* [U-Boot] [PATCH v2] add dm9000 eeprom read/write command
@ 2011-08-28 21:47 Eric Jarrige
2011-08-30 9:47 ` Stefano Babic
0 siblings, 1 reply; 6+ messages in thread
From: Eric Jarrige @ 2011-08-28 21:47 UTC (permalink / raw)
To: u-boot
Signed-off-by: Eric Jarrige <eric.jarrige@armadeus.org>
Signed-off-by: Stefano Babic <sbabic@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Detlev Zundel <dzu@denx.de>
Changes for v2:
- remove DM9000 driver dependant compilation flag
---
README | 1 +
common/Makefile | 1 +
common/cmd_dm9000ee.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 84 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..788e3bd
--- /dev/null
+++ b/common/cmd_dm9000ee.c
@@ -0,0 +1,82 @@
+/*
+ * (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>
+
+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");
+
^ permalink raw reply related [flat|nested] 6+ messages in thread* [U-Boot] [PATCH v2] add dm9000 eeprom read/write command
2011-08-28 21:47 [U-Boot] [PATCH v2] add dm9000 eeprom read/write command Eric Jarrige
@ 2011-08-30 9:47 ` Stefano Babic
2011-08-30 21:17 ` Eric Jarrige
0 siblings, 1 reply; 6+ messages in thread
From: Stefano Babic @ 2011-08-30 9:47 UTC (permalink / raw)
To: u-boot
On 08/28/2011 11:47 PM, Eric Jarrige wrote:
> Signed-off-by: Eric Jarrige <eric.jarrige@armadeus.org>
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Detlev Zundel <dzu@denx.de>
>
> Changes for v2:
> - remove DM9000 driver dependant compilation flag
> ---
Hi Eric,
why should this code put in a separate file and not inserted in the
driver itself ? I think this remains an open point from our previous
discussion.
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
=====================================================================
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] add dm9000 eeprom read/write command
2011-08-30 9:47 ` Stefano Babic
@ 2011-08-30 21:17 ` Eric Jarrige
2011-08-31 13:01 ` Stefano Babic
0 siblings, 1 reply; 6+ messages in thread
From: Eric Jarrige @ 2011-08-30 21:17 UTC (permalink / raw)
To: u-boot
Hi Stefano,
On 30 ao?t 2011, at 11:47, Stefano Babic wrote:
> On 08/28/2011 11:47 PM, Eric Jarrige wrote:
>> Signed-off-by: Eric Jarrige <eric.jarrige@armadeus.org>
>> Signed-off-by: Stefano Babic <sbabic@denx.de>
>> Cc: Wolfgang Denk <wd@denx.de>
>> Cc: Detlev Zundel <dzu@denx.de>
>>
>> Changes for v2:
>> - remove DM9000 driver dependant compilation flag
>> ---
>
> Hi Eric,
>
> why should this code put in a separate file and not inserted in the
> driver itself ? I think this remains an open point from our previous
> discussion.
>
Sorry for the confusion, I did not understood that your remark was
not related to the compilation flags.
Now, I've checked how to have this U-Boot commands in the driver
itself. I think it's doable if I can have a compilation
CONFIG_CMD_XXX to enable the command line feature.
Without this option, there is a direct impact one memory footprint for
every board featuring the DM9000 controller and I failed to find any
model of driver with an optional command line interface in a driver.
Why do you recommend to put the command interface in drivers
as it is seems to be something quite unusual in U-Boot ?
Best regards,
Eric
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] add dm9000 eeprom read/write command
2011-08-30 21:17 ` Eric Jarrige
@ 2011-08-31 13:01 ` Stefano Babic
2011-08-31 22:27 ` Eric Jarrige
0 siblings, 1 reply; 6+ messages in thread
From: Stefano Babic @ 2011-08-31 13:01 UTC (permalink / raw)
To: u-boot
On 08/30/2011 11:17 PM, Eric Jarrige wrote:
> Hi Stefano,
Hi Eric,
> Sorry for the confusion, I did not understood that your remark was
> not related to the compilation flags.
> Now, I've checked how to have this U-Boot commands in the driver
> itself. I think it's doable if I can have a compilation
> CONFIG_CMD_XXX to enable the command line feature.
You could add a CONFIG_DM9000_* switch. This set a switch only for the
DM9000 driver and the name remembers that it is used only inside the
driver itself, such as CONFIG_DM9000_DEBUG and CONFIG_DM9000_BASE that
are currently used in the driver.
> Without this option, there is a direct impact one memory footprint for
> every board featuring the DM9000 controller and I failed to find any
> model of driver with an optional command line interface in a driver.
Or you can reuse CONFIG_DM9000_NO_SROM (already in driver) and enable
your command if this switch is not defined. If the eeprom is present, it
makes sense to have this command enabled.
>
> Why do you recommend to put the command interface in drivers
> as it is seems to be something quite unusual in U-Boot ?
Not sure it is so unusual. I see that all commands under common/* are
general commands, and they are not related to a specific driver. There
are then two commands in drivers, drivers/misc/fsl_pmic.c and
drivers/qe/qe.c. These commands are only related to these drivers and
make no sense without the driver. I see then a lot of board related
commands stored in the board directories.
If I understand well the concept, each command resides where it is
thought: general commands in common, board commands in board
directories, driver commands in the driver itself. If your command is in
the driver file, there is also no need to check if CONFIG_DRIVER_DM9000
is set, because the file is simply not compiled if it is unset.
Best regards,
Stefano
--
=====================================================================
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
=====================================================================
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] add dm9000 eeprom read/write command
2011-08-31 13:01 ` Stefano Babic
@ 2011-08-31 22:27 ` Eric Jarrige
2011-09-01 8:05 ` Stefano Babic
0 siblings, 1 reply; 6+ messages in thread
From: Eric Jarrige @ 2011-08-31 22:27 UTC (permalink / raw)
To: u-boot
On 31 ao?t 2011, at 15:01, Stefano Babic wrote:
> Hi Eric,
>
>> Sorry for the confusion, I did not understood that your remark was
>> not related to the compilation flags.
>> Now, I've checked how to have this U-Boot commands in the driver
>> itself. I think it's doable if I can have a compilation
>> CONFIG_CMD_XXX to enable the command line feature.
>
> You could add a CONFIG_DM9000_* switch. This set a switch only for the
> DM9000 driver and the name remembers that it is used only inside the
> driver itself, such as CONFIG_DM9000_DEBUG and CONFIG_DM9000_BASE that
> are currently used in the driver.
In such a case MAKEALL fails to compile the trizepsiv board with a double definition
of 'do_dm9000_eeprom'.
>
>> Without this option, there is a direct impact one memory footprint for
>> every board featuring the DM9000 controller and I failed to find any
>> model of driver with an optional command line interface in a driver.
>
> Or you can reuse CONFIG_DM9000_NO_SROM (already in driver) and enable
> your command if this switch is not defined. If the eeprom is present, it
> makes sense to have this command enabled.
That makes sense but there is still a compilation failure and some impact on
7 other boards.
>
>>
>> Why do you recommend to put the command interface in drivers
>> as it is seems to be something quite unusual in U-Boot ?
>
> Not sure it is so unusual. I see that all commands under common/* are
> general commands, and they are not related to a specific driver. There
> are then two commands in drivers, drivers/misc/fsl_pmic.c and
> drivers/qe/qe.c. These commands are only related to these drivers and
> make no sense without the driver. I see then a lot of board related
> commands stored in the board directories.
>
> If I understand well the concept, each command resides where it is
> thought: general commands in common, board commands in board
> directories, driver commands in the driver itself. If your command is in
> the driver file, there is also no need to check if CONFIG_DRIVER_DM9000
> is set, because the file is simply not compiled if it is unset.
I understand your point of view but I am not sure at all that board maintainers will
want to give a command interface to modify the content of the DM9000 eeprom.
Best regards,
Eric
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] add dm9000 eeprom read/write command
2011-08-31 22:27 ` Eric Jarrige
@ 2011-09-01 8:05 ` Stefano Babic
0 siblings, 0 replies; 6+ messages in thread
From: Stefano Babic @ 2011-09-01 8:05 UTC (permalink / raw)
To: u-boot
On 09/01/2011 12:27 AM, Eric Jarrige wrote:
>
> On 31 ao?t 2011, at 15:01, Stefano Babic wrote:
>> You could add a CONFIG_DM9000_* switch. This set a switch only for the
>> DM9000 driver and the name remembers that it is used only inside the
>> driver itself, such as CONFIG_DM9000_DEBUG and CONFIG_DM9000_BASE that
>> are currently used in the driver.
>
> In such a case MAKEALL fails to compile the trizepsiv board with a double definition
> of 'do_dm9000_eeprom'.
Of course, the command must be dropped from the trizeps board. It makes
no sense to have such a duplication of identical code, even without
compilation errors.
Feel free to drop completely the eeprom.c file from the trizeps board.
> That makes sense but there is still a compilation failure and some impact on
> 7 other boards.
Sure, there is an incremented footprint, but I guess it is limited. And
at the moment, there are some dead code (dm9000_read_srom_word and
dm9000_write_srom_word) that is compiled in any case if
CONFIG_DM9000_NO_SROM is not set.
Because your code is a slightly modification of the code in the trizeps
board, we can assume we get the same impact. For the trizeps, I can see:
000221d4 T do_write_dm9000_eeprom
00022248 T do_read_dm9000_eeprom
000222b4 T do_dm9000_eeprom
0002234c T __aeabi_unwind_cpp_pr0
It seems to me that the code in TEXT for these functions increases for
only 376 bytes.
> I understand your point of view but I am not sure at all that board maintainers will
> want to give a command interface to modify the content of the DM9000 eeprom.
Why not ? If the eeprom is soldered on the board, it must be
preprogrammed, because there is no way to do this in the bootloader.
Anyway, we are currently discussing this topic on the ML, and everyone
can raise his objections if he disagree ;-).
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
=====================================================================
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-09-01 8:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-28 21:47 [U-Boot] [PATCH v2] add dm9000 eeprom read/write command Eric Jarrige
2011-08-30 9:47 ` Stefano Babic
2011-08-30 21:17 ` Eric Jarrige
2011-08-31 13:01 ` Stefano Babic
2011-08-31 22:27 ` Eric Jarrige
2011-09-01 8:05 ` Stefano Babic
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox