[U-Boot] eth/net driver documentation
is there any documentation that covers the NET_MULTI driver stack ? i.e. if i was a network chip vendor and wanted to write a u-boot driver for my part, where would i look ? or is it a matter of "the code is the documentation" ? i can start a small doc that covers the existing stack if that is the case ... more to keep my sanity so i dont have to keep going through the ethernet stack to remind myself of how things fit together ;). -mike
Hi Mike, Mike Frysinger wrote:
is there any documentation that covers the NET_MULTI driver stack ? i.e. if i was a network chip vendor and wanted to write a u-boot driver for my part, where would i look ? or is it a matter of "the code is the documentation" ? i can start a small doc that covers the existing stack if that is the case ... more to keep my sanity so i dont have to keep going through the ethernet stack to remind myself of how things fit together ;). -mike
Isn't the code nicely self-documenting? :) I think yours is a great idea. It's a pretty simple interface, but not necessarily intuitive. I'll be glad to help if needed.
regards, Ben
The net code is mostly consistent in using 'Packet' rather than 'Pkt', so rename the minor detractor to follow suite.
Signed-off-by: Mike Frysinger vapier@gentoo.org --- include/net.h | 4 ++-- net/bootp.c | 2 +- net/net.c | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/net.h b/include/net.h index 5a1d36e..88a9513 100644 --- a/include/net.h +++ b/include/net.h @@ -331,8 +331,8 @@ extern IPaddr_t NetOurIP; /* Our IP addr (0 = unknown) */ extern IPaddr_t NetServerIP; /* Server IP addr (0 = unknown) */ extern volatile uchar * NetTxPacket; /* THE transmit packet */ extern volatile uchar * NetRxPackets[PKTBUFSRX];/* Receive packets */ -extern volatile uchar * NetRxPkt; /* Current receive packet */ -extern int NetRxPktLen; /* Current rx packet length */ +extern volatile uchar * NetRxPacket; /* Current receive packet */ +extern int NetRxPacketLen; /* Current rx packet length */ extern unsigned NetIPID; /* IP ID (counting) */ extern uchar NetBcastAddr[6]; /* Ethernet boardcast address */ extern uchar NetEtherNullAddr[6]; diff --git a/net/bootp.c b/net/bootp.c index 77057c6..d5f9c4b 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -124,7 +124,7 @@ static void BootpCopyNetParams(Bootp_t *bp) NetCopyIP(&tmp_ip, &bp->bp_siaddr); if (tmp_ip != 0) NetCopyIP(&NetServerIP, &bp->bp_siaddr); - memcpy (NetServerEther, ((Ethernet_t *)NetRxPkt)->et_src, 6); + memcpy (NetServerEther, ((Ethernet_t *)NetRxPacket)->et_src, 6); #endif if (strlen(bp->bp_file) > 0) copy_filename (BootFile, bp->bp_file, sizeof(BootFile)); diff --git a/net/net.c b/net/net.c index 5637cf5..e215fd8 100644 --- a/net/net.c +++ b/net/net.c @@ -139,8 +139,8 @@ uchar NetServerEther[6] = /* Boot server enet address */ { 0, 0, 0, 0, 0, 0 }; IPaddr_t NetOurIP; /* Our IP addr (0 = unknown) */ IPaddr_t NetServerIP; /* Server IP addr (0 = unknown) */ -volatile uchar *NetRxPkt; /* Current receive packet */ -int NetRxPktLen; /* Current rx packet length */ +volatile uchar *NetRxPacket; /* Current receive packet */ +int NetRxPacketLen; /* Current rx packet length */ unsigned NetIPID; /* IP packet ID */ uchar NetBcastAddr[6] = /* Ethernet bcast address */ { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; @@ -1122,8 +1122,8 @@ NetReceive(volatile uchar * inpkt, int len) printf("packet received\n"); #endif
- NetRxPkt = inpkt; - NetRxPktLen = len; + NetRxPacket = inpkt; + NetRxPacketLen = len; et = (Ethernet_t *)inpkt;
/* too small packet? */
Mike,
Mike Frysinger wrote:
The net code is mostly consistent in using 'Packet' rather than 'Pkt', so rename the minor detractor to follow suite.
Signed-off-by: Mike Frysinger vapier@gentoo.org
include/net.h | 4 ++-- net/bootp.c | 2 +- net/net.c | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-)
Applied to net repo.
thanks, Ben
Signed-off-by: Mike Frysinger vapier@gentoo.org --- Ben: some things to note: - i adopted Jean's proposed naming scheme in the CONFIG section - i deprecated calling the driver-specific entry point "xxx_initialization()" in favor of "xxx_register()" because the former is way too confusing with everyone also having "xxx_init()"
doc/README.drivers.eth | 199 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 199 insertions(+), 0 deletions(-) create mode 100644 doc/README.drivers.eth
diff --git a/doc/README.drivers.eth b/doc/README.drivers.eth new file mode 100644 index 0000000..00b4eb1 --- /dev/null +++ b/doc/README.drivers.eth @@ -0,0 +1,199 @@ +----------------------- + Ethernet Driver Guide +----------------------- + +The networking stack in Das U-Boot is designed for multiple network devices +to be easily added and controlled at runtime. This guide is meant for people +who wish to review the net driver stack with an eye towards implementing your +own ethernet device driver. Here we will describe a new pseudo 'APE' driver. + +---------------- + CONFIG Options +---------------- + +The common config defines your device should respect (if applicable): + CONFIG_MII - configure in MII mode + CONFIG_RMII - configure in RMII mode + + CONFIG_CMD_MII - support the MII command + +If you want to add config options specific to your driver, you should use the +naming convention: + CONFIG_NET_<DRIVERNAME>_<OPTION> +For example, if the APE driver could operate in either 16bit or 32bit mode, +there would probably be the option: + CONFIG_NET_APE_32BIT - operate in 32bit mode rather than 16bit default + +And to control whether your driver is even enabled, you should use: + CONFIG_SYS_NET_<DRIVERNAME> +So the APE driver would look like: + CONFIG_SYS_NET_APE + +------------------ + Driver Functions +------------------ + +All functions you will be implementing in this document have the return value +meaning of 0 for success and non-zero for failure. + + ---------- + Register + ---------- + +When U-Boot initializes, it will call the common function eth_initialize(). +This will in turn call the board-specific board_eth_init() (or if that fails, +the cpu-specific cpu_eth_init()). These board-specific functions can do random +system handling, but ultimately they will call the driver-specific register +function which in turn takes care of initializing that particular instance. + +Keep in mind that you should code the driver to avoid storing state in global +data as someone might want to hook up two of the same devices to one board. If +the state is maintained as global data, it makes using both of those devices +impossible. + +So the call graph at this stage would look something like: +board_init() + eth_initialize() + board_eth_init() / cpu_eth_init() + driver_register() + initialize eth_device + eth_register() + +At this point in time, the only thing you need to worry about is the driver's +register function. The pseudo code would look something like: +int ape_register(bd_t *bis, int iobase) +{ + struct ape_priv *priv; + struct eth_device *dev; + + priv = malloc(sizeof(*priv)); + if (priv == NULL) + return 1; + + dev = malloc(sizeof(*dev)); + if (dev == NULL) { + free(priv); + return 1; + } + + /* setup whatever private state you need */ + + memset(dev, 0, sizeof(*dev)); + sprintf(dev->name, "APE"); + + /* if your device has dedicated hardware storage for the + * MAC, read it and initialize dev->enetaddr with it + */ + ape_mac_read(dev->enetaddr); + + dev->iobase = iobase; + dev->priv = priv; + dev->init = ape_init; + dev->halt = ape_halt; + dev->send = ape_send; + dev->recv = ape_recv; + + eth_register(dev); + +#ifdef CONFIG_CMD_MII) + miiphy_register(dev->name, ape_mii_read, ape_mii_write); +#endif + + return 0; +} + +The exact arguments needed to initialize your device are up to you. If you +need to pass more/less arguments, that's fine. You should also add the +prototype for your new register function to include/netdev.h. You might notice +that many drivers seem to use xxx_initialize() rather than xxx_register(). +This is the old naming convention and should be avoided as it causes confusion +with the driver-specific init function. + +Other than locating the MAC address in dedicated hardware storage, you should +not touch the hardware in anyway. That step is handled in the driver-specific +init function. Remember that we are only registering the device here, we are +not checking its state or doing random probing. + + ----------- + Callbacks + ----------- + +Now that we've registered with the ethernet layer, we can start getting some +real work done. You will need four functions: + int ape_init(struct eth_device *dev, bd_t *bis); + int ape_send(struct eth_device *dev, volatile void *packet, int length); + int ape_recv(struct eth_device *dev); + int ape_halt(struct eth_device *dev); + +The init function checks the hardware (probing/identifying) and gets it ready +for send/recv operations. You often do things here such as resetting the MAC +and/or PHY, and waiting for the link to autonegotiate. You should also take +the opportunity to program the device's MAC address with the dev->enetaddr +member. This allows the rest of U-Boot to dynamically change the MAC address +and have the new settings be respected. + +The send function does what you think -- transmit the specified packet whose +size is specified by length (in bytes). You should not return until the +transmission is complete, and you should leave the state such that the send +function can be called multiple times in a row. + +The recv function should process packets as long as the hardware has them +readily available before returning. i.e. you should drain the hardware fifo. +The common code sets up packet buffers for you already (NetRxPackets), so there +is no need to allocate your own. For each packet you receive, you should call +the NetReceive() function on it with the packet length. So the pseudo code +here would look something like: +int ape_recv(struct eth_device *dev) +{ + int length, i = 0; + ... + while (packets_are_available()) { + ... + length = ape_get_packet(&NetRxPackets[i]); + ... + NetReceive(&NetRxPackets[i], length); + ... + if (++i >= PKTBUFSRX) + i = 0; + ... + } + ... + return 0; +} + +The halt function should turn off / disable the hardware and place it back in +its reset state. + +So the call graph at this stage would look something like: +some net operation (ping / tftp / whatever...) + eth_init() + dev->init() + eth_send() + dev->send() + eth_rx() + dev->recv() + eth_halt() + dev->halt() + +---------------- + CONFIG_CMD_MII +---------------- + +If your device supports banging arbitrary values on the MII bus (pretty much +every device does), you should add support for the mii command. Doing so is +fairly trivial and makes debugging mii issues a lot easier at runtime. + +After you have called eth_register() in your driver's register function, add +a call to miiphy_register() like so: +#ifdef CONFIG_CMD_MII + miiphy_register(dev->name, mii_read, mii_write); +#endif + +And then define the mii_read and mii_write functions if you haven't already. +Their syntax is straightforward: + int mii_read(char *devname, uchar addr, uchar reg, ushort *val); + int mii_write(char *devname, uchar addr, uchar reg, ushort val); + +The read function should read the register 'reg' from the phy at address 'addr' +and store the result in the pointer 'val'. The implementation for the write +function should logically follow.
Mike Frysinger wrote:
Signed-off-by: Mike Frysinger vapier@gentoo.org
Ben: some things to note:
- i adopted Jean's proposed naming scheme in the CONFIG section
Is this a generally-accepted naming convention? I personally think it's crap, and since there isn't a single driver that uses it yet, you might say this is a bit ahead of the curve.
- i deprecated calling the driver-specific entry point "xxx_initialization()" in favor of "xxx_register()" because the former is way too confusing with everyone also having "xxx_init()"
That may be so, but since there isn't a single driver that uses this naming convention, you're wishing something that ain't so.
Other than that, nice writeup. thanks!
Ben
On Tuesday 21 July 2009 01:09:11 Ben Warren wrote:
Mike Frysinger wrote:
Signed-off-by: Mike Frysinger vapier@gentoo.org
Ben: some things to note:
- i adopted Jean's proposed naming scheme in the CONFIG section
Is this a generally-accepted naming convention? I personally think it's crap, and since there isn't a single driver that uses it yet, you might say this is a bit ahead of the curve.
some style needed to be suggested, and what Jean proposed is better than what we have today (which is nothing)
- i deprecated calling the driver-specific entry point "xxx_initialization()" in favor of "xxx_register()" because the former is way too confusing with everyone also having "xxx_init()"
That may be so, but since there isn't a single driver that uses this naming convention, you're wishing something that ain't so.
that's why i said "should", deprecated current naming, and noted existing practice. if you agree with the proposal, it's easy enough to run sed on a few files to fix one function name. you agree with my comment that today's behavior is confusing even if you stare and bang on the code day in and day out ? it's even worse for the occasional observer ... -mike
Dear Mike Frysinger,
In message 200907210228.09882.vapier@gentoo.org you wrote:
Is this a generally-accepted naming convention? I personally think it's crap, and since there isn't a single driver that uses it yet, you might say this is a bit ahead of the curve.
some style needed to be suggested, and what Jean proposed is better than what we have today (which is nothing)
Arent't we pretty much doing what Linux is doing here, too? I see lots of XXX_init functions in the Linux network code, for example.
that's why i said "should", deprecated current naming, and noted existing practice. if you agree with the proposal, it's easy enough to run sed on a few files to fix one function name. you agree with my comment that today's behavior is confusing even if you stare and bang on the code day in and day out ? it's even worse for the occasional observer ...
Hm... renaming something from "xxx_init()" into "xxx_register()" because other code is also also using "xxx_init()" does not really make anything clearer to me. Actually IMO it just adds confusion, because if other's use "xxx_init()" I'd expect from a consistence point of view that we use "xxx_init()", too.
Best regards,
Wolfgang Denk
On Tuesday 21 July 2009 03:32:55 Wolfgang Denk wrote:
Mike Frysinger wrote:
Is this a generally-accepted naming convention? I personally think it's crap, and since there isn't a single driver that uses it yet, you might say this is a bit ahead of the curve.
some style needed to be suggested, and what Jean proposed is better than what we have today (which is nothing)
Arent't we pretty much doing what Linux is doing here, too? I see lots of XXX_init functions in the Linux network code, for example.
that's why i said "should", deprecated current naming, and noted existing practice. if you agree with the proposal, it's easy enough to run sed on a few files to fix one function name. you agree with my comment that today's behavior is confusing even if you stare and bang on the code day in and day out ? it's even worse for the occasional observer ...
Hm... renaming something from "xxx_init()" into "xxx_register()" because other code is also also using "xxx_init()" does not really make anything clearer to me. Actually IMO it just adds confusion, because if other's use "xxx_init()" I'd expect from a consistence point of view that we use "xxx_init()", too.
your reply reinforces my point. i'm not talking about xxx_init(), i'm talking about xxx_initialize(). network drivers atm define both -- xxx_initialize() is to initialize the eth_driver structure and *register* with the eth layer, and xxx_init() to *initialize* the hardware. i'm proposing renaming xxx_initialize() to xxx_register(). -mike
Mike Frysinger wrote:
On Tuesday 21 July 2009 03:32:55 Wolfgang Denk wrote:
Mike Frysinger wrote:
Is this a generally-accepted naming convention? I personally think it's crap, and since there isn't a single driver that uses it yet, you might say this is a bit ahead of the curve.
some style needed to be suggested, and what Jean proposed is better than what we have today (which is nothing)
Arent't we pretty much doing what Linux is doing here, too? I see lots of XXX_init functions in the Linux network code, for example.
that's why i said "should", deprecated current naming, and noted existing practice. if you agree with the proposal, it's easy enough to run sed on a few files to fix one function name. you agree with my comment that today's behavior is confusing even if you stare and bang on the code day in and day out ? it's even worse for the occasional observer ...
Hm... renaming something from "xxx_init()" into "xxx_register()" because other code is also also using "xxx_init()" does not really make anything clearer to me. Actually IMO it just adds confusion, because if other's use "xxx_init()" I'd expect from a consistence point of view that we use "xxx_init()", too.
your reply reinforces my point. i'm not talking about xxx_init(), i'm talking about xxx_initialize(). network drivers atm define both -- xxx_initialize() is to initialize the eth_driver structure and *register* with the eth layer, and xxx_init() to *initialize* the hardware. i'm proposing renaming xxx_initialize() to xxx_register(). -mike
I understand what you're saying, and think in principle it's probably a good idea to rename to something other than xxx_initialize(). I just think a document that outlines best practices that are not in use *at all* seems a bit silly.
If we're going to go this way, IMHO we should change all function names at once. It would be easy to do, but would be a huge, potentially intrusive patch that I'm not sure buys us much.
regards, Ben
On Tuesday 21 July 2009 16:55:34 Ben Warren wrote:
Mike Frysinger wrote:
On Tuesday 21 July 2009 03:32:55 Wolfgang Denk wrote:
Mike Frysinger wrote:
Is this a generally-accepted naming convention? I personally think it's crap, and since there isn't a single driver that uses it yet, you might say this is a bit ahead of the curve.
some style needed to be suggested, and what Jean proposed is better than what we have today (which is nothing)
Arent't we pretty much doing what Linux is doing here, too? I see lots of XXX_init functions in the Linux network code, for example.
that's why i said "should", deprecated current naming, and noted existing practice. if you agree with the proposal, it's easy enough to run sed on a few files to fix one function name. you agree with my comment that today's behavior is confusing even if you stare and bang on the code day in and day out ? it's even worse for the occasional observer ...
Hm... renaming something from "xxx_init()" into "xxx_register()" because other code is also also using "xxx_init()" does not really make anything clearer to me. Actually IMO it just adds confusion, because if other's use "xxx_init()" I'd expect from a consistence point of view that we use "xxx_init()", too.
your reply reinforces my point. i'm not talking about xxx_init(), i'm talking about xxx_initialize(). network drivers atm define both -- xxx_initialize() is to initialize the eth_driver structure and *register* with the eth layer, and xxx_init() to *initialize* the hardware. i'm proposing renaming xxx_initialize() to xxx_register().
I understand what you're saying, and think in principle it's probably a good idea to rename to something other than xxx_initialize(). I just think a document that outlines best practices that are not in use *at all* seems a bit silly.
considering the document makes note of existing practice and suggests the new naming schema, i think it's fine and shouldnt hold up merging.
If we're going to go this way, IMHO we should change all function names at once. It would be easy to do, but would be a huge, potentially intrusive patch that I'm not sure buys us much.
how quickly we convert older drivers doesnt matter to me, but if you prefer sooner rather than later, that's easy enough to do and hand off to Wolfgang for the first patch in "next". -mike
Dear Ben Warren,
In message 4A654D77.4070901@gmail.com you wrote:
Mike Frysinger wrote:
Signed-off-by: Mike Frysinger vapier@gentoo.org
Ben: some things to note:
- i adopted Jean's proposed naming scheme in the CONFIG section
Is this a generally-accepted naming convention? I personally think it's crap, and since there isn't a single driver that uses it yet, you might say this is a bit ahead of the curve.
No, this is NOT a generally-accepted naming convention. It has been proposed, and rejected.
That may be so, but since there isn't a single driver that uses this naming convention, you're wishing something that ain't so.
Thanks for pointing out.
Best regards,
Wolfgang Denk
On Sat, Jul 18, 2009 at 8:04 PM, Mike Frysinger vapier@gentoo.org wrote:
Signed-off-by: Mike Frysinger vapier@gentoo.org
Ben: some things to note: - i adopted Jean's proposed naming scheme in the CONFIG section - i deprecated calling the driver-specific entry point "xxx_initialization()" in favor of "xxx_register()" because the former is way too confusing with everyone also having "xxx_init()"
doc/README.drivers.eth | 199 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 199 insertions(+), 0 deletions(-) create mode 100644 doc/README.drivers.eth
diff --git a/doc/README.drivers.eth b/doc/README.drivers.eth new file mode 100644 index 0000000..00b4eb1 --- /dev/null +++ b/doc/README.drivers.eth @@ -0,0 +1,199 @@ +-----------------------
- Ethernet Driver Guide
+-----------------------
+The networking stack in Das U-Boot is designed for multiple network devices +to be easily added and controlled at runtime. This guide is meant for people +who wish to review the net driver stack with an eye towards implementing your +own ethernet device driver. Here we will describe a new pseudo 'APE' driver.
+----------------
- CONFIG Options
+----------------
+The common config defines your device should respect (if applicable):
CONFIG_MII - configure in MII modeCONFIG_RMII - configure in RMII mode
That's awfully global, and inflexible. I don't think there's any convention here that should be encouraged. The data bus configuration is a per-device attribute. If some system architects choose to allow this into their config files, far be it from me to protest, but I'm against it.
Also, CONFIG_MII doesn't mean what you think it means. It stems from the obnoxious overlap in terms set forth by IEEE 802.3. The MII in this case refers to the MII Management bus. Defining CONFIG_MII will cause miiphyutil.c to be built, which is a bit-bang MII Management bus driver. It will also enable some MII Management features. CONFIG_MII is therefore a relevant CONFIG option for most drivers.
Andy
On Wednesday 22 July 2009 19:00:40 Andy Fleming wrote:
On Sat, Jul 18, 2009 at 8:04 PM, Mike Frysinger vapier@gentoo.org wrote:
+----------------
- CONFIG Options
+----------------
+The common config defines your device should respect (if applicable):
CONFIG_MII - configure in MII modeCONFIG_RMII - configure in RMII modeThat's awfully global, and inflexible. I don't think there's any convention here that should be encouraged. The data bus configuration is a per-device attribute. If some system architects choose to allow this into their config files, far be it from me to protest, but I'm against it.
feel free to fix it then. this is a case of me documenting existing practice and not feeling like doing the footwork to fix drivers. you can not like it all you want, but that doesnt change reality.
Also, CONFIG_MII doesn't mean what you think it means. It stems from the obnoxious overlap in terms set forth by IEEE 802.3. The MII in this case refers to the MII Management bus. Defining CONFIG_MII will cause miiphyutil.c to be built, which is a bit-bang MII Management bus driver. It will also enable some MII Management features. CONFIG_MII is therefore a relevant CONFIG option for most drivers.
yes, that's obvious, so i'm not sure why i missed it (especially considering i also wrote support for it in the Blackfin net driver). i'll update the doc. -mike
On 21:04 Sat 18 Jul , Mike Frysinger wrote:
Signed-off-by: Mike Frysinger vapier@gentoo.org
Ben: some things to note:
- i adopted Jean's proposed naming scheme in the CONFIG section
- i deprecated calling the driver-specific entry point "xxx_initialization()" in favor of "xxx_register()" because the former is way too confusing with everyone also having "xxx_init()"
doc/README.drivers.eth | 199 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 199 insertions(+), 0 deletions(-) create mode 100644 doc/README.drivers.eth
diff --git a/doc/README.drivers.eth b/doc/README.drivers.eth new file mode 100644 index 0000000..00b4eb1 --- /dev/null +++ b/doc/README.drivers.eth @@ -0,0 +1,199 @@ +-----------------------
- Ethernet Driver Guide
+-----------------------
+The networking stack in Das U-Boot is designed for multiple network devices +to be easily added and controlled at runtime. This guide is meant for people +who wish to review the net driver stack with an eye towards implementing your +own ethernet device driver. Here we will describe a new pseudo 'APE' driver.
+----------------
- CONFIG Options
+----------------
+The common config defines your device should respect (if applicable):
- CONFIG_MII - configure in MII mode
- CONFIG_RMII - configure in RMII mode
how will you deal you on your board you have a MII and a RMII chip used I think need to be configure via drivers specifc CONFIG if your driver can not deal with 2 different instance config or via driver init param
- CONFIG_CMD_MII - support the MII command
+If you want to add config options specific to your driver, you should use the +naming convention:
- CONFIG_NET_<DRIVERNAME>_<OPTION>
+For example, if the APE driver could operate in either 16bit or 32bit mode, +there would probably be the option:
- CONFIG_NET_APE_32BIT - operate in 32bit mode rather than 16bit default
+And to control whether your driver is even enabled, you should use:
- CONFIG_SYS_NET_<DRIVERNAME>
In my proposition CONFIG_SYS_ will be not use for driver enabling but for hardware specific information
+So the APE driver would look like:
- CONFIG_SYS_NET_APE
so it will be CONFIG_NET_APE
+------------------
- Driver Functions
+------------------
+All functions you will be implementing in this document have the return value +meaning of 0 for success and non-zero for failure.
<snip>
+----------------
- CONFIG_CMD_MII
+----------------
+If your device supports banging arbitrary values on the MII bus (pretty much +every device does), you should add support for the mii command. Doing so is +fairly trivial and makes debugging mii issues a lot easier at runtime.
+After you have called eth_register() in your driver's register function, add +a call to miiphy_register() like so: +#ifdef CONFIG_CMD_MII
- miiphy_register(dev->name, mii_read, mii_write);
+#endif
+And then define the mii_read and mii_write functions if you haven't already. +Their syntax is straightforward:
- int mii_read(char *devname, uchar addr, uchar reg, ushort *val);
- int mii_write(char *devname, uchar addr, uchar reg, ushort val);
+The read function should read the register 'reg' from the phy at address 'addr' +and store the result in the pointer 'val'. The implementation for the write +function should logically follow.
we will rewrite this api when we will push the phylib
Best Regards, J.
Dear Jean-Christophe PLAGNIOL-VILLARD,
In message 20090723214946.GE9480@game.jcrosoft.org you wrote:
+And to control whether your driver is even enabled, you should use:
- CONFIG_SYS_NET_<DRIVERNAME>
In my proposition CONFIG_SYS_ will be not use for driver enabling but for hardware specific information
Enabling or disabling a driver i ssomething that a dumb user can do, so it should be done using
CONFIG_<DRIVERNAME>
as in CONFIG_E1000, CONFIG_EEPRO100, etc.
+So the APE driver would look like:
- CONFIG_SYS_NET_APE
so it will be CONFIG_NET_APE
No. It should be CONFIG_APE
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Dear Jean-Christophe PLAGNIOL-VILLARD,
In message 20090723214946.GE9480@game.jcrosoft.org you wrote:
+And to control whether your driver is even enabled, you should use:
- CONFIG_SYS_NET_<DRIVERNAME>
In my proposition CONFIG_SYS_ will be not use for driver enabling but for hardware specific information
Enabling or disabling a driver i ssomething that a dumb user can do, so it should be done using
CONFIG_<DRIVERNAME>
as in CONFIG_E1000, CONFIG_EEPRO100, etc.
Fine with me, although I understand the argument that it doesn't convey enough information, especially when dealing with devices that are more obscure than Intel E1000. CONFIG_SYS_APE is definitely wrong, I think CONFIG_NET_APE should be for networking features such as selectable protocols. Maybe a compromise if one is needed would be CONFIG_NETDEV_APE ? Or is it too verbose?
regards, Ben
Dear Ben Warren,
In message 4A68E0C1.2000605@gmail.com you wrote:
Enabling or disabling a driver i ssomething that a dumb user can do, so it should be done using
CONFIG_<DRIVERNAME>
as in CONFIG_E1000, CONFIG_EEPRO100, etc.
Fine with me, although I understand the argument that it doesn't convey enough information, especially when dealing with devices that are more obscure than Intel E1000. CONFIG_SYS_APE is definitely wrong, I think
I understand the argument, too.
But then I find it extremely useful to use the very same config names as Linux, and Linux uses CONFIG_E100, CONFIG_NATSEMI, CONFIG_NE2K_PCI, CONFIG_8139CP, CONFIG_TLAN, CONFIG_ATP, CONFIG_E1000, ...
CONFIG_NET_APE should be for networking features such as selectable protocols. Maybe a compromise if one is needed would be CONFIG_NETDEV_APE ? Or is it too verbose?
I think we should remain compatible with Linux here, too.
Best regards,
Wolfgang Denk
Signed-off-by: Mike Frysinger vapier@gentoo.org --- v2 - drop CONFIG naming section - fix MII documentation
doc/README.drivers.eth | 177 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 177 insertions(+), 0 deletions(-) create mode 100644 doc/README.drivers.eth
diff --git a/doc/README.drivers.eth b/doc/README.drivers.eth new file mode 100644 index 0000000..7f21909 --- /dev/null +++ b/doc/README.drivers.eth @@ -0,0 +1,177 @@ +----------------------- + Ethernet Driver Guide +----------------------- + +The networking stack in Das U-Boot is designed for multiple network devices +to be easily added and controlled at runtime. This guide is meant for people +who wish to review the net driver stack with an eye towards implementing your +own ethernet device driver. Here we will describe a new pseudo 'APE' driver. + +------------------ + Driver Functions +------------------ + +All functions you will be implementing in this document have the return value +meaning of 0 for success and non-zero for failure. + + ---------- + Register + ---------- + +When U-Boot initializes, it will call the common function eth_initialize(). +This will in turn call the board-specific board_eth_init() (or if that fails, +the cpu-specific cpu_eth_init()). These board-specific functions can do random +system handling, but ultimately they will call the driver-specific register +function which in turn takes care of initializing that particular instance. + +Keep in mind that you should code the driver to avoid storing state in global +data as someone might want to hook up two of the same devices to one board. If +the state is maintained as global data, it makes using both of those devices +impossible. + +So the call graph at this stage would look something like: +board_init() + eth_initialize() + board_eth_init() / cpu_eth_init() + driver_register() + initialize eth_device + eth_register() + +At this point in time, the only thing you need to worry about is the driver's +register function. The pseudo code would look something like: +int ape_register(bd_t *bis, int iobase) +{ + struct ape_priv *priv; + struct eth_device *dev; + + priv = malloc(sizeof(*priv)); + if (priv == NULL) + return 1; + + dev = malloc(sizeof(*dev)); + if (dev == NULL) { + free(priv); + return 1; + } + + /* setup whatever private state you need */ + + memset(dev, 0, sizeof(*dev)); + sprintf(dev->name, "APE"); + + /* if your device has dedicated hardware storage for the + * MAC, read it and initialize dev->enetaddr with it + */ + ape_mac_read(dev->enetaddr); + + dev->iobase = iobase; + dev->priv = priv; + dev->init = ape_init; + dev->halt = ape_halt; + dev->send = ape_send; + dev->recv = ape_recv; + + eth_register(dev); + +#ifdef CONFIG_CMD_MII) + miiphy_register(dev->name, ape_mii_read, ape_mii_write); +#endif + + return 0; +} + +The exact arguments needed to initialize your device are up to you. If you +need to pass more/less arguments, that's fine. You should also add the +prototype for your new register function to include/netdev.h. You might notice +that many drivers seem to use xxx_initialize() rather than xxx_register(). +This is the old naming convention and should be avoided as it causes confusion +with the driver-specific init function. + +Other than locating the MAC address in dedicated hardware storage, you should +not touch the hardware in anyway. That step is handled in the driver-specific +init function. Remember that we are only registering the device here, we are +not checking its state or doing random probing. + + ----------- + Callbacks + ----------- + +Now that we've registered with the ethernet layer, we can start getting some +real work done. You will need four functions: + int ape_init(struct eth_device *dev, bd_t *bis); + int ape_send(struct eth_device *dev, volatile void *packet, int length); + int ape_recv(struct eth_device *dev); + int ape_halt(struct eth_device *dev); + +The init function checks the hardware (probing/identifying) and gets it ready +for send/recv operations. You often do things here such as resetting the MAC +and/or PHY, and waiting for the link to autonegotiate. You should also take +the opportunity to program the device's MAC address with the dev->enetaddr +member. This allows the rest of U-Boot to dynamically change the MAC address +and have the new settings be respected. + +The send function does what you think -- transmit the specified packet whose +size is specified by length (in bytes). You should not return until the +transmission is complete, and you should leave the state such that the send +function can be called multiple times in a row. + +The recv function should process packets as long as the hardware has them +readily available before returning. i.e. you should drain the hardware fifo. +The common code sets up packet buffers for you already (NetRxPackets), so there +is no need to allocate your own. For each packet you receive, you should call +the NetReceive() function on it with the packet length. So the pseudo code +here would look something like: +int ape_recv(struct eth_device *dev) +{ + int length, i = 0; + ... + while (packets_are_available()) { + ... + length = ape_get_packet(&NetRxPackets[i]); + ... + NetReceive(&NetRxPackets[i], length); + ... + if (++i >= PKTBUFSRX) + i = 0; + ... + } + ... + return 0; +} + +The halt function should turn off / disable the hardware and place it back in +its reset state. + +So the call graph at this stage would look something like: +some net operation (ping / tftp / whatever...) + eth_init() + dev->init() + eth_send() + dev->send() + eth_rx() + dev->recv() + eth_halt() + dev->halt() + +----------------------------- + CONFIG_MII / CONFIG_CMD_MII +----------------------------- + +If your device supports banging arbitrary values on the MII bus (pretty much +every device does), you should add support for the mii command. Doing so is +fairly trivial and makes debugging mii issues a lot easier at runtime. + +After you have called eth_register() in your driver's register function, add +a call to miiphy_register() like so: +#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) + miiphy_register(dev->name, mii_read, mii_write); +#endif + +And then define the mii_read and mii_write functions if you haven't already. +Their syntax is straightforward: + int mii_read(char *devname, uchar addr, uchar reg, ushort *val); + int mii_write(char *devname, uchar addr, uchar reg, ushort val); + +The read function should read the register 'reg' from the phy at address 'addr' +and store the result in the pointer 'val'. The implementation for the write +function should logically follow.
Dear Mike Frysinger,
In message 1252521682-5067-1-git-send-email-vapier@gentoo.org you wrote:
Signed-off-by: Mike Frysinger vapier@gentoo.org
v2
- drop CONFIG naming section
- fix MII documentation
doc/README.drivers.eth | 177 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 177 insertions(+), 0 deletions(-) create mode 100644 doc/README.drivers.eth
Acked-by: Wolfgang Denk wd@denx.de
Best regards,
Wolfgang Denk
Dear Ben,
In message 1252521682-5067-1-git-send-email-vapier@gentoo.org Mike Frysinger wrote:
Signed-off-by: Mike Frysinger vapier@gentoo.org
v2
- drop CONFIG naming section
- fix MII documentation
doc/README.drivers.eth | 177 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 177 insertions(+), 0 deletions(-) create mode 100644 doc/README.drivers.eth
Will you run this through the net repo, or shall I pick it up directly?
Best regards,
Wolfgang Denk
On Sun, Oct 4, 2009 at 12:12 PM, Wolfgang Denk wd@denx.de wrote:
Dear Ben,
In message 1252521682-5067-1-git-send-email-vapier@gentoo.org Mike Frysinger wrote:
Signed-off-by: Mike Frysinger vapier@gentoo.org
v2 - drop CONFIG naming section - fix MII documentation
doc/README.drivers.eth | 177
++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 177 insertions(+), 0 deletions(-) create mode 100644 doc/README.drivers.eth
Will you run this through the net repo, or shall I pick it up directly?
I'll take care of it.
Best regards,
Wolfgang Denk
regards, Ben
Hi Mike,
Mike Frysinger wrote:
Signed-off-by: Mike Frysinger vapier@gentoo.org
v2
- drop CONFIG naming section
- fix MII documentation
doc/README.drivers.eth | 177 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 177 insertions(+), 0 deletions(-) create mode 100644 doc/README.drivers.eth
Applied to net repo.
I'll add info about the return code of the initialize() functions in a separate patch. It wouldn't be fair to ask you to do that.
thanks, Ben
participants (5)
-
Andy Fleming -
Ben Warren -
Jean-Christophe PLAGNIOL-VILLARD -
Mike Frysinger -
Wolfgang Denk