| Message ID | 1411928001-3455-1-git-send-email-hdegoede@redhat.com |
|---|---|
| State | Superseded |
| Delegated to: | Ian Campbell |
| Headers | show |
On Sun, Sep 28, 2014 at 08:13:21PM +0200, Hans de Goede wrote: > In order for the gmac nic to work reliable on the Bananapi, we need to poke > these 2 undocumented bits in the gmac clk register. Since these are > undocumented, this commit only sets these bits on the Bananapi for now. > > I'll contact Allwinner to try and get these bits documented, once they > are documented we can hopefully replace this hack with a better patch. > > Reported-by: Karsten Merker <merker@debian.org> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Tested-by: Karsten Merker <merker@debian.org> > --- > board/sunxi/gmac.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/board/sunxi/gmac.c b/board/sunxi/gmac.c > index e7ff952..f58c963 100644 > --- a/board/sunxi/gmac.c > +++ b/board/sunxi/gmac.c > @@ -24,6 +24,15 @@ int sunxi_gmac_initialize(bd_t *bis) > CCM_GMAC_CTRL_GPIT_MII); > #endif > > + /* > + * HdG: this is necessary to get GMAC to work reliable on the > + * Bananapi. We don't know what these undocumented bits do, so this > + * is a Bananapi specific hack for now. > + */ > +#ifdef CONFIG_BANANAPI > + setbits_le32(&ccm->gmac_clk_cfg, 0x3 << 10); > +#endif > + > /* Configure pin mux settings for GMAC */ > for (pin = SUNXI_GPA(0); pin <= SUNXI_GPA(16); pin++) { > #ifdef CONFIG_RGMII > -- > 2.1.0 Regards, Karsten
On Sun, 28 Sep 2014, Karsten Merker wrote: > On Sun, Sep 28, 2014 at 08:13:21PM +0200, Hans de Goede wrote: >> In order for the gmac nic to work reliable on the Bananapi, we need to poke >> these 2 undocumented bits in the gmac clk register. Since these are >> undocumented, this commit only sets these bits on the Bananapi for now. >> >> I'll contact Allwinner to try and get these bits documented, once they >> are documented we can hopefully replace this hack with a better patch. >> >> Reported-by: Karsten Merker <merker@debian.org> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > Tested-by: Karsten Merker <merker@debian.org> Tested-by: Zoltan HERPAI <wigyori@openwrt.org> >> --- >> board/sunxi/gmac.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/board/sunxi/gmac.c b/board/sunxi/gmac.c >> index e7ff952..f58c963 100644 >> --- a/board/sunxi/gmac.c >> +++ b/board/sunxi/gmac.c >> @@ -24,6 +24,15 @@ int sunxi_gmac_initialize(bd_t *bis) >> CCM_GMAC_CTRL_GPIT_MII); >> #endif >> >> + /* >> + * HdG: this is necessary to get GMAC to work reliable on the >> + * Bananapi. We don't know what these undocumented bits do, so this >> + * is a Bananapi specific hack for now. >> + */ >> +#ifdef CONFIG_BANANAPI >> + setbits_le32(&ccm->gmac_clk_cfg, 0x3 << 10); >> +#endif >> + >> /* Configure pin mux settings for GMAC */ >> for (pin = SUNXI_GPA(0); pin <= SUNXI_GPA(16); pin++) { >> #ifdef CONFIG_RGMII >> -- >> 2.1.0 > > Regards, > Karsten > -- > Gem. Par. 28 Abs. 4 Bundesdatenschutzgesetz widerspreche ich der Nutzung > sowie der Weitergabe meiner personenbezogenen Daten für Zwecke der > Werbung sowie der Markt- oder Meinungsforschung. >
On Sun, 28 Sep 2014 20:13:21 +0200 Hans de Goede <hdegoede@redhat.com> wrote: > In order for the gmac nic to work reliable on the Bananapi, we need to poke > these 2 undocumented bits in the gmac clk register. Since these are > undocumented, this commit only sets these bits on the Bananapi for now. > > I'll contact Allwinner to try and get these bits documented, once they > are documented we can hopefully replace this hack with a better patch. Could you please provide a bit more details in the commit message? What are the symptoms of the problem? How did you come to the idea to poke these bits? Does the GMAC driver in the linux kernel need a similar fix/workaround? And if you apply it in the linux kernel, does it provide any visible changes in performance or anything else? Also as mentioned in another e-mail http://lists.denx.de/pipermail/u-boot/2014-September/190096.html u-boot configures the "912MHz @1.4V" CPU clock frequency/voltage setup for sun7i hardware. And according to the information from Tony Zhang, this is supposed to be unreliable for the Banana Pi. So what happens to this GMAC bug if you just increase the dcdc2 voltage in u-boot (or reduce the CPU clock frequency)? I remember that some users suffered from GMAC problems, which turned out to be caused by just overclocking the CPU to 1.2GHz (a "helful" SD card image creator decided that the users would like to enjoy better performance): http://www.cubieforums.com/index.php/topic,2590.0.html So it looks like GMAC might be one of those canaries, which die first on wrong CPU clock frequency/voltage settings. > Reported-by: Karsten Merker <merker@debian.org> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > board/sunxi/gmac.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/board/sunxi/gmac.c b/board/sunxi/gmac.c > index e7ff952..f58c963 100644 > --- a/board/sunxi/gmac.c > +++ b/board/sunxi/gmac.c > @@ -24,6 +24,15 @@ int sunxi_gmac_initialize(bd_t *bis) > CCM_GMAC_CTRL_GPIT_MII); > #endif > > + /* > + * HdG: this is necessary to get GMAC to work reliable on the > + * Bananapi. We don't know what these undocumented bits do, so this > + * is a Bananapi specific hack for now. > + */ > +#ifdef CONFIG_BANANAPI > + setbits_le32(&ccm->gmac_clk_cfg, 0x3 << 10); > +#endif > + > /* Configure pin mux settings for GMAC */ > for (pin = SUNXI_GPA(0); pin <= SUNXI_GPA(16); pin++) { > #ifdef CONFIG_RGMII
On Mon, Sep 29, 2014 at 09:13:37AM +0300, Siarhei Siamashka wrote: > On Sun, 28 Sep 2014 20:13:21 +0200 > Hans de Goede <hdegoede@redhat.com> wrote: > > > In order for the gmac nic to work reliable on the Bananapi, we need to poke > > these 2 undocumented bits in the gmac clk register. Since these are > > undocumented, this commit only sets these bits on the Bananapi for now. > > > > I'll contact Allwinner to try and get these bits documented, once they > > are documented we can hopefully replace this hack with a better patch. > > Could you please provide a bit more details in the commit message? > What are the symptoms of the problem? Without the patch, u-boot on the BananaPi shows massive packet loss on every network activity. The packet loss is so extreme that it is actually impossible to boot a kernel by tftp because u-boot gives up. The mainline kernel has (once booted) shown similar problems on the BananaPi. > How did you come to the idea to poke these bits? The linux 3.4 kernel from https://github.com/LeMaker/linux-bananapi (which is a patched linux-sunxi.org 3.4 kernel) does not show this problem, so we started looking into what is different there, and the relevant change that was found was setting these particular undocumented bits. > Does the GMAC driver in the linux kernel need a similar > fix/workaround? Not as far as I can see - once u-boot has set these two bits during system initialization, the mainline kernel works fine without any changes. > Also as mentioned in another e-mail > http://lists.denx.de/pipermail/u-boot/2014-September/190096.html > u-boot configures the "912MHz @1.4V" CPU clock frequency/voltage > setup for sun7i hardware. And according to the information from Tony > Zhang, this is supposed to be unreliable for the Banana Pi. So what > happens to this GMAC bug if you just increase the dcdc2 voltage in > u-boot (or reduce the CPU clock frequency)? I can test that if you could tell me what would have to be changed in u-boot to do that. Regards, Karsten
On Mon, Sep 29, 2014 at 2:16 PM, Karsten Merker <merker@debian.org> wrote: > On Mon, Sep 29, 2014 at 09:13:37AM +0300, Siarhei Siamashka wrote: > >> On Sun, 28 Sep 2014 20:13:21 +0200 >> Hans de Goede <hdegoede@redhat.com> wrote: >> >> > In order for the gmac nic to work reliable on the Bananapi, we need to poke >> > these 2 undocumented bits in the gmac clk register. Since these are >> > undocumented, this commit only sets these bits on the Bananapi for now. >> > >> > I'll contact Allwinner to try and get these bits documented, once they >> > are documented we can hopefully replace this hack with a better patch. >> >> Could you please provide a bit more details in the commit message? >> What are the symptoms of the problem? > > Without the patch, u-boot on the BananaPi shows massive packet loss > on every network activity. The packet loss is so extreme that it is > actually impossible to boot a kernel by tftp because u-boot gives up. > The mainline kernel has (once booted) shown similar problems on the > BananaPi. > >> How did you come to the idea to poke these bits? > > The linux 3.4 kernel from https://github.com/LeMaker/linux-bananapi > (which is a patched linux-sunxi.org 3.4 kernel) does not show this > problem, so we started looking into what is different there, and the > relevant change that was found was setting these particular > undocumented bits. File at issue here asking for the documentation https://github.com/allwinner-zh/documents > >> Does the GMAC driver in the linux kernel need a similar >> fix/workaround? > > Not as far as I can see - once u-boot has set these two bits during > system initialization, the mainline kernel works fine without > any changes. > >> Also as mentioned in another e-mail >> http://lists.denx.de/pipermail/u-boot/2014-September/190096.html >> u-boot configures the "912MHz @1.4V" CPU clock frequency/voltage >> setup for sun7i hardware. And according to the information from Tony >> Zhang, this is supposed to be unreliable for the Banana Pi. So what >> happens to this GMAC bug if you just increase the dcdc2 voltage in >> u-boot (or reduce the CPU clock frequency)? > > I can test that if you could tell me what would have to be changed > in u-boot to do that. > > Regards, > Karsten > -- > Gem. Par. 28 Abs. 4 Bundesdatenschutzgesetz widerspreche ich der Nutzung > sowie der Weitergabe meiner personenbezogenen Daten für Zwecke der > Werbung sowie der Markt- oder Meinungsforschung. > > -- > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout.
On Mon, 29 Sep 2014 20:16:35 +0200 Karsten Merker <merker@debian.org> wrote: > On Mon, Sep 29, 2014 at 09:13:37AM +0300, Siarhei Siamashka wrote: > > > On Sun, 28 Sep 2014 20:13:21 +0200 > > Hans de Goede <hdegoede@redhat.com> wrote: > > > > > In order for the gmac nic to work reliable on the Bananapi, we need to poke > > > these 2 undocumented bits in the gmac clk register. Since these are > > > undocumented, this commit only sets these bits on the Bananapi for now. > > > > > > I'll contact Allwinner to try and get these bits documented, once they > > > are documented we can hopefully replace this hack with a better patch. > > > > Could you please provide a bit more details in the commit message? > > What are the symptoms of the problem? > > Without the patch, u-boot on the BananaPi shows massive packet loss > on every network activity. The packet loss is so extreme that it is > actually impossible to boot a kernel by tftp because u-boot gives up. > The mainline kernel has (once booted) shown similar problems on the > BananaPi. > > > How did you come to the idea to poke these bits? > > The linux 3.4 kernel from https://github.com/LeMaker/linux-bananapi > (which is a patched linux-sunxi.org 3.4 kernel) does not show this > problem, so we started looking into what is different there, and the > relevant change that was found was setting these particular > undocumented bits. Thanks for the detailed explanations. They would look really great in the commit message. > > Does the GMAC driver in the linux kernel need a similar > > fix/workaround? > > Not as far as I can see - once u-boot has set these two bits during > system initialization, the mainline kernel works fine without > any changes. OK. I just wonder if it is a good idea for the kernel to rely on these bits being set appropriately by the bootloader. Not that it has anything to do with u-boot. > > Also as mentioned in another e-mail > > http://lists.denx.de/pipermail/u-boot/2014-September/190096.html > > u-boot configures the "912MHz @1.4V" CPU clock frequency/voltage > > setup for sun7i hardware. And according to the information from Tony > > Zhang, this is supposed to be unreliable for the Banana Pi. So what > > happens to this GMAC bug if you just increase the dcdc2 voltage in > > u-boot (or reduce the CPU clock frequency)? > > I can test that if you could tell me what would have to be changed > in u-boot to do that. The dcdc2 voltage is set here (under the CONFIG_AXP209_POWER ifdef): http://git.denx.de/?p=u-boot.git;a=blob;f=board/sunxi/board.c;h=2179e234e21d67b0fec064de792fca175db90ca5;hb=be9f643ae6aa9044c60fe80e3a2c10be8371c692#l140 Right now it is set to 1400, which means 1.4V (and the voltage can be changed in 0.025V steps). The CPU clock frequency for sun7i is set here: http://git.denx.de/?p=u-boot.git;a=blob;f=include/configs/sun7i.h;h=a902b845744707e64fb5a064c6039bc93b0150d9;hb=be9f643ae6aa9044c60fe80e3a2c10be8371c692#l16 Right now it is set to 912MHz This CPU clock frequency/voltage pair is then just used unmodified by the mainline kernel (until we get proper cpufreq support later). So it makes a lot of sense to be sure that it is sufficiently reliable on every Allwinner A20 device. You can try to increase the dcdc2 voltage or reduce the CPU clock frequency and check if the Banana Pi reliability problems disappear. By the way, the dcdc2 voltage is already set to the maximum, which is specified in the datasheet. You can have a look at VDD-CPU limits in https://github.com/allwinner-zh/documents/blob/master/A20/A20%20Datasheet%20V1.4%2020131230.pdf And VDD-CPU is provided from dcdc2, as can be seen, for example, in the A20-OLINUXINO-MICRO schematics (the Banana Pi does not seem to have its schematics document in public access, but all A20 devices have it designed in the same way): https://github.com/OLIMEX/OLINUXINO/blob/master/HARDWARE/A10-OLinuXino-MICRO/A20-OLINUXINO-MICRO_4GB_Rev_F1.pdf
On Sun, 2014-09-28 at 20:13 +0200, Hans de Goede wrote: > In order for the gmac nic to work reliable on the Bananapi, we need to poke > these 2 undocumented bits in the gmac clk register. Since these are > undocumented, this commit only sets these bits on the Bananapi for now. > > I'll contact Allwinner to try and get these bits documented, once they > are documented we can hopefully replace this hack with a better patch. > > Reported-by: Karsten Merker <merker@debian.org> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> It would be good to include some of Karsten's more detailed explanation (see his reply downthread) of what's going on, but other than that: Acked-by: Ian Campbell <ijc@hellion.org.uk> and ack to pushing it for v2014.10. > --- > board/sunxi/gmac.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/board/sunxi/gmac.c b/board/sunxi/gmac.c > index e7ff952..f58c963 100644 > --- a/board/sunxi/gmac.c > +++ b/board/sunxi/gmac.c > @@ -24,6 +24,15 @@ int sunxi_gmac_initialize(bd_t *bis) > CCM_GMAC_CTRL_GPIT_MII); > #endif > > + /* > + * HdG: this is necessary to get GMAC to work reliable on the > + * Bananapi. We don't know what these undocumented bits do, so this > + * is a Bananapi specific hack for now. > + */ > +#ifdef CONFIG_BANANAPI > + setbits_le32(&ccm->gmac_clk_cfg, 0x3 << 10); > +#endif > + > /* Configure pin mux settings for GMAC */ > for (pin = SUNXI_GPA(0); pin <= SUNXI_GPA(16); pin++) { > #ifdef CONFIG_RGMII
Hi, On 09/29/2014 08:35 PM, jonsmirl@gmail.com wrote: > On Mon, Sep 29, 2014 at 2:16 PM, Karsten Merker <merker@debian.org> wrote: >> On Mon, Sep 29, 2014 at 09:13:37AM +0300, Siarhei Siamashka wrote: >>> How did you come to the idea to poke these bits? >> >> The linux 3.4 kernel from https://github.com/LeMaker/linux-bananapi >> (which is a patched linux-sunxi.org 3.4 kernel) does not show this >> problem, so we started looking into what is different there, and the >> relevant change that was found was setting these particular >> undocumented bits. > > File at issue here asking for the documentation > https://github.com/allwinner-zh/documents I'm already talking to Allwinner about this, it seems that bits 10-11, or more likely 10-12 of the gmac clk register are the tx equivalent of bits 5-7, "Configure GMAC receive clock delay chain". I think this is there to deal with the pcb data and clk traces not having the same length, so that this is highly board specific. I'm waiting on confirmation on this from Allwinner, as well as some info on the unit of this 0-7 value. Regards, Hans
On Tue, Sep 30, 2014 at 8:02 AM, Hans de Goede <hdegoede@redhat.com> wrote: > Hi, > > On 09/29/2014 08:35 PM, jonsmirl@gmail.com wrote: >> On Mon, Sep 29, 2014 at 2:16 PM, Karsten Merker <merker@debian.org> wrote: >>> On Mon, Sep 29, 2014 at 09:13:37AM +0300, Siarhei Siamashka wrote: >>>> How did you come to the idea to poke these bits? >>> >>> The linux 3.4 kernel from https://github.com/LeMaker/linux-bananapi >>> (which is a patched linux-sunxi.org 3.4 kernel) does not show this >>> problem, so we started looking into what is different there, and the >>> relevant change that was found was setting these particular >>> undocumented bits. >> >> File at issue here asking for the documentation >> https://github.com/allwinner-zh/documents > > I'm already talking to Allwinner about this, it seems that bits > 10-11, or more likely 10-12 of the gmac clk register are the tx > equivalent of bits 5-7, "Configure GMAC receive clock delay > chain". I think this is there to deal with the pcb data and clk > traces not having the same length, so that this is highly board > specific. I'm waiting on confirmation on this from Allwinner, > as well as some info on the unit of this 0-7 value. Encourage them to update the manual and push a new copy to git hub immediately. Too much info like this is getting buried in mail archives. Electronic publishing is free - update as often as you want. The more they fix stuff like this in the manual, the fewer questions they will need to deal with in person. When it goes into the manual it won't get lost. > > Regards, > > Hans
Op 30 sep. 2014, om 14:02 heeft Hans de Goede <hdegoede@redhat.com> het volgende geschreven: > Hi, > > On 09/29/2014 08:35 PM, jonsmirl@gmail.com wrote: >> On Mon, Sep 29, 2014 at 2:16 PM, Karsten Merker <merker@debian.org> wrote: >>> On Mon, Sep 29, 2014 at 09:13:37AM +0300, Siarhei Siamashka wrote: >>>> How did you come to the idea to poke these bits? >>> >>> The linux 3.4 kernel from https://github.com/LeMaker/linux-bananapi >>> (which is a patched linux-sunxi.org 3.4 kernel) does not show this >>> problem, so we started looking into what is different there, and the >>> relevant change that was found was setting these particular >>> undocumented bits. >> >> File at issue here asking for the documentation >> https://github.com/allwinner-zh/documents > > I'm already talking to Allwinner about this, it seems that bits > 10-11, or more likely 10-12 of the gmac clk register are the tx > equivalent of bits 5-7, "Configure GMAC receive clock delay > chain". I think this is there to deal with the pcb data and clk > traces not having the same length, so that this is highly board > specific. I'm waiting on confirmation on this from Allwinner, > as well as some info on the unit of this 0-7 value. Gbit ethernet requires a delay between the lines, which you can implement by adding 10cm or so extra trace on the PCB or tell the MAC or PHY to do it in software. Very board specific, but so far I've only seen Intel reference designs use the extra trace length method most arm/ppc/mips gbit boards use a PHY that handles it in hardware on request like the GMAC is doing above. regards, Koen
On Tue, Sep 30, 2014 at 12:37:53AM +0300, Siarhei Siamashka wrote: > On Mon, 29 Sep 2014 20:16:35 +0200 > Karsten Merker <merker@debian.org> wrote: > > > Also as mentioned in another e-mail > > > http://lists.denx.de/pipermail/u-boot/2014-September/190096.html > > > u-boot configures the "912MHz @1.4V" CPU clock frequency/voltage > > > setup for sun7i hardware. And according to the information from Tony > > > Zhang, this is supposed to be unreliable for the Banana Pi. So what > > > happens to this GMAC bug if you just increase the dcdc2 voltage in > > > u-boot (or reduce the CPU clock frequency)? > > > > I can test that if you could tell me what would have to be changed > > in u-boot to do that. > > The dcdc2 voltage is set here (under the CONFIG_AXP209_POWER ifdef): > http://git.denx.de/?p=u-boot.git;a=blob;f=board/sunxi/board.c;h=2179e234e21d67b0fec064de792fca175db90ca5;hb=be9f643ae6aa9044c60fe80e3a2c10be8371c692#l140 > Right now it is set to 1400, which means 1.4V (and the voltage can > be changed in 0.025V steps). > > The CPU clock frequency for sun7i is set here: > http://git.denx.de/?p=u-boot.git;a=blob;f=include/configs/sun7i.h;h=a902b845744707e64fb5a064c6039bc93b0150d9;hb=be9f643ae6aa9044c60fe80e3a2c10be8371c692#l16 > Right now it is set to 912MHz Hello, I have now run tests with the dcdc2 voltage increased to 1.425V, with the CPU clock decreased to 800MHz and both at the same time. None of these changes shows any influence on the packet loss issue, so I assume that the GMAC problem is not related to the frequency/voltage setup. Regards, Karsten
diff --git a/board/sunxi/gmac.c b/board/sunxi/gmac.c index e7ff952..f58c963 100644 --- a/board/sunxi/gmac.c +++ b/board/sunxi/gmac.c @@ -24,6 +24,15 @@ int sunxi_gmac_initialize(bd_t *bis) CCM_GMAC_CTRL_GPIT_MII); #endif + /* + * HdG: this is necessary to get GMAC to work reliable on the + * Bananapi. We don't know what these undocumented bits do, so this + * is a Bananapi specific hack for now. + */ +#ifdef CONFIG_BANANAPI + setbits_le32(&ccm->gmac_clk_cfg, 0x3 << 10); +#endif + /* Configure pin mux settings for GMAC */ for (pin = SUNXI_GPA(0); pin <= SUNXI_GPA(16); pin++) { #ifdef CONFIG_RGMII
In order for the gmac nic to work reliable on the Bananapi, we need to poke these 2 undocumented bits in the gmac clk register. Since these are undocumented, this commit only sets these bits on the Bananapi for now. I'll contact Allwinner to try and get these bits documented, once they are documented we can hopefully replace this hack with a better patch. Reported-by: Karsten Merker <merker@debian.org> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- board/sunxi/gmac.c | 9 +++++++++ 1 file changed, 9 insertions(+)