[U-Boot] [PATCH v2 10/16] drivers: mtd: spi: sf_probe: add compatible for spansion spi flash

[Simon Glass]

Hi,

On 12 November 2015 at 05:48, Tom Rini <trini at konsulko.com> wrote:
> On Thu, Nov 12, 2015 at 02:42:41PM +0530, Mugunthan V N wrote:
>> On Friday 06 November 2015 05:37 PM, Simon Glass wrote:
>> > Hi Mugunthan,
>> >
>> > On 4 November 2015 at 01:16, Mugunthan V N <mugunthanvnm at ti.com> wrote:
>> >> Add compatible for spansion 32MiB spi flash s25fl256s1.
>> >>
>> >> Signed-off-by: Mugunthan V N <mugunthanvnm at ti.com>
>> >> ---
>> >>  drivers/mtd/spi/sf_probe.c | 1 +
>> >>  1 file changed, 1 insertion(+)
>> >>
>> >> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
>> >> index c000c53..9cfa9b6 100644
>> >> --- a/drivers/mtd/spi/sf_probe.c
>> >> +++ b/drivers/mtd/spi/sf_probe.c
>> >> @@ -502,6 +502,7 @@ static const struct dm_spi_flash_ops spi_flash_std_ops = {
>> >>
>> >>  static const struct udevice_id spi_flash_std_ids[] = {
>> >>         { .compatible = "spi-flash" },
>> >> +       { .compatible = "s25fl256s1" },
>> >
>> > Instead, is it possible to add "spi-flash" to the list of compatible
>> > strings in your device tree?
>> >
>>
>> The compatible "spi-flash" is not defined/documented in kernel and
>> compatible "s25fl256s1" is already documented and present in dt files.
>> So it will be good to follow the same dt compatibles in U-Boot so that
>> future merge/sync will be easier.
>
> Agreed.

I see this in kernel upstream at present:

git grep s25fl256s1
arch/arm/boot/dts/dra7-evm.dts:         compatible = "s25fl256s1";
arch/arm/boot/dts/dra72-evm.dts:                compatible = "s25fl256s1";
arch/arm/boot/dts/qcom-ipq8064-ap148.dts:
         compatible = "s25fl256s1";
arch/powerpc/boot/dts/fsl/kmcoge4.dts:
compatible = "spansion,s25fl256s1";
drivers/mtd/devices/m25p80.c:   {"s25fl256s1"}, {"s25fl512s"},
{"s25sl12801"}, {"s25fl008k"},
drivers/mtd/devices/st_spi_fsm.c:       { "s25fl256s1", 0x010219,
0x4d01,  64 * 1024, 512,
drivers/mtd/spi-nor/spi-nor.c:  { "s25fl256s1", INFO(0x010219, 0x4d01,
 64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },

There seems to be some confusion as to whether to include "spansion"
or not. I feel it should be included.

The problem is that in U-Boot at present this compatible string does
not affect any behaviour. We still scan the chip to figure out its
type from its ID. Do you want to add a .data value for it, and skip
the probing, or are you just trying to trigger the driver to bind?

Regards,
Simon