* [U-Boot] [PATCH 0/2] phylib: a couple of minor fixes in generic phy library @ 2011-09-05 17:24 Vladimir Zapolskiy 2011-09-05 17:24 ` [U-Boot] [PATCH 1/2] phylib: reset mii bus only if reset handler is registered Vladimir Zapolskiy 2011-09-05 17:24 ` [U-Boot] [PATCH 2/2] phylib: remove a couple of redundant code lines Vladimir Zapolskiy 0 siblings, 2 replies; 14+ messages in thread From: Vladimir Zapolskiy @ 2011-09-05 17:24 UTC (permalink / raw) To: u-boot Hello, recently I decided to port one (not integrated to U-boot yet) MAC to use phylib, and found that it serves quite good. I managed to find only one bug related to miiphy_register() and phylib cooperative usage, the small fix is provided. Additionally let me send a tiny readability improvement change. Vladimir Zapolskiy (2): phylib: reset mii bus only if reset handler is registered phylib: remove a couple of redundant code lines drivers/net/phy/phy.c | 10 ++++------ 1 files changed, 4 insertions(+), 6 deletions(-) -- 1.7.5.4 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH 1/2] phylib: reset mii bus only if reset handler is registered 2011-09-05 17:24 [U-Boot] [PATCH 0/2] phylib: a couple of minor fixes in generic phy library Vladimir Zapolskiy @ 2011-09-05 17:24 ` Vladimir Zapolskiy 2011-09-09 13:27 ` Detlev Zundel ` (2 more replies) 2011-09-05 17:24 ` [U-Boot] [PATCH 2/2] phylib: remove a couple of redundant code lines Vladimir Zapolskiy 1 sibling, 3 replies; 14+ messages in thread From: Vladimir Zapolskiy @ 2011-09-05 17:24 UTC (permalink / raw) To: u-boot 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); /* Wait 15ms to make sure the PHY has come out of hard reset */ udelay(15000); -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH 1/2] phylib: reset mii bus only if reset handler is registered 2011-09-05 17:24 ` [U-Boot] [PATCH 1/2] phylib: reset mii bus only if reset handler is registered Vladimir Zapolskiy @ 2011-09-09 13:27 ` Detlev Zundel 2011-09-09 22:08 ` Wolfgang Denk 2011-09-22 21:44 ` Andy Fleming 2 siblings, 0 replies; 14+ messages in thread From: Detlev Zundel @ 2011-09-09 13:27 UTC (permalink / raw) To: u-boot 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 -- But in terms of creative information, information that people can use or enjoy, and that will be used and enjoyed more the more people who have it, always we should encourage the copying. -- Richard M. Stallman -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de ^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH 1/2] phylib: reset mii bus only if reset handler is registered 2011-09-05 17:24 ` [U-Boot] [PATCH 1/2] phylib: reset mii bus only if reset handler is registered Vladimir Zapolskiy 2011-09-09 13:27 ` Detlev Zundel @ 2011-09-09 22:08 ` Wolfgang Denk 2011-09-22 21:44 ` Andy Fleming 2 siblings, 0 replies; 14+ messages in thread From: Wolfgang Denk @ 2011-09-09 22:08 UTC (permalink / raw) To: u-boot 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 -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de If a train station is a place where a train stops, then what's a workstation? ^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH 1/2] phylib: reset mii bus only if reset handler is registered 2011-09-05 17:24 ` [U-Boot] [PATCH 1/2] phylib: reset mii bus only if reset handler is registered Vladimir Zapolskiy 2011-09-09 13:27 ` Detlev Zundel 2011-09-09 22:08 ` Wolfgang Denk @ 2011-09-22 21:44 ` Andy Fleming 2011-09-22 22:13 ` Mike Frysinger 2 siblings, 1 reply; 14+ messages in thread From: Andy Fleming @ 2011-09-22 21:44 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH 1/2] phylib: reset mii bus only if reset handler is registered 2011-09-22 21:44 ` Andy Fleming @ 2011-09-22 22:13 ` Mike Frysinger 2011-09-22 22:19 ` Andy Fleming 0 siblings, 1 reply; 14+ messages in thread From: Mike Frysinger @ 2011-09-22 22:13 UTC (permalink / raw) To: u-boot 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 -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. Url : http://lists.denx.de/pipermail/u-boot/attachments/20110922/7f609d17/attachment.pgp ^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH 1/2] phylib: reset mii bus only if reset handler is registered 2011-09-22 22:13 ` Mike Frysinger @ 2011-09-22 22:19 ` Andy Fleming 0 siblings, 0 replies; 14+ messages in thread From: Andy Fleming @ 2011-09-22 22:19 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH 2/2] phylib: remove a couple of redundant code lines 2011-09-05 17:24 [U-Boot] [PATCH 0/2] phylib: a couple of minor fixes in generic phy library Vladimir Zapolskiy 2011-09-05 17:24 ` [U-Boot] [PATCH 1/2] phylib: reset mii bus only if reset handler is registered Vladimir Zapolskiy @ 2011-09-05 17:24 ` Vladimir Zapolskiy 2011-09-09 12:18 ` Detlev Zundel ` (3 more replies) 1 sibling, 4 replies; 14+ messages in thread From: Vladimir Zapolskiy @ 2011-09-05 17:24 UTC (permalink / raw) To: u-boot This change slightly improves readability of the phydev speed/duplex assignment logic. Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> --- drivers/net/phy/phy.c | 7 ++----- 1 files changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 8da7688..833a051 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -318,13 +318,10 @@ static int genphy_parse_link(struct phy_device *phydev) lpa = phy_read(phydev, MDIO_DEVAD_NONE, MII_ADVERTISE); lpa &= phy_read(phydev, MDIO_DEVAD_NONE, MII_LPA); - if (lpa & (LPA_100FULL | LPA_100HALF)) { + if (lpa & (LPA_100FULL | LPA_100HALF)) phydev->speed = SPEED_100; - if (lpa & LPA_100FULL) - phydev->duplex = DUPLEX_FULL; - - } else if (lpa & LPA_10FULL) + if (lpa & (LPA_100FULL | LPA_10FULL)) phydev->duplex = DUPLEX_FULL; } else { u32 bmcr = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMCR); -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH 2/2] phylib: remove a couple of redundant code lines 2011-09-05 17:24 ` [U-Boot] [PATCH 2/2] phylib: remove a couple of redundant code lines Vladimir Zapolskiy @ 2011-09-09 12:18 ` Detlev Zundel 2011-09-09 22:08 ` Wolfgang Denk ` (2 subsequent siblings) 3 siblings, 0 replies; 14+ messages in thread From: Detlev Zundel @ 2011-09-09 12:18 UTC (permalink / raw) To: u-boot Hi Vladimir, > This change slightly improves readability of the phydev speed/duplex > assignment logic. Agreed. > Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> Acked-by: Detlev Zundel <dzu@denx.de> Cheers Detlev -- Every time history repeats itself the price goes up. -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de ^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH 2/2] phylib: remove a couple of redundant code lines 2011-09-05 17:24 ` [U-Boot] [PATCH 2/2] phylib: remove a couple of redundant code lines Vladimir Zapolskiy 2011-09-09 12:18 ` Detlev Zundel @ 2011-09-09 22:08 ` Wolfgang Denk 2011-09-22 21:25 ` Andy Fleming 2011-09-28 19:05 ` Wolfgang Denk 3 siblings, 0 replies; 14+ messages in thread From: Wolfgang Denk @ 2011-09-09 22:08 UTC (permalink / raw) To: u-boot Dear Vladimir Zapolskiy, In message <1315243448-29959-3-git-send-email-vz@mleia.com> you wrote: > This change slightly improves readability of the phydev speed/duplex > assignment logic. > > Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> > --- > drivers/net/phy/phy.c | 7 ++----- > 1 files changed, 2 insertions(+), 5 deletions(-) Applied, thanks. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Philosophy is a game with objectives and no rules. Mathematics is a game with rules and no objectives. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH 2/2] phylib: remove a couple of redundant code lines 2011-09-05 17:24 ` [U-Boot] [PATCH 2/2] phylib: remove a couple of redundant code lines Vladimir Zapolskiy 2011-09-09 12:18 ` Detlev Zundel 2011-09-09 22:08 ` Wolfgang Denk @ 2011-09-22 21:25 ` Andy Fleming 2011-09-23 6:06 ` Wolfgang Denk 2011-09-28 19:05 ` Wolfgang Denk 3 siblings, 1 reply; 14+ messages in thread From: Andy Fleming @ 2011-09-22 21:25 UTC (permalink / raw) To: u-boot On Mon, Sep 5, 2011 at 12:24 PM, Vladimir Zapolskiy <vz@mleia.com> wrote: > This change slightly improves readability of the phydev speed/duplex > assignment logic. Shoot, I just saw this patch in my tree. It's incorrect. > @@ -318,13 +318,10 @@ static int genphy_parse_link(struct phy_device *phydev) > ? ? ? ? ? ? ? ?lpa = phy_read(phydev, MDIO_DEVAD_NONE, MII_ADVERTISE); > ? ? ? ? ? ? ? ?lpa &= phy_read(phydev, MDIO_DEVAD_NONE, MII_LPA); > > - ? ? ? ? ? ? ? if (lpa & (LPA_100FULL | LPA_100HALF)) { > + ? ? ? ? ? ? ? if (lpa & (LPA_100FULL | LPA_100HALF)) > ? ? ? ? ? ? ? ? ? ? ? ?phydev->speed = SPEED_100; > > - ? ? ? ? ? ? ? ? ? ? ? if (lpa & LPA_100FULL) > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? phydev->duplex = DUPLEX_FULL; > - > - ? ? ? ? ? ? ? } else if (lpa & LPA_10FULL) > + ? ? ? ? ? ? ? if (lpa & (LPA_100FULL | LPA_10FULL)) > ? ? ? ? ? ? ? ? ? ? ? ?phydev->duplex = DUPLEX_FULL; The lines weren't redundant. The logic is (and probably should be better commented): Find the intersection of the advertised capabilities of both sides of the link (lpa) ^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH 2/2] phylib: remove a couple of redundant code lines 2011-09-22 21:25 ` Andy Fleming @ 2011-09-23 6:06 ` Wolfgang Denk 2011-09-26 19:24 ` Vladimir Zapolskiy 0 siblings, 1 reply; 14+ messages in thread From: Wolfgang Denk @ 2011-09-23 6:06 UTC (permalink / raw) To: u-boot Dear Andy Fleming, In message <CAKWjMd5HGT9df76vPFs8B5sFQYWoAN1bGmt2vRihN0cTa1boug@mail.gmail.com> you wrote: > > Shoot, I just saw this patch in my tree. It's incorrect. Argh... > The lines weren't redundant. The logic is (and probably should be > better commented): > > Find the intersection of the advertised capabilities of both sides of > the link (lpa) > From that intersection, find the highest capability we can run at > (that will be the negotiated link) > > Now imagine that the intersection (lpa) is (LPA_100HALF | LPA_10FULL). > > The code will now set phydev->speed to 100, and phydev->duplex to 1, > but this link does not support 100FULL. Do we agree that I should revert this commit? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Felson's Law: To steal ideas from one person is plagiarism; to steal from many is research. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH 2/2] phylib: remove a couple of redundant code lines 2011-09-23 6:06 ` Wolfgang Denk @ 2011-09-26 19:24 ` Vladimir Zapolskiy 0 siblings, 0 replies; 14+ messages in thread From: Vladimir Zapolskiy @ 2011-09-26 19:24 UTC (permalink / raw) To: u-boot Hello Wolfgang, On 23.09.2011 09:06, Wolfgang Denk wrote: > Dear Andy Fleming, > > In message<CAKWjMd5HGT9df76vPFs8B5sFQYWoAN1bGmt2vRihN0cTa1boug@mail.gmail.com> you wrote: >> >> Shoot, I just saw this patch in my tree. It's incorrect. > > Argh... > >> The lines weren't redundant. The logic is (and probably should be >> better commented): >> >> Find the intersection of the advertised capabilities of both sides of >> the link (lpa) >> From that intersection, find the highest capability we can run at >> (that will be the negotiated link) >> >> Now imagine that the intersection (lpa) is (LPA_100HALF | LPA_10FULL). >> >> The code will now set phydev->speed to 100, and phydev->duplex to 1, >> but this link does not support 100FULL. > > Do we agree that I should revert this commit? > due to Andy's explanation I agree that this change is fishy and it should be reverted. As a U-Boot user I'd greatly appreciate, if you add the above note to the commit message. Sorry for inconvenience. -- With best wishes, Vladimir ^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH 2/2] phylib: remove a couple of redundant code lines 2011-09-05 17:24 ` [U-Boot] [PATCH 2/2] phylib: remove a couple of redundant code lines Vladimir Zapolskiy ` (2 preceding siblings ...) 2011-09-22 21:25 ` Andy Fleming @ 2011-09-28 19:05 ` Wolfgang Denk 3 siblings, 0 replies; 14+ messages in thread From: Wolfgang Denk @ 2011-09-28 19:05 UTC (permalink / raw) To: u-boot Dear Vladimir Zapolskiy, In message <1315243448-29959-3-git-send-email-vz@mleia.com> you wrote: > This change slightly improves readability of the phydev speed/duplex > assignment logic. > > Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> > --- > drivers/net/phy/phy.c | 7 ++----- > 1 files changed, 2 insertions(+), 5 deletions(-) Patch reverted. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "It is easier to port a shell than a shell script." - Larry Wall ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2011-09-28 19:05 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-09-05 17:24 [U-Boot] [PATCH 0/2] phylib: a couple of minor fixes in generic phy library Vladimir Zapolskiy 2011-09-05 17:24 ` [U-Boot] [PATCH 1/2] phylib: reset mii bus only if reset handler is registered Vladimir Zapolskiy 2011-09-09 13:27 ` Detlev Zundel 2011-09-09 22:08 ` Wolfgang Denk 2011-09-22 21:44 ` Andy Fleming 2011-09-22 22:13 ` Mike Frysinger 2011-09-22 22:19 ` Andy Fleming 2011-09-05 17:24 ` [U-Boot] [PATCH 2/2] phylib: remove a couple of redundant code lines Vladimir Zapolskiy 2011-09-09 12:18 ` Detlev Zundel 2011-09-09 22:08 ` Wolfgang Denk 2011-09-22 21:25 ` Andy Fleming 2011-09-23 6:06 ` Wolfgang Denk 2011-09-26 19:24 ` Vladimir Zapolskiy 2011-09-28 19:05 ` Wolfgang Denk
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.