netdev - Re: [RESEND PATCH net 02/10] net: ethernet: mediatek: fix incorrect return value of devm_clk_get with EPROBE

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: <e1e9ca7a-c33b-3a9e-fd47-48f80bb10a32@phrozen.org>
Date:   Thu, 25 Aug 2016 15:49:10 +0200
From:   John Crispin <john@...ozen.org>
To:     Sean Wang <sean.wang@...iatek.com>, davem@...emloft.net
Cc:     netdev@...r.kernel.org, nbd@...nwrt.org,
        linux-mediatek@...ts.infradead.org, keyhaede@...il.com
Subject: Re: [RESEND PATCH net 02/10] net: ethernet: mediatek: fix incorrect
 return value of devm_clk_get with EPROBE_DEFER



On 25/08/2016 12:44, Sean Wang wrote:
> If the return value of devm_clk_get is EPROBE_DEFER, we should
> defer probing the driver. The change is verified and works based
> on 4.8-rc1 staying with the latest clk-next code for MT7623.
> 
> Signed-off-by: Sean Wang <sean.wang@...iatek.com>
> ---
>  drivers/net/ethernet/mediatek/mtk_eth_soc.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index 6e4a6ca..02b048f 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -1851,8 +1851,15 @@ static int mtk_probe(struct platform_device *pdev)
>  	eth->clk_gp1 = devm_clk_get(&pdev->dev, "gp1");
>  	eth->clk_gp2 = devm_clk_get(&pdev->dev, "gp2");
>  	if (IS_ERR(eth->clk_esw) || IS_ERR(eth->clk_gp1) ||
> -	    IS_ERR(eth->clk_gp2) || IS_ERR(eth->clk_ethif))
> -		return -ENODEV;
> +	    IS_ERR(eth->clk_gp2) || IS_ERR(eth->clk_ethif)) {
> +		if (PTR_ERR(eth->clk_esw) == -EPROBE_DEFER ||
> +		    PTR_ERR(eth->clk_gp1) == -EPROBE_DEFER ||
> +		    PTR_ERR(eth->clk_gp1) == -EPROBE_DEFER ||
> +		    PTR_ERR(eth->clk_gp2) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +		else
> +			return -ENODEV;
> +	}

Hi Sean,

this looks a bit tedious. maybe a better solution would be to add an
array to struct mtk_eth for the clocks and an enum for the index
mapping. that would allow the usage of loops to work out if all clocks
are fine. the following code calling clk_prepare_enable() could then
also be turned into a loop

	John

>  
>  	clk_prepare_enable(eth->clk_ethif);
>  	clk_prepare_enable(eth->clk_esw);
>