[flashrom] [PATCH] do something useful with the dediprog LEDs
Mathias Krause
mathias.krause at secunet.com
Mon Jan 24 11:14:37 CET 2011
On 21.01.2011 20:09, Stefan Reinauer wrote:
> This patch makes use of the dediprog LEDs.
Nice!
> It would be nice to have some "more global" infrastructure for device
> dependent status reporting. Right now some scenarios can not be
> reported as an error, though the user would think of them as one. For
> example, if the chip is IDed successfully but not supported by
> flashrom, thus it can not be flashed. At best, the dediprog driver
> sees a successful RDID command, but has no knowledge about the failure
> to read/write the chip. Therefore it can not report it.
>
> However, the attached patch improves my work flow here quite a bit, so
> I thought it should be shared.
Thanks for the patch. Albeit it would be good to have a "noleds" option
to disable the functionality because we don't know if it breaks older
firmware versions.
> Implement proper LED handling for dediprog.
>
> ... so you don't have to stare at the screen to see when the write is
> finished and whether it worked.
>
> Signed-off-by: Stefan Reinauer <reinauer at google.com>
>
> Index: dediprog.c
> ===================================================================
> --- dediprog.c (revision 1254)
> +++ dediprog.c (working copy)
> @@ -58,6 +58,33 @@
>
> //int usb_control_msg(usb_dev_handle *dev, int requesttype, int request, int value, int index, char *bytes, int size, int timeout);
>
> +/* Set/clear LEDs on dediprog. */
> +#define PASS_ON (0 << 0)
> +#define PASS_OFF (1 << 0)
> +#define BUSY_ON (0 << 1)
> +#define BUSY_OFF (1 << 1)
> +#define ERROR_ON (0 << 2)
> +#define ERROR_OFF (1 << 2)
> +static int current_led_status = 0;
Should be initialized to -1, see below why.
> +
> +static int dediprog_set_leds(int leds)
> +{
> + int ret;
> +
> + if (leds < 0 || leds > 7)
> + leds = 0; // Bogus value, enable all LEDs
> +
We can skip sending the USB command when the LED already has the value
we want it to. Something like:
if (leds == current_led_status)
return 0;
> + ret = usb_control_msg(dediprog_handle, 0x42, 0x07, 0x09, leds, NULL, 0x0, DEFAULT_TIMEOUT);
> + if (ret != 0x0) {
> + msg_perr("Command Set LED 0x%x failed (%s)!\n", leds, usb_strerror());
> + return 1;
> + }
> +
> + current_led_status = leds;
> +
> + return 0;
> +}
> +
> static int dediprog_set_spi_voltage(int millivolt)
> {
> int ret;
> @@ -211,20 +238,26 @@
> int residue = start % chunksize ? chunksize - start % chunksize : 0;
> int bulklen;
>
> + dediprog_set_leds(PASS_OFF|BUSY_ON|ERROR_OFF);
> +
> if (residue) {
> msg_pdbg("Slow read for partial block from 0x%x, length 0x%x\n",
> start, residue);
> ret = spi_read_chunked(flash, buf, start, residue, 16);
> - if (ret)
> + if (ret) {
> + dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON);
> return ret;
> + }
> }
>
> /* Round down. */
> bulklen = (len - residue) / chunksize * chunksize;
> ret = dediprog_spi_bulk_read(flash, buf + residue, start + residue,
> bulklen);
> - if (ret)
> + if (ret) {
> + dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON);
> return ret;
> + }
>
> len -= residue + bulklen;
> if (len) {
> @@ -232,17 +265,29 @@
> start, len);
> ret = spi_read_chunked(flash, buf + residue + bulklen,
> start + residue + bulklen, len, 16);
> - if (ret)
> + if (ret) {
> + dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON);
> return ret;
> + }
> }
>
> + dediprog_set_leds(PASS_ON|BUSY_OFF|ERROR_OFF);
> return 0;
> }
>
> int dediprog_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len)
> {
> + int ret;
> +
> + dediprog_set_leds(PASS_OFF|BUSY_ON|ERROR_OFF);
> +
> /* No idea about the real limit. Maybe 12, maybe more, maybe less. */
> - return spi_write_chunked(flash, buf, start, len, 12);
> + ret = spi_write_chunked(flash, buf, start, len, 12);
> +
> + if (ret)
> + dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON);
> + else
> + dediprog_set_leds(PASS_ON|BUSY_OFF|ERROR_OFF);
Missing 'return ret;'
> }
>
> int dediprog_spi_send_command(unsigned int writecnt, unsigned int readcnt,
> @@ -410,81 +455,8 @@
> }
> return 0;
> }
> -
> -/* Start/stop blinking?
> - * Present in eng_detect_blink.log with firmware 3.1.8
> - * Preceded by Command J
> - */
> -static int dediprog_command_g(void)
> -{
> - int ret;
> -
> - ret = usb_control_msg(dediprog_handle, 0x42, 0x07, 0x09, 0x03, NULL, 0x0, DEFAULT_TIMEOUT);
> - if (ret != 0x0) {
> - msg_perr("Command G failed (%s)!\n", usb_strerror());
> - return 1;
> - }
> - return 0;
> -}
> -
> -/* Something.
> - * Present in all logs with firmware 5.1.5
> - * Always preceded by Command Receive Device String
> - * Always followed by Command Set SPI Voltage nonzero
> - */
> -static int dediprog_command_h(void)
> -{
> - int ret;
> -
> - ret = usb_control_msg(dediprog_handle, 0x42, 0x07, 0x09, 0x05, NULL, 0x0, DEFAULT_TIMEOUT);
> - if (ret != 0x0) {
> - msg_perr("Command H failed (%s)!\n", usb_strerror());
> - return 1;
> - }
> - return 0;
> -}
> #endif
>
> -/* Shutdown for firmware 5.x?
> - * This command swithces the "Pass" LED on.
> - * Present in all logs with firmware 5.1.5
> - * Often preceded by a SPI operation (Command Read SPI Bulk or Receive SPI)
> - * Always followed by Command Set SPI Voltage 0x0000
> - */
> -static int dediprog_command_i(void)
> -{
> - int ret;
> -
> - ret = usb_control_msg(dediprog_handle, 0x42, 0x07, 0x09, 0x06, NULL, 0x0, DEFAULT_TIMEOUT);
> - if (ret != 0x0) {
> - msg_perr("Command I failed (%s)!\n", usb_strerror());
> - return 1;
> - }
> - return 0;
> -}
> -
> -#if 0
> -/* Start/stop blinking?
> - * Present in all logs with firmware 5.1.5
> - * Always preceded by Command Receive Device String on 5.1.5
> - * Always followed by Command Set SPI Voltage nonzero on 5.1.5
> - * Present in eng_detect_blink.log with firmware 3.1.8
> - * Preceded by Command B in eng_detect_blink.log
> - * Followed by Command G in eng_detect_blink.log
> - */
> -static int dediprog_command_j(void)
> -{
> - int ret;
> -
> - ret = usb_control_msg(dediprog_handle, 0x42, 0x07, 0x09, 0x07, NULL, 0x0, DEFAULT_TIMEOUT);
> - if (ret != 0x0) {
> - msg_perr("Command J failed (%s)!\n", usb_strerror());
> - return 1;
> - }
> - return 0;
> -}
> -#endif
> -
> static int parse_voltage(char *voltage)
> {
> char *tmp = NULL;
> @@ -584,22 +556,35 @@
> return 1;
> }
> dediprog_endpoint = 2;
> +
> + dediprog_set_leds(PASS_ON|BUSY_ON|ERROR_ON);
> +
> /* URB 6. Command A. */
> - if (dediprog_command_a())
> + if (dediprog_command_a()) {
> + dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON);
> return 1;
> + }
> /* URB 7. Command A. */
> - if (dediprog_command_a())
> + if (dediprog_command_a()) {
> + dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON);
> return 1;
> + }
> /* URB 8. Command Prepare Receive Device String. */
> /* URB 9. Command Receive Device String. */
> - if (dediprog_check_devicestring())
> + if (dediprog_check_devicestring()) {
> + dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON);
> return 1;
> + }
> /* URB 10. Command C. */
> - if (dediprog_command_c())
> + if (dediprog_command_c()) {
> + dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON);
> return 1;
> + }
> /* URB 11. Command Set SPI Voltage. */
> - if (dediprog_set_spi_voltage(millivolt))
> + if (dediprog_set_spi_voltage(millivolt)) {
> + dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON);
> return 1;
> + }
>
> buses_supported = CHIP_BUSTYPE_SPI;
> spi_controller = SPI_CONTROLLER_DEDIPROG;
> @@ -610,6 +595,8 @@
> dediprog_do_stuff();
> #endif
>
> + dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_OFF);
> +
> return 0;
> }
>
> @@ -670,10 +657,6 @@
> {
> msg_pspew("%s\n", __func__);
>
> - /* Shutdown on firmware 5.x */
> - if (dediprog_firmwareversion == 5)
> - if (dediprog_command_i())
> - return 1;
> /* URB 28. Command Set SPI Voltage to 0. */
> if (dediprog_set_spi_voltage(0x0))
> return 1;
Otherwise looks good to me.
Tested-by: Mathias Krause <mathias.krause at secunet.com>
With the above mentioned changes:
Acked-by: Mathias Krause <mathias.krause at secunet.com>
More information about the flashrom
mailing list