[U-Boot] [PATCH] NET: add ENC28J60 driver using SPI framework

Reinhard Meyer u-boot at emk-elektronik.de
Thu Aug 26 08:19:07 CEST 2010


Dear Mike Frysinger,
>> From: Reinhard Meyer<info at emk-elektronik.de>

Give me a tip how I can change that info. The first patch to this was
committed while I used <info>. Squashing other patches into it does not
change that. rebase -i, edit, --amend does not present that line for
edit. The only idea I have now is making an empty commit and squashing
that before the original patch...

> also, this patch seems to depend on some other change not in mainline or Ben's
> net tree ...

Should have mentioned that the "move enc28j60 to sidetrack" patch is to be
applied first.

>> +static u8 enc_r8 (u16 regNo);
>> +static void enc_rbuf (u16 length, u8 *pBuff);
>
> this whole file needs to be checked for function style.  no space after the
> function name.

That's what happens if one reworks an existing driver. I'll recheck all this.

> why do you need to declare your own buffer ?  the common code already sets up
> one for you and it uses the standard 1518 size (PKTSIZE).

Ok, better. Just hint me on its name, browsing the common code none did catch
my eye:)

> pass the bus/cs/speed/mode in to the initialize function and store it in the
> per-device state.  then you can work fine with multiple enc28j60 devices in
> one board.

<joke>
Ok, I was already planning to build a 16 port hub using 16 such devices ;)
</joke>
But generalization is good. I might end up using two, actually.

> also, nowhere in this init func can i see error checking that the enc28j60
> device is actually out there.  the enc_init() should return an error if the
> expected phy's/etc... dont match, and this init func should key off that.

I'd like to even have the phy start auto-negotiation at that moment, I'm
just not sure that's ok. I won't wait for it to complete, of course, just
trigger it.

> once this func is fixed to take the spi bus/cs, the name can be based on that
> instead of an arbitrary integer.
> 	"enc%i.%i", bus, cs

setenv ethact enc1.13 - one can get used to that ;)

Reinhard


More information about the U-Boot mailing list