[U-Boot] [PATCH] MTD/SPI/FLASH: add support for Ramtron FRAMs using SPI

[Reinhard Meyer]

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