[PATCH] fix GENERIC_PHY match nothing

[PATCH] fix GENERIC_PHY match nothing

[张忠山]

According the match logic in function mdio_bus_match
generic phy driver matchs nothing.

If we want it match anything, We must let it's phy_id_mask
be all zeros

Signed-off-by: 张忠山 <zzs213@126.com>
---
 drivers/net/phy/generic.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/phy/generic.c b/drivers/net/phy/generic.c
index 3f5f127..c3743b9 100644
--- a/drivers/net/phy/generic.c
+++ b/drivers/net/phy/generic.c
@@ -25,7 +25,7 @@
 static struct phy_driver generic_phy = {
 	.drv.name = "Generic PHY",
 	.phy_id = PHY_ANY_UID,
-	.phy_id_mask = PHY_ANY_UID,
+	.phy_id_mask = 0,
 	.features = 0,
 };
 
-- 
1.7.4.4



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] fix GENERIC_PHY match nothing

[Sascha Hauer]

On Sat, Dec 08, 2012 at 09:58:52AM +0800, 张忠山 wrote:
> According the match logic in function mdio_bus_match
> generic phy driver matchs nothing.

This is done on purpose. The generic phy driver is probed manually
when no other phy matches, see:

> static int phy_register_device(struct phy_device* dev)
> {
>        int ret;
> 
>        ret = register_device(&dev->dev);
>        if (ret)
>                return ret;

register device...

> 
>        if (dev->dev.driver)
>                return 0;

... we have a driver specific to this phy, -> return

> 
>        dev->dev.driver = &genphy_driver.drv;
>        return device_probe(&dev->dev);

No driver, use generic phy driver.

>}

The problem this solves (and that you reintroduce with this patch) is
that if the phy driver matches every device, a more specific phy driver
will not be used if the generic driver is registered first.

Sascha

> 
> If we want it match anything, We must let it's phy_id_mask
> be all zeros
> 
> Signed-off-by: 张忠山 <zzs213@126.com>
> ---
>  drivers/net/phy/generic.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/phy/generic.c b/drivers/net/phy/generic.c
> index 3f5f127..c3743b9 100644
> --- a/drivers/net/phy/generic.c
> +++ b/drivers/net/phy/generic.c
> @@ -25,7 +25,7 @@
>  static struct phy_driver generic_phy = {
>  	.drv.name = "Generic PHY",
>  	.phy_id = PHY_ANY_UID,
> -	.phy_id_mask = PHY_ANY_UID,
> +	.phy_id_mask = 0,
>  	.features = 0,
>  };
>  
> -- 
> 1.7.4.4
> 
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] fix GENERIC_PHY match nothing

[Sascha Hauer]

On Sat, Dec 08, 2012 at 12:15:03PM +0100, Sascha Hauer wrote:
> On Sat, Dec 08, 2012 at 09:58:52AM +0800, 张忠山 wrote:
> > According the match logic in function mdio_bus_match
> > generic phy driver matchs nothing.
> 
> This is done on purpose. The generic phy driver is probed manually
> when no other phy matches, see:
> 
> > static int phy_register_device(struct phy_device* dev)
> > {
> >        int ret;
> > 
> >        ret = register_device(&dev->dev);
> >        if (ret)
> >                return ret;
> 
> register device...
> 
> > 
> >        if (dev->dev.driver)
> >                return 0;
> 
> ... we have a driver specific to this phy, -> return
> 
> > 
> >        dev->dev.driver = &genphy_driver.drv;
> >        return device_probe(&dev->dev);
> 
> No driver, use generic phy driver.
> 
> >}
> 
> The problem this solves (and that you reintroduce with this patch) is
> that if the phy driver matches every device, a more specific phy driver
> will not be used if the generic driver is registered first.

Now I understand what you mean.

The problem was introduced by a fix that went in right before the
release:

> commit 90806eea09672dd60c3a57e7d077ece613ff1cf3
> Author: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Date:   Sun Nov 18 13:49:41 2012 +0100
> 
>     mdio_bus: fix match
>     
>     on barebox we have the weird way the return 0 true on bus match
>     
>     Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>     Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> 
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index 35319b4..762adfe 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -106,7 +106,7 @@ static int mdio_bus_match(struct device_d *dev, struct driver_d *drv)
>         struct phy_device *phydev = to_phy_device(dev);
>         struct phy_driver *phydrv = to_phy_driver(drv);
>  
> -       return ((phydrv->phy_id & phydrv->phy_id_mask) ==
> +       return ((phydrv->phy_id & phydrv->phy_id_mask) !=
>                 (phydev->phy_id & phydrv->phy_id_mask));
>  }
> 

The patch is correct, but without the mechanism I described in my last
mail (which is in current master now, but not in the december release)
it has the effect that the generic phy will never be used in the last
release.

This renders the last release useless for every board using the generic
phy and makes a v2012.12.1 Release necessary.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] fix GENERIC_PHY match nothing

[张忠山]

In message <20121208123830.GM24458@pengutronix.de> Sascha Hauer wrote:
> On Sat, Dec 08, 2012 at 12:15:03PM +0100, Sascha Hauer wrote:
> > On Sat, Dec 08, 2012 at 09:58:52AM +0800, 张忠山 wrote:
> > > According the match logic in function mdio_bus_match
> > > generic phy driver matchs nothing.
> >
> > This is done on purpose. The generic phy driver is probed manually
> > when no other phy matches, see:
> >
> > > static int phy_register_device(struct phy_device* dev)
> > > {
> > >        int ret;
> > >
> > >        ret = register_device(&dev->dev);
> > >        if (ret)
> > >                return ret;
> >
> > register device...
> >
> > >
> > >        if (dev->dev.driver)
> > >                return 0;
> >
> > ... we have a driver specific to this phy, -> return
> >
> > >
> > >        dev->dev.driver = &genphy_driver.drv;
> > >        return device_probe(&dev->dev);
> >
> > No driver, use generic phy driver.
> >
> > >}
> >
> > The problem this solves (and that you reintroduce with this patch) is
> > that if the phy driver matches every device, a more specific phy driver
> > will not be used if the generic driver is registered first.
>
> Now I understand what you mean.
>

Sorry for not mention the source code version

-- 
Best Regards,
zzs



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox