Skip to contentSkip to conversation

add dm9000 eeprom read/write command

PATCH New Eric Jarrige 4 participants 8 messages Aug 22, 2011 – Oct 6, 2011

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 add dm9000 eeprom read/write command

---
 README                |    1 +
 common/Makefile       |    1 +
 common/cmd_dm9000ee.c |   84 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+), 0 deletions(-)
 create mode 100644 common/cmd_dm9000ee.c
README +1 -0
index 0886987..c6981bf 100644
@@ -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
common/Makefile +1 -0
index d662468..d22cbac 100644
@@ -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
common/cmd_dm9000ee.c new file +84 -0

84 lines not on this page · raw message

#1 lore raw

PATCH 1/1 add dm9000 eeprom read/write command

Quoted 1 line · 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

#2 lore raw

PATCH 1/1 add dm9000 eeprom read/write command

Hi Mike,

Quoted 5 lines · Mike Frysinger
Quoted 1 line · 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

#3 lore raw

PATCH 1/1 add dm9000 eeprom read/write command

Quoted 9 lines · On Tuesday, August 23, 2011 01:15:25 Eric Jarrige wrote:
Quoted 4 lines · Mike Frysinger
Quoted 1 line · 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

#4 lore raw

PATCH 1/1 add dm9000 eeprom read/write command

Quoted 7 lines · Mike Frysinger
Quoted 6 lines · On Tuesday, August 23, 2011 01:15:25 Eric Jarrige wrote:
Quoted 4 lines · Mike Frysinger
Quoted 1 line · 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 ?

Quoted 7 lines
Quoted 3 lines

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

Signature · 5 lines
=====================================================================
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
=====================================================================
#5 lore raw

PATCH 1/1 add dm9000 eeprom read/write command

Quoted 12 lines · Stefano Babic
Quoted 11 lines · Mike Frysinger
Quoted 9 lines · On Tuesday, August 23, 2011 01:15:25 Eric Jarrige wrote:
Quoted 4 lines · Mike Frysinger
Quoted 1 line · 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.

Quoted 7 lines
Quoted 3 lines

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

#6 lore raw

PATCH 1/1 add dm9000 eeprom read/write command

In reply to Eric Jarrige
Quoted 3 lines · Eric Jarrige

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.

Quoted 6 lines
---
 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

Signature · 5 lines
=====================================================================
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
=====================================================================
#7 lore raw

PATCH 1/1 add dm9000 eeprom read/write command

In reply to Eric Jarrige

Dear Eric Jarrige,

Quoted 9 lines · In message <[email protected]> you wrote:
---
 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

Signature · 5 lines
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!_