From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dirk Behme Date: Sun, 27 Dec 2009 19:55:02 +0100 Subject: [U-Boot] Remove board specific code from ENC28J60 network driver? In-Reply-To: References: <4B2E7B5B.3050603@googlemail.com> <200912201505.51636.vapier@gentoo.org> <4B350B33.3040702@googlemail.com> <200912261340.46179.vapier@gentoo.org> <4B3713D9.7060100@googlemail.com> Message-ID: <4B37AD86.4030501@googlemail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 27.12.2009 16:32, Ben Warren wrote: > Hi Dirk, > > On Sat, Dec 26, 2009 at 11:59 PM, Dirk Behmewrote: > >> 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