All of lore.kernel.org
 help / color / mirror / Atom feed
From: Reinhard Meyer <u-boot@emk-elektronik.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] NET: add ENC28J60 driver using SPI framework
Date: Thu, 26 Aug 2010 08:19:07 +0200	[thread overview]
Message-ID: <4C76075B.1080309@emk-elektronik.de> (raw)
In-Reply-To: <201008252355.22876.vapier@gentoo.org>

Dear Mike Frysinger,
>> From: Reinhard Meyer<info@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

  reply	other threads:[~2010-08-26  6:19 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-25 12:47 [U-Boot] [PATCH] NET: add ENC28J60 driver using SPI framework Reinhard Meyer
2010-08-26  3:55 ` Mike Frysinger
2010-08-26  6:19   ` Reinhard Meyer [this message]
2010-08-26  6:32     ` Mike Frysinger
2010-08-26  6:41       ` Reinhard Meyer
2010-08-26  7:11         ` Mike Frysinger
2010-09-03 17:50   ` Reinhard Meyer
2010-09-03 19:51     ` Mike Frysinger
2010-09-03 20:31       ` Reinhard Meyer
2010-09-03 21:03         ` Mike Frysinger
2010-09-03 21:23           ` Reinhard Meyer
2010-09-03 22:14             ` Mike Frysinger
2010-09-04  3:25               ` Reinhard Meyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C76075B.1080309@emk-elektronik.de \
    --to=u-boot@emk-elektronik.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.