Message ID | 1315243448-29959-2-git-send-email-vz@mleia.com |
---|---|
State | Accepted |
Commit | e3a77218a256edbe201112a39beeed8adcabae3f |
Headers | show |
Hi Vladimir, > This change allows to cope with a mii bus device registered using > miiphy_register(), which doesn't assign a default reset handler. Looks good, so Acked-by: Detlev Zundel <dzu@denx.de> > Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> Cheers Detlev
Dear Vladimir Zapolskiy, In message <1315243448-29959-2-git-send-email-vz@mleia.com> you wrote: > This change allows to cope with a mii bus device registered using > miiphy_register(), which doesn't assign a default reset handler. > > Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> > --- > drivers/net/phy/phy.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) Applied, thanks. Best regards, Wolfgang Denk
On Mon, Sep 5, 2011 at 12:24 PM, Vladimir Zapolskiy <vz@mleia.com> wrote: > This change allows to cope with a mii bus device registered using > miiphy_register(), which doesn't assign a default reset handler. > > Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> > --- > drivers/net/phy/phy.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c > index ce69c19..8da7688 100644 > --- a/drivers/net/phy/phy.c > +++ b/drivers/net/phy/phy.c > @@ -692,7 +692,8 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr, > struct phy_device *phydev; > > /* Reset the bus */ > - bus->reset(bus); > + if (bus->reset) > + bus->reset(bus); The change is a good idea, but I find the motivation for it strange. If you register a bus with miiphy_register, you are declaring your intent to use the legacy PHY interface. But phy_connect() is part of the new phylib API. It was not intended that combining the two work at all. Looking at the code, I see no reason it wouldn't work, but I question why you would do that, instead of creating a proper MDIO driver? Andy
On Thursday, September 22, 2011 17:44:11 Andy Fleming wrote: > On Mon, Sep 5, 2011 at 12:24 PM, Vladimir Zapolskiy <vz@mleia.com> wrote: > > This change allows to cope with a mii bus device registered using > > miiphy_register(), which doesn't assign a default reset handler. > > > > --- a/drivers/net/phy/phy.c > > +++ b/drivers/net/phy/phy.c > > @@ -692,7 +692,8 @@ struct phy_device *phy_connect(struct mii_dev *bus, > > int addr, struct phy_device *phydev; > > > > /* Reset the bus */ > > - bus->reset(bus); > > + if (bus->reset) > > + bus->reset(bus); > > The change is a good idea, but I find the motivation for it strange. > If you register a bus with miiphy_register, you are declaring your > intent to use the legacy PHY interface. But phy_connect() is part of > the new phylib API. It was not intended that combining the two work at > all. Looking at the code, I see no reason it wouldn't work, but I > question why you would do that, instead of creating a proper MDIO > driver? hmm, is there an #ifdef check we could add that would cause an #error if people mix the old and the new ? i think it makes sense to just force everyone to migrate to the new ... -mike
On Thu, Sep 22, 2011 at 5:13 PM, Mike Frysinger <vapier@gentoo.org> wrote: > On Thursday, September 22, 2011 17:44:11 Andy Fleming wrote: >> On Mon, Sep 5, 2011 at 12:24 PM, Vladimir Zapolskiy <vz@mleia.com> wrote: >> > /* Reset the bus */ >> > - bus->reset(bus); >> > + if (bus->reset) >> > + bus->reset(bus); >> >> The change is a good idea, but I find the motivation for it strange. >> If you register a bus with miiphy_register, you are declaring your >> intent to use the legacy PHY interface. But phy_connect() is part of >> the new phylib API. It was not intended that combining the two work at >> all. Looking at the code, I see no reason it wouldn't work, but I >> question why you would do that, instead of creating a proper MDIO >> driver? > > hmm, is there an #ifdef check we could add that would cause an #error if > people mix the old and the new ? i think it makes sense to just force > everyone to migrate to the new ... I suppose we could add a check in phy_connect to see whether the read function is actually legacy_miiphy_read. If you don't call phy_connect(), then you don't get a handle to the PHY device, and aren't using phylib. I don't think we're ready to disallow using the two concurrently in separate drivers. Andy
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index ce69c19..8da7688 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -692,7 +692,8 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr, struct phy_device *phydev; /* Reset the bus */ - bus->reset(bus); + if (bus->reset) + bus->reset(bus); /* Wait 15ms to make sure the PHY has come out of hard reset */ udelay(15000);
This change allows to cope with a mii bus device registered using miiphy_register(), which doesn't assign a default reset handler. Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> --- drivers/net/phy/phy.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)