[U-Boot] [PATCH] add dm9000 eeprom read/write command

[U-Boot] [PATCH] add dm9000 eeprom read/write command

[Eric Jarrige]

Signed-off-by: Eric Jarrige <eric.jarrige@armadeus.org>
Signed-off-by: Stefano Babic <sbabic@denx.de>
CC: Ben Warren <biggerbadderben@gmail.com>
---
 README                |    1 +
 common/Makefile       |    1 +
 common/cmd_dm9000ee.c |   84 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+), 0 deletions(-)
 create mode 100644 common/cmd_dm9000ee.c

diff --git a/README b/README
index 0886987..c6981bf 100644
--- a/README
+++ b/README
@@ -707,6 +707,7 @@ The following options need to be configured:
 		CONFIG_CMD_DATE		* support for RTC, date/time...
 		CONFIG_CMD_DHCP		* DHCP support
 		CONFIG_CMD_DIAG		* Diagnostics
+		CONFIG_CMD_DM9000EE	  DM9000 external EEPROM support
 		CONFIG_CMD_DS4510	* ds4510 I2C gpio commands
 		CONFIG_CMD_DS4510_INFO	* ds4510 I2C info command
 		CONFIG_CMD_DS4510_MEM	* ds4510 I2C eeprom/sram commansd
diff --git a/common/Makefile b/common/Makefile
index d662468..d22cbac 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -81,6 +81,7 @@ ifdef CONFIG_POST
 COBJS-$(CONFIG_CMD_DIAG) += cmd_diag.o
 endif
 COBJS-$(CONFIG_CMD_DISPLAY) += cmd_display.o
+COBJS-$(CONFIG_CMD_DM9000EE) += cmd_dm9000ee.o
 COBJS-$(CONFIG_CMD_DTT) += cmd_dtt.o
 COBJS-$(CONFIG_CMD_ECHO) += cmd_echo.o
 COBJS-$(CONFIG_ENV_IS_IN_EEPROM) += cmd_eeprom.o
diff --git a/common/cmd_dm9000ee.c b/common/cmd_dm9000ee.c
new file mode 100644
index 0000000..0d53c70
--- /dev/null
+++ b/common/cmd_dm9000ee.c
@@ -0,0 +1,84 @@
+/*
+ * (C) Copyright 2008-2011 Armadeus Project
+ * (C) Copyright 2007
+ * Stefano Babic, DENX Software Engineering, sbabic at denx.de.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <command.h>
+#include <dm9000.h>
+
+#if (!defined(CONFIG_DM9000_NO_SROM) && defined(CONFIG_DRIVER_DM9000))
+
+static int do_read_dm9000_eeprom(cmd_tbl_t *cmdtp, int flag, int argc,
+				 char *const argv[])
+{
+	unsigned int i;
+	u8 data[2];
+
+	for (i = 0; i < 0x40; i++) {
+		if (!(i % 0x10))
+			printf("\n%08x:", i);
+		dm9000_read_srom_word(i, data);
+		printf(" %02x%02x", data[1], data[0]);
+	}
+	printf("\n");
+	return 0;
+}
+
+static int do_write_dm9000_eeprom(cmd_tbl_t *cmdtp, int flag, int argc,
+				  char *const argv[])
+{
+	unsigned long offset, value;
+
+	if (argc < 4)
+		return cmd_usage(cmdtp);
+
+	strict_strtoul(argv[2], 16, &offset);
+	strict_strtoul(argv[3], 16, &value);
+	if (offset > 0x40) {
+		printf("Wrong offset : 0x%lx\n", offset);
+		return cmd_usage(cmdtp);
+	}
+	dm9000_write_srom_word(offset, value);
+	return 0;
+}
+
+int do_dm9000_eeprom(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
+{
+	if (argc < 2)
+		return cmd_usage(cmdtp);
+
+	if (strcmp(argv[1], "read") == 0)
+		return do_read_dm9000_eeprom(cmdtp, flag, argc, argv);
+	else if (strcmp(argv[1], "write") == 0)
+		return do_write_dm9000_eeprom(cmdtp, flag, argc, argv);
+	else
+		return cmd_usage(cmdtp);
+}
+
+U_BOOT_CMD(dm9000ee, 4, 1, do_dm9000_eeprom,
+	   "Read/Write eeprom connected to Ethernet Controller",
+	   "\ndm9000ee write <word offset> <value> \n"
+	   "\tdm9000ee read \n"
+	   "\tword:\t\t00-02 : MAC Address\n"
+	   "\t\t\t03-07 : DM9000 Configuration\n" "\t\t\t08-63 : User data");
+#endif /* !CONFIG_DM9000_NO_SROM && CONFIG_DRIVER_DM9000 */

[U-Boot] [PATCH] add dm9000 eeprom read/write command

[Mike Frysinger]

On Monday, August 22, 2011 17:37:42 Eric Jarrige wrote:
> +#if (!defined(CONFIG_DM9000_NO_SROM) && defined(CONFIG_DRIVER_DM9000))

why is this necessary ?
-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/20110822/c6153658/attachment.pgp 

[U-Boot] [PATCH] add dm9000 eeprom read/write command

[Eric Jarrige]

Hi Mike,

On 22 ao?t 2011, at 23:55, Mike Frysinger wrote:

> On Monday, August 22, 2011 17:37:42 Eric Jarrige wrote:
>> +#if (!defined(CONFIG_DM9000_NO_SROM) && defined(CONFIG_DRIVER_DM9000))
> 
> why is this necessary ?
> -mike

the methods dm9000_read_srom_word()/dm9000_write_srom_word() need these conditions to be compiled in dm9000x.c
Therefore this avoid dead code and/or compilation failure if these conditions are not met.

Best regards,
Eric

[U-Boot] [PATCH] add dm9000 eeprom read/write command

[Stefano Babic]

On 08/22/2011 11:37 PM, Eric Jarrige wrote:
> Signed-off-by: Eric Jarrige <eric.jarrige@armadeus.org>
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> CC: Ben Warren <biggerbadderben@gmail.com>

I think we should drop Ben's name as maintainer for the network because
he retired some times ago, but his name is still in the maintainer list.

Wolfgang is the maintainer at the moment, I added him in CC.

> ---
>  README                |    1 +
>  common/Makefile       |    1 +
>  common/cmd_dm9000ee.c |   84 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 86 insertions(+), 0 deletions(-)
>  create mode 100644 common/cmd_dm9000ee.c

Why do not we add this code simply to the DM9000 driver ? I think it
belongs to the driver and that is the right place to put this stuff.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH] add dm9000 eeprom read/write command

[Mike Frysinger]

On Tuesday, August 23, 2011 01:15:25 Eric Jarrige wrote:
> On 22 ao?t 2011, at 23:55, Mike Frysinger wrote:
> > On Monday, August 22, 2011 17:37:42 Eric Jarrige wrote:
> >> +#if (!defined(CONFIG_DM9000_NO_SROM) && defined(CONFIG_DRIVER_DM9000))
> > 
> > why is this necessary ?
> 
> the methods dm9000_read_srom_word()/dm9000_write_srom_word() need these
> conditions to be compiled in dm9000x.c Therefore this avoid dead code
> and/or compilation failure if these conditions are not met.

my point is that it is user error to select CONFIG_CMD_DM9000EE but not select 
the DM9000 settings.  in your case, the user silently gets a build that makes 
no sense.  in my case, they're forced to fix things.
-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/20110823/87b1505c/attachment.pgp 

[U-Boot] [PATCH] add dm9000 eeprom read/write command

[Stefano Babic]

On 08/23/2011 03:42 PM, Mike Frysinger wrote:
> On Tuesday, August 23, 2011 01:15:25 Eric Jarrige wrote:
>> On 22 ao?t 2011, at 23:55, Mike Frysinger wrote:
>>> On Monday, August 22, 2011 17:37:42 Eric Jarrige wrote:
>>>> +#if (!defined(CONFIG_DM9000_NO_SROM) && defined(CONFIG_DRIVER_DM9000))
>>>
>>> why is this necessary ?
>>
>> the methods dm9000_read_srom_word()/dm9000_write_srom_word() need these
>> conditions to be compiled in dm9000x.c Therefore this avoid dead code
>> and/or compilation failure if these conditions are not met.
> 
> my point is that it is user error to select CONFIG_CMD_DM9000EE but not select 
> the DM9000 settings.  in your case, the user silently gets a build that makes 
> no sense.  in my case, they're forced to fix things.

Agree. This is also my point to move this code inside the driver itself,
where CONFIG_DRIVER_DM9000 is set (or it is not compiled).

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH] add dm9000 eeprom read/write command

[Eric Jarrige]

On 23 ao?t 2011, at 16:21, Stefano Babic wrote:

> On 08/23/2011 03:42 PM, Mike Frysinger wrote:
>> On Tuesday, August 23, 2011 01:15:25 Eric Jarrige wrote:
>>> On 22 ao?t 2011, at 23:55, Mike Frysinger wrote:
>>>> On Monday, August 22, 2011 17:37:42 Eric Jarrige wrote:
>>>>> +#if (!defined(CONFIG_DM9000_NO_SROM) && defined(CONFIG_DRIVER_DM9000))
>>>> 
>>>> why is this necessary ?
>>> 
>>> the methods dm9000_read_srom_word()/dm9000_write_srom_word() need these
>>> conditions to be compiled in dm9000x.c Therefore this avoid dead code
>>> and/or compilation failure if these conditions are not met.
>> 
>> my point is that it is user error to select CONFIG_CMD_DM9000EE but not select 
>> the DM9000 settings.  in your case, the user silently gets a build that makes 
>> no sense.  in my case, they're forced to fix things.
> 
> Agree. This is also my point to move this code inside the driver itself,
> where CONFIG_DRIVER_DM9000 is set (or it is not compiled).
> 

That makes sense - I can remove this line.

Best regards,
Eric

[U-Boot] [PATCH] add dm9000 eeprom read/write command

[Wolfgang Denk]

Dear Eric Jarrige,

In message <20110822213742.28083.69514.stgit@shuttle2.etheralp.ch> you wrote:
> Signed-off-by: Eric Jarrige <eric.jarrige@armadeus.org>
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> CC: Ben Warren <biggerbadderben@gmail.com>
> ---
>  README                |    1 +
>  common/Makefile       |    1 +
>  common/cmd_dm9000ee.c |   84 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 86 insertions(+), 0 deletions(-)
>  create mode 100644 common/cmd_dm9000ee.c

Checkpatch says:

total: 0 errors, 2 warnings, 98 lines checked

Please clean up and resubmit.  Thanks.

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
If you can't beat it or corrupt it, you pretend it was your  idea  in
the first place.                 - Terry Pratchett, _Guards! Guards!_