[PATCH v6 25/35] iommu/mediatek: Migrate to aggregate driver

[Yong Wu]

On Thu, 2022-01-27 at 12:01 -0800, Stephen Boyd wrote:
> Use an aggregate driver instead of component ops so that we can get
> proper driver probe ordering of the aggregate device with respect to
> all
> the component devices that make up the aggregate device.
> 
> Cc: Yong Wu <yong.wu at mediatek.com>
> Cc: Joerg Roedel <joro at 8bytes.org>
> Cc: Will Deacon <will at kernel.org>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: "Rafael J. Wysocki" <rafael at kernel.org>
> Cc: Rob Clark <robdclark at gmail.com>
> Cc: Russell King <rmk+kernel at arm.linux.org.uk>
> Cc: Saravana Kannan <saravanak at google.com>
> Signed-off-by: Stephen Boyd <swboyd at chromium.org>

+ Krzysztof

The memory/mtk-smi.c is expected to get Ack from Krzysztof.

Tested-by: Yong Wu <yong.wu at mediatek.com>

> ---
>  drivers/iommu/mtk_iommu.c    | 14 +++++++++-----
>  drivers/iommu/mtk_iommu.h    |  6 ++++--
>  drivers/iommu/mtk_iommu_v1.c | 14 +++++++++-----
>  drivers/memory/mtk-smi.c     | 10 ++++------
>  4 files changed, 26 insertions(+), 18 deletions(-)

[...]

> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index e201e5976f34..0910fe109f53 100644
> --- a/drivers/memory/mtk-smi.c
> +++ b/drivers/memory/mtk-smi.c
> @@ -175,6 +175,8 @@ mtk_smi_larb_bind(struct device *dev, struct
> device *master, void *data)
>  			larb->larbid = i;
>  			larb->mmu = &larb_mmu[i].mmu;
>  			larb->bank = larb_mmu[i].bank;
> +
> +			pm_runtime_enable(dev);
>  			return 0;
>  		}
>  	}
> @@ -450,15 +452,11 @@ static int mtk_smi_larb_probe(struct
> platform_device *pdev)
>  	if (ret < 0)
>  		return ret;
>  
> -	pm_runtime_enable(dev);
>  	platform_set_drvdata(pdev, larb);
>  	ret = component_add(dev, &mtk_smi_larb_component_ops);
> -	if (ret)
> -		goto err_pm_disable;
> -	return 0;
> +	if (!ret)
> +		return 0;
>  
> -err_pm_disable:
> -	pm_runtime_disable(dev);
>  	device_link_remove(dev, larb->smi_common_dev);

Here is right. But at a glance code here, I was confused why it always
call device_link_remove here. If we have v7, Could you help keep the
original format? something like below. This may be helpful when we add
new error flow in future.

        if (ret)
              goto dev_link_remove;
        return ret;

dev_link_remove:
        device_link_remove(dev, larb->smi_common_dev);

Thanks.     

>  	return ret;
>  }