diff mbox series

[v3,13/15] memory: mtk-smi: Get rid of need_larbid

Message ID 1542422142-30688-14-git-send-email-yong.wu@mediatek.com (mailing list archive)
State New, archived
Headers show
Series MT8183 IOMMU SUPPORT | expand

Commit Message

Yong Wu (吴勇) Nov. 17, 2018, 2:35 a.m. UTC 
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.

Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
 drivers/memory/mtk-smi.c | 27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

Comments

Matthias Brugger Dec. 2, 2018, 11:04 p.m. UTC | #1 
On 17/11/2018 03:35, Yong Wu 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.

I'm trying to understand why we need the mediatek,larb-id at all. From what I
understand as long as the mediatek larbs described in the iommu are ordered
(larb0, larb1, larb2, etc) we actually get the same value as defined by
mediatek,larb-id. At least this holds for all present implementations.

On the other hand I don't see where the mtk_iommu_v1 driver actually parses the
larb-id, can you please help to understand:

1) why we need the larb-id at all
2) how this will work if we do not parse the larb-id for v1 iommu at all

Thanks a lot,
Matthias

> 
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> ---
>  drivers/memory/mtk-smi.c | 27 ++-------------------------
>  1 file changed, 2 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index e4daabb..e0265fe 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_special_mask; /* The special larbs mask. */
> @@ -152,18 +151,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;
>  		}
> @@ -242,7 +232,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
> @@ -251,13 +240,11 @@ 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_special_mask = BIT(8) | BIT(9), /* bdpsys */
>  };
>  
>  static const struct mtk_smi_larb_gen mtk_smi_larb_mt8183 = {
> -	.need_larbid = true,
>  	.config_port = mtk_smi_larb_config_port_gen2_general,
>  	.larb_special_mask = BIT(2) | BIT(3) | BIT(7), /* IPU0 | IPU1 | CCU */
>  };
> @@ -289,7 +276,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)
> @@ -316,15 +302,6 @@ static int mtk_smi_larb_probe(struct platform_device *pdev)
>  		return PTR_ERR(larb->smi.clk_gals0);
>  	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;
>
Yong Wu (吴勇) Dec. 3, 2018, 8:40 a.m. UTC | #2 
On Mon, 2018-12-03 at 00:04 +0100, Matthias Brugger wrote:
> 
> On 17/11/2018 03:35, Yong Wu 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.
> 
> I'm trying to understand why we need the mediatek,larb-id at all. From what I
> understand as long as the mediatek larbs described in the iommu are ordered
> (larb0, larb1, larb2, etc) we actually get the same value as defined by
> mediatek,larb-id. At least this holds for all present implementations.
> 
> On the other hand I don't see where the mtk_iommu_v1 driver actually parses the
> larb-id, can you please help to understand:
> 
> 1) why we need the larb-id at all

Actually only mt2712 which have 2 M4U HW need "mediatek,larb-id".

This is larbs in the m4u0/1 dtsi node of mt2712:
m4u0 { mediatek,larbs = <&larb0 &larb1 &larb2 &larb3 &larb6>;}
m4u1 { mediatek,larbs = <&larb4 &larb5 &larb7>;}

the id don't increase one by one, M4U have to get the larbid with the
help of "mediatek,larb-id".

(The m4u/smi dtsi patch of mt2712 will be send with some other modules,
maybe in this week.)


> 2) how this will work if we do not parse the larb-id for v1 iommu at all

As you said above and I also have wrote that the larbid "must sort
according to the local arbiter index" in the "mediatek,larbs"
description of dt-binding. All the M4U except mt2712 could ignore
"mediatek,larb-id". the v1 iommu also should be ok.

I'm not sure whether we should change [1], if only reserving mt2712
there, we also should change the dtsi file of mt2701 and mt7623.or keep
it as is.

[1]
https://elixir.bootlin.com/linux/v4.20-rc1/source/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt#L20

> 
> Thanks a lot,
> Matthias
> 
> > 

[snip]
diff mbox series

Patch

diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index e4daabb..e0265fe 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_special_mask; /* The special larbs mask. */
@@ -152,18 +151,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;
 		}
@@ -242,7 +232,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
@@ -251,13 +240,11 @@  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_special_mask = BIT(8) | BIT(9), /* bdpsys */
 };
 
 static const struct mtk_smi_larb_gen mtk_smi_larb_mt8183 = {
-	.need_larbid = true,
 	.config_port = mtk_smi_larb_config_port_gen2_general,
 	.larb_special_mask = BIT(2) | BIT(3) | BIT(7), /* IPU0 | IPU1 | CCU */
 };
@@ -289,7 +276,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)
@@ -316,15 +302,6 @@  static int mtk_smi_larb_probe(struct platform_device *pdev)
 		return PTR_ERR(larb->smi.clk_gals0);
 	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;