netdev - Re: [PATCH net-next] r8169: use devm_clk_get_optional

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]

Date:   Thu, 02 Feb 2023 14:59:31 +0100
From:   John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>
To:     hkallweit1@...il.com
Cc:     davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        netdev@...r.kernel.org, nic_swsd@...ltek.com, pabeni@...hat.com,
        linux-sh@...r.kernel.org,
        Yoshinori Sato <ysato@...rs.sourceforge.jp>,
        Rich Felker <dalias@...c.org>
Subject: Re: [PATCH net-next] r8169: use devm_clk_get_optional_enabled() to
 simplify the code

Hello Heiner!

> Now that we have devm_clk_get_optional_enabled(), we don't have to
> open-code it.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
> ---
>  drivers/net/ethernet/realtek/r8169_main.c | 37 ++---------------------
>  1 file changed, 3 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index a8b0070bb..e6fb6f223 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -5122,37 +5122,6 @@ static int rtl_jumbo_max(struct rtl8169_private *tp)
>  	}
>  }
>  
> -static void rtl_disable_clk(void *data)
> -{
> -	clk_disable_unprepare(data);
> -}
> -
> -static int rtl_get_ether_clk(struct rtl8169_private *tp)
> -{
> -	struct device *d = tp_to_dev(tp);
> -	struct clk *clk;
> -	int rc;
> -
> -	clk = devm_clk_get(d, "ether_clk");
> -	if (IS_ERR(clk)) {
> -		rc = PTR_ERR(clk);
> -		if (rc == -ENOENT)
> -			/* clk-core allows NULL (for suspend / resume) */
> -			rc = 0;
> -		else
> -			dev_err_probe(d, rc, "failed to get clk\n");
> -	} else {
> -		tp->clk = clk;
> -		rc = clk_prepare_enable(clk);
> -		if (rc)
> -			dev_err(d, "failed to enable clk: %d\n", rc);
> -		else
> -			rc = devm_add_action_or_reset(d, rtl_disable_clk, clk);
> -	}
> -
> -	return rc;
> -}
> -
>  static void rtl_init_mac_address(struct rtl8169_private *tp)
>  {
>  	u8 mac_addr[ETH_ALEN] __aligned(2) = {};
> @@ -5216,9 +5185,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  		return -ENOMEM;
>  
>  	/* Get the *optional* external "ether_clk" used on some boards */
> -	rc = rtl_get_ether_clk(tp);
> -	if (rc)
> -		return rc;
> +	tp->clk = devm_clk_get_optional_enabled(&pdev->dev, "ether_clk");
> +	if (IS_ERR(tp->clk))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(tp->clk), "failed to get ether_clk\n");
>  
>  	/* enable device (incl. PCI PM wakeup and hotplug setup) */
>  	rc = pcim_enable_device(pdev);
> -- 
> 2.37.3

This change broke the r8169 driver on my SH7785LCR SuperH Evaluation Board.

With your patch, the driver initialization fails with:

[    1.648000] r8169 0000:00:00.0: error -EINVAL: failed to get ether_clk
[    1.676000] r8169: probe of 0000:00:00.0 failed with error -22

Any idea what could be the problem?

Thanks,
Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913