Re: [PATCH] at803x: fix reset handling

[Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>]

On 03/23/2016 09:45 AM, Uwe Kleine-König wrote:

> On Wed, Mar 23, 2016 at 12:44:40AM +0300, Sergei Shtylyov wrote:
>> The driver of course "knows" that the chip's reset signal is active low,
>> so  it drives the GPIO to 0  to reset the PHY and to 1 otherwise; however
>> all this will only work iff the GPIO  is  specified as active-high in the
>> device tree!  I think both the driver and the device trees (if there are
>> any -- I was unable to find them) need to be fixed in this case...
>>
>> Fixes: 13a56b449325 ("net: phy: at803x: Add support for hardware reset")
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> ---
>> The patch is against DaveM's 'net.git' repo.
>
> Don't you need to work against net-next for non-urgent stuff? Or do you
> consider this urgent?

    The net-next.git is not for fixes, net.git is. And net-next is closed 
right now because of the merge window.

>>   drivers/net/phy/at803x.c |    6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> Index: net/drivers/net/phy/at803x.c
>> ===================================================================
>> --- net.orig/drivers/net/phy/at803x.c
>> +++ net/drivers/net/phy/at803x.c
>> @@ -277,7 +277,7 @@ static int at803x_probe(struct phy_devic
>>   	if (!priv)
>>   		return -ENOMEM;
>>
>> -	gpiod_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
>> +	gpiod_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
>>   	if (IS_ERR(gpiod_reset))
>>   		return PTR_ERR(gpiod_reset);
>>
>> @@ -362,10 +362,10 @@ static void at803x_link_change_notify(st
>>
>>   				at803x_context_save(phydev, &context);
>>
>> -				gpiod_set_value(priv->gpiod_reset, 0);
>> -				msleep(1);
>>   				gpiod_set_value(priv->gpiod_reset, 1);
>>   				msleep(1);
>> +				gpiod_set_value(priv->gpiod_reset, 0);
>> +				msleep(1);
>
> The new variant is better than the old one. The change however breaks
> existing device trees which is not so nice. Given there are no mainline
> users this is probably ok though. So:
>
> Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

    Thank you!

MBR, Sergei