On Thursday 03 December 2015 09:32 PM, Jagan Teki wrote: > Hi Mugunthan, > > On 23 November 2015 at 18:42, Jagan Teki <jt...@openedev.com> wrote: >> On 23 November 2015 at 16:59, Mugunthan V N <mugunthan...@ti.com> wrote: >>> On Friday 20 November 2015 05:57 PM, Jagan Teki wrote: >>>> On 20 November 2015 at 11:31, Mugunthan V N <mugunthan...@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 <mugunthan...@ti.com> wrote: >>>>>>> Add compatible for spansion 32MiB spi flash s25fl256s1. >>>>>>> >>>>>>> Signed-off-by: Mugunthan V N <mugunthan...@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, >> > > Comments?
Yes, I was pointing to this compatible only. > >> >>> >>> 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. > > Any comments? proceed with above suggestion. I agree, we can add compatibles, if needed we can work on trimming it later. Regards Mugunthan V N _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot