[U-Boot] [PATCH 4/5] e1000: New "e1000" commands for SPI EEPROM management

[Moffett, Kyle D]

On Apr 13, 2011, at 01:23, Wolfgang Denk wrote:
> In message <0EF7E520-FF4A-435F-AF2A-0D47C0951B34 at boeing.com> you wrote:
>> In particular, those other eeprom drivers simply have a single hardcoded
>> I/O base address that they assume is properly mapped, IE:
>>>  struct eth_device dev;
>>>  dev.iobase = CONFIG_SMC911X_BASE;
>> 
>> That won't really work with the way the existing E1000 driver is set
>> up; it expects to run with full PCI device access using the standard
>> U-Boot PCI calls.  It seems to get very confused if you poke at the
>> hardware behind its back.
> 
> I'm not really sure how this is related to the EEPROM access part of
> the code.  This is functionally separate from the network driver,
> isn't it?
> 

> And I don't really see where the EEPROM part needs to be PCI aware.

No, the EEPROM driver relies upon the network driver having initialized
the hardware correctly (including mapping PCI BARs, etc).  The SPI bus
to the EEPROM is also shared with the network hardware and firmware, and
requires the use of other shared arbitration register bits.

Given that the E1000 is always a PCI device, I can't see why you would
ever build an E1000 EEPROM programmer which was *not* PCI aware.

I honestly have no clue what IO address the E1000 BARs actually get
mapped at, nor do I really care; that is (and should always be) entirely
abstracted away by the U-Boot PCI layer.


>> Finally, from an actual operational standpoint, this EEPROM driver is
>> designed to allow the user to program up to 64kB of information to the
>> E1000 EEPROM, including manageability firmware and other stuff; speed
>> and copy-to/from-memory operations are very important.
> 
> These should be important to all kinds of drivers.

I agree, but for the other ethernet eeprom drivers I don't think it
matters.  From what I understand the existing programmers support at
most 256-byte EEPROMs (because that is what the hardware supports).
The E1000 is unique among the ethernet eeprom drivers in U-Boot because
of the size of the chip.


>> The other EEPROM "apps" only really support changing individual bytes
>> one-at-a-time and reprogramming the MAC address, which is insufficient
>> for initial hardware load.
> 
> I wonder if we could / should unify all this code.

It looks like most of the programmers right now use SPI, although from
what I remember of the Linux code some of the E1000 chipsets use a
different register interface to program other kinds of Flash memory
(I think the embedded E1000 in the ICH/PCH chipsets do this).

The only real obstacle right now is that there can only be one SPI host
driver in U-Boot at compile-time.  To fix this we would need to create a
"struct spi_host" abstraction with function pointers to allow multiple
different hosts to be registered as bus-0, bus-1, etc.

Then the "EEPROM" commands could just talk to the appropriate device on
each SPI bus.


>> If you agree then I will modify the patch to move the new code into a
>> separate e1000_eeprom.c file which is conditionally compiled, but
>> still linked into the main U-Boot image.
> 
> That's OK with me. [As long as it remains optional.]

Ok, I'll make the necessary changes and resubmit.

Cheers,
Kyle Moffett