[U-Boot] [PATCH v2] add dm9000 eeprom read/write command
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@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"); +
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
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
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
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
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
participants (2)
-
Eric Jarrige -
Stefano Babic