| Message ID | 1446624984-11033-11-git-send-email-mugunthanvnm@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
Hi Mugunthan, On 4 November 2015 at 01:16, Mugunthan V N <mugunthanvnm@ti.com> wrote: > Add compatible for spansion 32MiB spi flash s25fl256s1. > > Signed-off-by: Mugunthan V N <mugunthanvnm@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? > { } > }; > > -- > 2.6.2.280.g74301d6 > Regards, Simon
On Friday 06 November 2015 05:37 PM, Simon Glass wrote: > Hi Mugunthan, > > On 4 November 2015 at 01:16, Mugunthan V N <mugunthanvnm@ti.com> wrote: >> Add compatible for spansion 32MiB spi flash s25fl256s1. >> >> Signed-off-by: Mugunthan V N <mugunthanvnm@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. Regards Mugunthan V N
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@ti.com> wrote: > >> Add compatible for spansion 32MiB spi flash s25fl256s1. > >> > >> Signed-off-by: Mugunthan V N <mugunthanvnm@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.
Hi, On 12 November 2015 at 05:48, Tom Rini <trini@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@ti.com> wrote: >> >> Add compatible for spansion 32MiB spi flash s25fl256s1. >> >> >> >> Signed-off-by: Mugunthan V N <mugunthanvnm@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
On Tuesday 17 November 2015 02:38 AM, Simon Glass wrote: > Hi, > > On 12 November 2015 at 05:48, Tom Rini <trini@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@ti.com> wrote: >>>>> Add compatible for spansion 32MiB spi flash s25fl256s1. >>>>> >>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@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? > These compatibles are just used to bind the driver as spi-flash compatible is not present/documented in Kernel. Regards Mugunthan V N
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" }, { } };
Add compatible for spansion 32MiB spi flash s25fl256s1. Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> --- drivers/mtd/spi/sf_probe.c | 1 + 1 file changed, 1 insertion(+)