netdev - Re: [PATCH] at803x: fix reset handling

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]

Message-ID: <56F298D0.4080200@cogentembedded.com>
Date:	Wed, 23 Mar 2016 16:23:28 +0300
From:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To:	Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
Cc:	netdev@...r.kernel.org, f.fainelli@...il.com,
	Daniel Mack <zonque@...il.com>
Subject: Re: [PATCH] at803x: fix reset handling

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@...entembedded.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@...gutronix.de>

    Thank you!

MBR, Sergei