Re: [U-Boot] [PATCH] add dm9000 eeprom read/write command
Dear Eric Jarrige, In message 20110822213742.28083.69514.st...@shuttle2.etheralp.ch you wrote: Signed-off-by: Eric Jarrige eric.jarr...@armadeus.org Signed-off-by: Stefano Babic sba...@denx.de CC: Ben Warren biggerbadder...@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: w...@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!_ ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] add dm9000 eeprom read/write command
On 08/22/2011 11:37 PM, Eric Jarrige wrote: Signed-off-by: Eric Jarrige eric.jarr...@armadeus.org Signed-off-by: Stefano Babic sba...@denx.de CC: Ben Warren biggerbadder...@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: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] add dm9000 eeprom read/write command
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 signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] add dm9000 eeprom read/write command
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: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] add dm9000 eeprom read/write command
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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] add dm9000 eeprom read/write command
Signed-off-by: Eric Jarrige eric.jarr...@armadeus.org
Signed-off-by: Stefano Babic sba...@denx.de
CC: Ben Warren biggerbadder...@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 000..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, sba...@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] add dm9000 eeprom read/write command
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 signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] add dm9000 eeprom read/write command
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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
