On Tuesday, September 14, 2010 10:50:09 Reinhard Meyer wrote:
> JEDEC types
> non JEDEC types

this changelog is useless ... either make it say something, or just leave it 
empty

> +static int ramtron_common(struct spi_flash *flash,
> +             u32 offset, size_t len, void *buf, u8 command)
> +{
> +
> +     if (sn->params->addr_len == 3 && sn->params->merge_cmd == 0) {
> +             cmd[0] = command;
> +             cmd[1] = offset >> 16;
> +             cmd[2] = offset >> 8;
> +             cmd[3] = offset;
> +             cmd_len = 4;
> +     }
> +     else if (sn->params->addr_len == 2 && sn->params->merge_cmd == 0) {
> +             cmd[0] = command;
> +             cmd[1] = offset >> 8;
> +             cmd[2] = offset;
> +             cmd_len = 3;
> +     }
> +     else {

i thought i had mentioned this before, but those else statements need cuddling

> +     /* claim the bus */
> +     ret = spi_claim_bus(flash->spi);
> +     if (ret) {
> +             debug("SF: Unable to claim SPI bus\n");
> +             return ret;
> +     }

i'm thinking we should push the bus claim from the read/write funcs into the 
common spi flash layer ... not that this is specific to your flash.  we can do 
this after yours gets merged.

> +     sn = malloc(sizeof(struct ramtron_spi_fram));

sizeof(*sn)

otherwise, this looks good
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to