[U-Boot] [PATCH 0/5] smc911x driver fixes and additions
This patch serie miscelaneous fixes and additions for SMC911X driver.
Mike Rapoport (5): smc911x: return -1 when initialization fails smc911x: use dev->name in printfs smc911x: silence MAC mismatch warning smc911x: update SMC911X related configuration description smc911x: allow mac address to be kept after smc911x_halt
README | 15 ++++++++++----- drivers/net/smc911x.c | 41 ++++++++++++++++++++++++----------------- drivers/net/smc911x.h | 10 +++++----- 3 files changed, 39 insertions(+), 27 deletions(-)
Signed-off-by: Mike Rapoport mike@compulab.co.il --- drivers/net/smc911x.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index acc2306..57ad806 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -243,7 +243,7 @@ int smc911x_initialize(u8 dev_num, int base_addr) dev = malloc(sizeof(*dev)); if (!dev) { free(dev); - return 0; + return -1; } memset(dev, 0, sizeof(*dev));
@@ -252,7 +252,7 @@ int smc911x_initialize(u8 dev_num, int base_addr) /* Try to detect chip. Will fail if not present. */ if (smc911x_detect_chip(dev)) { free(dev); - return 0; + return -1; }
addrh = smc911x_get_mac_csr(dev, ADDRH);
On Wednesday 11 November 2009 03:03:00 Mike Rapoport wrote:
--- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -243,7 +243,7 @@ dev = malloc(sizeof(*dev)); if (!dev) { free(dev);
return 0;
return -1;
this is correct as this is an error
@@ -252,7 +252,7 @@ /* Try to detect chip. Will fail if not present. */ if (smc911x_detect_chip(dev)) { free(dev);
return 0;
return -1;
this is not -- we want it to return 0 if no parts are found. see recent net doc updates and discussions. -mike
Mike Frysinger wrote:
On Wednesday 11 November 2009 03:03:00 Mike Rapoport wrote:
--- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -243,7 +243,7 @@ dev = malloc(sizeof(*dev)); if (!dev) { free(dev);
return 0;
return -1;this is correct as this is an error
@@ -252,7 +252,7 @@ /* Try to detect chip. Will fail if not present. */ if (smc911x_detect_chip(dev)) { free(dev);
return 0;
return -1;this is not -- we want it to return 0 if no parts are found. see recent net doc updates and discussions. -mike
Hope this one is better:
From 4a9420704dd81a08f950017d365e0826880536ed Mon Sep 17 00:00:00 2001
From: Mike Rapoport mike@compulab.co.il Date: Tue, 10 Nov 2009 15:31:46 +0200 Subject: [PATCH] smc911x: make smc911x_initialize return correct value
Make smc911x_initialize return -1 on error and number of interfaces detected otherwise.
Signed-off-by: Mike Rapoport mike@compulab.co.il --- drivers/net/smc911x.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index acc2306..5d51406 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -243,7 +243,7 @@ int smc911x_initialize(u8 dev_num, int base_addr) dev = malloc(sizeof(*dev)); if (!dev) { free(dev); - return 0; + return -1; } memset(dev, 0, sizeof(*dev));
@@ -271,5 +271,5 @@ int smc911x_initialize(u8 dev_num, int base_addr) sprintf(dev->name, "%s-%hu", DRIVERNAME, dev_num);
eth_register(dev); - return 0; + return 1; }
On Thursday 12 November 2009 08:35:08 Mike Rapoport wrote:
Mike Frysinger wrote:
On Wednesday 11 November 2009 03:03:00 Mike Rapoport wrote:
--- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -243,7 +243,7 @@ dev = malloc(sizeof(*dev)); if (!dev) { free(dev);
return 0;
return -1;this is correct as this is an error
@@ -252,7 +252,7 @@ /* Try to detect chip. Will fail if not present. */ if (smc911x_detect_chip(dev)) { free(dev);
return 0;
return -1;this is not -- we want it to return 0 if no parts are found. see recent net doc updates and discussions.
Hope this one is better:
From 4a9420704dd81a08f950017d365e0826880536ed Mon Sep 17 00:00:00 2001 From: Mike Rapoport mike@compulab.co.il Date: Tue, 10 Nov 2009 15:31:46 +0200 Subject: [PATCH] smc911x: make smc911x_initialize return correct value
Make smc911x_initialize return -1 on error and number of interfaces detected otherwise.
Acked-by: Mike Frysinger vapier@gentoo.org -mike
Signed-off-by: Mike Rapoport mike@compulab.co.il --- drivers/net/smc911x.c | 17 ++++++++--------- drivers/net/smc911x.h | 10 +++++----- 2 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index 57ad806..9ff1288 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -47,7 +47,7 @@ static void smc911x_handle_mac_address(struct eth_device *dev) smc911x_set_mac_csr(dev, ADDRL, addrl); smc911x_set_mac_csr(dev, ADDRH, addrh);
- printf(DRIVERNAME ": MAC %pM\n", m); + printf("%s: MAC %pM\n", dev->name, m); }
static int smc911x_miiphy_read(struct eth_device *dev, @@ -119,12 +119,12 @@ static void smc911x_phy_configure(struct eth_device *dev) goto err_out; } while (!(status & PHY_BMSR_LS));
- printf(DRIVERNAME ": phy initialized\n"); + printf("%s: phy initialized\n", dev->name);
return;
err_out: - printf(DRIVERNAME ": autonegotiation timed out\n"); + printf("%s: autonegotiation timed out\n", dev->name); }
static void smc911x_enable(struct eth_device *dev) @@ -148,7 +148,7 @@ static int smc911x_init(struct eth_device *dev, bd_t * bd) { struct chip_id *id = dev->priv;
- printf(DRIVERNAME ": detected %s controller\n", id->name); + printf("%s: detected %s controller\n", dev->name, id->name);
smc911x_reset(dev);
@@ -193,7 +193,7 @@ static int smc911x_send(struct eth_device *dev, if (!status) return 0;
- printf(DRIVERNAME ": failed to send packet: %s%s%s%s%s\n", + printf("%s: failed to send packet: %s%s%s%s%s\n", dev->name, status & TX_STS_LOC ? "TX_STS_LOC " : "", status & TX_STS_LATE_COLL ? "TX_STS_LATE_COLL " : "", status & TX_STS_MANY_COLL ? "TX_STS_MANY_COLL " : "", @@ -225,9 +225,8 @@ static int smc911x_rx(struct eth_device *dev) *data++ = pkt_data_pull(dev, RX_DATA_FIFO);
if (status & RX_STS_ES) - printf(DRIVERNAME - ": dropped bad packet. Status: 0x%08x\n", - status); + printf("%s: dropped bad packet. Status: 0x%08x\n", + dev->name, status); else NetReceive(NetRxPackets[0], pktlen); } @@ -248,6 +247,7 @@ int smc911x_initialize(u8 dev_num, int base_addr) memset(dev, 0, sizeof(*dev));
dev->iobase = base_addr; + sprintf(dev->name, "%s-%hu", DRIVERNAME, dev_num);
/* Try to detect chip. Will fail if not present. */ if (smc911x_detect_chip(dev)) { @@ -268,7 +268,6 @@ int smc911x_initialize(u8 dev_num, int base_addr) dev->halt = smc911x_halt; dev->send = smc911x_send; dev->recv = smc911x_rx; - sprintf(dev->name, "%s-%hu", DRIVERNAME, dev_num);
eth_register(dev); return 0; diff --git a/drivers/net/smc911x.h b/drivers/net/smc911x.h index 05e007c..0ea6ac1 100644 --- a/drivers/net/smc911x.h +++ b/drivers/net/smc911x.h @@ -447,7 +447,7 @@ static int smc911x_detect_chip(struct eth_device *dev) /* Special case -- no chip present */ return -1; } else if (val != 0x87654321) { - printf(DRIVERNAME ": Invalid chip endian 0x%08lx\n", val); + printf("%s: Invalid chip endian 0x%08lx\n", __func__, val); return -1; }
@@ -456,7 +456,7 @@ static int smc911x_detect_chip(struct eth_device *dev) if (chip_ids[i].id == val) break; } if (!chip_ids[i].id) { - printf(DRIVERNAME ": Unknown chip ID %04lx\n", val); + printf("%s: Unknown chip ID %04lx\n", __func__, val); return -1; }
@@ -480,8 +480,8 @@ static void smc911x_reset(struct eth_device *dev) !(smc911x_reg_read(dev, PMT_CTRL) & PMT_CTRL_READY)) udelay(10); if (!timeout) { - printf(DRIVERNAME - ": timeout waiting for PM restore\n"); + printf("%s: timeout waiting for PM restore\n", + dev->name); return; } } @@ -496,7 +496,7 @@ static void smc911x_reset(struct eth_device *dev) udelay(10);
if (!timeout) { - printf(DRIVERNAME ": reset timeout\n"); + printf("%s: reset timeout\n", dev->name); return; }
On Wednesday 11 November 2009 03:03:01 Mike Rapoport wrote:
--- a/drivers/net/smc911x.h +++ b/drivers/net/smc911x.h @@ -480,8 +480,8 @@ static void smc911x_reset(struct eth_device *dev) !(smc911x_reg_read(dev, PMT_CTRL) & PMT_CTRL_READY)) udelay(10); if (!timeout) {
printf(DRIVERNAME": timeout waiting for PM restore\n");
printf("%s: timeout waiting for PM restore\n",dev->name); return;
these changes in general look good, but if you're going to modify the common header, you need to update the eeprom code as well to set up the name field -mike
On Wed, Nov 11, 2009 at 5:18 PM, Mike Frysinger vapier@gentoo.org wrote:
On Wednesday 11 November 2009 03:03:01 Mike Rapoport wrote:
--- a/drivers/net/smc911x.h +++ b/drivers/net/smc911x.h @@ -480,8 +480,8 @@ static void smc911x_reset(struct eth_device *dev) !(smc911x_reg_read(dev, PMT_CTRL) & PMT_CTRL_READY)) udelay(10); if (!timeout) {
- printf(DRIVERNAME
- ": timeout waiting for PM restore\n");
- printf("%s: timeout waiting for PM restore\n",
- dev->name);
return; } }
these changes in general look good, but if you're going to modify the common header, you need to update the eeprom code as well to set up the name field
It seems that eeprom code is broken since commit 736fead8fdbf8a8407048bebc373cd551d01ec98: "Convert SMC911X Ethernet driver to CONFIG_NET_MULTI API". I'll try to come up with a fix, but I have no way to test it.
-mike
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Wednesday 11 November 2009 16:56:57 Mike Rapoport wrote:
On Wed, Nov 11, 2009 at 5:18 PM, Mike Frysinger wrote:
On Wednesday 11 November 2009 03:03:01 Mike Rapoport wrote:
--- a/drivers/net/smc911x.h +++ b/drivers/net/smc911x.h @@ -480,8 +480,8 @@ static void smc911x_reset(struct eth_device *dev) !(smc911x_reg_read(dev, PMT_CTRL) & PMT_CTRL_READY)) udelay(10); if (!timeout) {
printf(DRIVERNAME": timeout waiting for PM restore\n");
printf("%s: timeout waiting for PM restore\n",dev->name); return; } }these changes in general look good, but if you're going to modify the common header, you need to update the eeprom code as well to set up the name field
It seems that eeprom code is broken since commit 736fead8fdbf8a8407048bebc373cd551d01ec98: "Convert SMC911X Ethernet driver to CONFIG_NET_MULTI API".
broken how ? i recall it working ...
I'll try to come up with a fix, but I have no way to test it.
any board that has a smc911x part can test this code easily. it doesnt require an eeprom to be hooked up to the smc911x part. you can still dump on- chip registers as well as poke the eeprorm interface. -mike
On Thu, Nov 12, 2009 at 12:11 AM, Mike Frysinger vapier@gentoo.org wrote:
On Wednesday 11 November 2009 16:56:57 Mike Rapoport wrote:
On Wed, Nov 11, 2009 at 5:18 PM, Mike Frysinger wrote:
On Wednesday 11 November 2009 03:03:01 Mike Rapoport wrote:
--- a/drivers/net/smc911x.h +++ b/drivers/net/smc911x.h @@ -480,8 +480,8 @@ static void smc911x_reset(struct eth_device *dev) !(smc911x_reg_read(dev, PMT_CTRL) & PMT_CTRL_READY)) udelay(10); if (!timeout) {
- printf(DRIVERNAME
- ": timeout waiting for PM restore\n");
- printf("%s: timeout waiting for PM restore\n",
- dev->name);
return; } }
these changes in general look good, but if you're going to modify the common header, you need to update the eeprom code as well to set up the name field
It seems that eeprom code is broken since commit 736fead8fdbf8a8407048bebc373cd551d01ec98: "Convert SMC911X Ethernet driver to CONFIG_NET_MULTI API".
broken how ? i recall it working ...
It gives pretty long list of compile errors. The smc911x.h header has now 'struct eth_device *dev' parameter in all the functions.
I'll try to come up with a fix, but I have no way to test it.
any board that has a smc911x part can test this code easily. it doesnt require an eeprom to be hooked up to the smc911x part. you can still dump on- chip registers as well as poke the eeprorm interface.
Ok, will try.
-mike
On Wednesday 11 November 2009 17:24:27 Mike Rapoport wrote:
On Thu, Nov 12, 2009 at 12:11 AM, Mike Frysinger vapier@gentoo.org wrote:
On Wednesday 11 November 2009 16:56:57 Mike Rapoport wrote:
It seems that eeprom code is broken since commit 736fead8fdbf8a8407048bebc373cd551d01ec98: "Convert SMC911X Ethernet driver to CONFIG_NET_MULTI API".
broken how ? i recall it working ...
It gives pretty long list of compile errors. The smc911x.h header has now 'struct eth_device *dev' parameter in all the functions.
yeah, i see that now. it wasnt noticed earlier as the config name changed but the eeprom code wasnt updated. i can take a look if you like since i wrote this sucker in the first place. -mike
Hi Mike,
Mike Frysinger wrote:
On Wednesday 11 November 2009 17:24:27 Mike Rapoport wrote:
On Thu, Nov 12, 2009 at 12:11 AM, Mike Frysinger vapier@gentoo.org wrote:
On Wednesday 11 November 2009 16:56:57 Mike Rapoport wrote:
It seems that eeprom code is broken since commit 736fead8fdbf8a8407048bebc373cd551d01ec98: "Convert SMC911X Ethernet driver to CONFIG_NET_MULTI API".
broken how ? i recall it working ...
It gives pretty long list of compile errors. The smc911x.h header has now 'struct eth_device *dev' parameter in all the functions.
yeah, i see that now. it wasnt noticed earlier as the config name changed but the eeprom code wasnt updated. i can take a look if you like since i wrote this sucker in the first place. -mike
You fixed the SMC91111 eeprom code by defining a 'struct eth_dev', but I guess not the SMC9111x. I'm responsible for making this mess, so if you don't have time I can take care of it (without being able to test, of course :)
We can probably still fit such a bug fix in this release if we act quickly.
regards, Ben
On Wednesday 11 November 2009 17:45:10 Ben Warren wrote:
Mike Frysinger wrote:
On Wednesday 11 November 2009 17:24:27 Mike Rapoport wrote:
On Thu, Nov 12, 2009 at 12:11 AM, Mike Frysinger wrote:
On Wednesday 11 November 2009 16:56:57 Mike Rapoport wrote:
It seems that eeprom code is broken since commit 736fead8fdbf8a8407048bebc373cd551d01ec98: "Convert SMC911X Ethernet driver to CONFIG_NET_MULTI API".
broken how ? i recall it working ...
It gives pretty long list of compile errors. The smc911x.h header has now 'struct eth_device *dev' parameter in all the functions.
yeah, i see that now. it wasnt noticed earlier as the config name changed but the eeprom code wasnt updated. i can take a look if you like since i wrote this sucker in the first place.
You fixed the SMC91111 eeprom code by defining a 'struct eth_dev', but I guess not the SMC9111x. I'm responsible for making this mess, so if you don't have time I can take care of it (without being able to test, of course :)
We can probably still fit such a bug fix in this release if we act quickly.
here's my [compile] tested change as the board i have with this part isnt readily accessible atm ... -mike
diff --git a/examples/standalone/smc911x_eeprom.c b/examples/standalone/smc911x_eeprom.c index bf22f0a..f2a6304 100644 --- a/examples/standalone/smc911x_eeprom.c +++ b/examples/standalone/smc911x_eeprom.c @@ -17,8 +17,8 @@ #include <common.h> #include <exports.h>
-#ifdef CONFIG_DRIVER_SMC911X - +/* the smc911x.h gets base addr through eth_device' iobase */ +struct eth_device { const char *name; unsigned long iobase; }; #include "../drivers/net/smc911x.h"
/** @@ -55,32 +55,32 @@ static void usage(void) * Registers 0x00 - 0x50 are FIFOs. The 0x50+ are the control registers * and they're all 32bits long. 0xB8+ are reserved, so don't bother. */ -static void dump_regs(void) +static void dump_regs(struct eth_device *dev) { u8 i, j = 0; for (i = 0x50; i < 0xB8; i += sizeof(u32)) printf("%02x: 0x%08x %c", i, - smc911x_reg_read(CONFIG_DRIVER_SMC911X_BASE + i), + smc911x_reg_read(dev, i), (j++ % 2 ? '\n' : ' ')); }
/** * do_eeprom_cmd - handle eeprom communication */ -static int do_eeprom_cmd(int cmd, u8 reg) +static int do_eeprom_cmd(struct eth_device *dev, int cmd, u8 reg) { - if (smc911x_reg_read(E2P_CMD) & E2P_CMD_EPC_BUSY) { + if (smc911x_reg_read(dev, E2P_CMD) & E2P_CMD_EPC_BUSY) { printf("eeprom_cmd: busy at start (E2P_CMD = 0x%08x)\n", - smc911x_reg_read(E2P_CMD)); + smc911x_reg_read(dev, E2P_CMD)); return -1; }
- smc911x_reg_write(E2P_CMD, E2P_CMD_EPC_BUSY | cmd | reg); + smc911x_reg_write(dev, E2P_CMD, E2P_CMD_EPC_BUSY | cmd | reg);
- while (smc911x_reg_read(E2P_CMD) & E2P_CMD_EPC_BUSY) + while (smc911x_reg_read(dev, E2P_CMD) & E2P_CMD_EPC_BUSY) if (smsc_ctrlc()) { printf("eeprom_cmd: timeout (E2P_CMD = 0x%08x)\n", - smc911x_reg_read(E2P_CMD)); + smc911x_reg_read(dev, E2P_CMD)); return -1; }
@@ -90,37 +90,37 @@ static int do_eeprom_cmd(int cmd, u8 reg) /** * read_eeprom_reg - read specified register in EEPROM */ -static u8 read_eeprom_reg(u8 reg) +static u8 read_eeprom_reg(struct eth_device *dev, u8 reg) { - int ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_READ, reg); - return (ret ? : smc911x_reg_read(E2P_DATA)); + int ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_READ, reg); + return (ret ? : smc911x_reg_read(dev, E2P_DATA)); }
/** * write_eeprom_reg - write specified value into specified register in EEPROM */ -static int write_eeprom_reg(u8 value, u8 reg) +static int write_eeprom_reg(struct eth_device *dev, u8 value, u8 reg) { int ret;
/* enable erasing/writing */ - ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_EWEN, reg); + ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_EWEN, reg); if (ret) goto done;
/* erase the eeprom reg */ - ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_ERASE, reg); + ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_ERASE, reg); if (ret) goto done;
/* write the eeprom reg */ - smc911x_reg_write(E2P_DATA, value); - ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_WRITE, reg); + smc911x_reg_write(dev, E2P_DATA, value); + ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_WRITE, reg); if (ret) goto done;
/* disable erasing/writing */ - ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_EWDS, reg); + ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_EWDS, reg);
done: return ret; @@ -139,7 +139,7 @@ static char *skip_space(char *buf) /** * write_stuff - handle writing of MAC registers / eeprom */ -static void write_stuff(char *line) +static void write_stuff(struct eth_device *dev, char *line) { char dest; char *endp; @@ -182,39 +182,39 @@ static void write_stuff(char *line) /* Finally, execute the command */ if (dest == 'E') { printf("Writing EEPROM register %02x with %02x\n", reg, value); - write_eeprom_reg(value, reg); + write_eeprom_reg(dev, value, reg); } else { printf("Writing MAC register %02x with %08x\n", reg, value); - smc911x_reg_write(CONFIG_DRIVER_SMC911X_BASE + reg, value); + smc911x_reg_write(dev, reg, value); } }
/** * copy_from_eeprom - copy MAC address in eeprom to address registers */ -static void copy_from_eeprom(void) +static void copy_from_eeprom(struct eth_device *dev) { ulong addrl = - read_eeprom_reg(0x01) | - read_eeprom_reg(0x02) << 8 | - read_eeprom_reg(0x03) << 16 | - read_eeprom_reg(0x04) << 24; + read_eeprom_reg(dev, 0x01) | + read_eeprom_reg(dev, 0x02) << 8 | + read_eeprom_reg(dev, 0x03) << 16 | + read_eeprom_reg(dev, 0x04) << 24; ulong addrh = - read_eeprom_reg(0x05) | - read_eeprom_reg(0x06) << 8; - smc911x_set_mac_csr(ADDRL, addrl); - smc911x_set_mac_csr(ADDRH, addrh); + read_eeprom_reg(dev, 0x05) | + read_eeprom_reg(dev, 0x06) << 8; + smc911x_set_mac_csr(dev, ADDRL, addrl); + smc911x_set_mac_csr(dev, ADDRH, addrh); puts("EEPROM contents copied to MAC\n"); }
/** * print_macaddr - print MAC address registers and MAC address in eeprom */ -static void print_macaddr(void) +static void print_macaddr(struct eth_device *dev) { puts("Current MAC Address in MAC: "); - ulong addrl = smc911x_get_mac_csr(ADDRL); - ulong addrh = smc911x_get_mac_csr(ADDRH); + ulong addrl = smc911x_get_mac_csr(dev, ADDRL); + ulong addrh = smc911x_get_mac_csr(dev, ADDRH); printf("%02x:%02x:%02x:%02x:%02x:%02x\n", (u8)(addrl), (u8)(addrl >> 8), (u8)(addrl >> 16), (u8)(addrl >> 24), (u8)(addrh), (u8)(addrh >> 8)); @@ -222,41 +222,42 @@ static void print_macaddr(void) puts("Current MAC Address in EEPROM: "); int i; for (i = 1; i < 6; ++i) - printf("%02x:", read_eeprom_reg(i)); - printf("%02x\n", read_eeprom_reg(i)); + printf("%02x:", read_eeprom_reg(dev, i)); + printf("%02x\n", read_eeprom_reg(dev, i)); }
/** * dump_eeprom - dump the whole content of the EEPROM */ -static void dump_eeprom(void) +static void dump_eeprom(struct eth_device *dev) { int i; puts("EEPROM:\n"); for (i = 0; i < 7; ++i) - printf("%02x: 0x%02x\n", i, read_eeprom_reg(i)); + printf("%02x: 0x%02x\n", i, read_eeprom_reg(dev, i)); }
/** * smc911x_init - get the MAC/EEPROM up and ready for use */ -static int smc911x_init(void) +static int smc911x_init(struct eth_device *dev) { /* See if there is anything there */ - if (!smc911x_detect_chip()) + if (!smc911x_detect_chip(dev)) return 1;
- smc911x_reset(); + smc911x_reset(dev);
/* Make sure we set EEDIO/EECLK to the EEPROM */ - if (smc911x_reg_read(GPIO_CFG) & GPIO_CFG_EEPR_EN) { - while (smc911x_reg_read(E2P_CMD) & E2P_CMD_EPC_BUSY) + if (smc911x_reg_read(dev, GPIO_CFG) & GPIO_CFG_EEPR_EN) { + while (smc911x_reg_read(dev, E2P_CMD) & E2P_CMD_EPC_BUSY) if (smsc_ctrlc()) { printf("init: timeout (E2P_CMD = 0x%08x)\n", - smc911x_reg_read(E2P_CMD)); + smc911x_reg_read(dev, E2P_CMD)); return 1; } - smc911x_reg_write(GPIO_CFG, smc911x_reg_read(GPIO_CFG) & ~GPIO_CFG_EEPR_EN); + smc911x_reg_write(dev, GPIO_CFG, + smc911x_reg_read(dev, GPIO_CFG) & ~GPIO_CFG_EEPR_EN); }
return 0; @@ -317,6 +318,11 @@ static char *getline(void) */ int smc911x_eeprom(int argc, char *argv[]) { + struct eth_device dev = { + .name = __func__, + .iobase = CONFIG_SMC911X_BASE, + }; + /* Print the ABI version */ app_startup(argv); if (XF_VERSION != get_version()) { @@ -328,7 +334,7 @@ int smc911x_eeprom(int argc, char *argv[])
/* Initialize the MAC/EEPROM somewhat */ puts("\n"); - if (smc911x_init()) + if (smc911x_init(&dev)) return 1;
/* Dump helpful usage information */ @@ -360,11 +366,11 @@ int smc911x_eeprom(int argc, char *argv[])
/* Now parse the command */ switch (line[0]) { - case 'W': write_stuff(line); break; - case 'D': dump_eeprom(); break; - case 'M': dump_regs(); break; - case 'C': copy_from_eeprom(); break; - case 'P': print_macaddr(); break; + case 'W': write_stuff(&dev, line); break; + case 'D': dump_eeprom(&dev); break; + case 'M': dump_regs(&dev); break; + case 'C': copy_from_eeprom(&dev); break; + case 'P': print_macaddr(&dev); break; unknown_cmd: default: puts("ERROR: Unknown command!\n\n"); case '?': @@ -373,11 +379,3 @@ int smc911x_eeprom(int argc, char *argv[]) } } } - -#else -int smc911x_eeprom(int argc, char *argv[]) -{ - puts("Not supported for this board\n"); - return 1; -} -#endif
Hi Mike,
Mike Frysinger wrote:
On Wednesday 11 November 2009 17:45:10 Ben Warren wrote:
Mike Frysinger wrote:
On Wednesday 11 November 2009 17:24:27 Mike Rapoport wrote:
On Thu, Nov 12, 2009 at 12:11 AM, Mike Frysinger wrote:
On Wednesday 11 November 2009 16:56:57 Mike Rapoport wrote:
It seems that eeprom code is broken since commit 736fead8fdbf8a8407048bebc373cd551d01ec98: "Convert SMC911X Ethernet driver to CONFIG_NET_MULTI API".
broken how ? i recall it working ...
It gives pretty long list of compile errors. The smc911x.h header has now 'struct eth_device *dev' parameter in all the functions.
yeah, i see that now. it wasnt noticed earlier as the config name changed but the eeprom code wasnt updated. i can take a look if you like since i wrote this sucker in the first place.
You fixed the SMC91111 eeprom code by defining a 'struct eth_dev', but I guess not the SMC9111x. I'm responsible for making this mess, so if you don't have time I can take care of it (without being able to test, of course :)
We can probably still fit such a bug fix in this release if we act quickly.
here's my [compile] tested change as the board i have with this part isnt readily accessible atm ... -mike
<snip>
return 0; @@ -317,6 +318,11 @@ static char *getline(void) */ int smc911x_eeprom(int argc, char *argv[]) {
- struct eth_device dev = {
.name = __func__,
Cute
This looks good to me.
Mike R - is there a chance that you can test this out? If not, I'd argue that even untested it has a fighting chance of working and as such is better than what's already in the code base, and should be picked up ASAP.
regards, Ben
When the smc911x driver was converted to NET_MULTI, the smc911x eeprom was missed. The config option needed updating as well as overhauling of the rergister read/write functions.
Signed-off-by: Mike Frysinger vapier@gentoo.org Tested-by: Mike Rapoport mike.rapoport@gmail.com --- examples/standalone/smc911x_eeprom.c | 122 +++++++++++++++++----------------- 1 files changed, 62 insertions(+), 60 deletions(-)
diff --git a/examples/standalone/smc911x_eeprom.c b/examples/standalone/smc911x_eeprom.c index bf22f0a..93d273e 100644 --- a/examples/standalone/smc911x_eeprom.c +++ b/examples/standalone/smc911x_eeprom.c @@ -2,7 +2,7 @@ * smc911x_eeprom.c - EEPROM interface to SMC911x parts. * Only tested on SMSC9118 though ... * - * Copyright 2004-2008 Analog Devices Inc. + * Copyright 2004-2009 Analog Devices Inc. * * Licensed under the GPL-2 or later. * @@ -17,8 +17,12 @@ #include <common.h> #include <exports.h>
-#ifdef CONFIG_DRIVER_SMC911X - +/* the smc911x.h gets base addr through eth_device' iobase */ +struct eth_device { + const char *name; + unsigned long iobase; + void *priv; +}; #include "../drivers/net/smc911x.h"
/** @@ -55,32 +59,32 @@ static void usage(void) * Registers 0x00 - 0x50 are FIFOs. The 0x50+ are the control registers * and they're all 32bits long. 0xB8+ are reserved, so don't bother. */ -static void dump_regs(void) +static void dump_regs(struct eth_device *dev) { u8 i, j = 0; for (i = 0x50; i < 0xB8; i += sizeof(u32)) printf("%02x: 0x%08x %c", i, - smc911x_reg_read(CONFIG_DRIVER_SMC911X_BASE + i), + smc911x_reg_read(dev, i), (j++ % 2 ? '\n' : ' ')); }
/** * do_eeprom_cmd - handle eeprom communication */ -static int do_eeprom_cmd(int cmd, u8 reg) +static int do_eeprom_cmd(struct eth_device *dev, int cmd, u8 reg) { - if (smc911x_reg_read(E2P_CMD) & E2P_CMD_EPC_BUSY) { + if (smc911x_reg_read(dev, E2P_CMD) & E2P_CMD_EPC_BUSY) { printf("eeprom_cmd: busy at start (E2P_CMD = 0x%08x)\n", - smc911x_reg_read(E2P_CMD)); + smc911x_reg_read(dev, E2P_CMD)); return -1; }
- smc911x_reg_write(E2P_CMD, E2P_CMD_EPC_BUSY | cmd | reg); + smc911x_reg_write(dev, E2P_CMD, E2P_CMD_EPC_BUSY | cmd | reg);
- while (smc911x_reg_read(E2P_CMD) & E2P_CMD_EPC_BUSY) + while (smc911x_reg_read(dev, E2P_CMD) & E2P_CMD_EPC_BUSY) if (smsc_ctrlc()) { printf("eeprom_cmd: timeout (E2P_CMD = 0x%08x)\n", - smc911x_reg_read(E2P_CMD)); + smc911x_reg_read(dev, E2P_CMD)); return -1; }
@@ -90,37 +94,37 @@ static int do_eeprom_cmd(int cmd, u8 reg) /** * read_eeprom_reg - read specified register in EEPROM */ -static u8 read_eeprom_reg(u8 reg) +static u8 read_eeprom_reg(struct eth_device *dev, u8 reg) { - int ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_READ, reg); - return (ret ? : smc911x_reg_read(E2P_DATA)); + int ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_READ, reg); + return (ret ? : smc911x_reg_read(dev, E2P_DATA)); }
/** * write_eeprom_reg - write specified value into specified register in EEPROM */ -static int write_eeprom_reg(u8 value, u8 reg) +static int write_eeprom_reg(struct eth_device *dev, u8 value, u8 reg) { int ret;
/* enable erasing/writing */ - ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_EWEN, reg); + ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_EWEN, reg); if (ret) goto done;
/* erase the eeprom reg */ - ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_ERASE, reg); + ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_ERASE, reg); if (ret) goto done;
/* write the eeprom reg */ - smc911x_reg_write(E2P_DATA, value); - ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_WRITE, reg); + smc911x_reg_write(dev, E2P_DATA, value); + ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_WRITE, reg); if (ret) goto done;
/* disable erasing/writing */ - ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_EWDS, reg); + ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_EWDS, reg);
done: return ret; @@ -139,7 +143,7 @@ static char *skip_space(char *buf) /** * write_stuff - handle writing of MAC registers / eeprom */ -static void write_stuff(char *line) +static void write_stuff(struct eth_device *dev, char *line) { char dest; char *endp; @@ -182,39 +186,39 @@ static void write_stuff(char *line) /* Finally, execute the command */ if (dest == 'E') { printf("Writing EEPROM register %02x with %02x\n", reg, value); - write_eeprom_reg(value, reg); + write_eeprom_reg(dev, value, reg); } else { printf("Writing MAC register %02x with %08x\n", reg, value); - smc911x_reg_write(CONFIG_DRIVER_SMC911X_BASE + reg, value); + smc911x_reg_write(dev, reg, value); } }
/** * copy_from_eeprom - copy MAC address in eeprom to address registers */ -static void copy_from_eeprom(void) +static void copy_from_eeprom(struct eth_device *dev) { ulong addrl = - read_eeprom_reg(0x01) | - read_eeprom_reg(0x02) << 8 | - read_eeprom_reg(0x03) << 16 | - read_eeprom_reg(0x04) << 24; + read_eeprom_reg(dev, 0x01) | + read_eeprom_reg(dev, 0x02) << 8 | + read_eeprom_reg(dev, 0x03) << 16 | + read_eeprom_reg(dev, 0x04) << 24; ulong addrh = - read_eeprom_reg(0x05) | - read_eeprom_reg(0x06) << 8; - smc911x_set_mac_csr(ADDRL, addrl); - smc911x_set_mac_csr(ADDRH, addrh); + read_eeprom_reg(dev, 0x05) | + read_eeprom_reg(dev, 0x06) << 8; + smc911x_set_mac_csr(dev, ADDRL, addrl); + smc911x_set_mac_csr(dev, ADDRH, addrh); puts("EEPROM contents copied to MAC\n"); }
/** * print_macaddr - print MAC address registers and MAC address in eeprom */ -static void print_macaddr(void) +static void print_macaddr(struct eth_device *dev) { puts("Current MAC Address in MAC: "); - ulong addrl = smc911x_get_mac_csr(ADDRL); - ulong addrh = smc911x_get_mac_csr(ADDRH); + ulong addrl = smc911x_get_mac_csr(dev, ADDRL); + ulong addrh = smc911x_get_mac_csr(dev, ADDRH); printf("%02x:%02x:%02x:%02x:%02x:%02x\n", (u8)(addrl), (u8)(addrl >> 8), (u8)(addrl >> 16), (u8)(addrl >> 24), (u8)(addrh), (u8)(addrh >> 8)); @@ -222,41 +226,42 @@ static void print_macaddr(void) puts("Current MAC Address in EEPROM: "); int i; for (i = 1; i < 6; ++i) - printf("%02x:", read_eeprom_reg(i)); - printf("%02x\n", read_eeprom_reg(i)); + printf("%02x:", read_eeprom_reg(dev, i)); + printf("%02x\n", read_eeprom_reg(dev, i)); }
/** * dump_eeprom - dump the whole content of the EEPROM */ -static void dump_eeprom(void) +static void dump_eeprom(struct eth_device *dev) { int i; puts("EEPROM:\n"); for (i = 0; i < 7; ++i) - printf("%02x: 0x%02x\n", i, read_eeprom_reg(i)); + printf("%02x: 0x%02x\n", i, read_eeprom_reg(dev, i)); }
/** * smc911x_init - get the MAC/EEPROM up and ready for use */ -static int smc911x_init(void) +static int smc911x_init(struct eth_device *dev) { /* See if there is anything there */ - if (!smc911x_detect_chip()) + if (!smc911x_detect_chip(dev)) return 1;
- smc911x_reset(); + smc911x_reset(dev);
/* Make sure we set EEDIO/EECLK to the EEPROM */ - if (smc911x_reg_read(GPIO_CFG) & GPIO_CFG_EEPR_EN) { - while (smc911x_reg_read(E2P_CMD) & E2P_CMD_EPC_BUSY) + if (smc911x_reg_read(dev, GPIO_CFG) & GPIO_CFG_EEPR_EN) { + while (smc911x_reg_read(dev, E2P_CMD) & E2P_CMD_EPC_BUSY) if (smsc_ctrlc()) { printf("init: timeout (E2P_CMD = 0x%08x)\n", - smc911x_reg_read(E2P_CMD)); + smc911x_reg_read(dev, E2P_CMD)); return 1; } - smc911x_reg_write(GPIO_CFG, smc911x_reg_read(GPIO_CFG) & ~GPIO_CFG_EEPR_EN); + smc911x_reg_write(dev, GPIO_CFG, + smc911x_reg_read(dev, GPIO_CFG) & ~GPIO_CFG_EEPR_EN); }
return 0; @@ -317,6 +322,11 @@ static char *getline(void) */ int smc911x_eeprom(int argc, char *argv[]) { + struct eth_device dev = { + .name = __func__, + .iobase = CONFIG_SMC911X_BASE, + }; + /* Print the ABI version */ app_startup(argv); if (XF_VERSION != get_version()) { @@ -328,7 +338,7 @@ int smc911x_eeprom(int argc, char *argv[])
/* Initialize the MAC/EEPROM somewhat */ puts("\n"); - if (smc911x_init()) + if (smc911x_init(&dev)) return 1;
/* Dump helpful usage information */ @@ -360,11 +370,11 @@ int smc911x_eeprom(int argc, char *argv[])
/* Now parse the command */ switch (line[0]) { - case 'W': write_stuff(line); break; - case 'D': dump_eeprom(); break; - case 'M': dump_regs(); break; - case 'C': copy_from_eeprom(); break; - case 'P': print_macaddr(); break; + case 'W': write_stuff(&dev, line); break; + case 'D': dump_eeprom(&dev); break; + case 'M': dump_regs(&dev); break; + case 'C': copy_from_eeprom(&dev); break; + case 'P': print_macaddr(&dev); break; unknown_cmd: default: puts("ERROR: Unknown command!\n\n"); case '?': @@ -373,11 +383,3 @@ int smc911x_eeprom(int argc, char *argv[]) } } } - -#else -int smc911x_eeprom(int argc, char *argv[]) -{ - puts("Not supported for this board\n"); - return 1; -} -#endif
When the smc911x driver was converted to NET_MULTI, the smc911x eeprom was missed. The config option needed updating as well as overhauling of the rergister read/write functions.
Signed-off-by: Mike Frysinger vapier@gentoo.org Tested-by: Mike Rapoport mike.rapoport@gmail.com --- v2 - fix building under certain gcc versions/compiler flags (struct init on stack)
examples/standalone/smc911x_eeprom.c | 122 +++++++++++++++++----------------- 1 files changed, 62 insertions(+), 60 deletions(-)
diff --git a/examples/standalone/smc911x_eeprom.c b/examples/standalone/smc911x_eeprom.c index bf22f0a..fff3123 100644 --- a/examples/standalone/smc911x_eeprom.c +++ b/examples/standalone/smc911x_eeprom.c @@ -2,7 +2,7 @@ * smc911x_eeprom.c - EEPROM interface to SMC911x parts. * Only tested on SMSC9118 though ... * - * Copyright 2004-2008 Analog Devices Inc. + * Copyright 2004-2009 Analog Devices Inc. * * Licensed under the GPL-2 or later. * @@ -17,8 +17,12 @@ #include <common.h> #include <exports.h>
-#ifdef CONFIG_DRIVER_SMC911X - +/* the smc911x.h gets base addr through eth_device' iobase */ +struct eth_device { + const char *name; + unsigned long iobase; + void *priv; +}; #include "../drivers/net/smc911x.h"
/** @@ -55,32 +59,32 @@ static void usage(void) * Registers 0x00 - 0x50 are FIFOs. The 0x50+ are the control registers * and they're all 32bits long. 0xB8+ are reserved, so don't bother. */ -static void dump_regs(void) +static void dump_regs(struct eth_device *dev) { u8 i, j = 0; for (i = 0x50; i < 0xB8; i += sizeof(u32)) printf("%02x: 0x%08x %c", i, - smc911x_reg_read(CONFIG_DRIVER_SMC911X_BASE + i), + smc911x_reg_read(dev, i), (j++ % 2 ? '\n' : ' ')); }
/** * do_eeprom_cmd - handle eeprom communication */ -static int do_eeprom_cmd(int cmd, u8 reg) +static int do_eeprom_cmd(struct eth_device *dev, int cmd, u8 reg) { - if (smc911x_reg_read(E2P_CMD) & E2P_CMD_EPC_BUSY) { + if (smc911x_reg_read(dev, E2P_CMD) & E2P_CMD_EPC_BUSY) { printf("eeprom_cmd: busy at start (E2P_CMD = 0x%08x)\n", - smc911x_reg_read(E2P_CMD)); + smc911x_reg_read(dev, E2P_CMD)); return -1; }
- smc911x_reg_write(E2P_CMD, E2P_CMD_EPC_BUSY | cmd | reg); + smc911x_reg_write(dev, E2P_CMD, E2P_CMD_EPC_BUSY | cmd | reg);
- while (smc911x_reg_read(E2P_CMD) & E2P_CMD_EPC_BUSY) + while (smc911x_reg_read(dev, E2P_CMD) & E2P_CMD_EPC_BUSY) if (smsc_ctrlc()) { printf("eeprom_cmd: timeout (E2P_CMD = 0x%08x)\n", - smc911x_reg_read(E2P_CMD)); + smc911x_reg_read(dev, E2P_CMD)); return -1; }
@@ -90,37 +94,37 @@ static int do_eeprom_cmd(int cmd, u8 reg) /** * read_eeprom_reg - read specified register in EEPROM */ -static u8 read_eeprom_reg(u8 reg) +static u8 read_eeprom_reg(struct eth_device *dev, u8 reg) { - int ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_READ, reg); - return (ret ? : smc911x_reg_read(E2P_DATA)); + int ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_READ, reg); + return (ret ? : smc911x_reg_read(dev, E2P_DATA)); }
/** * write_eeprom_reg - write specified value into specified register in EEPROM */ -static int write_eeprom_reg(u8 value, u8 reg) +static int write_eeprom_reg(struct eth_device *dev, u8 value, u8 reg) { int ret;
/* enable erasing/writing */ - ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_EWEN, reg); + ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_EWEN, reg); if (ret) goto done;
/* erase the eeprom reg */ - ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_ERASE, reg); + ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_ERASE, reg); if (ret) goto done;
/* write the eeprom reg */ - smc911x_reg_write(E2P_DATA, value); - ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_WRITE, reg); + smc911x_reg_write(dev, E2P_DATA, value); + ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_WRITE, reg); if (ret) goto done;
/* disable erasing/writing */ - ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_EWDS, reg); + ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_EWDS, reg);
done: return ret; @@ -139,7 +143,7 @@ static char *skip_space(char *buf) /** * write_stuff - handle writing of MAC registers / eeprom */ -static void write_stuff(char *line) +static void write_stuff(struct eth_device *dev, char *line) { char dest; char *endp; @@ -182,39 +186,39 @@ static void write_stuff(char *line) /* Finally, execute the command */ if (dest == 'E') { printf("Writing EEPROM register %02x with %02x\n", reg, value); - write_eeprom_reg(value, reg); + write_eeprom_reg(dev, value, reg); } else { printf("Writing MAC register %02x with %08x\n", reg, value); - smc911x_reg_write(CONFIG_DRIVER_SMC911X_BASE + reg, value); + smc911x_reg_write(dev, reg, value); } }
/** * copy_from_eeprom - copy MAC address in eeprom to address registers */ -static void copy_from_eeprom(void) +static void copy_from_eeprom(struct eth_device *dev) { ulong addrl = - read_eeprom_reg(0x01) | - read_eeprom_reg(0x02) << 8 | - read_eeprom_reg(0x03) << 16 | - read_eeprom_reg(0x04) << 24; + read_eeprom_reg(dev, 0x01) | + read_eeprom_reg(dev, 0x02) << 8 | + read_eeprom_reg(dev, 0x03) << 16 | + read_eeprom_reg(dev, 0x04) << 24; ulong addrh = - read_eeprom_reg(0x05) | - read_eeprom_reg(0x06) << 8; - smc911x_set_mac_csr(ADDRL, addrl); - smc911x_set_mac_csr(ADDRH, addrh); + read_eeprom_reg(dev, 0x05) | + read_eeprom_reg(dev, 0x06) << 8; + smc911x_set_mac_csr(dev, ADDRL, addrl); + smc911x_set_mac_csr(dev, ADDRH, addrh); puts("EEPROM contents copied to MAC\n"); }
/** * print_macaddr - print MAC address registers and MAC address in eeprom */ -static void print_macaddr(void) +static void print_macaddr(struct eth_device *dev) { puts("Current MAC Address in MAC: "); - ulong addrl = smc911x_get_mac_csr(ADDRL); - ulong addrh = smc911x_get_mac_csr(ADDRH); + ulong addrl = smc911x_get_mac_csr(dev, ADDRL); + ulong addrh = smc911x_get_mac_csr(dev, ADDRH); printf("%02x:%02x:%02x:%02x:%02x:%02x\n", (u8)(addrl), (u8)(addrl >> 8), (u8)(addrl >> 16), (u8)(addrl >> 24), (u8)(addrh), (u8)(addrh >> 8)); @@ -222,41 +226,42 @@ static void print_macaddr(void) puts("Current MAC Address in EEPROM: "); int i; for (i = 1; i < 6; ++i) - printf("%02x:", read_eeprom_reg(i)); - printf("%02x\n", read_eeprom_reg(i)); + printf("%02x:", read_eeprom_reg(dev, i)); + printf("%02x\n", read_eeprom_reg(dev, i)); }
/** * dump_eeprom - dump the whole content of the EEPROM */ -static void dump_eeprom(void) +static void dump_eeprom(struct eth_device *dev) { int i; puts("EEPROM:\n"); for (i = 0; i < 7; ++i) - printf("%02x: 0x%02x\n", i, read_eeprom_reg(i)); + printf("%02x: 0x%02x\n", i, read_eeprom_reg(dev, i)); }
/** * smc911x_init - get the MAC/EEPROM up and ready for use */ -static int smc911x_init(void) +static int smc911x_init(struct eth_device *dev) { /* See if there is anything there */ - if (!smc911x_detect_chip()) + if (!smc911x_detect_chip(dev)) return 1;
- smc911x_reset(); + smc911x_reset(dev);
/* Make sure we set EEDIO/EECLK to the EEPROM */ - if (smc911x_reg_read(GPIO_CFG) & GPIO_CFG_EEPR_EN) { - while (smc911x_reg_read(E2P_CMD) & E2P_CMD_EPC_BUSY) + if (smc911x_reg_read(dev, GPIO_CFG) & GPIO_CFG_EEPR_EN) { + while (smc911x_reg_read(dev, E2P_CMD) & E2P_CMD_EPC_BUSY) if (smsc_ctrlc()) { printf("init: timeout (E2P_CMD = 0x%08x)\n", - smc911x_reg_read(E2P_CMD)); + smc911x_reg_read(dev, E2P_CMD)); return 1; } - smc911x_reg_write(GPIO_CFG, smc911x_reg_read(GPIO_CFG) & ~GPIO_CFG_EEPR_EN); + smc911x_reg_write(dev, GPIO_CFG, + smc911x_reg_read(dev, GPIO_CFG) & ~GPIO_CFG_EEPR_EN); }
return 0; @@ -317,6 +322,11 @@ static char *getline(void) */ int smc911x_eeprom(int argc, char *argv[]) { + /* Avoid initializing on stack as gcc likes to call memset() */ + struct eth_device dev; + dev.name = __func__; + dev.iobase = CONFIG_SMC911X_BASE; + /* Print the ABI version */ app_startup(argv); if (XF_VERSION != get_version()) { @@ -328,7 +338,7 @@ int smc911x_eeprom(int argc, char *argv[])
/* Initialize the MAC/EEPROM somewhat */ puts("\n"); - if (smc911x_init()) + if (smc911x_init(&dev)) return 1;
/* Dump helpful usage information */ @@ -360,11 +370,11 @@ int smc911x_eeprom(int argc, char *argv[])
/* Now parse the command */ switch (line[0]) { - case 'W': write_stuff(line); break; - case 'D': dump_eeprom(); break; - case 'M': dump_regs(); break; - case 'C': copy_from_eeprom(); break; - case 'P': print_macaddr(); break; + case 'W': write_stuff(&dev, line); break; + case 'D': dump_eeprom(&dev); break; + case 'M': dump_regs(&dev); break; + case 'C': copy_from_eeprom(&dev); break; + case 'P': print_macaddr(&dev); break; unknown_cmd: default: puts("ERROR: Unknown command!\n\n"); case '?': @@ -373,11 +383,3 @@ int smc911x_eeprom(int argc, char *argv[]) } } } - -#else -int smc911x_eeprom(int argc, char *argv[]) -{ - puts("Not supported for this board\n"); - return 1; -} -#endif
Hi Mike,
On Thu, Nov 12, 2009 at 12:50 AM, Mike Frysinger vapier@gentoo.org wrote:
here's my [compile] tested change as the board i have with this part isnt readily accessible atm ... -mike
I've tested your changes, MAC register dumps works Ok. I needed to make some changes to make it work, see the comments below.
diff --git a/examples/standalone/smc911x_eeprom.c b/examples/standalone/smc911x_eeprom.c index bf22f0a..f2a6304 100644 --- a/examples/standalone/smc911x_eeprom.c +++ b/examples/standalone/smc911x_eeprom.c @@ -17,8 +17,8 @@ #include <common.h> #include <exports.h>
-#ifdef CONFIG_DRIVER_SMC911X
+/* the smc911x.h gets base addr through eth_device' iobase */ +struct eth_device { const char *name; unsigned long iobase; };
Needed to add 'void *priv' needed for smc911x_detect_chip, otherwise it does not compile with the latest U-Boot.
#include "../drivers/net/smc911x.h"
/** @@ -55,32 +55,32 @@ static void usage(void) * Registers 0x00 - 0x50 are FIFOs. The 0x50+ are the control registers * and they're all 32bits long. 0xB8+ are reserved, so don't bother. */ -static void dump_regs(void) +static void dump_regs(struct eth_device *dev) { u8 i, j = 0; for (i = 0x50; i < 0xB8; i += sizeof(u32)) printf("%02x: 0x%08x %c", i,
- smc911x_reg_read(CONFIG_DRIVER_SMC911X_BASE + i),
- smc911x_reg_read(dev, i),
(j++ % 2 ? '\n' : ' ')); }
/** * do_eeprom_cmd - handle eeprom communication */ -static int do_eeprom_cmd(int cmd, u8 reg) +static int do_eeprom_cmd(struct eth_device *dev, int cmd, u8 reg) {
- if (smc911x_reg_read(E2P_CMD) & E2P_CMD_EPC_BUSY) {
- if (smc911x_reg_read(dev, E2P_CMD) & E2P_CMD_EPC_BUSY) {
printf("eeprom_cmd: busy at start (E2P_CMD = 0x%08x)\n",
- smc911x_reg_read(E2P_CMD));
- smc911x_reg_read(dev, E2P_CMD));
return -1; }
- smc911x_reg_write(E2P_CMD, E2P_CMD_EPC_BUSY | cmd | reg);
- smc911x_reg_write(dev, E2P_CMD, E2P_CMD_EPC_BUSY | cmd | reg);
- while (smc911x_reg_read(E2P_CMD) & E2P_CMD_EPC_BUSY)
- while (smc911x_reg_read(dev, E2P_CMD) & E2P_CMD_EPC_BUSY)
if (smsc_ctrlc()) { printf("eeprom_cmd: timeout (E2P_CMD = 0x%08x)\n",
- smc911x_reg_read(E2P_CMD));
- smc911x_reg_read(dev, E2P_CMD));
return -1; }
@@ -90,37 +90,37 @@ static int do_eeprom_cmd(int cmd, u8 reg) /** * read_eeprom_reg - read specified register in EEPROM */ -static u8 read_eeprom_reg(u8 reg) +static u8 read_eeprom_reg(struct eth_device *dev, u8 reg) {
- int ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_READ, reg);
- return (ret ? : smc911x_reg_read(E2P_DATA));
- int ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_READ, reg);
- return (ret ? : smc911x_reg_read(dev, E2P_DATA));
}
/** * write_eeprom_reg - write specified value into specified register in EEPROM */ -static int write_eeprom_reg(u8 value, u8 reg) +static int write_eeprom_reg(struct eth_device *dev, u8 value, u8 reg) { int ret;
/* enable erasing/writing */
- ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_EWEN, reg);
- ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_EWEN, reg);
if (ret) goto done;
/* erase the eeprom reg */
- ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_ERASE, reg);
- ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_ERASE, reg);
if (ret) goto done;
/* write the eeprom reg */
- smc911x_reg_write(E2P_DATA, value);
- ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_WRITE, reg);
- smc911x_reg_write(dev, E2P_DATA, value);
- ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_WRITE, reg);
if (ret) goto done;
/* disable erasing/writing */
- ret = do_eeprom_cmd(E2P_CMD_EPC_CMD_EWDS, reg);
- ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_EWDS, reg);
done: return ret; @@ -139,7 +139,7 @@ static char *skip_space(char *buf) /** * write_stuff - handle writing of MAC registers / eeprom */ -static void write_stuff(char *line) +static void write_stuff(struct eth_device *dev, char *line) { char dest; char *endp; @@ -182,39 +182,39 @@ static void write_stuff(char *line) /* Finally, execute the command */ if (dest == 'E') { printf("Writing EEPROM register %02x with %02x\n", reg, value);
- write_eeprom_reg(value, reg);
- write_eeprom_reg(dev, value, reg);
} else { printf("Writing MAC register %02x with %08x\n", reg, value);
- smc911x_reg_write(CONFIG_DRIVER_SMC911X_BASE + reg, value);
- smc911x_reg_write(dev, reg, value);
} }
/** * copy_from_eeprom - copy MAC address in eeprom to address registers */ -static void copy_from_eeprom(void) +static void copy_from_eeprom(struct eth_device *dev) { ulong addrl =
- read_eeprom_reg(0x01) |
- read_eeprom_reg(0x02) << 8 |
- read_eeprom_reg(0x03) << 16 |
- read_eeprom_reg(0x04) << 24;
- read_eeprom_reg(dev, 0x01) |
- read_eeprom_reg(dev, 0x02) << 8 |
- read_eeprom_reg(dev, 0x03) << 16 |
- read_eeprom_reg(dev, 0x04) << 24;
ulong addrh =
- read_eeprom_reg(0x05) |
- read_eeprom_reg(0x06) << 8;
- smc911x_set_mac_csr(ADDRL, addrl);
- smc911x_set_mac_csr(ADDRH, addrh);
- read_eeprom_reg(dev, 0x05) |
- read_eeprom_reg(dev, 0x06) << 8;
- smc911x_set_mac_csr(dev, ADDRL, addrl);
- smc911x_set_mac_csr(dev, ADDRH, addrh);
puts("EEPROM contents copied to MAC\n"); }
/** * print_macaddr - print MAC address registers and MAC address in eeprom */ -static void print_macaddr(void) +static void print_macaddr(struct eth_device *dev) { puts("Current MAC Address in MAC: ");
- ulong addrl = smc911x_get_mac_csr(ADDRL);
- ulong addrh = smc911x_get_mac_csr(ADDRH);
- ulong addrl = smc911x_get_mac_csr(dev, ADDRL);
- ulong addrh = smc911x_get_mac_csr(dev, ADDRH);
printf("%02x:%02x:%02x:%02x:%02x:%02x\n", (u8)(addrl), (u8)(addrl >> 8), (u8)(addrl >> 16), (u8)(addrl >> 24), (u8)(addrh), (u8)(addrh >> 8)); @@ -222,41 +222,42 @@ static void print_macaddr(void) puts("Current MAC Address in EEPROM: "); int i; for (i = 1; i < 6; ++i)
- printf("%02x:", read_eeprom_reg(i));
- printf("%02x\n", read_eeprom_reg(i));
- printf("%02x:", read_eeprom_reg(dev, i));
- printf("%02x\n", read_eeprom_reg(dev, i));
}
/** * dump_eeprom - dump the whole content of the EEPROM */ -static void dump_eeprom(void) +static void dump_eeprom(struct eth_device *dev) { int i; puts("EEPROM:\n"); for (i = 0; i < 7; ++i)
- printf("%02x: 0x%02x\n", i, read_eeprom_reg(i));
- printf("%02x: 0x%02x\n", i, read_eeprom_reg(dev, i));
}
/** * smc911x_init - get the MAC/EEPROM up and ready for use */ -static int smc911x_init(void) +static int smc911x_init(struct eth_device *dev) { /* See if there is anything there */
- if (!smc911x_detect_chip())
- if (!smc911x_detect_chip(dev))
Should be if (smc911x_detect_chip(dev)) since smc911x_detect_chip returns 0 on success.
return 1;
- smc911x_reset();
- smc911x_reset(dev);
/* Make sure we set EEDIO/EECLK to the EEPROM */
- if (smc911x_reg_read(GPIO_CFG) & GPIO_CFG_EEPR_EN) {
- while (smc911x_reg_read(E2P_CMD) & E2P_CMD_EPC_BUSY)
- if (smc911x_reg_read(dev, GPIO_CFG) & GPIO_CFG_EEPR_EN) {
- while (smc911x_reg_read(dev, E2P_CMD) & E2P_CMD_EPC_BUSY)
if (smsc_ctrlc()) { printf("init: timeout (E2P_CMD = 0x%08x)\n",
- smc911x_reg_read(E2P_CMD));
- smc911x_reg_read(dev, E2P_CMD));
return 1; }
- smc911x_reg_write(GPIO_CFG, smc911x_reg_read(GPIO_CFG) &
~GPIO_CFG_EEPR_EN);
- smc911x_reg_write(dev, GPIO_CFG,
- smc911x_reg_read(dev, GPIO_CFG) & ~GPIO_CFG_EEPR_EN);
}
return 0; @@ -317,6 +318,11 @@ static char *getline(void) */ int smc911x_eeprom(int argc, char *argv[]) {
- struct eth_device dev = {
- .name = __func__,
- .iobase = CONFIG_SMC911X_BASE,
- };
This requires memset on arches that use C version (e.g. ARM). For my testing I've just copied the memset from lib_generic/string.c, but obviously it's not the correct solution.
/* Print the ABI version */ app_startup(argv); if (XF_VERSION != get_version()) { @@ -328,7 +334,7 @@ int smc911x_eeprom(int argc, char *argv[])
/* Initialize the MAC/EEPROM somewhat */ puts("\n");
- if (smc911x_init())
- if (smc911x_init(&dev))
return 1;
/* Dump helpful usage information */ @@ -360,11 +366,11 @@ int smc911x_eeprom(int argc, char *argv[])
/* Now parse the command */ switch (line[0]) {
- case 'W': write_stuff(line); break;
- case 'D': dump_eeprom(); break;
- case 'M': dump_regs(); break;
- case 'C': copy_from_eeprom(); break;
- case 'P': print_macaddr(); break;
- case 'W': write_stuff(&dev, line); break;
- case 'D': dump_eeprom(&dev); break;
- case 'M': dump_regs(&dev); break;
- case 'C': copy_from_eeprom(&dev); break;
- case 'P': print_macaddr(&dev); break;
unknown_cmd: default: puts("ERROR: Unknown command!\n\n"); case '?': @@ -373,11 +379,3 @@ int smc911x_eeprom(int argc, char *argv[]) } } }
-#else -int smc911x_eeprom(int argc, char *argv[]) -{
- puts("Not supported for this board\n");
- return 1;
-} -#endif
On Thursday 12 November 2009 04:13:48 Mike Rapoport wrote:
On Thu, Nov 12, 2009 at 12:50 AM, Mike Frysinger wrote:
here's my [compile] tested change as the board i have with this part isnt readily accessible atm ...
I've tested your changes, MAC register dumps works Ok. I needed to make some changes to make it work, see the comments below.
--- a/examples/standalone/smc911x_eeprom.c +++ b/examples/standalone/smc911x_eeprom.c @@ -17,8 +17,8 @@ #include <common.h> #include <exports.h>
-#ifdef CONFIG_DRIVER_SMC911X
+/* the smc911x.h gets base addr through eth_device' iobase */ +struct eth_device { const char *name; unsigned long iobase; };
Needed to add 'void *priv' needed for smc911x_detect_chip, otherwise it does not compile with the latest U-Boot.
i wrote it against last release which didnt need this. looking at smc911x.h, obviously priv is needed now and shouldnt be a problem adding it. -mike
If there is no SROM attached to the SMSC chip it's MAC address is initialized to ff:ff:ff:ff:ff:ff and it causes the following warning:
Warning: smc911x-0 MAC addresses don't match: Address in SROM is ff:ff:ff:ff:ff:ff Address in environment is 00:01:ba:dc:0d:03
Set dev->enetaddr only if MAC address is valid, and thus avoid the above case.
Signed-off-by: Mike Rapoport mike@compulab.co.il --- drivers/net/smc911x.c | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index 9ff1288..f5d984d 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -237,6 +237,7 @@ static int smc911x_rx(struct eth_device *dev) int smc911x_initialize(u8 dev_num, int base_addr) { unsigned long addrl, addrh; + unsigned char enetaddr[6]; struct eth_device *dev;
dev = malloc(sizeof(*dev)); @@ -257,12 +258,15 @@ int smc911x_initialize(u8 dev_num, int base_addr)
addrh = smc911x_get_mac_csr(dev, ADDRH); addrl = smc911x_get_mac_csr(dev, ADDRL); - dev->enetaddr[0] = addrl; - dev->enetaddr[1] = addrl >> 8; - dev->enetaddr[2] = addrl >> 16; - dev->enetaddr[3] = addrl >> 24; - dev->enetaddr[4] = addrh; - dev->enetaddr[5] = addrh >> 8; + enetaddr[0] = addrl; + enetaddr[1] = addrl >> 8; + enetaddr[2] = addrl >> 16; + enetaddr[3] = addrl >> 24; + enetaddr[4] = addrh; + enetaddr[5] = addrh >> 8; + + if (is_valid_ether_addr(enetaddr)) + memcpy(dev->enetaddr, enetaddr, 6);
dev->init = smc911x_init; dev->halt = smc911x_halt;
On Wednesday 11 November 2009 03:03:02 Mike Rapoport wrote:
If there is no SROM attached to the SMSC chip it's MAC address is initialized to ff:ff:ff:ff:ff:ff and it causes the following warning:
Warning: smc911x-0 MAC addresses don't match: Address in SROM is ff:ff:ff:ff:ff:ff Address in environment is 00:01:ba:dc:0d:03Set dev->enetaddr only if MAC address is valid, and thus avoid the above case.
someone already posted a patch for this issue: NET: Fix MAC addr handling for smc911x
i think the approach they took is better -- they check for explicit values that indicate 'no srom is attached' rather than 'is the mac valid' as your code could ignore attached srom's but bad addresses. in this latter case, i think we want the warning. -mike
Mike Frysinger wrote:
On Wednesday 11 November 2009 03:03:02 Mike Rapoport wrote:
If there is no SROM attached to the SMSC chip it's MAC address is initialized to ff:ff:ff:ff:ff:ff and it causes the following warning:
Warning: smc911x-0 MAC addresses don't match: Address in SROM is ff:ff:ff:ff:ff:ff Address in environment is 00:01:ba:dc:0d:03Set dev->enetaddr only if MAC address is valid, and thus avoid the above case.
someone already posted a patch for this issue: NET: Fix MAC addr handling for smc911x
i think the approach they took is better -- they check for explicit values that indicate 'no srom is attached' rather than 'is the mac valid' as your code could ignore attached srom's but bad addresses. in this latter case, i think we want the warning.
agree
-mike
Since commit 736fead8fdbf8a8407048bebc373cd551d01ec98 "Convert SMC911X Ethernet driver to CONFIG_NET_MULTI API" SMC911X configration options are called CONFIG_SMC911X rather than CONFIG_DRIVER_SMC911X. Update README to reflect that change.
Signed-off-by: Mike Rapoport mike@compulab.co.il --- README | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/README b/README index 2c77687..84d7f16 100644 --- a/README +++ b/README @@ -842,20 +842,20 @@ The following options need to be configured: Define this to use i/o functions instead of macros (some hardware wont work with macros)
- CONFIG_DRIVER_SMC911X + CONFIG_SMC911X Support for SMSC's LAN911x and LAN921x chips
- CONFIG_DRIVER_SMC911X_BASE + CONFIG_SMC911X_BASE Define this to hold the physical address of the device (I/O space)
- CONFIG_DRIVER_SMC911X_32_BIT + CONFIG_SMC911X_32_BIT Define this if data bus is 32 bits
- CONFIG_DRIVER_SMC911X_16_BIT + CONFIG_SMC911X_16_BIT Define this if data bus is 16 bits. If your processor automatically converts one 32 bit word to two 16 bit - words you may also try CONFIG_DRIVER_SMC911X_32_BIT. + words you may also try CONFIG_SMC911X_32_BIT.
- USB Support: At the moment only the UHCI host controller is
Dear Mike Rapoport,
In message 6b76074bd24b92947049829d60eff2f5441ac221.1257861401.git.mike@compulab.co.il you wrote:
Since commit 736fead8fdbf8a8407048bebc373cd551d01ec98 "Convert SMC911X Ethernet driver to CONFIG_NET_MULTI API" SMC911X configration options are called CONFIG_SMC911X rather than CONFIG_DRIVER_SMC911X. Update README to reflect that change.
Signed-off-by: Mike Rapoport mike@compulab.co.il
README | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
Applied, thanks.
Ben, I hope this is OK with you.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Dear Mike Rapoport,
In message 6b76074bd24b92947049829d60eff2f5441ac221.1257861401.git.mike@compulab.co.il you wrote:
Since commit 736fead8fdbf8a8407048bebc373cd551d01ec98 "Convert SMC911X Ethernet driver to CONFIG_NET_MULTI API" SMC911X configration options are called CONFIG_SMC911X rather than CONFIG_DRIVER_SMC911X. Update README to reflect that change.
Signed-off-by: Mike Rapoport mike@compulab.co.il
README | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
Applied, thanks.
Ben, I hope this is OK with you.
Best regards,
Wolfgang Denk
Sure. You're making my to-do list smaller, so, yeah it's OK!
regards, Ben
The smc911x_halt gets called after completion of network opration and resets the chip. When there is no SROM attached to the SMSC, MAC address gets reset as well. Add CONFIG_SMC911X_KEEP_MAC option to allow boards with no SROM instruct the SMSC driver to keep mac address after the reset
Signed-off-by: Mike Rapoport mike@compulab.co.il --- README | 5 +++++ drivers/net/smc911x.c | 4 ++++ 2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/README b/README index 84d7f16..a5956b2 100644 --- a/README +++ b/README @@ -857,6 +857,11 @@ The following options need to be configured: automatically converts one 32 bit word to two 16 bit words you may also try CONFIG_SMC911X_32_BIT.
+ CONFIG_SMC911X_KEEP_MAC + Define this if your board does not have SROM + attached to SMC chip and the MAC address is + stored elsewhere + - USB Support: At the moment only the UHCI host controller is supported (PIP405, MIP405, MPC5200); define diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index f5d984d..eee34f0 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -206,6 +206,10 @@ static int smc911x_send(struct eth_device *dev, static void smc911x_halt(struct eth_device *dev) { smc911x_reset(dev); + +#ifdef CONFIG_SMC911X_KEEP_MAC + smc911x_handle_mac_address(dev); +#endif }
static int smc911x_rx(struct eth_device *dev)
On Wednesday 11 November 2009 03:03:04 Mike Rapoport wrote:
The smc911x_halt gets called after completion of network opration and resets the chip. When there is no SROM attached to the SMSC, MAC address gets reset as well. Add CONFIG_SMC911X_KEEP_MAC option to allow boards with no SROM instruct the SMSC driver to keep mac address after the reset
the mac already has proper storage -- the environment. please read doc/REAdME.enetaddr. so i dont thinkt his option should be added. -mike
Mike Frysinger wrote:
On Wednesday 11 November 2009 03:03:04 Mike Rapoport wrote:
The smc911x_halt gets called after completion of network opration and resets the chip. When there is no SROM attached to the SMSC, MAC address gets reset as well. Add CONFIG_SMC911X_KEEP_MAC option to allow boards with no SROM instruct the SMSC driver to keep mac address after the reset
the mac already has proper storage -- the environment. please read doc/REAdME.enetaddr. so i dont thinkt his option should be added.
If you have no network activity in U-Boot the mac address never gets from the environment to the SMSC chip. So, when the kernel boots there's no mac address neither in the chip registers nor in (absent) SROM.
-mike
On Wednesday 11 November 2009 11:05:24 Mike Rapoport wrote:
Mike Frysinger wrote:
On Wednesday 11 November 2009 03:03:04 Mike Rapoport wrote:
The smc911x_halt gets called after completion of network opration and resets the chip. When there is no SROM attached to the SMSC, MAC address gets reset as well. Add CONFIG_SMC911X_KEEP_MAC option to allow boards with no SROM instruct the SMSC driver to keep mac address after the reset
the mac already has proper storage -- the environment. please read doc/REAdME.enetaddr. so i dont thinkt his option should be added.
If you have no network activity in U-Boot the mac address never gets from the environment to the SMSC chip. So, when the kernel boots there's no mac address neither in the chip registers nor in (absent) SROM.
this is correct u-boot behavior. please read the faq. http://www.denx.de/wiki/view/DULG/EthernetDoesNotWorkInLinux -mike
participants (5)
-
Ben Warren -
Mike Frysinger -
Mike Rapoport -
Mike Rapoport -
Wolfgang Denk