at91 SoC separation

[Sylvestre Gallon]

On Mon, May 25, 2009 at 12:38 AM, M. Warner Losh <imp at bsdimp.com> wrote:
>
> I've been thinking about the issues for a long time.  And there's a
> number of interrelated problems.  All of them are related to the fact
> that we have the core, SoC and board smashed into one file right now.
> We really need to finish separating out the core support (it is mostly
> done right now, except for some of the bus space stuff).  The SoC and
> board support is all in at91.c, which really should be about
> infrastructure not about boards...
>
> First, you need to know what board you are on when you boot.  uboot
> and others pass this information into the kernel using register r3,
> IIRC.  So, there would need to be a way for the early boot code to
> probe for one of a number of different boards.  For SoC, this could be
> just one board compiled into the kernel (eg, you don't have run time
> select), but Sam has done some work in this area on the xscale parts
> you may be able to snag.

It is what I've tried to do for SoC:

Each board must have a device with a SoC name :

http://perforce.freebsd.org/fileViewer.cgi?FSPC=//depot/projects/soc2009/syl_usb/src/sys/arm/at91/std.bwct&REV=2
http://perforce.freebsd.org/fileViewer.cgi?FSPC=//depot/projects/soc2009/syl_usb/src/sys/arm/at91/std.at91sam9261ek&REV=1

This allow the compile of the good SoC file :

http://perforce.freebsd.org/fileViewer.cgi?FSPC=//depot/projects/soc2009/syl_usb/src/sys/arm/at91/files.at91&REV=3

The SoC file contains the code SoC dependent :

http://perforce.freebsd.org/fileViewer.cgi?FSPC=//depot/projects/soc2009/syl_usb/src/sys/arm/at91/soc_at91rm9200.c&REV=4
http://perforce.freebsd.org/fileViewer.cgi?FSPC=//depot/projects/soc2009/syl_usb/src/sys/arm/at91/soc_at91sam9261.c&REV=3

All this changes allow me to move at91.c into a code SoC independent :

http://perforce.freebsd.org/fileViewer.cgi?FSPC=//depot/projects/soc2009/syl_usb/src/sys/arm/at91/at91.c&REV=3

I think this is important to have a SoC file because there are some
big changes between each SoC the at91_master_clock, cpu_devs
and the pmap_devmap are not the same on each SoC.

>
> If we assume that there's a board init routine, we can assume that
> init routine knows what kind of SoC is installed on the board and can
> populate the atmelbus with the right devices by calling something like
> at91rm9200_soc_init().  Then, that board routine can also do the
> proper routing of the pins to the different peripherals as well,
> possibly calling some generic peripheral init routines as well
> (although that's a lot harder to arrange than you might otherwise
> think given just how flexible these things are).  The board routine
> would also be responsible for calling cninit() as well.
>
> If we go this route, we can eliminate much of the #ifdef soup that you
> were otherwise looking at doing.  The AT91RM9200 registers would be
> defined in one place, but we'd try to use the base + offset approach
> to isolate the number places that need this information.  That's one
> of the problems with the current set of patches for the at91sam
> boards: they just do ifdefs rather than do the proper refactoring to
> get us the most amount of support.
>
> So the typical board package would look something like:
>
> int
> kb9202_probe()
> {
>        if (arm_board_id_from_loader() != KB9202_BOARD_ID)
>                return ENXIO;
>        return 0;
> }
>
> int
> kb9202_init()
> {
>        at91rm9200_soc_init();
>        /*
>         * <insert the contents from board_kb920x.c's board_init()
>         * here>
>         */
>        /*
>         * Map in the pmap entries needed on this board for flash, CF,
>         * etc
>         */
>        /*
>         * Create some kind of descriptor to tell the SD card what GPIO
>         * pins are used for different things, what the MAC address
>         * of the board is, etc.  Maybe need one per driver and a list
>         * of them for this board
>         */
> }
>
> ARM_BOARD_TYPE(kb9202_probe, kb9202_init);
>
> As an aside, you may also need to make the different boot loaders
> pluggable as well, or maybe we would do that as part of another pass.
> You could assume they were all uboot compatible or something...  Linux
> defines a standard here, and we'd be wise to follow it...
>
> Obviously, you'd need to rewrite arm_init as well to defer some of the
> things it does now.  You'd need to move pmap init stuff to the
> at91rm9200_soc_init(), except for the CF/FLASH stuff, which would move
> to a board-level init.  Some of the mappings might need to change that
> are done currently inline (which may mean we'd have to do some much
> needed refactoring here).  Right now board_init is in a very special
> place, and we'd likely need to move the cninit() into each board init
> so that the boards could control which console was used.

The cninit has not move now but I have also moved pmap init stuff into
the SoC file :

http://perforce.freebsd.org/fileViewer.cgi?FSPC=//depot/projects/soc2009/syl_usb/src/sys/arm/at91/soc_at91rm9200.c&REV=4
http://perforce.freebsd.org/fileViewer.cgi?FSPC=//depot/projects/soc2009/syl_usb/src/sys/arm/at91/soc_at91sam9261.c&REV=3

For that have done some little change in the machedep.c to be
SoC independent :

http://perforce.freebsd.org/fileViewer.cgi?FSPC=//depot/projects/soc2009/syl_usb/src/sys/arm/at91/at91_machdep.c&REV=2

>
> There's also some ideas going around about being able to load hints
> dynamically at run time and the foo_soc_init() and board_init routines
> would just load them up correctly.  That's another orthogonal way to
> move the tables out of at91rm9200_soc_init and into an easier to
> manage text form, but at the expense of needing to add code to the
> hints mechanism, and fight the bike-sheds that surround its
> replacement.  So I'd frankly avoid this for the SoC project.  Getting
> good separation is more important than having a perfect mechanism the
> separation can use.

I am totally agree.

Thanks for your help :)

Cheers,

-- 
Sylvestre Gallon (http://devsyl.blogspot.com)
Fifth Grade Student @ Epitech & Researcher @ LSE
R&D @ Rathaxes (http://www.rathaxes.org)