diff mbox

phylib: Make PHYs children of their MDIO bus, not the bus' parent.

Message ID 1440198963-20080-1-git-send-email-ddaney.cavm@gmail.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

David Daney Aug. 21, 2015, 11:16 p.m. UTC 
From: David Daney <david.daney@cavium.com>

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 <buytenh@wantstofly.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: David Daney <david.daney@cavium.com>
---
 drivers/net/phy/phy_device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Florian Fainelli Aug. 21, 2015, 11:59 p.m. UTC | #1 
On 21/08/15 16:16, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> 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 <buytenh@wantstofly.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: David Daney <david.daney@cavium.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
David Miller Aug. 25, 2015, 6:31 p.m. UTC | #2 
From: David Daney <ddaney.cavm@gmail.com>
Date: Fri, 21 Aug 2015 16:16:03 -0700

> From: David Daney <david.daney@cavium.com>
> 
> 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 <buytenh@wantstofly.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: David Daney <david.daney@cavium.com>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov Nov. 19, 2015, 8:51 p.m. UTC | #3 
Hello.

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

> From: David Daney <david.daney@cavium.com>
>
> 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 <buytenh@wantstofly.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>   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

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrew Lunn Nov. 19, 2015, 9:06 p.m. UTC | #4 
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 <david.daney@cavium.com>
> >
> >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 <buytenh@wantstofly.org>
> >Cc: Grant Likely <grant.likely@secretlab.ca>
> >Signed-off-by: David Daney <david.daney@cavium.com>
> >---
> >  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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov Nov. 19, 2015, 9:33 p.m. UTC | #5 
Hello.

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

>>> From: David Daney <david.daney@cavium.com>
>>>
>>> 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 <buytenh@wantstofly.org>
>>> Cc: Grant Likely <grant.likely@secretlab.ca>
>>> Signed-off-by: David Daney <david.daney@cavium.com>
>>> ---
>>>   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

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrew Lunn Nov. 19, 2015, 11:04 p.m. UTC | #6 
> >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/netdev@vger.kernel.org/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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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);