Message ID | 3349780.3Prc3uV314@wasted.cogentembedded.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On 03/22/2016 10:27 PM, Sergei Shtylyov wrote: > The driver calls gpiod_set_value() with GPIOD_OUT_* instead of 0 and 1, as > a result the PHY isn't really put back into reset state in macb_remove(). > Moreover, the driver assumes that something else has set the GPIO direction > to output, so if it has not, the PHY wouldn't be taken out of reset in s/wouldn't/may not/, sorry. Do I need to resend? > macb_probe() either... > > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> MBR, Sergei
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Date: Tue, 22 Mar 2016 22:34:05 +0300 > On 03/22/2016 10:27 PM, Sergei Shtylyov wrote: > >> The driver calls gpiod_set_value() with GPIOD_OUT_* instead of 0 and >> 1, as >> a result the PHY isn't really put back into reset state in >> macb_remove(). >> Moreover, the driver assumes that something else has set the GPIO >> direction >> to output, so if it has not, the PHY wouldn't be taken out of reset in > > s/wouldn't/may not/, sorry. Do I need to resend? No need, I fixed it up by hand. Applied, thanks.
On 03/22/2016 11:07 PM, David Miller wrote: >>> The driver calls gpiod_set_value() with GPIOD_OUT_* instead of 0 and >>> 1, as >>> a result the PHY isn't really put back into reset state in >>> macb_remove(). >>> Moreover, the driver assumes that something else has set the GPIO >>> direction >>> to output, so if it has not, the PHY wouldn't be taken out of reset in >> >> s/wouldn't/may not/, sorry. Do I need to resend? > > No need, I fixed it up by hand. > > Applied, thanks. Thank you! gpio_request() or smth of that sort wouldn't hurt too (the pin won't be switched to the GPIO mode on the pin function controller that I have here... Anyway, this code is doomed if my phylib reset GPIO patch (to be posted yet) is accepted ... MBR, Sergei
On 03/22/2016 11:07 PM, David Miller wrote: >> On 03/22/2016 10:27 PM, Sergei Shtylyov wrote: >> >>> The driver calls gpiod_set_value() with GPIOD_OUT_* instead of 0 and >>> 1, as >>> a result the PHY isn't really put back into reset state in >>> macb_remove(). >>> Moreover, the driver assumes that something else has set the GPIO >>> direction >>> to output, so if it has not, the PHY wouldn't be taken out of reset in >> >> s/wouldn't/may not/, sorry. Do I need to resend? > > No need, I fixed it up by hand. > > Applied, thanks. Oops, forgot another tag: Fixes: 270c499f0993 ("net/macb: Update device tree binding for resetting PHY using GPIO") Too late probably... :-( MBR, Sergei
Le 22/03/2016 22:33, Sergei Shtylyov a écrit : > On 03/22/2016 11:07 PM, David Miller wrote: > >>> On 03/22/2016 10:27 PM, Sergei Shtylyov wrote: >>> >>>> The driver calls gpiod_set_value() with GPIOD_OUT_* instead of 0 and >>>> 1, as >>>> a result the PHY isn't really put back into reset state in >>>> macb_remove(). >>>> Moreover, the driver assumes that something else has set the GPIO >>>> direction >>>> to output, so if it has not, the PHY wouldn't be taken out of reset in >>> >>> s/wouldn't/may not/, sorry. Do I need to resend? >> >> No need, I fixed it up by hand. >> >> Applied, thanks. > > Oops, forgot another tag: > > Fixes: 270c499f0993 ("net/macb: Update device tree binding for resetting PHY > using GPIO") > > Too late probably... :-( Too late also: Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com> Thanks Sergei! Bye,
Index: net/drivers/net/ethernet/cadence/macb.c =================================================================== --- net.orig/drivers/net/ethernet/cadence/macb.c +++ net/drivers/net/ethernet/cadence/macb.c @@ -2959,7 +2959,7 @@ static int macb_probe(struct platform_de int gpio = of_get_named_gpio(phy_node, "reset-gpios", 0); if (gpio_is_valid(gpio)) bp->reset_gpio = gpio_to_desc(gpio); - gpiod_set_value(bp->reset_gpio, GPIOD_OUT_HIGH); + gpiod_direction_output(bp->reset_gpio, 1); } of_node_put(phy_node); @@ -3029,7 +3029,7 @@ static int macb_remove(struct platform_d mdiobus_free(bp->mii_bus); /* Shutdown the PHY if there is a GPIO reset */ - gpiod_set_value(bp->reset_gpio, GPIOD_OUT_LOW); + gpiod_set_value(bp->reset_gpio, 0); unregister_netdev(dev); clk_disable_unprepare(bp->tx_clk);
The driver calls gpiod_set_value() with GPIOD_OUT_* instead of 0 and 1, as a result the PHY isn't really put back into reset state in macb_remove(). Moreover, the driver assumes that something else has set the GPIO direction to output, so if it has not, the PHY wouldn't be taken out of reset in macb_probe() either... Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- The patch is against David Miller's 'net.git' repo. drivers/net/ethernet/cadence/macb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)