Re: [PATCH v2 7/9] mailbox: mediatek: Add secure CMDQ driver support for CMDQ driver

Fei Shao
2023-10-23T10:48:18Z

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
>
>