Dear Peter Tyser,
In message <[EMAIL PROTECTED]> you wrote:
> Initial support for the DS4510, a CPU supervisor with
> integrated EEPROM, SRAM, and 4 programmable non-volatile
> GPIO pins. The CONFIG_DS4510 define enables support
> for the device while the CONFIG_CMD_DS4510 define
> enables the ds4510 command.
>
> Signed-off-by: Peter Tyser <[EMAIL PROTECTED]>
> ---
> README | 2 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/ds4510.c | 344
> +++++++++++++++++++++++++++++++++++++++++++++++++
> include/gpio/ds4510.h | 75 +++++++++++
> 4 files changed, 422 insertions(+), 0 deletions(-)
> create mode 100644 drivers/gpio/ds4510.c
> create mode 100644 include/gpio/ds4510.h
This should go to devices/i2c/
> + /* This delay isn't needed for SRAM writes but shouldn't delay
> + * things too much, so do it unconditionally for simplicity */
Please fix multiline comment style.
> +int do_ds4510(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
> +{
...
> + case 4:
> + val = simple_strtoul(argv[3], NULL, 16);
> +
> + if (strcmp(argv[2], "nv") == 0)
> + return ds4510_see_write(chip, val);
> + else if (strcmp(argv[2], "rstdelay") == 0)
> + return ds4510_rstdelay_write(chip, val);
> + else if (strcmp(argv[2], "input") == 0)
> + return (ds4510_gpio_read_val(chip) & (1 << val)) != 0;
> + break;
We have generic code for processing multiple command selections.
Please use that.
> + case 5:
...
> + if (strcmp(argv[2], "output") == 0) {
> + tmp = ds4510_gpio_read(chip);
> + if (val)
> + tmp |= (1 << pin);
> + else
> + tmp &= ~(1 << pin);
> + return ds4510_gpio_write(chip, tmp);
> + } else if (strcmp(argv[2], "pullup") == 0) {
Ditto.
> + case 7:
...
> + if (strcmp(argv[3], "read") == 0)
> + rw_func = ds4510_mem_read;
> + else if (strcmp(argv[3], "write") == 0)
> + rw_func = ds4510_mem_write;
> + else
> + break;
> +
> + if (strcmp(argv[2], "eeprom") == 0) {
> + end = DS4510_EEPROM + DS4510_EEPROM_SIZE;
> + off += DS4510_EEPROM;
> + } else if (strcmp(argv[2], "seeprom") == 0) {
> + end = DS4510_SEEPROM + DS4510_SEEPROM_SIZE;
> + off += DS4510_SEEPROM;
> + } else if (strcmp(argv[2], "sram") == 0) {
> + end = DS4510_SRAM + DS4510_SRAM_SIZE;
> + off += DS4510_SRAM;
> + } else {
> + break;
> + }
Ditto.
> +U_BOOT_CMD(
> + ds4510, 7, 2, do_ds4510,
> + "ds4510 - ds4510 eeprom/seeprom/sram/gpio access\n",
> + "chip info\n"
> + " - display ds4510 info\n"
> + "ds4510 chip nv 0|1\n"
> + " - make gpio and seeprom writes volatile/non-volatile\n"
> + "ds4510 chip rstdelay 0-3\n"
> + " - set reset output delay\n"
> + "ds4510 chip output pin 0|1\n"
> + " - set pin low or high-Z\n"
> + "ds4510 chip input pin\n"
> + " - read value of pin\n"
> + "ds4510 chip pullup pin 0|1\n"
> + " - disable/enable pullup on specified pin\n"
> + "ds4510 chip eeprom read addr off cnt\n"
> + "ds4510 chip eeprom write addr off cnt\n"
> + " - read/write 'cnt' bytes at EEPROM offset 'off'\n"
> + "ds4510 chip seeprom read addr off cnt\n"
> + "ds4510 chip seeprom write addr off cnt\n"
> + " - read/write 'cnt' bytes at SRAM-shadowed EEPROM offset 'off'\n"
> + "ds4510 chip sram read addr off cnt\n"
> + "ds4510 chip sram write addr off cnt\n"
> + " - read/write 'cnt' bytes at SRAM offset 'off'\n"
Why do we need the "chip" argument? It just adds typing...
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: [EMAIL PROTECTED]
You speak of courage. Obviously you do not know the difference bet-
ween courage and foolhardiness. Always it is the brave ones who die,
the soldiers.
-- Kor, the Klingon Commander, "Errand of Mercy",
stardate 3201.7
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
