Revert "mdio_bus: Remove unneeded gpiod NULL check"
Subsystems (3)
PATCH 1/1 Revert "mdio_bus: Remove unneeded gpiod NULL check"
This reverts commit 95b80bf3db03c2bf572a357cf74b9a6aefef0a4a ("mdio_bus:
Remove unneeded gpiod NULL check"), this commit assumed that GPIOLIB checks for NULL descriptors, so it's safe to drop them, but it is not when CONFIG_GPIOLIB is disabled in the kernel. If we do call gpiod_set_value_cansleep() on a GPIO descriptor we will issue warnings coming from the inline stubs declared in include/linux/gpio/consumer.h.
--- drivers/net/phy/mdio_bus.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/net/phy/mdio_bus.c +4 -2
@@ -399,7 +399,8 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner) }/* Put PHYs in RESET to save power */ -gpiod_set_value_cansleep (bus->reset_gpiod,1 ); +if (bus->reset_gpiod) +gpiod_set_value_cansleep (bus->reset_gpiod,1 );device_del (&bus->dev);return err;
@@ -424,7 +425,8 @@ void mdiobus_unregister(struct mii_bus *bus) }/* Put PHYs in RESET to save power */ -gpiod_set_value_cansleep (bus->reset_gpiod,1 ); +if (bus->reset_gpiod) +gpiod_set_value_cansleep (bus->reset_gpiod,1 );device_del (&bus->dev); }
Signature · 1 line
2.9.3
Quoted 10 lines · Florian Fainelli
This reverts commit
95b80bf3db03c2bf572a357cf74b9a6aefef0a4a("mdio_bus:
Remove unneeded gpiod NULL check"), this commit assumed that GPIOLIB checks for NULL descriptors, so it's safe to drop them, but it is not whenCONFIG_GPIOLIBis disabled in the kernel. If we do callgpiod_set_value_cansleep()on a GPIO descriptor we will issue warnings coming from the inline stubs declared ininclude/linux/gpio/consumer.h.Fixes 95b80bf3db03 ("mdio_bus: Remove unneeded gpiod NULL check")Reported-by Woojung Huh <[email protected]>Signed-off-by Florian Fainelli <[email protected]>
Yeah I guess you don't wanna have these messages spewing in the console. :/
But what about simply doing this:
Quoted 1 line
From
150e4f3c1f227c9a4c31bbb48d44220e25b792ecMon Sep 17 00:00:00 2001
From: Linus Walleij <[email protected]>
Date: Sat, 9 Sep 2017 01:07:22 +0200
Subject: [PATCH] net: phy: mdio_bus: mandate gpiolib
The core mdio bus unconditionally uses gpiolib APIs to handle reset lines which results in runtime errors when it is compiled out. It is not supposed to be possible to stub out gpiolib like this, the error messages from the stubs are a sign that something is wrong.
So just select it. Fix some unneeded #includes while we're at it.
--- drivers/net/phy/Kconfig | 1 + drivers/net/phy/mdio_bus.c | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-)
drivers/net/phy/Kconfig +1 -0
@@ -4,6 +4,7 @@ menuconfig MDIO_DEVICE tristate "MDIO bus device drivers" + select GPIOLIB help MDIO devices and driver infrastructure code.
drivers/net/phy/mdio_bus.c +0 -2
@@ -22,11 +22,9 @@#include <linux/init.h> #include <linux/delay.h> #include <linux/device.h> -#include <linux/gpio.h> #include <linux/gpio/consumer.h> #include <linux/of_device.h> #include <linux/of_mdio.h> -#include <linux/of_gpio.h> #include <linux/netdevice.h> #include <linux/etherdevice.h> #include <linux/skbuff.h>
Signature · 4 lines
2.13.5 Yours, Linus Walleij
Quoted 9 lines · Linus Walleij
Quoted 7 lines · Florian Fainelli
This reverts commit
95b80bf3db03c2bf572a357cf74b9a6aefef0a4a("mdio_bus:
Remove unneeded gpiod NULL check"), this commit assumed that GPIOLIB checks for NULL descriptors, so it's safe to drop them, but it is not whenCONFIG_GPIOLIBis disabled in the kernel. If we do callgpiod_set_value_cansleep()on a GPIO descriptor we will issue warnings coming from the inline stubs declared ininclude/linux/gpio/consumer.h.
Quoted 8 lines
Quoted 3 lines
Fixes 95b80bf3db03 ("mdio_bus: Remove unneeded gpiod NULL check")Reported-by Woojung Huh <[email protected]>Signed-off-by Florian Fainelli <[email protected]>Yeah I guess you don't wanna have these messages spewing in the console. :/
But what about simply doing this:
That would create another dependency which really does not need to be there as it really prevents you from testing more configurations, including but not limited to having CONFIG_GPIOLIB=n w/ CONFIG_MDIO_DEVICE=[ym].
The GPIOLIB=n inline stubs have a "contract" with the code calling them that is fairly clear, which is what this revert is leveraging.
Instead of doing what you suggest, we could also encapsulate the offending code within an #IS_ENABLED(CONFIG_GPIOLIB) block, which would be virtually the same thing in terms of object code generated, but would make it clear there is a functional dependency, I would personally be keener on that approach than having a select or depends.
Thanks
Quoted 50 lines
From
150e4f3c1f227c9a4c31bbb48d44220e25b792ecMon Sep 17 00:00:00 2001
From: Linus Walleij <[email protected]>
Date: Sat, 9 Sep 2017 01:07:22 +0200
Subject: [PATCH] net: phy:mdio_bus: mandate gpiolibThe core mdio bus unconditionally uses gpiolib APIs to handle reset lines which results in runtime errors when it is compiled out. It is not supposed to be possible to stub out gpiolib like this, the error messages from the stubs are a sign that something is wrong.
So just select it. Fix some unneeded #includes while we're at it.>
Signed-off-by Linus Walleij <[email protected]>--- drivers/net/phy/Kconfig | 1 + drivers/net/phy/mdio_bus.c | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-)drivers/net/phy/Kconfig +1
-0@@ -4,6 +4,7 @@ menuconfig MDIO_DEVICE tristate "MDIO bus device drivers" + select GPIOLIB help MDIO devices and driver infrastructure code.drivers/net/phy/mdio_bus.c +0
-2@@ -22,11 +22,9 @@#include <linux/init.h> #include <linux/delay.h> #include <linux/device.h> -#include <linux/gpio.h> #include <linux/gpio/consumer.h> #include <linux/of_device.h> #include <linux/of_mdio.h> -#include <linux/of_gpio.h> #include <linux/netdevice.h> #include <linux/etherdevice.h> #include <linux/skbuff.h>
Signature · 1 line
Florian
Linus Walleij
Acked-byQuoted 19 lines · Florian Fainelli
Quoted 9 lines · Linus Walleij
Quoted 7 lines · Florian Fainelli
This reverts commit
95b80bf3db03c2bf572a357cf74b9a6aefef0a4a("mdio_bus:
Remove unneeded gpiod NULL check"), this commit assumed that GPIOLIB checks for NULL descriptors, so it's safe to drop them, but it is not whenCONFIG_GPIOLIBis disabled in the kernel. If we do callgpiod_set_value_cansleep()on a GPIO descriptor we will issue warnings coming from the inline stubs declared ininclude/linux/gpio/consumer.h.Quoted 8 lines
Quoted 3 lines
Fixes 95b80bf3db03 ("mdio_bus: Remove unneeded gpiod NULL check")Reported-by Woojung Huh <[email protected]>Signed-off-by Florian Fainelli <[email protected]>Yeah I guess you don't wanna have these messages spewing in the console. :/
But what about simply doing this:
Quoted 7 lines
That would create another dependency which really does not need to be there as it really prevents you from testing more configurations, including but not limited to having
CONFIG_GPIOLIB=nw/CONFIG_MDIO_DEVICE=[ym].The GPIOLIB=n inline stubs have a "contract" with the code calling them that is fairly clear, which is what this revert is leveraging.
Ayeah the contract is something like "you can call us if compiled out, but then you get warnings".
Yeah NULL checks does the trick as well as #ifdef:in in that sense. It's no big deal so if you prefer this:
Yours,
Linus Walleij
From: Florian Fainelli <[email protected]>
Date: Fri, 8 Sep 2017 15:38:07 -0700
Quoted 10 lines
This reverts commit
95b80bf3db03c2bf572a357cf74b9a6aefef0a4a("mdio_bus:
Remove unneeded gpiod NULL check"), this commit assumed that GPIOLIB checks for NULL descriptors, so it's safe to drop them, but it is not whenCONFIG_GPIOLIBis disabled in the kernel. If we do callgpiod_set_value_cansleep()on a GPIO descriptor we will issue warnings coming from the inline stubs declared ininclude/linux/gpio/consumer.h.Fixes 95b80bf3db03 ("mdio_bus: Remove unneeded gpiod NULL check")Reported-by Woojung Huh <[email protected]>Signed-off-by Florian Fainelli <[email protected]>
Applied.