* [U-Boot] [PATCH] net: macb: sama5d4 is not gigabit capable @ 2015-12-14 16:37 Gregory CLEMENT 2015-12-15 19:59 ` Joe Hershberger 0 siblings, 1 reply; 8+ messages in thread From: Gregory CLEMENT @ 2015-12-14 16:37 UTC (permalink / raw) To: u-boot During the initialization of PHY the gigabit bit capable is set if the controller is a GEM. However, for sama5d4, the GEM is not gigabit capable. Improperly setting the GBE capability leads to an unresponsive MAC controller. This patch fix this behavior allowing to use the gmac with the sama5d4. Suggested-by: Nicolas Ferre <nicolas.ferre@atmel.com> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> --- drivers/net/macb.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/net/macb.c b/drivers/net/macb.c index a5c1880..642717d 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -480,8 +480,11 @@ static int macb_phy_init(struct macb_device *macb) return 0; } - /* First check for GMAC */ - if (macb_is_gem(macb)) { + /* + * First check for GMAC, but not the one on SAMA5D4 which is + * not gigabit capabale + */ + if (macb_is_gem(macb) && ! cpu_is_sama5d4()) { lpa = macb_mdio_read(macb, MII_STAT1000); if (lpa & (LPA_1000FULL | LPA_1000HALF)) { -- 2.5.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] net: macb: sama5d4 is not gigabit capable 2015-12-14 16:37 [U-Boot] [PATCH] net: macb: sama5d4 is not gigabit capable Gregory CLEMENT @ 2015-12-15 19:59 ` Joe Hershberger 2015-12-16 9:19 ` Nicolas Ferre 2015-12-16 9:32 ` Gregory CLEMENT 0 siblings, 2 replies; 8+ messages in thread From: Joe Hershberger @ 2015-12-15 19:59 UTC (permalink / raw) To: u-boot Hi Gregory, On Mon, Dec 14, 2015 at 10:37 AM, Gregory CLEMENT <gregory.clement@free-electrons.com> wrote: > During the initialization of PHY the gigabit bit capable is set if the > controller is a GEM. However, for sama5d4, the GEM is not gigabit > capable. Improperly setting the GBE capability leads to an unresponsive > MAC controller. This patch fix this behavior allowing to use the gmac > with the sama5d4. > > Suggested-by: Nicolas Ferre <nicolas.ferre@atmel.com> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > --- > drivers/net/macb.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/macb.c b/drivers/net/macb.c > index a5c1880..642717d 100644 > --- a/drivers/net/macb.c > +++ b/drivers/net/macb.c > @@ -480,8 +480,11 @@ static int macb_phy_init(struct macb_device *macb) > return 0; > } > > - /* First check for GMAC */ > - if (macb_is_gem(macb)) { > + /* > + * First check for GMAC, but not the one on SAMA5D4 which is > + * not gigabit capabale > + */ > + if (macb_is_gem(macb) && ! cpu_is_sama5d4()) { Is there not some other property that can identify the lack of Gigabit support in the "GEM"? It would be better to not have to keep track of the next processor and the one after that which has the same situation. > lpa = macb_mdio_read(macb, MII_STAT1000); > > if (lpa & (LPA_1000FULL | LPA_1000HALF)) { > -- > 2.5.0 > > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] net: macb: sama5d4 is not gigabit capable 2015-12-15 19:59 ` Joe Hershberger @ 2015-12-16 9:19 ` Nicolas Ferre 2015-12-16 9:35 ` Andreas Bießmann 2015-12-16 9:32 ` Gregory CLEMENT 1 sibling, 1 reply; 8+ messages in thread From: Nicolas Ferre @ 2015-12-16 9:19 UTC (permalink / raw) To: u-boot Le 15/12/2015 20:59, Joe Hershberger a ?crit : > Hi Gregory, > > On Mon, Dec 14, 2015 at 10:37 AM, Gregory CLEMENT > <gregory.clement@free-electrons.com> wrote: >> During the initialization of PHY the gigabit bit capable is set if the >> controller is a GEM. However, for sama5d4, the GEM is not gigabit >> capable. Improperly setting the GBE capability leads to an unresponsive >> MAC controller. This patch fix this behavior allowing to use the gmac >> with the sama5d4. >> >> Suggested-by: Nicolas Ferre <nicolas.ferre@atmel.com> >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> >> --- >> drivers/net/macb.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/macb.c b/drivers/net/macb.c >> index a5c1880..642717d 100644 >> --- a/drivers/net/macb.c >> +++ b/drivers/net/macb.c >> @@ -480,8 +480,11 @@ static int macb_phy_init(struct macb_device *macb) >> return 0; >> } >> >> - /* First check for GMAC */ >> - if (macb_is_gem(macb)) { >> + /* >> + * First check for GMAC, but not the one on SAMA5D4 which is >> + * not gigabit capabale >> + */ >> + if (macb_is_gem(macb) && ! cpu_is_sama5d4()) { > > Is there not some other property that can identify the lack of Gigabit > support in the "GEM"? It would be better to not have to keep track of > the next processor and the one after that which has the same > situation. Actually, sama5d2 is in the same situation already... Bye, > >> lpa = macb_mdio_read(macb, MII_STAT1000); >> >> if (lpa & (LPA_1000FULL | LPA_1000HALF)) { >> -- >> 2.5.0 >> >> _______________________________________________ >> U-Boot mailing list >> U-Boot at lists.denx.de >> http://lists.denx.de/mailman/listinfo/u-boot > -- Nicolas Ferre ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] net: macb: sama5d4 is not gigabit capable 2015-12-16 9:19 ` Nicolas Ferre @ 2015-12-16 9:35 ` Andreas Bießmann 2015-12-16 9:41 ` Gregory CLEMENT 0 siblings, 1 reply; 8+ messages in thread From: Andreas Bießmann @ 2015-12-16 9:35 UTC (permalink / raw) To: u-boot Hi all, On 16.12.2015 10:19, Nicolas Ferre wrote: > Le 15/12/2015 20:59, Joe Hershberger a ?crit : >> Hi Gregory, >> >> On Mon, Dec 14, 2015 at 10:37 AM, Gregory CLEMENT >> <gregory.clement@free-electrons.com> wrote: >>> During the initialization of PHY the gigabit bit capable is set if the >>> controller is a GEM. However, for sama5d4, the GEM is not gigabit >>> capable. Improperly setting the GBE capability leads to an unresponsive >>> MAC controller. This patch fix this behavior allowing to use the gmac >>> with the sama5d4. >>> >>> Suggested-by: Nicolas Ferre <nicolas.ferre@atmel.com> >>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> >>> --- >>> drivers/net/macb.c | 7 +++++-- >>> 1 file changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/net/macb.c b/drivers/net/macb.c >>> index a5c1880..642717d 100644 >>> --- a/drivers/net/macb.c >>> +++ b/drivers/net/macb.c >>> @@ -480,8 +480,11 @@ static int macb_phy_init(struct macb_device *macb) >>> return 0; >>> } >>> >>> - /* First check for GMAC */ >>> - if (macb_is_gem(macb)) { >>> + /* >>> + * First check for GMAC, but not the one on SAMA5D4 which is >>> + * not gigabit capabale >>> + */ >>> + if (macb_is_gem(macb) && ! cpu_is_sama5d4()) { >> >> Is there not some other property that can identify the lack of Gigabit >> support in the "GEM"? It would be better to not have to keep track of >> the next processor and the one after that which has the same >> situation. > > Actually, sama5d2 is in the same situation already... would it be a compromise to patch the macb_is_gem() in favor of the different usages of this function? On the other hand I also wonder why the MACB IP tells it is GiB capable by the IDNUM bitfield. Isn't there a possibility to fix this detection? Andreas > > Bye, > >> >>> lpa = macb_mdio_read(macb, MII_STAT1000); >>> >>> if (lpa & (LPA_1000FULL | LPA_1000HALF)) { >>> -- >>> 2.5.0 >>> >>> _______________________________________________ >>> U-Boot mailing list >>> U-Boot at lists.denx.de >>> http://lists.denx.de/mailman/listinfo/u-boot >> > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] net: macb: sama5d4 is not gigabit capable 2015-12-16 9:35 ` Andreas Bießmann @ 2015-12-16 9:41 ` Gregory CLEMENT 2015-12-16 9:52 ` Nicolas Ferre 0 siblings, 1 reply; 8+ messages in thread From: Gregory CLEMENT @ 2015-12-16 9:41 UTC (permalink / raw) To: u-boot Hi Andreas, On mer., d?c. 16 2015, "Andreas Bie?mann" <andreas.devel@googlemail.com> wrote: > Hi all, > > On 16.12.2015 10:19, Nicolas Ferre wrote: >> Le 15/12/2015 20:59, Joe Hershberger a ?crit : >>> Hi Gregory, >>> >>> On Mon, Dec 14, 2015 at 10:37 AM, Gregory CLEMENT >>> <gregory.clement@free-electrons.com> wrote: >>>> During the initialization of PHY the gigabit bit capable is set if the >>>> controller is a GEM. However, for sama5d4, the GEM is not gigabit >>>> capable. Improperly setting the GBE capability leads to an unresponsive >>>> MAC controller. This patch fix this behavior allowing to use the gmac >>>> with the sama5d4. >>>> >>>> Suggested-by: Nicolas Ferre <nicolas.ferre@atmel.com> >>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> >>>> --- >>>> drivers/net/macb.c | 7 +++++-- >>>> 1 file changed, 5 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/net/macb.c b/drivers/net/macb.c >>>> index a5c1880..642717d 100644 >>>> --- a/drivers/net/macb.c >>>> +++ b/drivers/net/macb.c >>>> @@ -480,8 +480,11 @@ static int macb_phy_init(struct macb_device *macb) >>>> return 0; >>>> } >>>> >>>> - /* First check for GMAC */ >>>> - if (macb_is_gem(macb)) { >>>> + /* >>>> + * First check for GMAC, but not the one on SAMA5D4 which is >>>> + * not gigabit capabale >>>> + */ >>>> + if (macb_is_gem(macb) && ! cpu_is_sama5d4()) { >>> >>> Is there not some other property that can identify the lack of Gigabit >>> support in the "GEM"? It would be better to not have to keep track of >>> the next processor and the one after that which has the same >>> situation. >> >> Actually, sama5d2 is in the same situation already... > > would it be a compromise to patch the macb_is_gem() in favor of the > different usages of this function? I was thinking of introducing a is_gb_capable() function. > > On the other hand I also wonder why the MACB IP tells it is GiB capable > by the IDNUM bitfield. Isn't there a possibility to fix this > detection? the first part of the IDNUM identify the generation of this IP and 0x1 is MACB whereas 0x2 is GEM. As I wrote in my previous email, I _think_ that the GEM is really GiB capabale but the SoC around do not provde the needed ressource for this (such as line running at GHz). Gregory > > Andreas > >> >> Bye, >> >>> >>>> lpa = macb_mdio_read(macb, MII_STAT1000); >>>> >>>> if (lpa & (LPA_1000FULL | LPA_1000HALF)) { >>>> -- >>>> 2.5.0 >>>> >>>> _______________________________________________ >>>> U-Boot mailing list >>>> U-Boot at lists.denx.de >>>> http://lists.denx.de/mailman/listinfo/u-boot >>> >> >> > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] net: macb: sama5d4 is not gigabit capable 2015-12-16 9:41 ` Gregory CLEMENT @ 2015-12-16 9:52 ` Nicolas Ferre 0 siblings, 0 replies; 8+ messages in thread From: Nicolas Ferre @ 2015-12-16 9:52 UTC (permalink / raw) To: u-boot Le 16/12/2015 10:41, Gregory CLEMENT a ?crit : > Hi Andreas, > > On mer., d?c. 16 2015, "Andreas Bie?mann" <andreas.devel@googlemail.com> wrote: > >> Hi all, >> >> On 16.12.2015 10:19, Nicolas Ferre wrote: >>> Le 15/12/2015 20:59, Joe Hershberger a ?crit : >>>> Hi Gregory, >>>> >>>> On Mon, Dec 14, 2015 at 10:37 AM, Gregory CLEMENT >>>> <gregory.clement@free-electrons.com> wrote: >>>>> During the initialization of PHY the gigabit bit capable is set if the >>>>> controller is a GEM. However, for sama5d4, the GEM is not gigabit >>>>> capable. Improperly setting the GBE capability leads to an unresponsive >>>>> MAC controller. This patch fix this behavior allowing to use the gmac >>>>> with the sama5d4. >>>>> >>>>> Suggested-by: Nicolas Ferre <nicolas.ferre@atmel.com> >>>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> >>>>> --- >>>>> drivers/net/macb.c | 7 +++++-- >>>>> 1 file changed, 5 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/net/macb.c b/drivers/net/macb.c >>>>> index a5c1880..642717d 100644 >>>>> --- a/drivers/net/macb.c >>>>> +++ b/drivers/net/macb.c >>>>> @@ -480,8 +480,11 @@ static int macb_phy_init(struct macb_device *macb) >>>>> return 0; >>>>> } >>>>> >>>>> - /* First check for GMAC */ >>>>> - if (macb_is_gem(macb)) { >>>>> + /* >>>>> + * First check for GMAC, but not the one on SAMA5D4 which is >>>>> + * not gigabit capabale >>>>> + */ >>>>> + if (macb_is_gem(macb) && ! cpu_is_sama5d4()) { >>>> >>>> Is there not some other property that can identify the lack of Gigabit >>>> support in the "GEM"? It would be better to not have to keep track of >>>> the next processor and the one after that which has the same >>>> situation. >>> >>> Actually, sama5d2 is in the same situation already... >> >> would it be a compromise to patch the macb_is_gem() in favor of the >> different usages of this function? > > I was thinking of introducing a is_gb_capable() function. > >> >> On the other hand I also wonder why the MACB IP tells it is GiB capable >> by the IDNUM bitfield. Isn't there a possibility to fix this >> detection? > > the first part of the IDNUM identify the generation of this IP and 0x1 > is MACB whereas 0x2 is GEM. As I wrote in my previous email, I _think_ > that the GEM is really GiB capabale but the SoC around do not provde the > needed ressource for this (such as line running at GHz). Actually, the GEM IP from Cadence can be configured to only do 10/100Mbs. So the IP ID doesn't tell that and the design configuration registers don't seem to reflect that. Bye, -- Nicolas Ferre ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] net: macb: sama5d4 is not gigabit capable 2015-12-15 19:59 ` Joe Hershberger 2015-12-16 9:19 ` Nicolas Ferre @ 2015-12-16 9:32 ` Gregory CLEMENT 2015-12-16 9:40 ` Andreas Bießmann 1 sibling, 1 reply; 8+ messages in thread From: Gregory CLEMENT @ 2015-12-16 9:32 UTC (permalink / raw) To: u-boot Hi Joe, On mar., d?c. 15 2015, Joe Hershberger <joe.hershberger@gmail.com> wrote: > Hi Gregory, > > On Mon, Dec 14, 2015 at 10:37 AM, Gregory CLEMENT > <gregory.clement@free-electrons.com> wrote: >> During the initialization of PHY the gigabit bit capable is set if the >> controller is a GEM. However, for sama5d4, the GEM is not gigabit >> capable. Improperly setting the GBE capability leads to an unresponsive >> MAC controller. This patch fix this behavior allowing to use the gmac >> with the sama5d4. >> >> Suggested-by: Nicolas Ferre <nicolas.ferre@atmel.com> >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> >> --- >> drivers/net/macb.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/macb.c b/drivers/net/macb.c >> index a5c1880..642717d 100644 >> --- a/drivers/net/macb.c >> +++ b/drivers/net/macb.c >> @@ -480,8 +480,11 @@ static int macb_phy_init(struct macb_device *macb) >> return 0; >> } >> >> - /* First check for GMAC */ >> - if (macb_is_gem(macb)) { >> + /* >> + * First check for GMAC, but not the one on SAMA5D4 which is >> + * not gigabit capabale >> + */ >> + if (macb_is_gem(macb) && ! cpu_is_sama5d4()) { > > Is there not some other property that can identify the lack of Gigabit > support in the "GEM"? It would be better to not have to keep track of > the next processor and the one after that which has the same > situation. I agree and I started to looked for this kind of information. But there is not such information documented inside the controller. I also think of the controller ID but I doubt that there is a link between the gigabit capability and the version of the GEM. On sama5d3 the revision is 0x119 and the one in sama5d4 is 0x120, so this really reflects the version of the controller itself and not his capabilities. I also had a look on how it was done in the kernel, and they relies on the compatible string which is different for each SoC. Last thing, but it is only speculation, I think that the gigabit capability depend on the SoC around the controller and not the controller itself. Being able to do gigabit means being able to have high speed dedicated lines and it is out of the scope of the controller itself. An other hint is actually the fact that we have to set this GBE bit in the configuration register of the controller, for me that means that the controller is not aware of it. Thanks, Gregory > >> lpa = macb_mdio_read(macb, MII_STAT1000); >> >> if (lpa & (LPA_1000FULL | LPA_1000HALF)) { >> -- >> 2.5.0 >> >> _______________________________________________ >> U-Boot mailing list >> U-Boot at lists.denx.de >> http://lists.denx.de/mailman/listinfo/u-boot -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] net: macb: sama5d4 is not gigabit capable 2015-12-16 9:32 ` Gregory CLEMENT @ 2015-12-16 9:40 ` Andreas Bießmann 0 siblings, 0 replies; 8+ messages in thread From: Andreas Bießmann @ 2015-12-16 9:40 UTC (permalink / raw) To: u-boot Hi Gregory, On 16.12.2015 10:32, Gregory CLEMENT wrote: > Hi Joe, > > On mar., d?c. 15 2015, Joe Hershberger <joe.hershberger@gmail.com> wrote: > >> Hi Gregory, >> >> On Mon, Dec 14, 2015 at 10:37 AM, Gregory CLEMENT >> <gregory.clement@free-electrons.com> wrote: >>> During the initialization of PHY the gigabit bit capable is set if the >>> controller is a GEM. However, for sama5d4, the GEM is not gigabit >>> capable. Improperly setting the GBE capability leads to an unresponsive >>> MAC controller. This patch fix this behavior allowing to use the gmac >>> with the sama5d4. >>> >>> Suggested-by: Nicolas Ferre <nicolas.ferre@atmel.com> >>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> >>> --- >>> drivers/net/macb.c | 7 +++++-- >>> 1 file changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/net/macb.c b/drivers/net/macb.c >>> index a5c1880..642717d 100644 >>> --- a/drivers/net/macb.c >>> +++ b/drivers/net/macb.c >>> @@ -480,8 +480,11 @@ static int macb_phy_init(struct macb_device *macb) >>> return 0; >>> } >>> >>> - /* First check for GMAC */ >>> - if (macb_is_gem(macb)) { >>> + /* >>> + * First check for GMAC, but not the one on SAMA5D4 which is >>> + * not gigabit capabale >>> + */ >>> + if (macb_is_gem(macb) && ! cpu_is_sama5d4()) { >> >> Is there not some other property that can identify the lack of Gigabit >> support in the "GEM"? It would be better to not have to keep track of >> the next processor and the one after that which has the same >> situation. > > I agree and I started to looked for this kind of information. But there > is not such information documented inside the controller. I also think > of the controller ID but I doubt that there is a link between the > gigabit capability and the version of the GEM. On sama5d3 the revision is > 0x119 and the one in sama5d4 is 0x120, so this really reflects the > version of the controller itself and not his capabilities. You where faster ... answer my question before I was asking, at least to the timestamp of the mail ;) > I also had a look on how it was done in the kernel, and they relies on > the compatible string which is different for each SoC. We will get this also if the network detection has moved to DM completely. > Last thing, but it is only speculation, I think that the gigabit > capability depend on the SoC around the controller and not the > controller itself. Being able to do gigabit means being able to have > high speed dedicated lines and it is out of the scope of the controller > itself. An other hint is actually the fact that we have to set this GBE > bit in the configuration register of the controller, for me that means > that the controller is not aware of it. Good point. So could we please patch the macb_is_gem() then? I think we should also add the sama5d2 as Nicolas said this has also no GiB capability. Andreas > > Thanks, > > Gregory > >> >>> lpa = macb_mdio_read(macb, MII_STAT1000); >>> >>> if (lpa & (LPA_1000FULL | LPA_1000HALF)) { >>> -- >>> 2.5.0 >>> >>> _______________________________________________ >>> U-Boot mailing list >>> U-Boot at lists.denx.de >>> http://lists.denx.de/mailman/listinfo/u-boot > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-12-16 9:52 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-12-14 16:37 [U-Boot] [PATCH] net: macb: sama5d4 is not gigabit capable Gregory CLEMENT 2015-12-15 19:59 ` Joe Hershberger 2015-12-16 9:19 ` Nicolas Ferre 2015-12-16 9:35 ` Andreas Bießmann 2015-12-16 9:41 ` Gregory CLEMENT 2015-12-16 9:52 ` Nicolas Ferre 2015-12-16 9:32 ` Gregory CLEMENT 2015-12-16 9:40 ` Andreas Bießmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox