2015-08-21 23:16:13

by David Daney

[permalink] [raw]
Subject: [PATCH] phylib: Make PHYs children of their MDIO bus, not the bus' parent.

From: David Daney <[email protected]>

commit 18ee49ddb0d2 ("phylib: rename mii_bus::dev to mii_bus::parent")
changed the parent of PHY devices from the bus to the bus parent.

Then, commit 4dea547fef1b ("phylib: rework to prepare for OF
registration of PHYs") moved the code into phy_device.c

At this point, it is somewhat unclear why the change was seen as
necessary. But, when we look at the device model tree in
/sys/devices, it is clearly incorrect. The PHYs should be children of
their MDIO bus.

Change the PHY's parent device to be the MDIO bus device.

Cc: Lennert Buytenhek <[email protected]>
Cc: Grant Likely <[email protected]>
Signed-off-by: David Daney <[email protected]>
---
drivers/net/phy/phy_device.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 0302483..55f0178 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -176,7 +176,7 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
if (c45_ids)
dev->c45_ids = *c45_ids;
dev->bus = bus;
- dev->dev.parent = bus->parent;
+ dev->dev.parent = &bus->dev;
dev->dev.bus = &mdio_bus_type;
dev->irq = bus->irq != NULL ? bus->irq[addr] : PHY_POLL;
dev_set_name(&dev->dev, PHY_ID_FMT, bus->id, addr);
--
1.7.11.7


2015-08-22 00:02:17

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH] phylib: Make PHYs children of their MDIO bus, not the bus' parent.

On 21/08/15 16:16, David Daney wrote:
> From: David Daney <[email protected]>
>
> commit 18ee49ddb0d2 ("phylib: rename mii_bus::dev to mii_bus::parent")
> changed the parent of PHY devices from the bus to the bus parent.
>
> Then, commit 4dea547fef1b ("phylib: rework to prepare for OF
> registration of PHYs") moved the code into phy_device.c
>
> At this point, it is somewhat unclear why the change was seen as
> necessary. But, when we look at the device model tree in
> /sys/devices, it is clearly incorrect. The PHYs should be children of
> their MDIO bus.
>
> Change the PHY's parent device to be the MDIO bus device.
>
> Cc: Lennert Buytenhek <[email protected]>
> Cc: Grant Likely <[email protected]>
> Signed-off-by: David Daney <[email protected]>

Acked-by: Florian Fainelli <[email protected]>
--
Florian

2015-08-25 18:31:39

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] phylib: Make PHYs children of their MDIO bus, not the bus' parent.

From: David Daney <[email protected]>
Date: Fri, 21 Aug 2015 16:16:03 -0700

> From: David Daney <[email protected]>
>
> commit 18ee49ddb0d2 ("phylib: rename mii_bus::dev to mii_bus::parent")
> changed the parent of PHY devices from the bus to the bus parent.
>
> Then, commit 4dea547fef1b ("phylib: rework to prepare for OF
> registration of PHYs") moved the code into phy_device.c
>
> At this point, it is somewhat unclear why the change was seen as
> necessary. But, when we look at the device model tree in
> /sys/devices, it is clearly incorrect. The PHYs should be children of
> their MDIO bus.
>
> Change the PHY's parent device to be the MDIO bus device.
>
> Cc: Lennert Buytenhek <[email protected]>
> Cc: Grant Likely <[email protected]>
> Signed-off-by: David Daney <[email protected]>

Applied, thanks.

2015-11-19 20:51:43

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH] phylib: Make PHYs children of their MDIO bus, not the bus' parent.

Hello.

On 08/22/2015 02:16 AM, David Daney wrote:

> From: David Daney <[email protected]>
>
> commit 18ee49ddb0d2 ("phylib: rename mii_bus::dev to mii_bus::parent")
> changed the parent of PHY devices from the bus to the bus parent.
>
> Then, commit 4dea547fef1b ("phylib: rework to prepare for OF
> registration of PHYs") moved the code into phy_device.c
>
> At this point, it is somewhat unclear why the change was seen as
> necessary. But, when we look at the device model tree in
> /sys/devices, it is clearly incorrect. The PHYs should be children of
> their MDIO bus.
>
> Change the PHY's parent device to be the MDIO bus device.
>
> Cc: Lennert Buytenhek <[email protected]>
> Cc: Grant Likely <[email protected]>
> Signed-off-by: David Daney <[email protected]>
> ---
> drivers/net/phy/phy_device.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 0302483..55f0178 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -176,7 +176,7 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
> if (c45_ids)
> dev->c45_ids = *c45_ids;
> dev->bus = bus;
> - dev->dev.parent = bus->parent;
> + dev->dev.parent = &bus->dev;
> dev->dev.bus = &mdio_bus_type;
> dev->irq = bus->irq != NULL ? bus->irq[addr] : PHY_POLL;
> dev_set_name(&dev->dev, PHY_ID_FMT, bus->id, addr);

This patch makes my sh_eth driver fail to connect to PHY usinjg
of_phy_connect(). (The ravb driver fails too but for some other reason.)

MBR, Sergei

2015-11-19 21:06:56

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH] phylib: Make PHYs children of their MDIO bus, not the bus' parent.

On Thu, Nov 19, 2015 at 11:51:37PM +0300, Sergei Shtylyov wrote:
> Hello.
>
> On 08/22/2015 02:16 AM, David Daney wrote:
>
> >From: David Daney <[email protected]>
> >
> >commit 18ee49ddb0d2 ("phylib: rename mii_bus::dev to mii_bus::parent")
> >changed the parent of PHY devices from the bus to the bus parent.
> >
> >Then, commit 4dea547fef1b ("phylib: rework to prepare for OF
> >registration of PHYs") moved the code into phy_device.c
> >
> >At this point, it is somewhat unclear why the change was seen as
> >necessary. But, when we look at the device model tree in
> >/sys/devices, it is clearly incorrect. The PHYs should be children of
> >their MDIO bus.
> >
> >Change the PHY's parent device to be the MDIO bus device.
> >
> >Cc: Lennert Buytenhek <[email protected]>
> >Cc: Grant Likely <[email protected]>
> >Signed-off-by: David Daney <[email protected]>
> >---
> > drivers/net/phy/phy_device.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> >index 0302483..55f0178 100644
> >--- a/drivers/net/phy/phy_device.c
> >+++ b/drivers/net/phy/phy_device.c
> >@@ -176,7 +176,7 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
> > if (c45_ids)
> > dev->c45_ids = *c45_ids;
> > dev->bus = bus;
> >- dev->dev.parent = bus->parent;
> >+ dev->dev.parent = &bus->dev;
> > dev->dev.bus = &mdio_bus_type;
> > dev->irq = bus->irq != NULL ? bus->irq[addr] : PHY_POLL;
> > dev_set_name(&dev->dev, PHY_ID_FMT, bus->id, addr);
>
> This patch makes my sh_eth driver fail to connect to PHY usinjg
> of_phy_connect(). (The ravb driver fails too but for some other
> reason.)

Hi Sergei

What phy is it?

Do you have phy DT properties in the MAC node?

Andrew

2015-11-19 21:33:18

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH] phylib: Make PHYs children of their MDIO bus, not the bus' parent.

Hello.

On 11/20/2015 12:06 AM, Andrew Lunn wrote:

>>> From: David Daney <[email protected]>
>>>
>>> commit 18ee49ddb0d2 ("phylib: rename mii_bus::dev to mii_bus::parent")
>>> changed the parent of PHY devices from the bus to the bus parent.
>>>
>>> Then, commit 4dea547fef1b ("phylib: rework to prepare for OF
>>> registration of PHYs") moved the code into phy_device.c
>>>
>>> At this point, it is somewhat unclear why the change was seen as
>>> necessary. But, when we look at the device model tree in
>>> /sys/devices, it is clearly incorrect. The PHYs should be children of
>>> their MDIO bus.
>>>
>>> Change the PHY's parent device to be the MDIO bus device.
>>>
>>> Cc: Lennert Buytenhek <[email protected]>
>>> Cc: Grant Likely <[email protected]>
>>> Signed-off-by: David Daney <[email protected]>
>>> ---
>>> drivers/net/phy/phy_device.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>>> index 0302483..55f0178 100644
>>> --- a/drivers/net/phy/phy_device.c
>>> +++ b/drivers/net/phy/phy_device.c
>>> @@ -176,7 +176,7 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
>>> if (c45_ids)
>>> dev->c45_ids = *c45_ids;
>>> dev->bus = bus;
>>> - dev->dev.parent = bus->parent;
>>> + dev->dev.parent = &bus->dev;
>>> dev->dev.bus = &mdio_bus_type;
>>> dev->irq = bus->irq != NULL ? bus->irq[addr] : PHY_POLL;
>>> dev_set_name(&dev->dev, PHY_ID_FMT, bus->id, addr);
>>
>> This patch makes my sh_eth driver fail to connect to PHY usinjg
>> of_phy_connect(). (The ravb driver fails too but for some other
>> reason.)
>
> Hi Sergei
>
> What phy is it?

Micrel KSZ8041RNLI for sh_eth, KSZ9031 for ravb.

> Do you have phy DT properties in the MAC node?

I have PHY subnodes (with props) in the MAC node.

> Andrew

MBR, Sergei

2015-11-19 23:04:13

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH] phylib: Make PHYs children of their MDIO bus, not the bus' parent.

> >What phy is it?
>
> Micrel KSZ8041RNLI for sh_eth, KSZ9031 for ravb.
>
> >Do you have phy DT properties in the MAC node?
>
> I have PHY subnodes (with props) in the MAC node.

O.K, so this could be the same problems as

https://lkml.org/lkml/2015/10/15/726

https://www.mail-archive.com/[email protected]/msg83183.html

We never got a resolution to that problem.

It needs somebody with none working hardware to do some debugging to
figure out exactly what is going on and why my idea to fix it does not
work.

Andrew