[PATCH v2 1/2] Documentation: devicetree: m25p80: add "nor-jedec" binding

[PATCH v2 1/2] Documentation: devicetree: m25p80: add "nor-jedec" binding

[Brian Norris]

Almost all flash that are "compatible" with m25p80 support the JEDEC
READ ID opcode (0x9F), and in fact, that is often the only thing that is
used to differentiate them. Let's add a compatible string that
represents this lowest common denominator of compatibility.

Device trees can still specify manufacturer/device names in addition,
but (until some reason is found to differentiate between them through
device tree) software will likely want to bind just against the generic
name, and avoid unnecessarily growing its device ID binding tables.

This is related to the work of commit a5b7616c55e1 ("mtd:
m25p80,spi-nor: Fix module aliases for m25p80"), which showed that
maintaining these device tables as stable device-tree/modalias binding
tables is not a worthwhile burden for mostly-comptatible flash.

At the same time, let's update the binding doc to point to the
m25p_ids[] ID list instead of spi_nor_ids[]. The former can be used for
device tree bindings, but the latter cannot. In the future, we should
pare down the m25p_ids[] list to only those IDs which are actually used
in device trees.

Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
---
v2: get the opdcode correct (0x9F, not 0x95)
    include the "nor-jedec" language after the more-specific device name
    binding

 Documentation/devicetree/bindings/mtd/m25p80.txt | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/m25p80.txt b/Documentation/devicetree/bindings/mtd/m25p80.txt
index 4611aa83531b..f20b111b502a 100644
--- a/Documentation/devicetree/bindings/mtd/m25p80.txt
+++ b/Documentation/devicetree/bindings/mtd/m25p80.txt
@@ -3,10 +3,13 @@
 Required properties:
 - #address-cells, #size-cells : Must be present if the device has sub-nodes
   representing partitions.
-- compatible : Should be the manufacturer and the name of the chip. Bear in mind
-               the DT binding is not Linux-only, but in case of Linux, see the
-               "spi_nor_ids" table in drivers/mtd/spi-nor/spi-nor.c for the list
-               of supported chips.
+- 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.
+               Must also include "nor-jedec" for any SPI NOR flash that can be
+               identified by the JEDEC READ ID opcode (0x9F).
 - reg : Chip-Select number
 - spi-max-frequency : Maximum frequency of the SPI bus the chip can operate at
 
@@ -22,7 +25,7 @@ Example:
 	flash: m25p80@0 {
 		#address-cells = <1>;
 		#size-cells = <1>;
-		compatible = "spansion,m25p80";
+		compatible = "spansion,m25p80", "nor-jedec";
 		reg = <0>;
 		spi-max-frequency = <40000000>;
 		m25p,fast-read;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] Documentation: devicetree: m25p80: add "nor-jedec" binding

[Brian Norris]

Almost all flash that are "compatible" with m25p80 support the JEDEC
READ ID opcode (0x9F), and in fact, that is often the only thing that is
used to differentiate them. Let's add a compatible string that
represents this lowest common denominator of compatibility.

Device trees can still specify manufacturer/device names in addition,
but (until some reason is found to differentiate between them through
device tree) software will likely want to bind just against the generic
name, and avoid unnecessarily growing its device ID binding tables.

This is related to the work of commit a5b7616c55e1 ("mtd:
m25p80,spi-nor: Fix module aliases for m25p80"), which showed that
maintaining these device tables as stable device-tree/modalias binding
tables is not a worthwhile burden for mostly-comptatible flash.

At the same time, let's update the binding doc to point to the
m25p_ids[] ID list instead of spi_nor_ids[]. The former can be used for
device tree bindings, but the latter cannot. In the future, we should
pare down the m25p_ids[] list to only those IDs which are actually used
in device trees.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Cc: Rafał Miłecki <zajec5@gmail.com>
Reviewed-by: Marek Vasut <marex@denx.de>
---
v2: get the opdcode correct (0x9F, not 0x95)
    include the "nor-jedec" language after the more-specific device name
    binding

 Documentation/devicetree/bindings/mtd/m25p80.txt | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/m25p80.txt b/Documentation/devicetree/bindings/mtd/m25p80.txt
index 4611aa83531b..f20b111b502a 100644
--- a/Documentation/devicetree/bindings/mtd/m25p80.txt
+++ b/Documentation/devicetree/bindings/mtd/m25p80.txt
@@ -3,10 +3,13 @@
 Required properties:
 - #address-cells, #size-cells : Must be present if the device has sub-nodes
   representing partitions.
-- compatible : Should be the manufacturer and the name of the chip. Bear in mind
-               the DT binding is not Linux-only, but in case of Linux, see the
-               "spi_nor_ids" table in drivers/mtd/spi-nor/spi-nor.c for the list
-               of supported chips.
+- 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.
+               Must also include "nor-jedec" for any SPI NOR flash that can be
+               identified by the JEDEC READ ID opcode (0x9F).
 - reg : Chip-Select number
 - spi-max-frequency : Maximum frequency of the SPI bus the chip can operate at
 
@@ -22,7 +25,7 @@ Example:
 	flash: m25p80@0 {
 		#address-cells = <1>;
 		#size-cells = <1>;
-		compatible = "spansion,m25p80";
+		compatible = "spansion,m25p80", "nor-jedec";
 		reg = <0>;
 		spi-max-frequency = <40000000>;
 		m25p,fast-read;
-- 
1.9.1

[PATCH v2 2/2] mtd: m25p80: bind to "nor-jedec" ID, for auto-detection

[Brian Norris]

Use the new 'nor-jedec' binding to provide automatic detection of flash
that use the 0x9F READ ID opcode. This can help for use cases where
platforms just specify compatibility with "m25p80", and then see
messages like this:

  m25p80 spi32766.0: found s25fl256s1, expected m25p80

Instead, they can just specify the generic string and see this:

  m25p80 spi32766.0: s25fl256s1 (32768 Kbytes)

Also, update the language about m25p_ids[] to straighten out the
expectations here. We should no longer need to continuously grow the
m25p_ids[] table, and in fact, we might want to start removing entries
which are not used in device trees so far, so we can just default to
auto-detection as much as possible in the future.

Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
v2: no change

 drivers/mtd/devices/m25p80.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 85e35467fba6..7c8b1694a134 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -223,6 +223,8 @@ static int m25p_probe(struct spi_device *spi)
 	 */
 	if (data && data->type)
 		flash_name = data->type;
+	else if (!strcmp(spi->modalias, "nor-jedec"))
+		flash_name = NULL; /* auto-detect */
 	else
 		flash_name = spi->modalias;
 
@@ -247,9 +249,16 @@ static int m25p_remove(struct spi_device *spi)
 }
 
 /*
- * XXX This needs to be kept in sync with spi_nor_ids.  We can't share
- * it with spi-nor, because if this is built as a module then modpost
- * won't be able to read it and add appropriate aliases.
+ * Do NOT add to this array without reading the following:
+ *
+ * Historically, many flash devices are bound to this driver by their name. But
+ * since most of these flash are compatible to some extent, and their
+ * differences can often be differentiated by the JEDEC read-ID command, we
+ * encourage new users to add support to the spi-nor library, and simply bind
+ * against a generic string here (e.g., "nor-jedec").
+ *
+ * Many flash names are kept here in this list (as well as in spi-nor.c) to
+ * keep them available as module aliases for existing platforms.
  */
 static const struct spi_device_id m25p_ids[] = {
 	{"at25fs010"},	{"at25fs040"},	{"at25df041a"},	{"at25df321a"},
@@ -291,6 +300,12 @@ static const struct spi_device_id m25p_ids[] = {
 	{"w25x64"},	{"w25q64"},	{"w25q80"},	{"w25q80bl"},
 	{"w25q128"},	{"w25q256"},	{"cat25c11"},
 	{"cat25c03"},	{"cat25c09"},	{"cat25c17"},	{"cat25128"},
+
+	/*
+	 * Generic support for SPI NOR that can be identified by the JEDEC READ
+	 * ID opcode (0x9F). Use this, if possible.
+	 */
+	{"nor-jedec"},
 	{ },
 };
 MODULE_DEVICE_TABLE(spi, m25p_ids);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] mtd: m25p80: bind to "nor-jedec" ID, for auto-detection

[Brian Norris]

Use the new 'nor-jedec' binding to provide automatic detection of flash
that use the 0x9F READ ID opcode. This can help for use cases where
platforms just specify compatibility with "m25p80", and then see
messages like this:

  m25p80 spi32766.0: found s25fl256s1, expected m25p80

Instead, they can just specify the generic string and see this:

  m25p80 spi32766.0: s25fl256s1 (32768 Kbytes)

Also, update the language about m25p_ids[] to straighten out the
expectations here. We should no longer need to continuously grow the
m25p_ids[] table, and in fact, we might want to start removing entries
which are not used in device trees so far, so we can just default to
auto-detection as much as possible in the future.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Cc: Rafał Miłecki <zajec5@gmail.com>
---
v2: no change

 drivers/mtd/devices/m25p80.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 85e35467fba6..7c8b1694a134 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -223,6 +223,8 @@ static int m25p_probe(struct spi_device *spi)
 	 */
 	if (data && data->type)
 		flash_name = data->type;
+	else if (!strcmp(spi->modalias, "nor-jedec"))
+		flash_name = NULL; /* auto-detect */
 	else
 		flash_name = spi->modalias;
 
@@ -247,9 +249,16 @@ static int m25p_remove(struct spi_device *spi)
 }
 
 /*
- * XXX This needs to be kept in sync with spi_nor_ids.  We can't share
- * it with spi-nor, because if this is built as a module then modpost
- * won't be able to read it and add appropriate aliases.
+ * Do NOT add to this array without reading the following:
+ *
+ * Historically, many flash devices are bound to this driver by their name. But
+ * since most of these flash are compatible to some extent, and their
+ * differences can often be differentiated by the JEDEC read-ID command, we
+ * encourage new users to add support to the spi-nor library, and simply bind
+ * against a generic string here (e.g., "nor-jedec").
+ *
+ * Many flash names are kept here in this list (as well as in spi-nor.c) to
+ * keep them available as module aliases for existing platforms.
  */
 static const struct spi_device_id m25p_ids[] = {
 	{"at25fs010"},	{"at25fs040"},	{"at25df041a"},	{"at25df321a"},
@@ -291,6 +300,12 @@ static const struct spi_device_id m25p_ids[] = {
 	{"w25x64"},	{"w25q64"},	{"w25q80"},	{"w25q80bl"},
 	{"w25q128"},	{"w25q256"},	{"cat25c11"},
 	{"cat25c03"},	{"cat25c09"},	{"cat25c17"},	{"cat25128"},
+
+	/*
+	 * Generic support for SPI NOR that can be identified by the JEDEC READ
+	 * ID opcode (0x9F). Use this, if possible.
+	 */
+	{"nor-jedec"},
 	{ },
 };
 MODULE_DEVICE_TABLE(spi, m25p_ids);
-- 
1.9.1

Re: [PATCH v2 2/2] mtd: m25p80: bind to "nor-jedec" ID, for auto-detection

[Brian Norris]

On Fri, Mar 27, 2015 at 10:29:50AM -0700, Brian Norris wrote:
> Use the new 'nor-jedec' binding to provide automatic detection of flash
> that use the 0x9F READ ID opcode. This can help for use cases where
> platforms just specify compatibility with "m25p80", and then see
> messages like this:
> 
>   m25p80 spi32766.0: found s25fl256s1, expected m25p80
> 
> Instead, they can just specify the generic string and see this:
> 
>   m25p80 spi32766.0: s25fl256s1 (32768 Kbytes)
> 
> Also, update the language about m25p_ids[] to straighten out the
> expectations here. We should no longer need to continuously grow the
> m25p_ids[] table, and in fact, we might want to start removing entries
> which are not used in device trees so far, so we can just default to
> auto-detection as much as possible in the future.
> 
> Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> v2: no change

Saw no comments last week, and this patch is blocking several other
trivial patches. See comments below.

> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 85e35467fba6..7c8b1694a134 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
[...]
> @@ -247,9 +249,16 @@ static int m25p_remove(struct spi_device *spi)
>  }
>  
>  /*
> - * XXX This needs to be kept in sync with spi_nor_ids.  We can't share
> - * it with spi-nor, because if this is built as a module then modpost
> - * won't be able to read it and add appropriate aliases.

People patching the spi-nor.c table are not heeding this comment. So
either I need to get them to all rework their patches, or else I can
pull this in to get this all straightened out.

Given I fixed up all the comments from v1 and saw no major objections on
v2 or my plan for cleaning up the DT ABI for this class of devices...

Applied this and the binding documentation change to l2-mtd.git.

Brian

> + * Do NOT add to this array without reading the following:
> + *
> + * Historically, many flash devices are bound to this driver by their name. But
> + * since most of these flash are compatible to some extent, and their
> + * differences can often be differentiated by the JEDEC read-ID command, we
> + * encourage new users to add support to the spi-nor library, and simply bind
> + * against a generic string here (e.g., "nor-jedec").
> + *
> + * Many flash names are kept here in this list (as well as in spi-nor.c) to
> + * keep them available as module aliases for existing platforms.
>   */
>  static const struct spi_device_id m25p_ids[] = {
>  	{"at25fs010"},	{"at25fs040"},	{"at25df041a"},	{"at25df321a"},
> @@ -291,6 +300,12 @@ static const struct spi_device_id m25p_ids[] = {
>  	{"w25x64"},	{"w25q64"},	{"w25q80"},	{"w25q80bl"},
>  	{"w25q128"},	{"w25q256"},	{"cat25c11"},
>  	{"cat25c03"},	{"cat25c09"},	{"cat25c17"},	{"cat25128"},
> +
> +	/*
> +	 * Generic support for SPI NOR that can be identified by the JEDEC READ
> +	 * ID opcode (0x9F). Use this, if possible.
> +	 */
> +	{"nor-jedec"},
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(spi, m25p_ids);
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] mtd: m25p80: bind to "nor-jedec" ID, for auto-detection

[Brian Norris]

On Fri, Mar 27, 2015 at 10:29:50AM -0700, Brian Norris wrote:
> Use the new 'nor-jedec' binding to provide automatic detection of flash
> that use the 0x9F READ ID opcode. This can help for use cases where
> platforms just specify compatibility with "m25p80", and then see
> messages like this:
> 
>   m25p80 spi32766.0: found s25fl256s1, expected m25p80
> 
> Instead, they can just specify the generic string and see this:
> 
>   m25p80 spi32766.0: s25fl256s1 (32768 Kbytes)
> 
> Also, update the language about m25p_ids[] to straighten out the
> expectations here. We should no longer need to continuously grow the
> m25p_ids[] table, and in fact, we might want to start removing entries
> which are not used in device trees so far, so we can just default to
> auto-detection as much as possible in the future.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> Cc: Rafał Miłecki <zajec5@gmail.com>
> ---
> v2: no change

Saw no comments last week, and this patch is blocking several other
trivial patches. See comments below.

> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 85e35467fba6..7c8b1694a134 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
[...]
> @@ -247,9 +249,16 @@ static int m25p_remove(struct spi_device *spi)
>  }
>  
>  /*
> - * XXX This needs to be kept in sync with spi_nor_ids.  We can't share
> - * it with spi-nor, because if this is built as a module then modpost
> - * won't be able to read it and add appropriate aliases.

People patching the spi-nor.c table are not heeding this comment. So
either I need to get them to all rework their patches, or else I can
pull this in to get this all straightened out.

Given I fixed up all the comments from v1 and saw no major objections on
v2 or my plan for cleaning up the DT ABI for this class of devices...

Applied this and the binding documentation change to l2-mtd.git.

Brian

> + * Do NOT add to this array without reading the following:
> + *
> + * Historically, many flash devices are bound to this driver by their name. But
> + * since most of these flash are compatible to some extent, and their
> + * differences can often be differentiated by the JEDEC read-ID command, we
> + * encourage new users to add support to the spi-nor library, and simply bind
> + * against a generic string here (e.g., "nor-jedec").
> + *
> + * Many flash names are kept here in this list (as well as in spi-nor.c) to
> + * keep them available as module aliases for existing platforms.
>   */
>  static const struct spi_device_id m25p_ids[] = {
>  	{"at25fs010"},	{"at25fs040"},	{"at25df041a"},	{"at25df321a"},
> @@ -291,6 +300,12 @@ static const struct spi_device_id m25p_ids[] = {
>  	{"w25x64"},	{"w25q64"},	{"w25q80"},	{"w25q80bl"},
>  	{"w25q128"},	{"w25q256"},	{"cat25c11"},
>  	{"cat25c03"},	{"cat25c09"},	{"cat25c17"},	{"cat25128"},
> +
> +	/*
> +	 * Generic support for SPI NOR that can be identified by the JEDEC READ
> +	 * ID opcode (0x9F). Use this, if possible.
> +	 */
> +	{"nor-jedec"},
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(spi, m25p_ids);
> -- 
> 1.9.1
>