[U-Boot] [PATCH 0/5] smc911x driver fixes and additions

[U-Boot] [PATCH 0/5] smc911x driver fixes and additions

[Mike Rapoport]

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(-)

[U-Boot] [PATCH 1/5] smc911x: return -1 when initialization fails

[Mike Rapoport]

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);
-- 
1.6.0.6

[U-Boot] [PATCH 2/5] smc911x: use dev->name in printfs

[Mike Rapoport]

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;
 	}
 
-- 
1.6.0.6

[U-Boot] [PATCH 3/5] smc911x: silence MAC mismatch warning

[Mike Rapoport]

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;
-- 
1.6.0.6

[U-Boot] [PATCH 4/5] smc911x: update SMC911X related configuration description

[Mike Rapoport]

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
-- 
1.6.0.6

[U-Boot] [PATCH 5/5] smc911x: allow mac address to be kept after smc911x_halt

[Mike Rapoport]

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)
-- 
1.6.0.6

[U-Boot] [PATCH 1/5] smc911x: return -1 when initialization fails

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091111/032d095f/attachment.pgp 

[U-Boot] [PATCH 2/5] smc911x: use dev->name in printfs

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091111/7c5d0578/attachment.pgp 

[U-Boot] [PATCH 3/5] smc911x: silence MAC mismatch warning

[Mike Frysinger]

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:03
> 
> Set 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091111/e283b240/attachment.pgp 

[U-Boot] [PATCH 5/5] smc911x: allow mac address to be kept after smc911x_halt

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091111/2b22c8d9/attachment.pgp 

[U-Boot] [PATCH 5/5] smc911x: allow mac address to be kept after smc911x_halt

[Mike Rapoport]

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

-- 
Sincerely yours,
Mike.

[U-Boot] [PATCH 3/5] smc911x: silence MAC mismatch warning

[Mike Rapoport]

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:03
>>
>> Set 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

-- 
Sincerely yours,
Mike.

[U-Boot] [PATCH 5/5] smc911x: allow mac address to be kept after smc911x_halt

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091111/1ea9a2f5/attachment.pgp 

[U-Boot] [PATCH 2/5] smc911x: use dev->name in printfs

[Mike Rapoport]

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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>



-- 
	Sincerely Yours,
		Mike.

[U-Boot] [PATCH 2/5] smc911x: use dev->name in printfs

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091111/d5e7a2a8/attachment.pgp 

[U-Boot] [PATCH 2/5] smc911x: use dev->name in printfs

[Mike Rapoport]

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
>



-- 
	Sincerely Yours,
		Mike.

[U-Boot] [PATCH 2/5] smc911x: use dev->name in printfs

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091111/274e7b1c/attachment.pgp 

[U-Boot] [PATCH 2/5] smc911x: use dev->name in printfs

[Ben Warren]

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

[U-Boot] [PATCH 2/5] smc911x: use dev->name in printfs

[Mike Frysinger]

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@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

[U-Boot] [PATCH 2/5] smc911x: use dev->name in printfs

[Ben Warren]

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

[U-Boot] [PATCH 2/5] smc911x: use dev->name in printfs

[Mike Rapoport]

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
>



-- 
	Sincerely Yours,
		Mike.

[U-Boot] [PATCH] smc911x: make smc911x_initialize return correct value (Was: Re: [PATCH 1/5] smc911x: return -1 when initialization fails)

[Mike Rapoport]

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:

[U-Boot] [PATCH 2/5] smc911x: use dev->name in printfs

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091112/7bc039a5/attachment.pgp 

[U-Boot] [PATCH] smc911x_eeprom: fix building after smc911x overhaul

[Mike Frysinger]

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@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
-- 
1.6.5.2

[U-Boot] [PATCH v2] smc911x_eeprom: fix building after smc911x overhaul

[Mike Frysinger]

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@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
-- 
1.6.5.2

[U-Boot] [PATCH] smc911x: make smc911x_initialize return correct value (Was: Re: [PATCH 1/5] smc911x: return -1 when initialization fails)

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091112/9c076d13/attachment.pgp 

[U-Boot] [PATCH 4/5] smc911x: update SMC911X related configuration description

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There you go man, Keep as cool as you can. It riles them  to  believe
that you perceive the web they weave. Keep on being free!

[U-Boot] [PATCH 4/5] smc911x: update SMC911X related configuration description

[Ben Warren]

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