| Message ID | 20231023043751.17114-8-jason-jh.lin@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add CMDQ secure driver for SVP | expand |
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()") > + 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? > + > 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_event) == 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. Regards, Fei > + > u8 cmdq_get_shift_pa(struct mbox_chan *chan); > > #endif /* __MTK_CMDQ_MAILBOX_H__ */ > -- > 2.18.0 > >
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 > > > > >
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); + 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); + 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_event) == 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; +}; + u8 cmdq_get_shift_pa(struct mbox_chan *chan); #endif /* __MTK_CMDQ_MAILBOX_H__ */
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(-)