lkml.org 
[lkml]   [2016]   [May]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH RFT 1/2] phylib: add device reset GPIO support
From
Date
Hello.

On 05/13/2016 12:35 AM, Sergei Shtylyov wrote:

>> [we already talked about this patch in #armlinux, I'm now just
>> forwarding my comments on the list. Background was that I sent an easier
>> and less complete patch with the same idea. See
>> http://patchwork.ozlabs.org/patch/621418/]
>>
>> [added Linus Walleij to Cc, there is a question for you/him below]
>>
>> On Fri, Apr 29, 2016 at 01:12:54AM +0300, Sergei Shtylyov wrote:
>>> --- net-next.orig/Documentation/devicetree/bindings/net/phy.txt
>>> +++ net-next/Documentation/devicetree/bindings/net/phy.txt
>>> @@ -35,6 +35,8 @@ Optional Properties:
>>> - broken-turn-around: If set, indicates the PHY device does not correctly
>>> release the turn around line low at the end of a MDIO transaction.
>>>
>>> +- reset-gpios: The GPIO phandle and specifier for the PHY reset signal.
>>> +
>>> Example:
>>>
>>> ethernet-phy@0 {
>>
>> This is great.
>>
>>> Index: net-next/drivers/net/phy/at803x.c
>>> ===================================================================
>>> --- net-next.orig/drivers/net/phy/at803x.c
>>> +++ net-next/drivers/net/phy/at803x.c
>>> @@ -65,7 +65,6 @@ MODULE_LICENSE("GPL");
>>> [...]
>>
>> My patch breaks this driver. I wasn't aware of it.
>
> I tried to be as careful as I could but still it looks that I didn't
> succeed at that too...

Hm, I'm starting to forget the vital details about my patch...

> [...]
>>> Index: net-next/drivers/net/phy/mdio_device.c
>>> ===================================================================
>>> --- net-next.orig/drivers/net/phy/mdio_device.c
>>> +++ net-next/drivers/net/phy/mdio_device.c
> [...]
>>> @@ -117,9 +126,16 @@ static int mdio_probe(struct device *dev
>>> struct mdio_driver *mdiodrv = to_mdio_driver(drv);
>>> int err = 0;
>>>
>>> - if (mdiodrv->probe)
>>> + if (mdiodrv->probe) {
>>> + /* Deassert the reset signal */
>>> + mdio_device_reset(mdiodev, 0);
>>> +
>>> err = mdiodrv->probe(mdiodev);
>>>
>>> + /* Assert the reset signal */
>>> + mdio_device_reset(mdiodev, 1);
>>
>> I wonder if it's safe to do this in general. What if ->probe does
>> something with the phy that is lost by resetting but that is relied on
>> later?
>
> Well, I thought that config_init() method is designed for that but indeed
> the LXT driver writes to BMCR in its probe() method and hence is broken.
> Thank you for noticing...

It's broken even without my patch. The phylib will cause a PHY soft reset
when attaching to the PHY device, so all BMCR programming dpone in the probe()
method will be lost. My patch does make sense as is. Looks like I should also
look into fixing lxt.c. Florian, what do you think?

[...]

MBR, Sergei

\
 
 \ /
  Last update: 2016-05-13 21:41    [W:0.174 / U:0.424 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site