linux-kernel - Re: [PATCH v6 25/35] iommu/mediatek: Migrate to aggregate driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7c517b787d1dd768372d0141f5078e3089e883cb.camel@mediatek.com>
Date:   Thu, 10 Feb 2022 19:03:35 +0800
From:   Yong Wu <yong.wu@...iatek.com>
To:     Stephen Boyd <swboyd@...omium.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Douglas Anderson <dianders@...omium.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>
CC:     <linux-kernel@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
        <dri-devel@...ts.freedesktop.org>,
        <freedreno@...ts.freedesktop.org>,
        "Joerg Roedel" <joro@...tes.org>, Will Deacon <will@...nel.org>,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        "Rob Clark" <robdclark@...il.com>,
        Russell King <rmk+kernel@....linux.org.uk>,
        Saravana Kannan <saravanak@...gle.com>
Subject: Re: [PATCH v6 25/35] iommu/mediatek: Migrate to aggregate driver

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@...iatek.com>
> Cc: Joerg Roedel <joro@...tes.org>
> Cc: Will Deacon <will@...nel.org>
> Cc: Daniel Vetter <daniel.vetter@...ll.ch>
> Cc: "Rafael J. Wysocki" <rafael@...nel.org>
> Cc: Rob Clark <robdclark@...il.com>
> Cc: Russell King <rmk+kernel@....linux.org.uk>
> Cc: Saravana Kannan <saravanak@...gle.com>
> Signed-off-by: Stephen Boyd <swboyd@...omium.org>

+ Krzysztof

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

Tested-by: Yong Wu <yong.wu@...iatek.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;
>  }