[U-Boot] [PATCH v2 0/6] Add support for MIPS Creator CI20

[Ezequiel Garcia]

Hi Daniel,

On Wed, 2018-12-12 at 18:27 +0100, Daniel Schwierzeck wrote:
> Hi Ezequiel,
> 
> Am 12.12.18 um 14:58 schrieb Ezequiel Garcia:
> > A new round.
> > 
> > For this new round:
> > 
> >  * Replaced infinite while loop with wait_for_bit.
> >  * Added a MAINTAINERS file. If anyone wants to co-maintain this,
> >    please let me know.
> > 
> > This is based on top of yesterday's master (ee168783ae8) and has
> > been tested by SD-card booting both U-Boot and Linux. Booting
> > Linux via TFTP was also tested.
> > 
> > Toolchain used to test:
> > 
> > $ mips-linux-gcc -v
> > Using built-in specs.
> > COLLECT_GCC=mips-linux-gcc
> > COLLECT_LTO_WRAPPER=/home/zeta/.buildman-toolchains/gcc-7.3.0-nolibc/mips-linux/bin/../libexec/gcc/mips-linux/7.3.0/lto-wrapper
> > Target: mips-linux
> > Configured with: /home/arnd/git/gcc/configure --target=mips-linux --enable-targets=all --prefix=/opt/crosstool/gcc-7.3.0-nolibc/mips-linux --
> > enable-languages=c --without-headers --disable-bootstrap --disable-nls --disable-threads --disable-shared --disable-libmudflap --disable-libssp --
> > disable-libgomp --disable-decimal-float --disable-libquadmath --disable-libatomic --disable-libcc1 --disable-libmpx --enable-checking=release
> > Thread model: single
> > gcc version 7.3.0 (GCC) 
> > 
> > SPL size:
> > 
> > $ size spl/u-boot-spl
> >    text	   data	    bss	    dec	    hex	filename
> >    9252	    752	    736	  10740	   29f4	spl/u-boot-spl
> > 
> > I've pushed a branch to https://github.com/ezequielgarcia/u-boot/tree/ci20-v2
> > and made sure travis passed.
> > 
> 
> Thanks for working on this but the problem with the original patch
> series was that it was not fully driver model compatible and that stuff
> like watchdog, reset and pin-muxing was open-coded instead of using the
> proper frameworks. As the ship is sailing towards having only driver
> model and device tree in U-Boot proper, it doesn't make sense to accept
> new SoC/boards with legacy code especially if there is no active
> maintainer who would do the conversion work. And with the upcoming dead
> lines for DM conversions, boards with legacy code will be removed when
> not converted. This is also one of the reasons why the Ingenic SoC
> support we already had in mainline was removed. And readding parts of
> the removed code doesn't make sense either because the patch series is
> based on that old Ingenic SoC code.
> 

I have to say I disagree with all of this.

The open coded drivers are used in SPL, which most likely can't affort
any DM. Or **if** it can't afford it, it's a separate question that
needs research.

The BootROM for this SoC can only load 14 KiB of code, and we are already
at roughly 11 KiB.

This series, on the other side, is quite clean, quite small, has quite
simple drivers (with even a DM MMC driver for the second stage)...
... and most importantly it works.

Given how much history this particular series has, if we delay it now
with yet more requirements, we risk not ever having it.

We want to put this board on kernelCI labs, and having upstream U-Boot
would be really nice for that, so we don't rely on ancient vendor forks.

As for a maintainer doing the work, I'm here and not going anywhere.
I can do any follow-up work that's considered needed, including any DM
conversions.

But, it would be really important to have a working starting point.

> I wanted to work on Marek's patch series to fix those issues because
> it's quite some work which I couldn't expect from Marek ;)
> But I got stuck with EJTAG because halting the CPU didn't work. Then I
> hadn't time anymore.
> 
> I'm little busy right now so I can't do a detailed review. But the
> issues which should be addressed at first are:
> 
> - fix all SPDX license identifiers
> - convert GPIO driver to DM
> - add a DM watchdog driver
> - implement machine reset with the generic watchdog reset (see BMIPS for
> an example)
> - don't use SoC specific start.S and u-boot-spl.lds, the generic code is
> now configurable enough and non-generic things could be done in a
> lowlevel_init.S
> - reduce the hundreds of definitions of register addresses to the ones
> really needed in assembly or SPL. This stuff should come from device tree
> - define the remaining register base addresses as physical addresses and
> establish a mapping with ioremap_nocache()
> 
> 

This really sounds that work that can be done as follow-up patches.
And, we don't know if any of this will bloat the SPL beyond the current
limit :-)

I must admit, I have a hard time understanding this blocker policy,
for a series that gives support to a new board, and affects no other platforms.

Thanks,
Ezequiel