Patchwork [v5,17/20] memory: mtk-smi: Get rid of need_larbid

login
register
mail settings
Submitter Yong Wu
Date Jan. 1, 2019, 3:55 a.m.
Message ID <1546314952-15990-18-git-send-email-yong.wu@mediatek.com>
Download mbox | patch
Permalink /patch/691303/
State New
Headers show

Comments

Yong Wu - Jan. 1, 2019, 3:55 a.m.
The "mediatek,larb-id" has already been parsed in MTK IOMMU driver.
It's no need to parse it again in SMI driver. Only clean some codes.
This patch is fit for all the current mt2701, mt2712, mt7623, mt8173
and mt8183.

After this patch, the "mediatek,larb-id" only be needed for mt2712
which have 2 M4Us. In the other SoCs, we can get the larb-id from M4U
in which the larbs in the "mediatek,larbs" always are ordered.

CC: Matthias Brugger <matthias.bgg@gmail.com>
Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
 drivers/memory/mtk-smi.c | 26 ++------------------------
 1 file changed, 2 insertions(+), 24 deletions(-)
Evan Green - Jan. 30, 2019, 7:11 p.m.
On Mon, Dec 31, 2018 at 7:59 PM Yong Wu <yong.wu@mediatek.com> wrote:
>
> The "mediatek,larb-id" has already been parsed in MTK IOMMU driver.
> It's no need to parse it again in SMI driver. Only clean some codes.
> This patch is fit for all the current mt2701, mt2712, mt7623, mt8173
> and mt8183.
>
> After this patch, the "mediatek,larb-id" only be needed for mt2712
> which have 2 M4Us. In the other SoCs, we can get the larb-id from M4U
> in which the larbs in the "mediatek,larbs" always are ordered.
>
> CC: Matthias Brugger <matthias.bgg@gmail.com>
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> ---
>  drivers/memory/mtk-smi.c | 26 ++------------------------
>  1 file changed, 2 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index 08cf40d..10e6493 100644
> --- a/drivers/memory/mtk-smi.c
> +++ b/drivers/memory/mtk-smi.c
> @@ -67,7 +67,6 @@ struct mtk_smi_common_plat {
>  };
>
>  struct mtk_smi_larb_gen {
> -       bool need_larbid;
>         int port_in_larb[MTK_LARB_NR_MAX + 1];
>         void (*config_port)(struct device *);
>         unsigned int larb_direct_to_common_mask;
> @@ -153,18 +152,9 @@ void mtk_smi_larb_put(struct device *larbdev)
>         struct mtk_smi_iommu *smi_iommu = data;
>         unsigned int         i;
>
> -       if (larb->larb_gen->need_larbid) {
> -               larb->mmu = &smi_iommu->larb_imu[larb->larbid].mmu;
> -               return 0;
> -       }
> -
> -       /*
> -        * If there is no larbid property, Loop to find the corresponding
> -        * iommu information.
> -        */
> -       for (i = 0; i < smi_iommu->larb_nr; i++) {
> +       for (i = 0; i < MTK_LARB_NR_MAX; i++) {

Looks like this was the only use of mtk_smi_iommu.larb_nr. Should we
remove that now?
Yong Wu - Jan. 31, 2019, 3:22 a.m.
On Wed, 2019-01-30 at 11:11 -0800, Evan Green wrote:
> On Mon, Dec 31, 2018 at 7:59 PM Yong Wu <yong.wu@mediatek.com> wrote:
> >
> > The "mediatek,larb-id" has already been parsed in MTK IOMMU driver.
> > It's no need to parse it again in SMI driver. Only clean some codes.
> > This patch is fit for all the current mt2701, mt2712, mt7623, mt8173
> > and mt8183.
> >
> > After this patch, the "mediatek,larb-id" only be needed for mt2712
> > which have 2 M4Us. In the other SoCs, we can get the larb-id from M4U
> > in which the larbs in the "mediatek,larbs" always are ordered.
> >
> > CC: Matthias Brugger <matthias.bgg@gmail.com>
> > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > ---
> >  drivers/memory/mtk-smi.c | 26 ++------------------------
> >  1 file changed, 2 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> > index 08cf40d..10e6493 100644
> > --- a/drivers/memory/mtk-smi.c
> > +++ b/drivers/memory/mtk-smi.c
> > @@ -67,7 +67,6 @@ struct mtk_smi_common_plat {
> >  };
> >
> >  struct mtk_smi_larb_gen {
> > -       bool need_larbid;
> >         int port_in_larb[MTK_LARB_NR_MAX + 1];
> >         void (*config_port)(struct device *);
> >         unsigned int larb_direct_to_common_mask;
> > @@ -153,18 +152,9 @@ void mtk_smi_larb_put(struct device *larbdev)
> >         struct mtk_smi_iommu *smi_iommu = data;
> >         unsigned int         i;
> >
> > -       if (larb->larb_gen->need_larbid) {
> > -               larb->mmu = &smi_iommu->larb_imu[larb->larbid].mmu;
> > -               return 0;
> > -       }
> > -
> > -       /*
> > -        * If there is no larbid property, Loop to find the corresponding
> > -        * iommu information.
> > -        */
> > -       for (i = 0; i < smi_iommu->larb_nr; i++) {
> > +       for (i = 0; i < MTK_LARB_NR_MAX; i++) {
> 
> Looks like this was the only use of mtk_smi_iommu.larb_nr. Should we
> remove that now?

This is necessary since the mt2712 which has two M4U HW.

From its dtsi[1], take iommu1 as a example, its larb_nr only is 3, but
we need scan all the larb.

[1]
http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016119.html

> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Evan Green - Jan. 31, 2019, 5:45 p.m.
On Wed, Jan 30, 2019 at 7:22 PM Yong Wu <yong.wu@mediatek.com> wrote:
>
> On Wed, 2019-01-30 at 11:11 -0800, Evan Green wrote:
> > On Mon, Dec 31, 2018 at 7:59 PM Yong Wu <yong.wu@mediatek.com> wrote:
> > >
> > > The "mediatek,larb-id" has already been parsed in MTK IOMMU driver.
> > > It's no need to parse it again in SMI driver. Only clean some codes.
> > > This patch is fit for all the current mt2701, mt2712, mt7623, mt8173
> > > and mt8183.
> > >
> > > After this patch, the "mediatek,larb-id" only be needed for mt2712
> > > which have 2 M4Us. In the other SoCs, we can get the larb-id from M4U
> > > in which the larbs in the "mediatek,larbs" always are ordered.
> > >
> > > CC: Matthias Brugger <matthias.bgg@gmail.com>
> > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > ---
> > >  drivers/memory/mtk-smi.c | 26 ++------------------------
> > >  1 file changed, 2 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> > > index 08cf40d..10e6493 100644
> > > --- a/drivers/memory/mtk-smi.c
> > > +++ b/drivers/memory/mtk-smi.c
> > > @@ -67,7 +67,6 @@ struct mtk_smi_common_plat {
> > >  };
> > >
> > >  struct mtk_smi_larb_gen {
> > > -       bool need_larbid;
> > >         int port_in_larb[MTK_LARB_NR_MAX + 1];
> > >         void (*config_port)(struct device *);
> > >         unsigned int larb_direct_to_common_mask;
> > > @@ -153,18 +152,9 @@ void mtk_smi_larb_put(struct device *larbdev)
> > >         struct mtk_smi_iommu *smi_iommu = data;
> > >         unsigned int         i;
> > >
> > > -       if (larb->larb_gen->need_larbid) {
> > > -               larb->mmu = &smi_iommu->larb_imu[larb->larbid].mmu;
> > > -               return 0;
> > > -       }
> > > -
> > > -       /*
> > > -        * If there is no larbid property, Loop to find the corresponding
> > > -        * iommu information.
> > > -        */
> > > -       for (i = 0; i < smi_iommu->larb_nr; i++) {
> > > +       for (i = 0; i < MTK_LARB_NR_MAX; i++) {
> >
> > Looks like this was the only use of mtk_smi_iommu.larb_nr. Should we
> > remove that now?
>
> This is necessary since the mt2712 which has two M4U HW.
>
> From its dtsi[1], take iommu1 as a example, its larb_nr only is 3, but
> we need scan all the larb.
>
> [1]
> http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016119.html

I'm not sure I follow. My point was that this structure member is only
ever set but never read:
$ git grep '[.>]larb_nr'
drivers/iommu/mtk_iommu.c:      data->smi_imu.larb_nr = larb_nr;
drivers/iommu/mtk_iommu_v1.c:   data->smi_imu.larb_nr = larb_nr;

Maybe I've applied the patches to the wrong tree, and there is a use
of this member I'm not seeing?
-Evan
Yong Wu - Feb. 1, 2019, 9:42 a.m.
On Thu, 2019-01-31 at 09:45 -0800, Evan Green wrote:
> On Wed, Jan 30, 2019 at 7:22 PM Yong Wu <yong.wu@mediatek.com> wrote:
> >
> > On Wed, 2019-01-30 at 11:11 -0800, Evan Green wrote:
> > > On Mon, Dec 31, 2018 at 7:59 PM Yong Wu <yong.wu@mediatek.com> wrote:
> > > >
> > > > The "mediatek,larb-id" has already been parsed in MTK IOMMU driver.
> > > > It's no need to parse it again in SMI driver. Only clean some codes.
> > > > This patch is fit for all the current mt2701, mt2712, mt7623, mt8173
> > > > and mt8183.
> > > >
> > > > After this patch, the "mediatek,larb-id" only be needed for mt2712
> > > > which have 2 M4Us. In the other SoCs, we can get the larb-id from M4U
> > > > in which the larbs in the "mediatek,larbs" always are ordered.
> > > >
> > > > CC: Matthias Brugger <matthias.bgg@gmail.com>
> > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > > ---
> > > >  drivers/memory/mtk-smi.c | 26 ++------------------------
> > > >  1 file changed, 2 insertions(+), 24 deletions(-)
> > > >
> > > > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> > > > index 08cf40d..10e6493 100644
> > > > --- a/drivers/memory/mtk-smi.c
> > > > +++ b/drivers/memory/mtk-smi.c
> > > > @@ -67,7 +67,6 @@ struct mtk_smi_common_plat {
> > > >  };
> > > >
> > > >  struct mtk_smi_larb_gen {
> > > > -       bool need_larbid;
> > > >         int port_in_larb[MTK_LARB_NR_MAX + 1];
> > > >         void (*config_port)(struct device *);
> > > >         unsigned int larb_direct_to_common_mask;
> > > > @@ -153,18 +152,9 @@ void mtk_smi_larb_put(struct device *larbdev)
> > > >         struct mtk_smi_iommu *smi_iommu = data;
> > > >         unsigned int         i;
> > > >
> > > > -       if (larb->larb_gen->need_larbid) {
> > > > -               larb->mmu = &smi_iommu->larb_imu[larb->larbid].mmu;
> > > > -               return 0;
> > > > -       }
> > > > -
> > > > -       /*
> > > > -        * If there is no larbid property, Loop to find the corresponding
> > > > -        * iommu information.
> > > > -        */
> > > > -       for (i = 0; i < smi_iommu->larb_nr; i++) {
> > > > +       for (i = 0; i < MTK_LARB_NR_MAX; i++) {
> > >
> > > Looks like this was the only use of mtk_smi_iommu.larb_nr. Should we
> > > remove that now?
> >
> > This is necessary since the mt2712 which has two M4U HW.
> >
> > From its dtsi[1], take iommu1 as a example, its larb_nr only is 3, but
> > we need scan all the larb.
> >
> > [1]
> > http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016119.html
> 
> I'm not sure I follow. My point was that this structure member is only
> ever set but never read:
> $ git grep '[.>]larb_nr'
> drivers/iommu/mtk_iommu.c:      data->smi_imu.larb_nr = larb_nr;
> drivers/iommu/mtk_iommu_v1.c:   data->smi_imu.larb_nr = larb_nr;
> 
> Maybe I've applied the patches to the wrong tree, and there is a use
> of this member I'm not seeing?

Thanks. I misunderstood. It looks right, this variable can be deleted I
didn't realize this. Maybe I need use a new patch for this.

> -Evan

Patch

diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index 08cf40d..10e6493 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -67,7 +67,6 @@  struct mtk_smi_common_plat {
 };
 
 struct mtk_smi_larb_gen {
-	bool need_larbid;
 	int port_in_larb[MTK_LARB_NR_MAX + 1];
 	void (*config_port)(struct device *);
 	unsigned int larb_direct_to_common_mask;
@@ -153,18 +152,9 @@  void mtk_smi_larb_put(struct device *larbdev)
 	struct mtk_smi_iommu *smi_iommu = data;
 	unsigned int         i;
 
-	if (larb->larb_gen->need_larbid) {
-		larb->mmu = &smi_iommu->larb_imu[larb->larbid].mmu;
-		return 0;
-	}
-
-	/*
-	 * If there is no larbid property, Loop to find the corresponding
-	 * iommu information.
-	 */
-	for (i = 0; i < smi_iommu->larb_nr; i++) {
+	for (i = 0; i < MTK_LARB_NR_MAX; i++) {
 		if (dev == smi_iommu->larb_imu[i].dev) {
-			/* The 'mmu' may be updated in iommu-attach/detach. */
+			larb->larbid = i;
 			larb->mmu = &smi_iommu->larb_imu[i].mmu;
 			return 0;
 		}
@@ -243,7 +233,6 @@  static void mtk_smi_larb_config_port_gen1(struct device *dev)
 };
 
 static const struct mtk_smi_larb_gen mtk_smi_larb_mt2701 = {
-	.need_larbid = true,
 	.port_in_larb = {
 		LARB0_PORT_OFFSET, LARB1_PORT_OFFSET,
 		LARB2_PORT_OFFSET, LARB3_PORT_OFFSET
@@ -252,7 +241,6 @@  static void mtk_smi_larb_config_port_gen1(struct device *dev)
 };
 
 static const struct mtk_smi_larb_gen mtk_smi_larb_mt2712 = {
-	.need_larbid = true,
 	.config_port                = mtk_smi_larb_config_port_gen2_general,
 	.larb_direct_to_common_mask = BIT(8) | BIT(9),      /* bdpsys */
 };
@@ -291,7 +279,6 @@  static int mtk_smi_larb_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *smi_node;
 	struct platform_device *smi_pdev;
-	int err;
 
 	larb = devm_kzalloc(dev, sizeof(*larb), GFP_KERNEL);
 	if (!larb)
@@ -321,15 +308,6 @@  static int mtk_smi_larb_probe(struct platform_device *pdev)
 	}
 	larb->smi.dev = dev;
 
-	if (larb->larb_gen->need_larbid) {
-		err = of_property_read_u32(dev->of_node, "mediatek,larb-id",
-					   &larb->larbid);
-		if (err) {
-			dev_err(dev, "missing larbid property\n");
-			return err;
-		}
-	}
-
 	smi_node = of_parse_phandle(dev->of_node, "mediatek,smi", 0);
 	if (!smi_node)
 		return -EINVAL;