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

[Mugunthan V N]

On Friday 20 November 2015 05:57 PM, Jagan Teki wrote:
> On 20 November 2015 at 11:31, Mugunthan V N <mugunthanvnm at ti.com> wrote:
>> Jagan
>>
>> On Thursday 19 November 2015 03:40 PM, Jagan Teki wrote:
>>> On 19 November 2015 at 12:35, 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 bc05d30..ffbce59 100644
>>>> --- a/drivers/mtd/spi/sf_probe.c
>>>> +++ b/drivers/mtd/spi/sf_probe.c
>>>> @@ -515,6 +515,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" },
>>>>         { }
>>>
>>> Never compare with spi-flash vs s25fl256s1 here former is driver
>>> compatible string and later one is probed flash string name. we don't
>>> need to add them in compatible list, better to add it on dts node
>>> otherwise no issue the connected flash will detect while 'sf probe'
>>>
>>
>> I agree that spi flash will be probed even without addition of compatibles.
>>
>> If there is no matching compatible between the dtb and driver,  the dt
>> properties like spi-max-frequency, spi clock phase/polarity and spi
>> bus-width are not parsed from DT as the of_offset of the spi device will
>> be '-1'. This check and dt parse is done in spi_child_post_bind() in
>> spi-uclass driver.
>>
>> Since spi bus-width is not read from DT, by default spi transfers are
>> done in one wire mode and read throughput drops to 5.3Mbps. With Quad
>> mode read throughput is 16.6Mbps in DRA74x EVM.
>>
> 
> Why can't we try something like this [1] [2] I do agree with
> documentation missing, may be we add add that well.
> 
> [1] arch/arm/dts/socfpga_cyclone5_sockit.dts

This dts file is not in sync with kernel dts. There is no qspi support
in kernel. It is going to be really tough time to sync this dts between
kernel and u-boot ;)

> [2] arch/arm/dts/zynqmp-ep108.dts

I don't see compatible string "spi-flash" in zynqmp-ep108.dts

Adding spi-flash compatible will be tough as spi-flash works with the
existing compatibles and convincing spi maintainer (Mark Brown) for
adding a generic compatible will be difficult.

If you still insist to add spi-flash compatible to the dts file and *Tom
Rini* is also fine, I will re-spin the series with compatible changes to
the dts files.

Regards
Mugunthan V N