[U-Boot] Remove board specific code from ENC28J60 network driver?

[Dirk Behme]

On 27.12.2009 16:32, Ben Warren wrote:
> Hi Dirk,
>
> On Sat, Dec 26, 2009 at 11:59 PM, Dirk Behme<dirk.behme at googlemail.com>wrote:
>
>> On 26.12.2009 19:40, Mike Frysinger wrote:
>>> On Friday 25 December 2009 13:57:55 Dirk Behme wrote:
>>>> I started to convert the enc28j60.c to common SPI framework. Do you
>>>> like to have a look at attachment (and maybe test it?)?
>>>>
>>>> It is compile tested only. And for the moment it just re-uses the
>>>> existing driver. When we know that it basically works this way, doing
>>>> it in a clean way as you describe above would be the next step.
>>>> CONFIG_NET_MULTI is still missing, too.
>>>
>>> spi_lock/spi_unlock should redirect to spi_claim_bus/spi_release_bus.
>>   this
>>> isnt so much an "interrupts" issue as the process of claiming the bus
>>> reprograms the controller with the slave settings.
>>>
>>>> In your config file you have to set and configure
>>>>
>>>> #define CONFIG_xxx_SPI
>>>> #define CONFIG_ENC28J60
>>>> #define CONFIG_ENC28J60_SPI_BUS              0
>>>> #define CONFIG_ENC28J60_SPI_CS               0
>>>> #define CONFIG_ENC28J60_SPI_CLK              1000000
>>>> #define CONFIG_CMD_NET
>>>>
>>>> for your board.
>>>
>>> this is ok with the current design, but broken for NET_MULTI.  when
>> converted
>>> to NET_MULTI, the new enc28j60_register() function will take the spi
>> settings
>>> as function arguments.  so the function would look something like:
>>> int enc28j60_register(bd_t *bis, unsigned int spi_bus, unsigned int
>> spi_cs,
>>> unsigned int max_hz, unsigned int mode);
>>>
>>> and it'd be up to the board to call it with the settings it wants
>>
>> Both changes, enc28j60_initialize() (NET_MULTI enabled) and
>> spi_claim_bus/spi_release_bus done in below.
>>
>> In the the board file I now have
>>
>> int board_eth_init(bd_t *bis)
>> {
>>         int rc = 0;
>> #ifdef CONFIG_ENC28J60
>>         rc = enc28j60_initialize(bis,
>>                                  CONFIG_ENC28J60_SPI_BUS,
>>                                  CONFIG_ENC28J60_SPI_CS,
>>                                  CONFIG_ENC28J60_SPI_CLK,
>>                                  SPI_MODE_3);
>> #endif
>>         return rc;
>> }
>>
>> This is the right way to do it.  Thanks for taking on this task.  One
> comment is that I'm not sure you need to pass 'bis' to the initialize()
> function.

 From functionality point of view it's not needed. The API was 
proposed by Mike this way. But yes, 'bis' can be removed.

>> Do you like to test? Any further comments?
>>
> I'm sure you know that converting to CONFIG_NET_MULTI involves a lot more
> than renaming the initialize() functions.

Could you give some examples or hints, what you like to see? Just in 
case I missed the details ;)

> Looking forward to the full
> version.

First I'd like to hear if it basically works for someone. If not, it 
makes no sense to go on ;)

> I wish I had hardware to help you, but will be happy to review
> once you're ready.

Mike mentioned that he eventually could test it on Blackfin boards.

Many thanks for review,

Dirk