| Message ID | 68bd1e34-4251-4306-cc7d-e5ccc578acd9@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 599566c1c369205286b1a22e1b3c2e9dea0e3744 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] r8169: use devm_clk_get_optional_enabled() to simplify the code | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Single patches do not need cover letters |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 7 of 7 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | warning | WARNING: line length of 96 exceeds 80 columns |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
Hello: This patch was applied to netdev/net-next.git (master) by David S. Miller <davem@davemloft.net>: On Fri, 2 Sep 2022 22:52:34 +0200 you wrote: > Now that we have devm_clk_get_optional_enabled(), we don't have to > open-code it. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/net/ethernet/realtek/r8169_main.c | 37 ++--------------------- > 1 file changed, 3 insertions(+), 34 deletions(-) Here is the summary with links: - [net-next] r8169: use devm_clk_get_optional_enabled() to simplify the code https://git.kernel.org/netdev/net-next/c/599566c1c369 You are awesome, thank you!
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@gmail.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
Hi Adrian, On Thu, Feb 2, 2023 at 3:11 PM John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> wrote: > > Now that we have devm_clk_get_optional_enabled(), we don't have to > > open-code it. > > > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > --- a/drivers/net/ethernet/realtek/r8169_main.c > > +++ b/drivers/net/ethernet/realtek/r8169_main.c > > @@ -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? SH's clk_enable() returns -EINVAL if clk == NULL, which is wrong. Preparing a patch... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert! On Thu, 2023-02-02 at 17:09 +0100, Geert Uytterhoeven wrote: > > 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? > > SH's clk_enable() returns -EINVAL if clk == NULL, which is wrong. > Preparing a patch... Ah, that explains the error message then. Adrian
On Thu, Feb 2, 2023 at 5:09 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Thu, Feb 2, 2023 at 3:11 PM John Paul Adrian Glaubitz > <glaubitz@physik.fu-berlin.de> wrote: > > > Now that we have devm_clk_get_optional_enabled(), we don't have to > > > open-code it. > > > > > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > > > --- a/drivers/net/ethernet/realtek/r8169_main.c > > > +++ b/drivers/net/ethernet/realtek/r8169_main.c > > > > @@ -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? > > SH's clk_enable() returns -EINVAL if clk == NULL, which is wrong. > Preparing a patch... https://lore.kernel.org/r/b53e6b557b4240579933b3359dda335ff94ed5af.1675354849.git.geert+renesas@glider.be Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
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);
Now that we have devm_clk_get_optional_enabled(), we don't have to open-code it. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/ethernet/realtek/r8169_main.c | 37 ++--------------------- 1 file changed, 3 insertions(+), 34 deletions(-)