[U-Boot] [PATCH 1/3] Add nitrogen6x board

[Eric Nelson]

Hi Stefano,

On 10/08/2012 05:54 AM, Stefano Babic wrote:
> On 05/10/2012 22:03, Eric Nelson wrote:
>> On 10/05/2012 12:00 PM, Tom Rini wrote:
>>> On Fri, Oct 05, 2012 at 11:42:19AM -0700, Eric Nelson wrote:
>>>> On 10/05/2012 10:24 AM, Albert ARIBAUD wrote:
>>>>> Hi Eric,
>>>>>
>>>>> On Thu,  4 Oct 2012 12:49:07 -0700, Eric Nelson
>>>>> <eric.nelson at boundarydevices.com>    wrote:
>>>>>
>>>>>> Signed-off-by: Eric Nelson<eric.nelson at boundarydevices.com>
>>>>>> ---
>>>>>>    board/boundary/nitrogen6x/Makefile     |   41 ++
>>>>>>    board/boundary/nitrogen6x/README       |   77 +++
>>>>>>    board/boundary/nitrogen6x/nitrogen6x.c |  840
>>>>>> ++++++++++++++++++++++++++++++++
>>>>>>    boards.cfg                             |    1 +
>>>>>>    include/configs/nitrogen6x.h           |  242 +++++++++
>>>>>>    5 files changed, 1201 insertions(+), 0 deletions(-)
>>>>>>    create mode 100644 board/boundary/nitrogen6x/Makefile
>>>>>>    create mode 100644 board/boundary/nitrogen6x/README
>>>>>>    create mode 100644 board/boundary/nitrogen6x/nitrogen6x.c
>>>>>>    create mode 100644 include/configs/nitrogen6x.h
>>>>>
>>>>> If this is essentially a copy of sabrelite, I am surprised that git
>>>>> does not report any copies. Did you use -C with git format-patch ?
>>>>>
>>>>
>>>> Hi Albert,
>>>>
>>>> I didn't use '-C' (didn't know about it: thanks for the tip!).
>>>>
>>>> It wouldn't have detected copies though, because I made slight
>>>> changes in each of these files, replacing the board names and
>>>> file names in board/boundary/ and altering the default environment
>>>> (policy bits) in nitrogen6x.h.
>>>
>>> And we can't deal with this by factoring the code differently?
>>>
>> Hi Tom,
>>
>> There are two bits to this question:
>>      - Can we represent the policy differences outside of a
>>      board structure? These differences are all inside of
>>      include/configs/nitrogen6x.
>>
>
> What we have already done is to set a common config header that is
> included by both include/configs/nitrogen6x and include/configs/sabrelite.
> See for example mx6qsabre_common.h, with the boards mx6qsabresd and
> mx6qsabreauto.
>

The question I have is "what is common?". Looking at mx6qsabre_common.h,
it seems that there's a mixture of things that might be common to
all i.MX6-based boards (inclusion of imx-regs is the clear example),
but most of this header file is enforcing policy like
	#define CONFIG_CMDLINE_EDITING
and
	#define CONFIG_ENV_IS_IN_MMC

>>      - Can we represent the board differences without a
>>      board structure? This is a bit harder, since the
>>      boards are slightly different. The Nitrogen6X has
>>      a different ethernet PHY reset pin and an optional
>>      SDIO Wi-Fi module.
>
> Ona major point is *how* you want to represent your board into U-boot,
> even if it is derived from a Freescale's evaluation board. In many case
> a vendor directory is desired (freescale vs boundary).
>

It's nice to have some brand exposure, but mostly we just want
to make things simple to define and maintain so we can help
get products to market.

>>
>>      We could add code to SABRE Lite to accommodate these,
>>      but it seems that sets a bad precedent. Would this
>>      be done for every vendor that bases a design on
>>      SABRE Lite?
>
> I am afraid that it is easy to reach the case when changes for a vendor
> will break other boards, and getting all in sync can be problematic.
>

I agree. That, and the question of policy are the primary
drivers for the separate board directory.

>>
>> The precise diffs for the configs and sources is attached for
>> reference.
>>
>> I've also been pondering how to simply re-use the code within
>> the board setup file (mx6qsabrelite.c), but I haven't figured
>> anything out. Clearly a lot of the code is duplicated, but at
>> the same time it's board-specific.
>>
>> For example, we could create a common module that sets up
>> the SD card pads "like SABRE Lite", and a similar one to
>> configure ethernet pads. Since SABRE Lite is a reference design,
>> perhaps that makes sense.
>
> I think that another example in u-boot doing this is for davinci (8xx)
> boards, At least three boards share the same board structure, see
> board/davinci/da8xxevm/Makefile.
>
> Very board related functions are compiled using CONFIG_MACH:
>
> COBJS-$(CONFIG_MACH_DAVINCI_DA830_EVM)  += da830evm.o
> COBJS-$(CONFIG_MACH_DAVINCI_DA850_EVM)  += da850evm.o
> COBJS-$(CONFIG_MACH_DAVINCI_HAWK)       += hawkboard.o
>

Thanks for the pointers. I was about to ask Tom whether he
could point me at specific cases to help illustrate what he's
thinking.

That said, I'm not quite grokking how the commonality of these
boards is expressed.

In some ways, I think Nitrogen6X is a case study in how a
board derived from a reference design can be supported and
maintained in U-Boot, either in the main-line tree or off-line
as many vendors do.

-- The schematics are almost identical, so 99% of the pad settings
are the same.

-- The native peripheral set for Nitrogen6X is a superset of those on
Sabre Lite.

-- The set of accessories supplied for both is the same (Android
button board, LVDS and RGB displays).

It seems a useful exercise at least to look at what can be done
to express this in a useful and scalable way.

Board designers often cut & paste big hunks of schematic when
building a custom board based on a reference design. For example,
the mx6qsabreauto design also seems to have DISP0 mapped to a
40-pin parallel display connector with the same I2C interface
as SABRE Lite and Nitrogen6x.

For our part, we actively encourage that for our Nitrogen6X
SOM customers to help speed them to market.

I think there's a way of mapping these decisions through to
board-specific files.

Because talking about in the abstract is difficult, I took a
stab at coding this over the weekend and found that the easiest
bits to express are the pad/pinmux settings.

This also has the biggest impact.

I'd like to propose that we place something like this
in a common header corresponding to the reference design:

	#define SABRELITE_RGB_PADS \
		MX6Q_PAD_DI0_DISP_CLK__IPU1_DI0_DISP_CLK,	\
		MX6Q_PAD_DI0_PIN15__IPU1_DI0_PIN15,		\
		...

Then, in board-specific file, the implementer could re-use
them like so:
	static iomux_v3_cfg_t const rgb_pads[] = {
	        SABRELITE_RGB_PADS
	};

To follow through, if a board designer uses the Atheros ethernet
PHY, they might use bits from the SABREAUTO design:

	iomux_v3_cfg_t const enet_pads[] = {
		SABREAUTO_ENET_PADS
	};

If another board really does require different muxing or
pad setup, there's nothing precluding cut & paste.

As much as I dislike excessive macro-fu, using macros rather
than external data declarations will avoid any symbol name
conflicts or data segment bloat for unused bits.

If changes are needed, it should also be simple to grep for
boards using, say "SABRELITE_RGB_PADS" if an when an
update is needed. For example, it's possible that the
choice of 120 ohm drive strength is sub-optimal. If we
determine that 80 ohms is better in the general case,
we could change it in one place, but a board could
easily override it if they have a particular panel
that needs something else.

As Stefano pointed out, other bits of re-factoring of the
SABRE Lite code are possible and appropriate. In particular:

	- keyboard handling via GPIOs could go into drivers/input
	as a proper "gpio_keyboard" driver or somesuch.

	- some parts of display support could be moved into
	separate modules (though I'm not yet sure where all
	of the dividing lines should go).

If this is done, it seems that very little is left.

For example, is there a way to meaningfully make this bit of
common code between SABRE Lite and Nitrogen6X modular?

	int misc_init_r(void)
	{
	#ifdef CONFIG_PREBOOT
		preboot_keys();
	#endif

	#ifdef CONFIG_CMD_BMODE
		add_board_boot_modes(board_boot_modes);
	#endif
		return 0;
	}

If so, is there a point? I'm not sure.

As always, let me know your thoughts.

Clearly, all of this needs feedback from Fabio and Jason.

Regards,


Eric