[flashrom] [PATCH 5/8] ichspi.c: refactor filling and reading the fdata/spid registers
David Hendricks
dhendrix at google.com
Thu Jun 30 00:06:00 CEST 2011
A couple quick comments in-line below...
On Mon, Jun 27, 2011 at 8:33 PM, Stefan Tauner <
stefan.tauner at student.tuwien.ac.at> wrote:
>
> + static void ich_read_data(uint8_t *data, uint8_t len, int reg0_off)
> + {
> + int a;
> + uint32_t temp32 = 0;
> +
> + if (len > spi_programmer->max_data_read)
> + len = spi_programmer->max_data_read;
> +
> + for (a = 0; a < len; a++) {
> + if ((a % 4) == 0)
> + temp32 = REGREAD32(reg0_off + (a));
> +
> + data[a] = (temp32 & (((uint32_t) 0xff) << ((a % 4) * 8)))
> + >> ((a % 4) * 8);
>
How about "data[a] = (temp32 >> ((a % 4) * 8)) & 0xff" instead? That is
clearer IMHO, and you won't even need to break the line. (80 columns really
*is* enough :-P)
On Mon, Jun 27, 2011 at 8:33 PM, Stefan Tauner <
stefan.tauner at student.tuwien.ac.at> wrote:
> +static uint8_t ich_fill_data(const uint8_t *data, uint8_t len, int
> reg0_off)
As mentioned on IRC, the caller tends to pass in a "len" value > 255, e.g. 1
block of 4096 bytes or more. Since len is handled as an int in the caller,
it should probably get passed in as an int here.
--
David Hendricks (dhendrix)
Systems Software Engineer, Google Inc.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.flashrom.org/pipermail/flashrom/attachments/20110629/0f53fc9b/attachment.html>
More information about the flashrom
mailing list