| Message ID | 1401991217-1252-3-git-send-email-ijc@hellion.org.uk |
|---|---|
| State | Accepted |
| Delegated to: | Ian Campbell |
| Headers | show |
On Thu, 5 Jun 2014 19:00:14 +0100 Ian Campbell <ijc@hellion.org.uk> wrote: > This is a sun7i (A20) based followup to the sun4i (A10) > Cubieboard. It has GMAC using MII mode. > > Signed-off-by: Ian Campbell <ijc@hellion.org.uk> > Acked-by: Hans de Goede <hdegoede@redhat.com> This board is using exactly the same PCB as the Cubieboard1. And only the SoC is different (Allwinner A20 instead of the pin-compatible Allwinner A10). Before piling up more board configurations, we might want to consider supporting both Cubieboard1 and Cubieboard2 with a single u-boot binary (and perhaps keep Cubieboard1 and Cubieboard2 as aliases in boards.cfg). The Allwinner SoCs have support for runtime identification of the SoC type (sun4i/sun5i/sun7i) via the VER_REG (Version Register) located at the address 0x01C00024 as explained in the Allwinner A20 user manual. This requires replacing all the CONFIG_SUN4I/CONFIG_SUN5I/CONFIG_SUN7I ifdefs in the u-boot code with a runtime SoC type checks, but there are not too many places affected (mostly just the DRAM code). Here is a quick and dirty example (not a patch submission yet), which allows to boot the Cubieboard2 hardware using the existing Cubieboard1 config: https://github.com/ssvb/u-boot-sunxi-dram/commit/3153905e0221 If the u-boot code is further extended to define a variable with the relevant dtb file name in the u-boot environment (depending on the runtime detected SoC type and selecting from "sun4i-a10-cubieboard.dtb" and "sun7i-a20-cubieboard2.dtb"), then we can have the same SD card with the whole pre-installed Linux system usable on both Cubieboard1 and Cubieboard2 hardware by just swapping the card. Also the Cubieboards are not alone. Sharing the same PCB happens for the LIME boards from Olimex too: https://www.olimex.com/Products/OLinuXino/A10/A10-OLinuXino-LIME https://www.olimex.com/Products/OLinuXino/A20/A20-OLinuXino-LIME > Active arm armv7 sunxi - sunxi Cubieboard sun4i:CUBIEBOARD,SPL,AXP209_POWER,SUNXI_EMAC Hans de Goede <hdegoede@redhat.com> > +Active arm armv7 sunxi - sunxi Cubieboard2 sun7i:CUBIEBOARD2,SPL,SUNXI_GMAC Ian Campbell <ijc@hellion.org.uk>:Hans de Goede <hdegoede@redhat.com> > +Active arm armv7 sunxi - sunxi Cubieboard2_FEL sun7i:CUBIEBOARD2,SPL_FEL,SUNXI_GMAC Ian Campbell <ijc@hellion.org.uk>:Hans de Goede <hdegoede@redhat.com> The newly added Cubieboard2 from your patch appears to be missing the important AXP209_POWER option. So the patch is not good enough to be pushed anywhere in its current form.
On Thu, Jul 24, 2014 at 11:12 AM, Siarhei Siamashka <siarhei.siamashka@gmail.com> wrote: > On Thu, 5 Jun 2014 19:00:14 +0100 > Ian Campbell <ijc@hellion.org.uk> wrote: > >> This is a sun7i (A20) based followup to the sun4i (A10) >> Cubieboard. It has GMAC using MII mode. >> >> Signed-off-by: Ian Campbell <ijc@hellion.org.uk> >> Acked-by: Hans de Goede <hdegoede@redhat.com> > > This board is using exactly the same PCB as the Cubieboard1. And only > the SoC is different (Allwinner A20 instead of the pin-compatible > Allwinner A10). > > Before piling up more board configurations, we might want to consider > supporting both Cubieboard1 and Cubieboard2 with a single u-boot binary > (and perhaps keep Cubieboard1 and Cubieboard2 as aliases in boards.cfg). > The Allwinner SoCs have support for runtime identification of the SoC > type (sun4i/sun5i/sun7i) via the VER_REG (Version Register) located at > the address 0x01C00024 as explained in the Allwinner A20 user manual. > This requires replacing all the CONFIG_SUN4I/CONFIG_SUN5I/CONFIG_SUN7I > ifdefs in the u-boot code with a runtime SoC type checks, but there > are not too many places affected (mostly just the DRAM code). A20 will need PSCI for SMP and virtualization support. (I know the related code isn't in there yet.) Won't this be slightly hard to do if you mix them together? Just a though. Cheers ChenYu > Here is a quick and dirty example (not a patch submission yet), which > allows to boot the Cubieboard2 hardware using the existing Cubieboard1 > config: > https://github.com/ssvb/u-boot-sunxi-dram/commit/3153905e0221 > > If the u-boot code is further extended to define a variable with the > relevant dtb file name in the u-boot environment (depending on the > runtime detected SoC type and selecting from "sun4i-a10-cubieboard.dtb" > and "sun7i-a20-cubieboard2.dtb"), then we can have the same SD card > with the whole pre-installed Linux system usable on both Cubieboard1 > and Cubieboard2 hardware by just swapping the card. > > Also the Cubieboards are not alone. Sharing the same PCB happens for > the LIME boards from Olimex too: > https://www.olimex.com/Products/OLinuXino/A10/A10-OLinuXino-LIME > https://www.olimex.com/Products/OLinuXino/A20/A20-OLinuXino-LIME > >> Active arm armv7 sunxi - sunxi Cubieboard sun4i:CUBIEBOARD,SPL,AXP209_POWER,SUNXI_EMAC Hans de Goede <hdegoede@redhat.com> >> +Active arm armv7 sunxi - sunxi Cubieboard2 sun7i:CUBIEBOARD2,SPL,SUNXI_GMAC Ian Campbell <ijc@hellion.org.uk>:Hans de Goede <hdegoede@redhat.com> >> +Active arm armv7 sunxi - sunxi Cubieboard2_FEL sun7i:CUBIEBOARD2,SPL_FEL,SUNXI_GMAC Ian Campbell <ijc@hellion.org.uk>:Hans de Goede <hdegoede@redhat.com> > > The newly added Cubieboard2 from your patch appears to be missing the > important AXP209_POWER option. So the patch is not good enough to be > pushed anywhere in its current form.
On Thu, 2014-07-24 at 06:12 +0300, Siarhei Siamashka wrote: > On Thu, 5 Jun 2014 19:00:14 +0100 > Ian Campbell <ijc@hellion.org.uk> wrote: > > > This is a sun7i (A20) based followup to the sun4i (A10) > > Cubieboard. It has GMAC using MII mode. > > > > Signed-off-by: Ian Campbell <ijc@hellion.org.uk> > > Acked-by: Hans de Goede <hdegoede@redhat.com> > > This board is using exactly the same PCB as the Cubieboard1. And only > the SoC is different (Allwinner A20 instead of the pin-compatible > Allwinner A10). > > Before piling up more board configurations, we might want to consider > supporting both Cubieboard1 and Cubieboard2 with a single u-boot binary > (and perhaps keep Cubieboard1 and Cubieboard2 as aliases in boards.cfg). > The Allwinner SoCs have support for runtime identification of the SoC > type (sun4i/sun5i/sun7i) via the VER_REG (Version Register) located at > the address 0x01C00024 as explained in the Allwinner A20 user manual. > This requires replacing all the CONFIG_SUN4I/CONFIG_SUN5I/CONFIG_SUN7I > ifdefs in the u-boot code with a runtime SoC type checks, but there > are not too many places affected (mostly just the DRAM code). This all sounds nice but is very certainly a future piece of work not related to this patch submission. > The newly added Cubieboard2 from your patch appears to be missing the > important AXP209_POWER option. So the patch is not good enough to be > pushed anywhere in its current form. It works for me regardless and always has. The AXP209 config is trivial to add now that Hans has added the relevant code. Ian.
On Thu, 24 Jul 2014 11:18:15 +0800 Chen-Yu Tsai <wens@csie.org> wrote: > On Thu, Jul 24, 2014 at 11:12 AM, Siarhei Siamashka > <siarhei.siamashka@gmail.com> wrote: > > On Thu, 5 Jun 2014 19:00:14 +0100 > > Ian Campbell <ijc@hellion.org.uk> wrote: > > > >> This is a sun7i (A20) based followup to the sun4i (A10) > >> Cubieboard. It has GMAC using MII mode. > >> > >> Signed-off-by: Ian Campbell <ijc@hellion.org.uk> > >> Acked-by: Hans de Goede <hdegoede@redhat.com> > > > > This board is using exactly the same PCB as the Cubieboard1. And only > > the SoC is different (Allwinner A20 instead of the pin-compatible > > Allwinner A10). > > > > Before piling up more board configurations, we might want to consider > > supporting both Cubieboard1 and Cubieboard2 with a single u-boot binary > > (and perhaps keep Cubieboard1 and Cubieboard2 as aliases in boards.cfg). > > The Allwinner SoCs have support for runtime identification of the SoC > > type (sun4i/sun5i/sun7i) via the VER_REG (Version Register) located at > > the address 0x01C00024 as explained in the Allwinner A20 user manual. > > This requires replacing all the CONFIG_SUN4I/CONFIG_SUN5I/CONFIG_SUN7I > > ifdefs in the u-boot code with a runtime SoC type checks, but there > > are not too many places affected (mostly just the DRAM code). > > A20 will need PSCI for SMP and virtualization support. > (I know the related code isn't in there yet.) > Won't this be slightly hard to do if you mix them together? Thanks, that's a good point. Do you expect any special challenges other than just having to handle runtime SoC detection in more places and deal with larger final binaries on the hardware, which does not need PSCI itself?
On Thu, Jul 24, 2014 at 03:47:53PM +0300, Siarhei Siamashka wrote: > On Thu, 24 Jul 2014 11:18:15 +0800 > Chen-Yu Tsai <wens@csie.org> wrote: > > > On Thu, Jul 24, 2014 at 11:12 AM, Siarhei Siamashka > > <siarhei.siamashka@gmail.com> wrote: > > > On Thu, 5 Jun 2014 19:00:14 +0100 > > > Ian Campbell <ijc@hellion.org.uk> wrote: > > > > > >> This is a sun7i (A20) based followup to the sun4i (A10) > > >> Cubieboard. It has GMAC using MII mode. > > >> > > >> Signed-off-by: Ian Campbell <ijc@hellion.org.uk> > > >> Acked-by: Hans de Goede <hdegoede@redhat.com> > > > > > > This board is using exactly the same PCB as the Cubieboard1. And only > > > the SoC is different (Allwinner A20 instead of the pin-compatible > > > Allwinner A10). > > > > > > Before piling up more board configurations, we might want to consider > > > supporting both Cubieboard1 and Cubieboard2 with a single u-boot binary > > > (and perhaps keep Cubieboard1 and Cubieboard2 as aliases in boards.cfg). > > > The Allwinner SoCs have support for runtime identification of the SoC > > > type (sun4i/sun5i/sun7i) via the VER_REG (Version Register) located at > > > the address 0x01C00024 as explained in the Allwinner A20 user manual. > > > This requires replacing all the CONFIG_SUN4I/CONFIG_SUN5I/CONFIG_SUN7I > > > ifdefs in the u-boot code with a runtime SoC type checks, but there > > > are not too many places affected (mostly just the DRAM code). > > > > A20 will need PSCI for SMP and virtualization support. > > (I know the related code isn't in there yet.) > > Won't this be slightly hard to do if you mix them together? > > Thanks, that's a good point. Do you expect any special challenges other > than just having to handle runtime SoC detection in more places and > deal with larger final binaries on the hardware, which does not need > PSCI itself? We have similar issues to deal with on most TI SoCs as well. We want to share code as much as possible to avoid duplication, etc. As for run-time sharing, that's where it gets trickier. You're going to have some sort of size constraint on your binary so just how close are you to hitting it today on the smaller of the parts that you would run this on? In some cases, for the first part of the problem, we may want to do "if (is_sun4i()) { ... }" code that boils down to a compile-time check anyhow. I expect Simon to possibly chime in here as he's done some work in the past on this concept, for all CONFIG options (but I'm not sold on the idea for use everywhere, just yet anyhow).
On Thu, 24 Jul 2014 07:45:44 +0100 Ian Campbell <ijc@hellion.org.uk> wrote: > On Thu, 2014-07-24 at 06:12 +0300, Siarhei Siamashka wrote: > > On Thu, 5 Jun 2014 19:00:14 +0100 > > Ian Campbell <ijc@hellion.org.uk> wrote: > > > > > This is a sun7i (A20) based followup to the sun4i (A10) > > > Cubieboard. It has GMAC using MII mode. > > > > > > Signed-off-by: Ian Campbell <ijc@hellion.org.uk> > > > Acked-by: Hans de Goede <hdegoede@redhat.com> > > > > This board is using exactly the same PCB as the Cubieboard1. And only > > the SoC is different (Allwinner A20 instead of the pin-compatible > > Allwinner A10). > > > > Before piling up more board configurations, we might want to consider > > supporting both Cubieboard1 and Cubieboard2 with a single u-boot binary > > (and perhaps keep Cubieboard1 and Cubieboard2 as aliases in boards.cfg). > > The Allwinner SoCs have support for runtime identification of the SoC > > type (sun4i/sun5i/sun7i) via the VER_REG (Version Register) located at > > the address 0x01C00024 as explained in the Allwinner A20 user manual. > > This requires replacing all the CONFIG_SUN4I/CONFIG_SUN5I/CONFIG_SUN7I > > ifdefs in the u-boot code with a runtime SoC type checks, but there > > are not too many places affected (mostly just the DRAM code). > > This all sounds nice but is very certainly a future piece of work not > related to this patch submission. The multi-soc support (within the Allwinner A10/A13/A20 family) is the feature, which is scheduled for this merge window. It is a present piece of work. Your patch is related in the sense that it is detrimental to this goal. > > The newly added Cubieboard2 from your patch appears to be missing the > > important AXP209_POWER option. So the patch is not good enough to be > > pushed anywhere in its current form. > > It works for me regardless and always has. This simply means that your board is not very sensitive to the use of wrong voltages and may tolerate some abuse. You are just betting on luck. If you have been tracking the linux-sunxi mailing list, wrong voltages (dcdc3 in particular) have caused some very real reliability problems for some fraction of users. "Works for me" is not the right answer. > The AXP209 config is trivial to add now that Hans has added the relevant code. Yes. You can fix the problem after the fact, or you can avoid pushing the problematic commit in the first place and do something better.
On Thu, 24 Jul 2014 11:00:31 -0400 Tom Rini <trini@ti.com> wrote: > On Thu, Jul 24, 2014 at 03:47:53PM +0300, Siarhei Siamashka wrote: > > On Thu, 24 Jul 2014 11:18:15 +0800 > > Chen-Yu Tsai <wens@csie.org> wrote: > > > > > On Thu, Jul 24, 2014 at 11:12 AM, Siarhei Siamashka > > > <siarhei.siamashka@gmail.com> wrote: > > > > On Thu, 5 Jun 2014 19:00:14 +0100 > > > > Ian Campbell <ijc@hellion.org.uk> wrote: > > > > > > > >> This is a sun7i (A20) based followup to the sun4i (A10) > > > >> Cubieboard. It has GMAC using MII mode. > > > >> > > > >> Signed-off-by: Ian Campbell <ijc@hellion.org.uk> > > > >> Acked-by: Hans de Goede <hdegoede@redhat.com> > > > > > > > > This board is using exactly the same PCB as the Cubieboard1. And only > > > > the SoC is different (Allwinner A20 instead of the pin-compatible > > > > Allwinner A10). > > > > > > > > Before piling up more board configurations, we might want to consider > > > > supporting both Cubieboard1 and Cubieboard2 with a single u-boot binary > > > > (and perhaps keep Cubieboard1 and Cubieboard2 as aliases in boards.cfg). > > > > The Allwinner SoCs have support for runtime identification of the SoC > > > > type (sun4i/sun5i/sun7i) via the VER_REG (Version Register) located at > > > > the address 0x01C00024 as explained in the Allwinner A20 user manual. > > > > This requires replacing all the CONFIG_SUN4I/CONFIG_SUN5I/CONFIG_SUN7I > > > > ifdefs in the u-boot code with a runtime SoC type checks, but there > > > > are not too many places affected (mostly just the DRAM code). > > > > > > A20 will need PSCI for SMP and virtualization support. > > > (I know the related code isn't in there yet.) > > > Won't this be slightly hard to do if you mix them together? > > > > Thanks, that's a good point. Do you expect any special challenges other > > than just having to handle runtime SoC detection in more places and > > deal with larger final binaries on the hardware, which does not need > > PSCI itself? > > We have similar issues to deal with on most TI SoCs as well. We want to > share code as much as possible to avoid duplication, etc. As for > run-time sharing, that's where it gets trickier. You're going to have > some sort of size constraint on your binary so just how close are you to > hitting it today on the smaller of the parts that you would run this on? As far as I know, the most SRAM space constrained case on Allwinner hardware used to be the FEL boot mode, where we only had roughly ~15K for the SPL (code, data, stack). This can be improved though: http://lists.denx.de/pipermail/u-boot/2014-July/183985.html But I need to verify the SRAM size constraints again and come up with the detailed numbers. And also have a closer look at how this all interacts with the PSCI support. > In some cases, for the first part of the problem, we may want to do "if > (is_sun4i()) { ... }" code that boils down to a compile-time check > anyhow. Right, if the 'is_sun4i()' is a macro (should it be lower or upper case?), which expands to a compile time constant 0 or 1 in some configurations instead of the runtime checks, then the compiler is normally able to remove the unreachable parts of code and save space. This is a more flexible approach than using ifdefs. > I expect Simon to possibly chime in here as he's done some work > in the past on this concept, for all CONFIG options (but I'm not sold on > the idea for use everywhere, just yet anyhow).
On Fri, 2014-07-25 at 00:12 +0300, Siarhei Siamashka wrote: > The multi-soc support (within the Allwinner A10/A13/A20 family) is the > feature, which is scheduled for this merge window. It is a present > piece of work. I have never seen any such code, nor am I aware of any such thing being "scheduled" for this merge window (who by?). As far as I'm concerned this is not a goal for this merge window, if it lands then that would be nice but I think you are either underestimating the work involved or over estimating the size of the merge window. > Your patch is related in the sense that it is detrimental to this goal. No it is not. Whoever eventually wants to work on multi SoC support can trivially build on this series. > > > The newly added Cubieboard2 from your patch appears to be missing the > > > important AXP209_POWER option. So the patch is not good enough to be > > > pushed anywhere in its current form. > > > > It works for me regardless and always has. > > This simply means that your board is not very sensitive to the use of > wrong voltages and may tolerate some abuse. You are just betting on > luck. The sunxi github tree had exactly the same lack of power controller config issue and it appears to be fine for plenty of people. Note that we have not yet merged FAST_MBUS into mainline. > If you have been tracking the linux-sunxi mailing list, wrong voltages > (dcdc3 in particular) have caused some very real reliability problems > for some fraction of users. "Works for me" is not the right answer. > > > The AXP209 config is trivial to add now that Hans has added the relevant code. And I have already sent a patch to do so. > Yes. You can fix the problem after the fact, or you can avoid pushing > the problematic commit in the first place and do something better. We are not going to redo all of these patches from scratch as you seem to be asking repeatedly for every trivial issue you have found in your review of this series which I'm afraid is several weeks to late to be useful. Please base your work on what is currently in u-boot-sunxi#master, that is now the baseline. Even if we *were* to redo the current stuff as you are asking your work will still have to be based on that. Ian.
On Fri, Jul 25, 2014 at 12:40:04AM +0300, Siarhei Siamashka wrote: > On Thu, 24 Jul 2014 11:00:31 -0400 > Tom Rini <trini@ti.com> wrote: > > > On Thu, Jul 24, 2014 at 03:47:53PM +0300, Siarhei Siamashka wrote: > > > On Thu, 24 Jul 2014 11:18:15 +0800 > > > Chen-Yu Tsai <wens@csie.org> wrote: > > > > > > > On Thu, Jul 24, 2014 at 11:12 AM, Siarhei Siamashka > > > > <siarhei.siamashka@gmail.com> wrote: > > > > > On Thu, 5 Jun 2014 19:00:14 +0100 > > > > > Ian Campbell <ijc@hellion.org.uk> wrote: > > > > > > > > > >> This is a sun7i (A20) based followup to the sun4i (A10) > > > > >> Cubieboard. It has GMAC using MII mode. > > > > >> > > > > >> Signed-off-by: Ian Campbell <ijc@hellion.org.uk> > > > > >> Acked-by: Hans de Goede <hdegoede@redhat.com> > > > > > > > > > > This board is using exactly the same PCB as the Cubieboard1. And only > > > > > the SoC is different (Allwinner A20 instead of the pin-compatible > > > > > Allwinner A10). > > > > > > > > > > Before piling up more board configurations, we might want to consider > > > > > supporting both Cubieboard1 and Cubieboard2 with a single u-boot binary > > > > > (and perhaps keep Cubieboard1 and Cubieboard2 as aliases in boards.cfg). > > > > > The Allwinner SoCs have support for runtime identification of the SoC > > > > > type (sun4i/sun5i/sun7i) via the VER_REG (Version Register) located at > > > > > the address 0x01C00024 as explained in the Allwinner A20 user manual. > > > > > This requires replacing all the CONFIG_SUN4I/CONFIG_SUN5I/CONFIG_SUN7I > > > > > ifdefs in the u-boot code with a runtime SoC type checks, but there > > > > > are not too many places affected (mostly just the DRAM code). > > > > > > > > A20 will need PSCI for SMP and virtualization support. > > > > (I know the related code isn't in there yet.) > > > > Won't this be slightly hard to do if you mix them together? > > > > > > Thanks, that's a good point. Do you expect any special challenges other > > > than just having to handle runtime SoC detection in more places and > > > deal with larger final binaries on the hardware, which does not need > > > PSCI itself? > > > > We have similar issues to deal with on most TI SoCs as well. We want to > > share code as much as possible to avoid duplication, etc. As for > > run-time sharing, that's where it gets trickier. You're going to have > > some sort of size constraint on your binary so just how close are you to > > hitting it today on the smaller of the parts that you would run this on? > > As far as I know, the most SRAM space constrained case on Allwinner > hardware used to be the FEL boot mode, where we only had roughly ~15K > for the SPL (code, data, stack). This can be improved though: > > http://lists.denx.de/pipermail/u-boot/2014-July/183985.html > > But I need to verify the SRAM size constraints again and come up with > the detailed numbers. And also have a closer look at how this all > interacts with the PSCI support. Yeah, it's important to get the worst-case numbers just right. OMAP4 is ours and that's pretty tight :( > > In some cases, for the first part of the problem, we may want to do "if > > (is_sun4i()) { ... }" code that boils down to a compile-time check > > anyhow. > > Right, if the 'is_sun4i()' is a macro (should it be lower or upper > case?), which expands to a compile time constant 0 or 1 in some > configurations instead of the runtime checks, then the compiler is > normally able to remove the unreachable parts of code and save space. > This is a more flexible approach than using ifdefs. I use board_is_foo() in the TI SoC cases where we have one config that supports many boards of the same SoC family (ie am335x_evm supports beaglebone black/white, GP EVM, EVM SK and IDK EVM, and various revs of the boards too as needed). So board_is_sun4i(), etc sounds fine with me. The only tricky part of compile-time consolidation is making sure you don't get bit by https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54303
On Fri, Jul 25, 2014 at 07:52:28AM +0100, Ian Campbell wrote: > On Fri, 2014-07-25 at 00:12 +0300, Siarhei Siamashka wrote: > > The multi-soc support (within the Allwinner A10/A13/A20 family) is the > > feature, which is scheduled for this merge window. It is a present > > piece of work. > > I have never seen any such code, nor am I aware of any such thing being > "scheduled" for this merge window (who by?). > > As far as I'm concerned this is not a goal for this merge window, if it > lands then that would be nice but I think you are either underestimating > the work involved or over estimating the size of the merge window. FWIW, merge window closes 02 August 2014, and that means that v1 must be posted by then (and stable enough after that time that the relevant custodians feel comfortable pulling in the patches for a release that happens 13 October). And Ian and Hans _are_ the custodians of record.
On Fri, 25 Jul 2014 07:52:28 +0100 Ian Campbell <ijc@hellion.org.uk> wrote: > On Fri, 2014-07-25 at 00:12 +0300, Siarhei Siamashka wrote: > > The multi-soc support (within the Allwinner A10/A13/A20 family) is the > > feature, which is scheduled for this merge window. It is a present > > piece of work. > > I have never seen any such code, nor am I aware of any such thing being > "scheduled" for this merge window (who by?). Sorry for not stating this clear enough. I'm am taking care of this particular part of work. And this has been the plan since the start. > As far as I'm concerned this is not a goal for this merge window, You have not seen all the sunxi patches yet. And the merge window is still open. > if it lands then that would be nice but I think you are either underestimating > the work involved or over estimating the size of the merge window. We'll see. > > Your patch is related in the sense that it is detrimental to this goal. > > No it is not. Whoever eventually wants to work on multi SoC support can > trivially build on this series. Right. Anyway, I have both Cubieboard1 and Cubieboard2 hardware. So the code in u-boot is still fully testable by me, and I'm not dependent on the cooperation from the nominal maintainers of these boards. > > > > The newly added Cubieboard2 from your patch appears to be missing the > > > > important AXP209_POWER option. So the patch is not good enough to be > > > > pushed anywhere in its current form. > > > > > > It works for me regardless and always has. > > > > This simply means that your board is not very sensitive to the use of > > wrong voltages and may tolerate some abuse. You are just betting on > > luck. > > The sunxi github tree had exactly the same lack of power controller > config issue and it appears to be fine for plenty of people. Okay, that's totally convincing. Not. > Note that we have not yet merged FAST_MBUS into mainline. FAST_MBUS was just the use of 400MHz MBUS clock frequency instead of 300MHz. It required the dcdc3 voltage increase from 1.25V to 1.3V. Not doing so caused troubles for a *small* fraction of users. The odds of *you* being in this group are indeed rather small. Now we are talking about the 300MHz MBUS clock frequency in the mainline u-boot, which is normally used with the dcdc3 voltage 1.25V. But the default AXP209 dcdc3 voltage after reset appears to be only 1.2V (measured on the Cubietruck, where the tests pads are easily accessible). > > If you have been tracking the linux-sunxi mailing list, wrong voltages > > (dcdc3 in particular) have caused some very real reliability problems > > for some fraction of users. "Works for me" is not the right answer. > > > > > The AXP209 config is trivial to add now that Hans has added the relevant code. > > And I have already sent a patch to do so. Thanks for addressing the problem. That was the exactly the action I expected from you. You don't need to go full length explaining how minor or insignificant it was. Really. > > Yes. You can fix the problem after the fact, or you can avoid pushing > > the problematic commit in the first place and do something better. > > We are not going to redo all of these patches from scratch as you seem > to be asking repeatedly for every trivial issue you have found in your > review of this series which I'm afraid is several weeks to late to be > useful. I am reviewing your patches because it is a good development practice, and actually a part of the u-boot development process. I'm not doing this personally for you. This is just needed to ensure code quality. As for redoing the patches from scratch, you totally got the wrong idea. I'm not the one to tell you what to do. I can only share my opinion and you don't have any obligations to pay attention. It is your responsibility as a custodian to make decisions in the best interests of the sunxi project and do a proper job. > Please base your work on what is currently in u-boot-sunxi#master, that > is now the baseline. OK. Let's go this route. > Even if we *were* to redo the current stuff as you > are asking your work will still have to be based on that. I just want to mitigate risks and ensure that you don't screw up something as an inexperienced custodian. And was just waiting for the upstream custodians to confirm that they are really going to accept your strange early pull request. Now the comments from Tom seem to be reassuring.
diff --git a/board/sunxi/Makefile b/board/sunxi/Makefile index 7083632..62acb8f 100644 --- a/board/sunxi/Makefile +++ b/board/sunxi/Makefile @@ -12,5 +12,6 @@ obj-y += board.o obj-$(CONFIG_SUNXI_GMAC) += gmac.o obj-$(CONFIG_A13_OLINUXINOM) += dram_a13_oli_micro.o obj-$(CONFIG_CUBIEBOARD) += dram_cubieboard.o +obj-$(CONFIG_CUBIEBOARD2) += dram_cubieboard2.o obj-$(CONFIG_CUBIETRUCK) += dram_cubietruck.o obj-$(CONFIG_R7DONGLE) += dram_r7dongle.o diff --git a/board/sunxi/dram_cubieboard2.c b/board/sunxi/dram_cubieboard2.c new file mode 100644 index 0000000..9e75367 --- /dev/null +++ b/board/sunxi/dram_cubieboard2.c @@ -0,0 +1,31 @@ +/* this file is generated, don't edit it yourself */ + +#include <common.h> +#include <asm/arch/dram.h> + +static struct dram_para dram_para = { + .clock = 480, + .type = 3, + .rank_num = 1, + .density = 4096, + .io_width = 16, + .bus_width = 32, + .cas = 9, + .zq = 0x7f, + .odt_en = 0, + .size = 1024, + .tpr0 = 0x42d899b7, + .tpr1 = 0xa090, + .tpr2 = 0x22a00, + .tpr3 = 0x0, + .tpr4 = 0x1, + .tpr5 = 0x0, + .emr1 = 0x4, + .emr2 = 0x10, + .emr3 = 0x0, +}; + +unsigned long sunxi_dram_init(void) +{ + return dramc_init(&dram_para); +} diff --git a/boards.cfg b/boards.cfg index 18a8400..ddb105e 100644 --- a/boards.cfg +++ b/boards.cfg @@ -383,6 +383,8 @@ Active arm armv7 s5pc1xx samsung smdkc100 Active arm armv7 socfpga altera socfpga socfpga_cyclone5 - - Active arm armv7 sunxi - sunxi A13-OLinuXinoM sun5i:A13_OLINUXINOM,SPL,CONS_INDEX=2 Hans de Goede <hdegoede@redhat.com> Active arm armv7 sunxi - sunxi Cubieboard sun4i:CUBIEBOARD,SPL,AXP209_POWER,SUNXI_EMAC Hans de Goede <hdegoede@redhat.com> +Active arm armv7 sunxi - sunxi Cubieboard2 sun7i:CUBIEBOARD2,SPL,SUNXI_GMAC Ian Campbell <ijc@hellion.org.uk>:Hans de Goede <hdegoede@redhat.com> +Active arm armv7 sunxi - sunxi Cubieboard2_FEL sun7i:CUBIEBOARD2,SPL_FEL,SUNXI_GMAC Ian Campbell <ijc@hellion.org.uk>:Hans de Goede <hdegoede@redhat.com> Active arm armv7 sunxi - sunxi Cubietruck sun7i:CUBIETRUCK,SPL,AXP209_POWER,SUNXI_GMAC,RGMII Ian Campbell <ijc@hellion.org.uk>:Hans de Goede <hdegoede@redhat.com> Active arm armv7 sunxi - sunxi Cubietruck_FEL sun7i:CUBIETRUCK,SPL_FEL,AXP209_POWER,SUNXI_GMAC,RGMII Ian Campbell <ijc@hellion.org.uk>:Hans de Goede <hdegoede@redhat.com> Active arm armv7 sunxi - sunxi r7-tv-dongle sun5i:R7DONGLE,SPL,AXP152_POWER Hans de Goede <hdegoede@redhat.com>