diff mbox series

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

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

Commit Message

Jason-JH.Lin Oct. 23, 2023, 4:37 a.m. UTC 
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(-)

Comments

Fei Shao Oct. 23, 2023, 10:47 a.m. UTC | #1 
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
>
>
Jason-JH.Lin Oct. 25, 2023, 2:08 a.m. UTC | #2 
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 mbox series

Patch

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__ */