[U-Boot] [RFC][PATCH 3/3] Add board support for the eXMeritusHWW-1U-1A devices

[Moffett, Kyle D]

On Sep 07, 2010, at 18:09, Peter Tyser wrote:
>>> The GPIO functions above aren't hww1u1a specific.  What about adding
>>> generic 85xx GPIO functions so others can use them too?
>> 
>> I can do that.  Do you have any particular place you recommend I put them?
> 
> The 2 places that jump to mind are drivers/gpio, or
> arch/powerpc/cpu/mpc8xxx.  My personal preference would be drivers/gpio.

Hmm, given that they all seem to inline smaller than a function call would be, I've put them as "static inline" functions in:
  arch/powerpc/include/asm/mpc85xx_gpio.h

Is that OK?


>>> Is the above check the same as the hww1u1a_is_cpu_a() function
>>> previously added?
>> 
>> Oops, yes it is, will fix.  That reminds me of one other question,
>> though.  Several variables are different between the CPU A and the CPU
>> B on the board but maintaining 2 nearly-identical device trees is kind
>> of a pain.  Is there any good way to manage that?  Should I try to
>> poke the device tree file during boot, or is there some kind of macro
>> language I can use in the dts file? (EG: "#ifdef CPU_A", etc)
> 
> There isn't a macro language in the dts file that you can use.  Its hard
> to say if you should tweak the dtb in U-Boot.  People's opinions vary:
> http://www.mail-archive.com/u-boot@lists.denx.de/msg36029.html
> 
> I'd give modifying the dtb in U-Boot a try, and people will let you know
> if its outside of the scope of U-Boot's responsibility.

Hmm, seems like a lot of extra work to try fiddling with the DTB for very little gain.

I think at this point I'll just implement an "is_cpu_a" command and use it from a hush script something like this:

  if is_cpu_a; then
    bootm ${flkernel} ${flinitramfs} ${fldevicetree_cpua}
  else
    bootm ${flkernel} ${flinitramfs} ${fldevicetree_cpub}
  fi

Does that seem OK?


>>>> +/* Flash configuration registers */
>>>> +#define CONFIG_SYS_BR0_PRELIM (BR_PHYS_ADDR(FLASH0_PHYS) | BR_PS_16 | BR_V)
>>>> +#define CONFIG_SYS_BR1_PRELIM (BR_PHYS_ADDR(FLASH1_PHYS) | BR_PS_16 | BR_V)
>>>> +#define CONFIG_SYS_OR0_PRELIM 0xf8000ff7
>>>> +#define CONFIG_SYS_OR1_PRELIM 0xf8000ff7
>>> 
>>> There are ORx defines.  They should be used to make it clear what the
>>> settings above are at a glance.
>> 
>> Ok, I'll have to go look up what those bits mean since I copied them
>> unmodified from the P2020DS board.
> 
> If you are using different localbus devices and/or board clocking from
> the p2020ds, it'd be good to review the settings as they likely could be
> improved for your specific setup.

The default settings are the most pessimistic configuration parameters for the onboard FLASH.  As it is plenty fast for our purposes right now, I'm disinclined to risk breaking this particular piece of code right now.  I did rewrite it so it doesn't use magic numbers anymore, but I'm not sure it makes it much more readable :-D.


>>>> +/* PCI-E dual-port E1000 (external ethernet ports) */
>>>> +#define CONFIG_E1000 1
>>>> +#define CONFIG_E1000_FALLBACK_MAC { 0x00, 0x05, 0x93, 0x81, 0xff, 0xfe }
>>> 
>>> The general U-Boot policy is to not set default MACs.
>> 
>> The e1000e chips on our board need their EEPROMs to be programmed by
>> software during provisioning.  At one point I think we needed this in
>> order for the programming to work successfully, but it's possible that
>> no longer applies; I'll go verify it.  Regardless, this particular MAC
>> address is one I selected from our suballocation specifically for this
>> purpose.
> 
> The general philosophy is that U-Boot shouldn't include default MAC
> addresses as inevitably at some point you'll have multiple boards on the
> network with identical MACs, or customers will corrupt their EEPROM and
> fall back to the default MAC, etc.

Well, due to the design of the E1000E hardware, a corrupted EEPROM (as opposed to an unprogrammed EEPROM) usually just causes the whole E1000E chip to drop off the PCI bus...

I'm having other functional annoyances with our programming process right now, so I'll drop this part for now and revisit it later once I've got those isolated.

Cheers,
Kyle Moffett