Messages in this thread Patch in this message | | | From | Jason-JH Lin (林睿祥) <> | Subject | Re: [PATCH v2 7/9] mailbox: mediatek: Add secure CMDQ driver support for CMDQ driver | Date | Wed, 25 Oct 2023 02:08:46 +0000 |
| |
Hi Fei,
Thanks for the reviews.
On Mon, 2023-10-23 at 18:47 +0800, Fei Shao wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > Hi Jason, > > On Mon, Oct 23, 2023 at 12:39 PM Jason-JH.Lin < > jason-jh.lin@mediatek.com> wrote: > > > > CMDQ driver will probe a secure CMDQ driver when has_sec flag > > in platform data is true and its device node in dts has defined a > > event id of CMDQ_SYNC_TOKEN_SEC_EOF. > > > > Secure CMDQ driver support on mt8188 and mt8195 currently. > > So add a has_sec flag to their driver data to probe it. > > > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > > --- > > drivers/mailbox/mtk-cmdq-mailbox.c | 42 > ++++++++++++++++++++++-- > > include/linux/mailbox/mtk-cmdq-mailbox.h | 11 +++++++ > > 2 files changed, 51 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c > b/drivers/mailbox/mtk-cmdq-mailbox.c > > index 3bdfb9a60614..4db5eb76f353 100644 > > --- a/drivers/mailbox/mtk-cmdq-mailbox.c > > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c > > @@ -87,6 +87,7 @@ struct gce_plat { > > u8 shift; > > bool control_by_sw; > > bool sw_ddr_en; > > + bool has_sec; > > u32 gce_num; > > }; > > > > @@ -560,14 +561,23 @@ static int cmdq_probe(struct platform_device > *pdev) > > int alias_id = 0; > > static const char * const clk_name = "gce"; > > static const char * const clk_names[] = { "gce0", "gce1" }; > > + struct resource *res; > > + struct platform_device *mtk_cmdq_sec; > > + u32 hwid = 0; > > > > cmdq = devm_kzalloc(dev, sizeof(*cmdq), GFP_KERNEL); > > if (!cmdq) > > return -ENOMEM; > > > > - cmdq->base = devm_platform_ioremap_resource(pdev, 0); > > - if (IS_ERR(cmdq->base)) > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > The devm_platform_ioremap_resource() helper was added on purpose [1]. > Please stick to it unless you have a strong reason to re-split it. > > [1]: a04f30356e75 ("mailbox: mtk-cmdq: Make use of the helper > function > devm_platform_ioremap_resource()") > OK, because want to get the pa of GCE by resource like this: gce_sec_plat.base_pa = res->start;
I'll find another way to do that.
> > + if (!res) > > + return -EINVAL; > > + > > + cmdq->base = devm_ioremap_resource(dev, res); > > + if (IS_ERR(cmdq->base)) { > > + dev_err(dev, "failed to ioremap cmdq\n"); > > return PTR_ERR(cmdq->base); > > + } > > > > cmdq->irq = platform_get_irq(pdev, 0); > > if (cmdq->irq < 0) > > @@ -585,6 +595,8 @@ static int cmdq_probe(struct platform_device > *pdev) > > dev, cmdq->base, cmdq->irq); > > > > if (cmdq->pdata->gce_num > 1) { > > + hwid = of_alias_get_id(dev->of_node, clk_name); > Why get hwid here while it's only used in the has_sec branch?
Because I want to pass the hwid to secure world and let it know what GCE pa should use in TEE rather than passing GCE pa.
> > > + > > for_each_child_of_node(phandle->parent, node) { > > alias_id = of_alias_get_id(node, clk_name); > > if (alias_id >= 0 && alias_id < cmdq- > >pdata->gce_num) { > > @@ -653,6 +665,30 @@ static int cmdq_probe(struct platform_device > *pdev) > > return err; > > } > > > > + if (cmdq->pdata->has_sec) { > > + struct cmdq_sec_plat gce_sec_plat; > > + > > + if (of_property_read_u32_index(dev->of_node, > "mediatek,gce-events", 0, > > > + &gce_sec_plat.cmdq_eve > nt) == 0) { > > + gce_sec_plat.gce_dev = dev; > > + gce_sec_plat.base = cmdq->base; > > + gce_sec_plat.base_pa = res->start; > > + gce_sec_plat.hwid = hwid; > > + gce_sec_plat.gce_num = cmdq->pdata- > >gce_num; > > + gce_sec_plat.clocks = cmdq->clocks; > > + gce_sec_plat.thread_nr = cmdq->pdata- > >thread_nr; > > + > > + mtk_cmdq_sec = > platform_device_register_data(dev, "mtk_cmdq_sec", > > > + > PLATFORM_DEVID_AUTO, > > > + > &gce_sec_plat, > > > + > sizeof(gce_sec_plat)); > > + if (IS_ERR(mtk_cmdq_sec)) { > > + dev_err(dev, "failed to register > platform_device mtk_cmdq_sec\n"); > > + return PTR_ERR(mtk_cmdq_sec); > > + } > > + } > > + } > > + > > return 0; > > } > > > > @@ -693,6 +729,7 @@ static const struct gce_plat gce_plat_v6 = { > > .thread_nr = 24, > > .shift = 3, > > .control_by_sw = true, > > + .has_sec = true, > > .gce_num = 2 > > }; > > > > @@ -708,6 +745,7 @@ static const struct gce_plat gce_plat_v8 = { > > .thread_nr = 32, > > .shift = 3, > > .control_by_sw = true, > > + .has_sec = true, > > .gce_num = 2 > > }; > > > > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h > b/include/linux/mailbox/mtk-cmdq-mailbox.h > > index f78a08e7c6ed..fdda995a69ce 100644 > > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h > > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h > > @@ -79,6 +79,17 @@ struct cmdq_pkt { > > bool loop; > > }; > > > > +struct cmdq_sec_plat { > > + struct device *gce_dev; > > + void __iomem *base; > > + dma_addr_t base_pa; > > + u32 hwid; > > + u32 gce_num; > > + struct clk_bulk_data *clocks; > > + u32 thread_nr; > > + u32 cmdq_event; > > +}; > I feel this should be in mtk-cmdq-sec-mailbox.h. > That means we have to include <mtk-cmdq-sec-mailbox.h> in this file, but actually mtk-cmdq-sec-mailbox.h have to include <mtk-cmdq- mailbox.h>. So I add this here finally.
But I agree with you, I'll try to move this struct into mtk-cmdq-sec- mailbox.h and include <mtk-cmdq-sec-mailbox.h> in mtk-cmdq-mailbox.c.
Regards, Jason-JH.Lin
> Regards, > Fei > > > > > + > > u8 cmdq_get_shift_pa(struct mbox_chan *chan); > > > > #endif /* __MTK_CMDQ_MAILBOX_H__ */ > > -- > > 2.18.0 > > > > >
| |