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

[Jagan Teki]

On 23 November 2015 at 16:59, Mugunthan V N <mugunthanvnm at ti.com> wrote:
> 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 ;)

Since we are in starting point of adding dts files (unlike Linux)
these dts files may not sync with Linux, we started adding these to
make dm or related drivers need to work. Yes I agreed with your point
and we really need to work on that as well.

>
>> [2] arch/arm/dts/zynqmp-ep108.dts
>
> I don't see compatible string "spi-flash" in zynqmp-ep108.dts

Yes, cant you add "s25fl256s1" like n25q512a11 added in this 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.

This is what you pointed out - id_table.

static struct spi_driver m25p80_driver = {
        .driver = {
                .name   = "m25p80",
                .of_match_table = m25p_of_table,
        },
        .id_table       = m25p_ids,


>
> 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.

I'm saying like s25fl256s1 few of compatible strings were added
already on existing dts files, probably we may proceed with the same
for now. will trimming up later.

thanks!
-- 
Jagan | openedev.