Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1270659 > unrolled thread
| Started by | Brian Norris <computersforpeace@gmail.com> |
|---|---|
| First post | 2015-11-16 23:40 +0100 |
| Last post | 2015-11-20 01:50 +0100 |
| Articles | 11 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH 0/3] mtd: m25p80: fix some module and documentation issues Brian Norris <computersforpeace@gmail.com> - 2015-11-16 23:40 +0100
[PATCH 2/3] mtd: m25p80: replace leftover "nor-jedec" with "spi-nor" in comments Brian Norris <computersforpeace@gmail.com> - 2015-11-16 23:40 +0100
Re: [PATCH 2/3] mtd: m25p80: replace leftover "nor-jedec" with "spi-nor" in comments Javier Martinez Canillas <javier@osg.samsung.com> - 2015-11-17 15:00 +0100
[PATCH 1/3] mtd: m25p80: fix module autoloading for "jedec,spi-nor" and "spi-nor" Brian Norris <computersforpeace@gmail.com> - 2015-11-16 23:40 +0100
Re: [PATCH 1/3] mtd: m25p80: fix module autoloading for "jedec,spi-nor" and "spi-nor" Javier Martinez Canillas <javier@osg.samsung.com> - 2015-11-17 15:00 +0100
[PATCH 3/3] doc: dt: mtd: stop referring to driver code for spi-nor IDs Brian Norris <computersforpeace@gmail.com> - 2015-11-16 23:40 +0100
Re: [PATCH 3/3] doc: dt: mtd: stop referring to driver code for spi-nor IDs Rob Herring <robh@kernel.org> - 2015-11-17 00:20 +0100
Re: [PATCH 3/3] doc: dt: mtd: stop referring to driver code for spi-nor IDs Javier Martinez Canillas <javier@osg.samsung.com> - 2015-11-17 15:10 +0100
Re: [PATCH 3/3] doc: dt: mtd: stop referring to driver code for spi-nor IDs Brian Norris <computersforpeace@gmail.com> - 2015-11-18 20:50 +0100
Re: [PATCH 3/3] doc: dt: mtd: stop referring to driver code for spi-nor IDs Javier Martinez Canillas <javier@osg.samsung.com> - 2015-11-18 21:30 +0100
Re: [PATCH 0/3] mtd: m25p80: fix some module and documentation issues Brian Norris <computersforpeace@gmail.com> - 2015-11-20 01:50 +0100
| From | Brian Norris <computersforpeace@gmail.com> |
|---|---|
| Date | 2015-11-16 23:40 +0100 |
| Subject | [PATCH 0/3] mtd: m25p80: fix some module and documentation issues |
| Message-ID | <qvABk-4Q6-21@gated-at.bofh.it> |
Hi,
There were a few mistakes and improvements pointed out at various points in
this thread, subject:
spi: OF module autoloading is still broken
(was: Re: m25p80: Commit "allow arbitrary OF matching for "jedec,spi-nor""
breaks module autoloading)
http://lists.infradead.org/pipermail/linux-mtd/2015-October/062369.html
https://lkml.org/lkml/2015/11/12/574
That thread covers some other interesting issues that are still not solved, but
let's fix the ones we can here in MTD right now.
Regards,
Brian
Brian Norris (3):
mtd: m25p80: fix module autoloading for "jedec,spi-nor" and "spi-nor"
mtd: m25p80: replace leftover "nor-jedec" with "spi-nor" in comments
doc: dt: mtd: stop referring to driver code for spi-nor IDs
.../devicetree/bindings/mtd/jedec,spi-nor.txt | 56 ++++++++++++++++++++--
drivers/mtd/devices/m25p80.c | 17 +++++--
2 files changed, 64 insertions(+), 9 deletions(-)
--
2.6.0.rc2.230.g3dd15c0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Brian Norris <computersforpeace@gmail.com> |
|---|---|
| Date | 2015-11-16 23:40 +0100 |
| Subject | [PATCH 2/3] mtd: m25p80: replace leftover "nor-jedec" with "spi-nor" in comments |
| Message-ID | <qvABk-4Q6-27@gated-at.bofh.it> |
| In reply to | #1270659 |
I overlooked a few comments in commit 8947e396a829 ("Documentation: dt:
mtd: replace "nor-jedec" binding with "jedec, spi-nor""). Fix these up
now.
Suggested-by: Javier Martinez Canillas <javier@osg.samsung.com>
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
drivers/mtd/devices/m25p80.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 151b453d0867..6cf5702706a6 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -263,13 +263,13 @@ static const struct spi_device_id m25p_ids[] = {
/*
* Entries not used in DTs that should be safe to drop after replacing
- * them with "nor-jedec" in platform data.
+ * them with "spi-nor" in platform data.
*/
{"s25sl064a"}, {"w25x16"}, {"m25p10"}, {"m25px64"},
/*
- * Entries that were used in DTs without "nor-jedec" fallback and should
- * be kept for backward compatibility.
+ * Entries that were used in DTs without "jedec,spi-nor" fallback and
+ * should be kept for backward compatibility.
*/
{"at25df321a"}, {"at25df641"}, {"at26df081a"},
{"mr25h256"},
--
2.6.0.rc2.230.g3dd15c0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Javier Martinez Canillas <javier@osg.samsung.com> |
|---|---|
| Date | 2015-11-17 15:00 +0100 |
| Subject | Re: [PATCH 2/3] mtd: m25p80: replace leftover "nor-jedec" with "spi-nor" in comments |
| Message-ID | <qvOXF-5L7-29@gated-at.bofh.it> |
| In reply to | #1270662 |
Hello Brian,
On 11/16/2015 07:34 PM, Brian Norris wrote:
> I overlooked a few comments in commit 8947e396a829 ("Documentation: dt:
> mtd: replace "nor-jedec" binding with "jedec, spi-nor""). Fix these up
> now.
>
> Suggested-by: Javier Martinez Canillas <javier@osg.samsung.com>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Brian Norris <computersforpeace@gmail.com> |
|---|---|
| Date | 2015-11-16 23:40 +0100 |
| Subject | [PATCH 1/3] mtd: m25p80: fix module autoloading for "jedec,spi-nor" and "spi-nor" |
| Message-ID | <qvABk-4Q6-31@gated-at.bofh.it> |
| In reply to | #1270659 |
Commit 43163022927b ("mtd: m25p80: allow arbitrary OF matching for
"jedec,spi-nor"") moved the "jedec,spi-nor" handling from the
spi_device_id table to the of_match_table, to better handle matching
complex device tree compatible strings. With that patch, device tree
support works as expected when m25p80.c is built into the kernel.
However, that commit ignored the fact that:
(1) (non-DT) platform devices might want to use the "spi-nor" string
for matching with this driver, rather than picking an arbitrary one
like "m25p80"
(2) the core SPI uevent/modalias code doesn't yet support kernel module
autoloading via of_match_table strings; so for DT-based devices, it
will only report (part of) the first compatible string used
Problem (1) has been reported previously, and I forgot to patch it up
afterward.
Problem (2) was noticed recently here:
http://lists.infradead.org/pipermail/linux-mtd/2015-October/062369.html
https://lkml.org/lkml/2015/11/12/574
Specifically, this patch fixes m25p80.ko module autoloading for cases
like this:
flash@xxx {
compatible = "jedec,spi-nor";
...
};
because modalias of "spi:spi-nor" (the only module loading info provided
by the SPI core for this device) will now be listed as an alias in
m25p80.ko.
Notably, it does *not* help cases like this:
flash@xxx {
compatible = "vendor,shiny-new-device", "jedec,spi-nor";
...
};
unless we also list "shiny-new-device" in m25p_ids[]. There has been
discussion on future work for this issue here:
https://lkml.org/lkml/2015/11/12/574
Cc: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
Tested module autoload with each of the following:
compatible = "m25p80";
compatible = "jedec,spi-nor";
drivers/mtd/devices/m25p80.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index f002a8f75374..151b453d0867 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -179,7 +179,7 @@ static int m25p_probe(struct spi_device *spi)
struct m25p *flash;
struct spi_nor *nor;
enum read_mode mode = SPI_NOR_NORMAL;
- char *flash_name = NULL;
+ char *flash_name;
int ret;
data = dev_get_platdata(&spi->dev);
@@ -219,6 +219,8 @@ static int m25p_probe(struct spi_device *spi)
*/
if (data && data->type)
flash_name = data->type;
+ else if (!strcmp(spi->modalias, "spi-nor"))
+ flash_name = NULL; /* auto-detect */
else
flash_name = spi->modalias;
@@ -253,6 +255,13 @@ static int m25p_remove(struct spi_device *spi)
*/
static const struct spi_device_id m25p_ids[] = {
/*
+ * Allow non-DT platform devices to bind to the "spi-nor" modalias, and
+ * hack around the fact that the SPI core does not provide uevent
+ * matching for .of_match_table
+ */
+ {"spi-nor"},
+
+ /*
* Entries not used in DTs that should be safe to drop after replacing
* them with "nor-jedec" in platform data.
*/
--
2.6.0.rc2.230.g3dd15c0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Javier Martinez Canillas <javier@osg.samsung.com> |
|---|---|
| Date | 2015-11-17 15:00 +0100 |
| Subject | Re: [PATCH 1/3] mtd: m25p80: fix module autoloading for "jedec,spi-nor" and "spi-nor" |
| Message-ID | <qvOXG-5L7-31@gated-at.bofh.it> |
| In reply to | #1270663 |
Hello Brian,
On 11/16/2015 07:34 PM, Brian Norris wrote:
> Commit 43163022927b ("mtd: m25p80: allow arbitrary OF matching for
> "jedec,spi-nor"") moved the "jedec,spi-nor" handling from the
> spi_device_id table to the of_match_table, to better handle matching
> complex device tree compatible strings. With that patch, device tree
> support works as expected when m25p80.c is built into the kernel.
>
> However, that commit ignored the fact that:
>
> (1) (non-DT) platform devices might want to use the "spi-nor" string
> for matching with this driver, rather than picking an arbitrary one
> like "m25p80"
> (2) the core SPI uevent/modalias code doesn't yet support kernel module
> autoloading via of_match_table strings; so for DT-based devices, it
> will only report (part of) the first compatible string used
>
Very nice changelog message!
Patch looks good to me.
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Brian Norris <computersforpeace@gmail.com> |
|---|---|
| Date | 2015-11-16 23:40 +0100 |
| Subject | [PATCH 3/3] doc: dt: mtd: stop referring to driver code for spi-nor IDs |
| Message-ID | <qvABl-4Q6-39@gated-at.bofh.it> |
| In reply to | #1270659 |
Pull the supported chip names from drivers/mtd/devices/m25p80.c and stop
pointing readers to Linux code.
Also (although I see this habit repeated throughout the
Documentation/devicetree/bindings/ tree), stop using the title "driver"
in this file, when we're trying explicitly to describe hardware, not
software.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Cc: <devicetree@vger.kernel.org>
---
.../devicetree/bindings/mtd/jedec,spi-nor.txt | 56 ++++++++++++++++++++--
1 file changed, 51 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
index 2bee68103b01..2c91c03e7eb0 100644
--- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
+++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
@@ -1,15 +1,61 @@
-* MTD SPI driver for ST M25Pxx (and similar) serial flash chips
+* SPI NOR flash: ST M25Pxx (and similar) serial flash chips
Required properties:
- #address-cells, #size-cells : Must be present if the device has sub-nodes
representing partitions.
- compatible : May include a device-specific string consisting of the
- manufacturer and name of the chip. Bear in mind the DT binding
- is not Linux-only, but in case of Linux, see the "m25p_ids"
- table in drivers/mtd/devices/m25p80.c for the list of supported
- chips.
+ manufacturer and name of the chip. A list of supported chip
+ names follows.
Must also include "jedec,spi-nor" for any SPI NOR flash that can
be identified by the JEDEC READ ID opcode (0x9F).
+
+ Supported chip names:
+ at25df321a
+ at25df641
+ at26df081a
+ mr25h256
+ mx25l4005a
+ mx25l1606e
+ mx25l6405d
+ mx25l12805d
+ mx25l25635e
+ n25q064
+ n25q128a11
+ n25q128a13
+ n25q512a
+ s25fl256s1
+ s25fl512s
+ s25sl12801
+ s25fl008k
+ s25fl064k
+ sst25vf040b
+ m25p40
+ m25p80
+ m25p16
+ m25p32
+ m25p64
+ m25p128
+ w25x80
+ w25x32
+ w25q32
+ w25q32dw
+ w25q80bl
+ w25q128
+ w25q256
+
+ The following chip names have been used historically to
+ designate quirky versions of flash chips that do not support the
+ JEDEC READ ID opcode (0x9F):
+ m25p05-nonjedec
+ m25p10-nonjedec
+ m25p20-nonjedec
+ m25p40-nonjedec
+ m25p80-nonjedec
+ m25p16-nonjedec
+ m25p32-nonjedec
+ m25p64-nonjedec
+ m25p128-nonjedec
+
- reg : Chip-Select number
- spi-max-frequency : Maximum frequency of the SPI bus the chip can operate at
--
2.6.0.rc2.230.g3dd15c0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2015-11-17 00:20 +0100 |
| Subject | Re: [PATCH 3/3] doc: dt: mtd: stop referring to driver code for spi-nor IDs |
| Message-ID | <qvBe1-5kg-17@gated-at.bofh.it> |
| In reply to | #1270665 |
On Mon, Nov 16, 2015 at 02:34:52PM -0800, Brian Norris wrote: > Pull the supported chip names from drivers/mtd/devices/m25p80.c and stop > pointing readers to Linux code. > > Also (although I see this habit repeated throughout the > Documentation/devicetree/bindings/ tree), stop using the title "driver" > in this file, when we're trying explicitly to describe hardware, not > software. > > Signed-off-by: Brian Norris <computersforpeace@gmail.com> > Cc: <devicetree@vger.kernel.org> Thanks for the clean-up. Acked-by: Rob Herring <robh@kernel.org> > --- > .../devicetree/bindings/mtd/jedec,spi-nor.txt | 56 ++++++++++++++++++++-- > 1 file changed, 51 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > index 2bee68103b01..2c91c03e7eb0 100644 > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > @@ -1,15 +1,61 @@ > -* MTD SPI driver for ST M25Pxx (and similar) serial flash chips > +* SPI NOR flash: ST M25Pxx (and similar) serial flash chips > > Required properties: > - #address-cells, #size-cells : Must be present if the device has sub-nodes > representing partitions. > - compatible : May include a device-specific string consisting of the > - manufacturer and name of the chip. Bear in mind the DT binding > - is not Linux-only, but in case of Linux, see the "m25p_ids" > - table in drivers/mtd/devices/m25p80.c for the list of supported > - chips. > + manufacturer and name of the chip. A list of supported chip > + names follows. > Must also include "jedec,spi-nor" for any SPI NOR flash that can > be identified by the JEDEC READ ID opcode (0x9F). > + > + Supported chip names: > + at25df321a > + at25df641 > + at26df081a > + mr25h256 > + mx25l4005a > + mx25l1606e > + mx25l6405d > + mx25l12805d > + mx25l25635e > + n25q064 > + n25q128a11 > + n25q128a13 > + n25q512a > + s25fl256s1 > + s25fl512s > + s25sl12801 > + s25fl008k > + s25fl064k > + sst25vf040b > + m25p40 > + m25p80 > + m25p16 > + m25p32 > + m25p64 > + m25p128 > + w25x80 > + w25x32 > + w25q32 > + w25q32dw > + w25q80bl > + w25q128 > + w25q256 > + > + The following chip names have been used historically to > + designate quirky versions of flash chips that do not support the > + JEDEC READ ID opcode (0x9F): > + m25p05-nonjedec > + m25p10-nonjedec > + m25p20-nonjedec > + m25p40-nonjedec > + m25p80-nonjedec > + m25p16-nonjedec > + m25p32-nonjedec > + m25p64-nonjedec > + m25p128-nonjedec > + > - reg : Chip-Select number > - spi-max-frequency : Maximum frequency of the SPI bus the chip can operate at > > -- > 2.6.0.rc2.230.g3dd15c0 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Javier Martinez Canillas <javier@osg.samsung.com> |
|---|---|
| Date | 2015-11-17 15:10 +0100 |
| Subject | Re: [PATCH 3/3] doc: dt: mtd: stop referring to driver code for spi-nor IDs |
| Message-ID | <qvP7j-64d-25@gated-at.bofh.it> |
| In reply to | #1270665 |
Hello Brian, On 11/16/2015 07:34 PM, Brian Norris wrote: > Pull the supported chip names from drivers/mtd/devices/m25p80.c and stop > pointing readers to Linux code. > > Also (although I see this habit repeated throughout the > Documentation/devicetree/bindings/ tree), stop using the title "driver" Agreed. > in this file, when we're trying explicitly to describe hardware, not > software. > > Signed-off-by: Brian Norris <computersforpeace@gmail.com> > Cc: <devicetree@vger.kernel.org> > --- > .../devicetree/bindings/mtd/jedec,spi-nor.txt | 56 ++++++++++++++++++++-- > 1 file changed, 51 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > index 2bee68103b01..2c91c03e7eb0 100644 > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > @@ -1,15 +1,61 @@ > -* MTD SPI driver for ST M25Pxx (and similar) serial flash chips > +* SPI NOR flash: ST M25Pxx (and similar) serial flash chips > > Required properties: > - #address-cells, #size-cells : Must be present if the device has sub-nodes > representing partitions. > - compatible : May include a device-specific string consisting of the > - manufacturer and name of the chip. Bear in mind the DT binding > - is not Linux-only, but in case of Linux, see the "m25p_ids" > - table in drivers/mtd/devices/m25p80.c for the list of supported > - chips. > + manufacturer and name of the chip. A list of supported chip > + names follows. Here says that the compatible string consists of manufacturer and name... > Must also include "jedec,spi-nor" for any SPI NOR flash that can > be identified by the JEDEC READ ID opcode (0x9F). > + > + Supported chip names: > + at25df321a > + at25df641 > + at26df081a > + mr25h256 > + mx25l4005a > + mx25l1606e > + mx25l6405d > + mx25l12805d > + mx25l25635e > + n25q064 > + n25q128a11 > + n25q128a13 > + n25q512a > + s25fl256s1 > + s25fl512s > + s25sl12801 > + s25fl008k > + s25fl064k > + sst25vf040b > + m25p40 > + m25p80 > + m25p16 > + m25p32 > + m25p64 > + m25p128 > + w25x80 > + w25x32 > + w25q32 > + w25q32dw > + w25q80bl > + w25q128 > + w25q256 > + ... but the entries in the list don't have a manufacturer. I know this is due backward compatibility because as we discussed in the thread mentioned in the cover letter, the SPI core didn't use the manufacturer and that implementation detail leaked into the DTBs. But I think that either only the correct list with vendor should be added to the DT binding docs (but make sure that backward compatibility in the driver and SPI core is maintained) or both the wrong and correct list should be documented and the former be marked as deprecated. > + The following chip names have been used historically to > + designate quirky versions of flash chips that do not support the > + JEDEC READ ID opcode (0x9F): > + m25p05-nonjedec > + m25p10-nonjedec > + m25p20-nonjedec > + m25p40-nonjedec > + m25p80-nonjedec > + m25p16-nonjedec > + m25p32-nonjedec > + m25p64-nonjedec > + m25p128-nonjedec > + Same here, I would prefer if the DT binding make it clear that not having a vendor is wrong and is only documented to maintain backward compatibility. > - reg : Chip-Select number > - spi-max-frequency : Maximum frequency of the SPI bus the chip can operate at > > Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Brian Norris <computersforpeace@gmail.com> |
|---|---|
| Date | 2015-11-18 20:50 +0100 |
| Subject | Re: [PATCH 3/3] doc: dt: mtd: stop referring to driver code for spi-nor IDs |
| Message-ID | <qwgTU-7l2-15@gated-at.bofh.it> |
| In reply to | #1271237 |
Hi, On Tue, Nov 17, 2015 at 11:04:55AM -0300, Javier Martinez Canillas wrote: > On 11/16/2015 07:34 PM, Brian Norris wrote: > > diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > > index 2bee68103b01..2c91c03e7eb0 100644 > > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > > @@ -1,15 +1,61 @@ > > -* MTD SPI driver for ST M25Pxx (and similar) serial flash chips > > +* SPI NOR flash: ST M25Pxx (and similar) serial flash chips > > > > Required properties: > > - #address-cells, #size-cells : Must be present if the device has sub-nodes > > representing partitions. > > - compatible : May include a device-specific string consisting of the > > - manufacturer and name of the chip. Bear in mind the DT binding > > - is not Linux-only, but in case of Linux, see the "m25p_ids" > > - table in drivers/mtd/devices/m25p80.c for the list of supported > > - chips. > > + manufacturer and name of the chip. A list of supported chip > > + names follows. > > Here says that the compatible string consists of manufacturer and name... > > > Must also include "jedec,spi-nor" for any SPI NOR flash that can > > be identified by the JEDEC READ ID opcode (0x9F). > > + > > + Supported chip names: > > + at25df321a > > + at25df641 [...] > + > > ... but the entries in the list don't have a manufacturer. I know this is > due backward compatibility because as we discussed in the thread mentioned > in the cover letter, the SPI core didn't use the manufacturer and that > implementation detail leaked into the DTBs. > > But I think that either only the correct list with vendor should be added > to the DT binding docs (but make sure that backward compatibility in the > driver and SPI core is maintained) or both the wrong and correct list should > be documented and the former be marked as deprecated. First, note that the list says "Supported chip *names*" (not "Supported compatible values"). It does not attempt to specify the full compatible value, and that's intentional. Second, I believe it is hard to determine after-the-fact what all the reasonable pairings of vendors might be. For some of these parts, various companies have produced parts under the same chip ID -- usually because one company bought another. For most chips though, this probably isn't a problem, so I could probably pick reasonable vendor pairings. IOW, there isn't just "a wrong" and "a correct" list; there's a (probably?) correct list and an enormous space of "I don't know what people might have put here" list. It's nearly unbounded, but even a reasonable list might get pretty large. So in practice, we'd essentially be sacrificing completeness for...what reason? > > + The following chip names have been used historically to > > + designate quirky versions of flash chips that do not support the > > + JEDEC READ ID opcode (0x9F): > > + m25p05-nonjedec > > + m25p10-nonjedec > > + m25p20-nonjedec > > + m25p40-nonjedec > > + m25p80-nonjedec > > + m25p16-nonjedec > > + m25p32-nonjedec > > + m25p64-nonjedec > > + m25p128-nonjedec > > + > > Same here, I would prefer if the DT binding make it clear that not having > a vendor is wrong and is only documented to maintain backward compatibility. The doc never says anything about not including the vendor. It says "May include a device-specific string consisting of the manufacturer and name of the chip" and it lists the chip names. So if someone is actually following the documentation, they will include a vendor. The vendor names are not listed because they're really not relevant to the implementation. But I could try to include them. > > - reg : Chip-Select number > > - spi-max-frequency : Maximum frequency of the SPI bus the chip can operate at > > > > So, what makes sense? I can make a separate list of vendors (my preference), or even try to pair up vendors with chip names (even if it's sometimes an N:1 relationship). But I don't see that really buying us much, since the implementation never has (and probably never will) enforce this. What do you think? Regards, Brian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Javier Martinez Canillas <javier@osg.samsung.com> |
|---|---|
| Date | 2015-11-18 21:30 +0100 |
| Subject | Re: [PATCH 3/3] doc: dt: mtd: stop referring to driver code for spi-nor IDs |
| Message-ID | <qwhwC-7V2-11@gated-at.bofh.it> |
| In reply to | #1272513 |
Hello Brian, On 11/18/2015 04:43 PM, Brian Norris wrote: > Hi, > > On Tue, Nov 17, 2015 at 11:04:55AM -0300, Javier Martinez Canillas wrote: >> On 11/16/2015 07:34 PM, Brian Norris wrote: >>> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt >>> index 2bee68103b01..2c91c03e7eb0 100644 >>> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt >>> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt >>> @@ -1,15 +1,61 @@ >>> -* MTD SPI driver for ST M25Pxx (and similar) serial flash chips >>> +* SPI NOR flash: ST M25Pxx (and similar) serial flash chips >>> >>> Required properties: >>> - #address-cells, #size-cells : Must be present if the device has sub-nodes >>> representing partitions. >>> - compatible : May include a device-specific string consisting of the >>> - manufacturer and name of the chip. Bear in mind the DT binding >>> - is not Linux-only, but in case of Linux, see the "m25p_ids" >>> - table in drivers/mtd/devices/m25p80.c for the list of supported >>> - chips. >>> + manufacturer and name of the chip. A list of supported chip >>> + names follows. >> >> Here says that the compatible string consists of manufacturer and name... >> >>> Must also include "jedec,spi-nor" for any SPI NOR flash that can >>> be identified by the JEDEC READ ID opcode (0x9F). >>> + >>> + Supported chip names: >>> + at25df321a >>> + at25df641 > [...] >> + >> >> ... but the entries in the list don't have a manufacturer. I know this is >> due backward compatibility because as we discussed in the thread mentioned >> in the cover letter, the SPI core didn't use the manufacturer and that >> implementation detail leaked into the DTBs. >> >> But I think that either only the correct list with vendor should be added >> to the DT binding docs (but make sure that backward compatibility in the >> driver and SPI core is maintained) or both the wrong and correct list should >> be documented and the former be marked as deprecated. > > First, note that the list says "Supported chip *names*" (not "Supported > compatible values"). It does not attempt to specify the full compatible > value, and that's intentional. > Right, sorry I missed that subtlety. > Second, I believe it is hard to determine after-the-fact what all the > reasonable pairings of vendors might be. For some of these parts, > various companies have produced parts under the same chip ID -- usually > because one company bought another. For most chips though, this probably > isn't a problem, so I could probably pick reasonable vendor pairings. > > IOW, there isn't just "a wrong" and "a correct" list; there's a > (probably?) correct list and an enormous space of "I don't know what > people might have put here" list. It's nearly unbounded, but even a > reasonable list might get pretty large. So in practice, we'd essentially > be sacrificing completeness for...what reason? > I see. >>> + The following chip names have been used historically to >>> + designate quirky versions of flash chips that do not support the >>> + JEDEC READ ID opcode (0x9F): >>> + m25p05-nonjedec >>> + m25p10-nonjedec >>> + m25p20-nonjedec >>> + m25p40-nonjedec >>> + m25p80-nonjedec >>> + m25p16-nonjedec >>> + m25p32-nonjedec >>> + m25p64-nonjedec >>> + m25p128-nonjedec >>> + >> >> Same here, I would prefer if the DT binding make it clear that not having >> a vendor is wrong and is only documented to maintain backward compatibility. > > The doc never says anything about not including the vendor. It says > > "May include a device-specific string consisting of the manufacturer > and name of the chip" > > and it lists the chip names. So if someone is actually following the > documentation, they will include a vendor. The vendor names are not > listed because they're really not relevant to the implementation. But I > could try to include them. > My goal was to start forcing people to use a complete compatible string to avoid the "garbage,chip-name" that you mentioned in the other thread. The vendor are not relevant in the current implementation because how the SPI core is implemented but I think that shouldn't affect the accuracy on which hardware is described in the DT. >>> - reg : Chip-Select number >>> - spi-max-frequency : Maximum frequency of the SPI bus the chip can operate at >>> >>> > > So, what makes sense? I can make a separate list of vendors (my > preference), or even try to pair up vendors with chip names (even if > it's sometimes an N:1 relationship). But I don't see that really buying > us much, since the implementation never has (and probably never will) > enforce this. What do you think? > Yes, you are right that is hard to come with a reasonable list specially since the vendor and chip relation could be N:1 as you said. $SUBJECT is definitely an improvement over the current doc that mentions the "m25p_ids" table in the driver though. So we can update later the DT binding once / if the SPI core is modified to report proper OF modalias so OF-only drivers can get rid of their SPI id table. So for this patch: Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com> Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Brian Norris <computersforpeace@gmail.com> |
|---|---|
| Date | 2015-11-20 01:50 +0100 |
| Message-ID | <qwI3M-8m5-21@gated-at.bofh.it> |
| In reply to | #1270659 |
On Mon, Nov 16, 2015 at 02:34:49PM -0800, Brian Norris wrote: > Hi, > > There were a few mistakes and improvements pointed out at various points in > this thread, subject: > > spi: OF module autoloading is still broken > (was: Re: m25p80: Commit "allow arbitrary OF matching for "jedec,spi-nor"" > breaks module autoloading) > > http://lists.infradead.org/pipermail/linux-mtd/2015-October/062369.html > https://lkml.org/lkml/2015/11/12/574 > > That thread covers some other interesting issues that are still not solved, but > let's fix the ones we can here in MTD right now. > > Regards, > Brian Pushed all to l2-mtd.git -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web