Dear Mike Frysinger,
> i dont have a problem with going through the env as a hook, but it doesnt seem
> to scale. what if you have 2 fram devices and a winbond spi flash ? perhaps
> the name spec should be:
> fram_dev.<bus>.<cs>
>
> then you can signal that only certain<bus>.<cs> locations should get special
> treatment. and it wont inadvertently clobber other devices or detect devices
> that dont exist without explicit permission.
Makes sense.
>
>> +{
>> + struct ramtron_spi_fram *sn = to_ramtron_spi_fram(flash);
>> + u8 cmd[4];
>> + u8 cmd_len = 1;
>> + int ret;
>
> the majority of this function seems to match the write() func ... perhaps the
> body should be unified in a local static "setup" func and let gcc determine
> whether to inline its body ?
OK
> no space after "getenv". i'd also delay the call to the point where you
> actually check "device_name" ...
Right.
>> --- a/drivers/mtd/spi/spi_flash.c
>> +++ b/drivers/mtd/spi/spi_flash.c
>> @@ -116,6 +116,19 @@
>> goto err_claim_bus;
>> }
>>
>> +#ifdef CONFIG_SPI_FRAM_RAMTRON
>> + /*
>> + * not all RAMTRON FRAMs do have a READ_ID command,
>> + * let the ramtron code figure out details
>> + */
>> + flash = spi_fram_probe_ramtron(spi);
>> + if (flash) {
>> + spi_release_bus(spi);
>> + return flash;
>> + }
>> + /* if spi_fram_probe did not find anything, continue normal probe */
>> +#endif
>> +
>> /* Read the ID codes */
>> ret = spi_flash_cmd(spi, CMD_READ_ID,&idcode, sizeof(idcode));
>> if (ret)
>
> you hook early to handle extended idcode reads and for devices that dont
> respect the idcode command.
>
> for the first, we can extend the idcode length yet again, but perhaps this
> time temper it:
> #if defined(CONFIG_SPI_FRAM_RAMTRON)
> # define IDCODE_LEN 10
> #else
> # define IDCODE_LEN 5
> #endif
OK, see below. Can't we have it 10 generally? The impact should be negligible?
>
> for the second, what do you get back when you issue the idcode ? 0xff ? we
> already have a fall back case for this with stmicro, so perhaps we should
> generalize this further too. after the vendor id switch statement, we do:
If MISO has no pull-up the result is indeterminate, the chip simply lets MISO
float when it does not honor the read-id command.
I'll add a comment to that file that a pull-up is required for non-standard
devices to be detected. Otherwise, depending on random noise, a false
detection of a standard device is not entirely impossible.
On a side note: its beyond my comprehension why ramtron's newest and densest
variant FM25H20 has no read-id command (while all devices before that except
for the really old and small ones do have it)....
Reinhard
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
