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

[PATCH 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 (0x95), 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>
---
 Documentation/devicetree/bindings/mtd/m25p80.txt | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/m25p80.txt b/Documentation/devicetree/bindings/mtd/m25p80.txt
index 4611aa83531b..1b2997d4cee4 100644
--- a/Documentation/devicetree/bindings/mtd/m25p80.txt
+++ b/Documentation/devicetree/bindings/mtd/m25p80.txt
@@ -3,9 +3,12 @@
 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
+- compatible : Should be "nor-jedec" for any SPI NOR flash that can be
+               identified by the JEDEC READ ID opcode (0x95).
+               Additionally, 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
-               "spi_nor_ids" table in drivers/mtd/spi-nor/spi-nor.c for the list
+               "m25p_ids" table in drivers/mtd/devices/m25p80.c for the list
                of supported chips.
 - 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 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>
---
 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

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

[Marek Vasut]

On Wednesday, March 11, 2015 at 10:57:25 PM, Brian Norris wrote:
> Almost all flash that are "compatible" with m25p80 support the JEDEC
> READ ID opcode (0x95)

On the 2/2 patch, you claim READ ID is 0x9F . You might want to sort
this inconsistency :) Otherwise ...

Reviewed-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>

Best regards,
Marek Vasut
--
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 1/2] Documentation: devicetree: m25p80: add "nor-jedec" binding

[Brian Norris]

On Wed, Mar 11, 2015 at 11:22:46PM +0100, Marek Vasut wrote:
> On Wednesday, March 11, 2015 at 10:57:25 PM, Brian Norris wrote:
> > Almost all flash that are "compatible" with m25p80 support the JEDEC
> > READ ID opcode (0x95)
> 
> On the 2/2 patch, you claim READ ID is 0x9F . You might want to sort
> this inconsistency :) Otherwise ...

Ugh, it should be 0x9F of course. Thanks for noticing. I'll fix this
in the commit message and doc before applying, if I don't get other
complaints.

> Reviewed-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>

Thanks for the review!

Brian
--
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 1/2] Documentation: devicetree: m25p80: add "nor-jedec" binding

[Geert Uytterhoeven]

Hi Brian,

On Wed, Mar 11, 2015 at 10:57 PM, Brian Norris
<computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> diff --git a/Documentation/devicetree/bindings/mtd/m25p80.txt b/Documentation/devicetree/bindings/mtd/m25p80.txt
> index 4611aa83531b..1b2997d4cee4 100644
> --- a/Documentation/devicetree/bindings/mtd/m25p80.txt
> +++ b/Documentation/devicetree/bindings/mtd/m25p80.txt
> @@ -3,9 +3,12 @@
>  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
> +- compatible : Should be "nor-jedec" for any SPI NOR flash that can be
> +               identified by the JEDEC READ ID opcode (0x95).
> +               Additionally, may include a device-specific string consisting of
> +               the manufacturer and name of the chip. Bear in mind

For the casual reader, this suggests putting "nor-jedec" first, which is not
what we want. So I would write it like e.g.

"Should be the manufacturer and the name of the chip. Additionally,
 should contain "nor-jedec" for any SPI NOR flash that can be
 identified by the JEDEC READ ID opcode (0x95)."

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
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 1/2] Documentation: devicetree: m25p80: add "nor-jedec" binding

[Rafał Miłecki]

On 12 March 2015 at 11:19, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> On Wed, Mar 11, 2015 at 10:57 PM, Brian Norris
> <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> diff --git a/Documentation/devicetree/bindings/mtd/m25p80.txt b/Documentation/devicetree/bindings/mtd/m25p80.txt
>> index 4611aa83531b..1b2997d4cee4 100644
>> --- a/Documentation/devicetree/bindings/mtd/m25p80.txt
>> +++ b/Documentation/devicetree/bindings/mtd/m25p80.txt
>> @@ -3,9 +3,12 @@
>>  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
>> +- compatible : Should be "nor-jedec" for any SPI NOR flash that can be
>> +               identified by the JEDEC READ ID opcode (0x95).
>> +               Additionally, may include a device-specific string consisting of
>> +               the manufacturer and name of the chip. Bear in mind
>
> For the casual reader, this suggests putting "nor-jedec" first, which is not
> what we want. So I would write it like e.g.
>
> "Should be the manufacturer and the name of the chip. Additionally,
>  should contain "nor-jedec" for any SPI NOR flash that can be
>  identified by the JEDEC READ ID opcode (0x95)."

I don't really like above. It suggests using manufacturer,name
always/in most cases. This is what we want to avoid. Let's use
"nor-jedec" wherever possible (when dealing with JEDEC compatible
flashes).

-- 
Rafał
--
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 1/2] Documentation: devicetree: m25p80: add "nor-jedec" binding

[Geert Uytterhoeven]

On Thu, Mar 12, 2015 at 11:36 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
> On 12 March 2015 at 11:19, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
>> On Wed, Mar 11, 2015 at 10:57 PM, Brian Norris
>> <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> diff --git a/Documentation/devicetree/bindings/mtd/m25p80.txt b/Documentation/devicetree/bindings/mtd/m25p80.txt
>>> index 4611aa83531b..1b2997d4cee4 100644
>>> --- a/Documentation/devicetree/bindings/mtd/m25p80.txt
>>> +++ b/Documentation/devicetree/bindings/mtd/m25p80.txt
>>> @@ -3,9 +3,12 @@
>>>  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
>>> +- compatible : Should be "nor-jedec" for any SPI NOR flash that can be
>>> +               identified by the JEDEC READ ID opcode (0x95).
>>> +               Additionally, may include a device-specific string consisting of
>>> +               the manufacturer and name of the chip. Bear in mind
>>
>> For the casual reader, this suggests putting "nor-jedec" first, which is not
>> what we want. So I would write it like e.g.
>>
>> "Should be the manufacturer and the name of the chip. Additionally,
>>  should contain "nor-jedec" for any SPI NOR flash that can be
>>  identified by the JEDEC READ ID opcode (0x95)."
>
> I don't really like above. It suggests using manufacturer,name
> always/in most cases. This is what we want to avoid. Let's use
> "nor-jedec" wherever possible (when dealing with JEDEC compatible
> flashes).

Why? It's a basic DT principle to always specify the real device.
One day you might find a deviation from the nor-jedec spec, which you cannot
handle if the device node doesn't claim compatibility with the real device.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
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 1/2] Documentation: devicetree: m25p80: add "nor-jedec" binding

[Mark Rutland]

On Thu, Mar 12, 2015 at 10:36:29AM +0000, Rafał Miłecki wrote:
> On 12 March 2015 at 11:19, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> > On Wed, Mar 11, 2015 at 10:57 PM, Brian Norris
> > <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> diff --git a/Documentation/devicetree/bindings/mtd/m25p80.txt b/Documentation/devicetree/bindings/mtd/m25p80.txt
> >> index 4611aa83531b..1b2997d4cee4 100644
> >> --- a/Documentation/devicetree/bindings/mtd/m25p80.txt
> >> +++ b/Documentation/devicetree/bindings/mtd/m25p80.txt
> >> @@ -3,9 +3,12 @@
> >>  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
> >> +- compatible : Should be "nor-jedec" for any SPI NOR flash that can be
> >> +               identified by the JEDEC READ ID opcode (0x95).
> >> +               Additionally, may include a device-specific string consisting of
> >> +               the manufacturer and name of the chip. Bear in mind
> >
> > For the casual reader, this suggests putting "nor-jedec" first, which is not
> > what we want. So I would write it like e.g.
> >
> > "Should be the manufacturer and the name of the chip. Additionally,
> >  should contain "nor-jedec" for any SPI NOR flash that can be
> >  identified by the JEDEC READ ID opcode (0x95)."
> 
> I don't really like above. It suggests using manufacturer,name
> always/in most cases. This is what we want to avoid. Let's use
> "nor-jedec" wherever possible (when dealing with JEDEC compatible
> flashes).

The compatible property is a list. We should have "nor-jedec" in the
list, but a more specific string should come first. If we don't
recognise the more specific string, we'll still recognise nor-jedec, if
we do recognise it then we can do more advacned things with the HW (or
work around errata).

Mark.
--
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 1/2] Documentation: devicetree: m25p80: add "nor-jedec" binding

[Brian Norris]

On Thu, Mar 12, 2015 at 10:53:52AM +0000, Mark Rutland wrote:
> On Thu, Mar 12, 2015 at 10:36:29AM +0000, Rafał Miłecki wrote:
> > On 12 March 2015 at 11:19, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> > > On Wed, Mar 11, 2015 at 10:57 PM, Brian Norris
> > > <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > >> diff --git a/Documentation/devicetree/bindings/mtd/m25p80.txt b/Documentation/devicetree/bindings/mtd/m25p80.txt
> > >> index 4611aa83531b..1b2997d4cee4 100644
> > >> --- a/Documentation/devicetree/bindings/mtd/m25p80.txt
> > >> +++ b/Documentation/devicetree/bindings/mtd/m25p80.txt
> > >> @@ -3,9 +3,12 @@
> > >>  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
> > >> +- compatible : Should be "nor-jedec" for any SPI NOR flash that can be
> > >> +               identified by the JEDEC READ ID opcode (0x95).
> > >> +               Additionally, may include a device-specific string consisting of
> > >> +               the manufacturer and name of the chip. Bear in mind
> > >
> > > For the casual reader, this suggests putting "nor-jedec" first, which is not
> > > what we want. So I would write it like e.g.
> > >
> > > "Should be the manufacturer and the name of the chip. Additionally,
> > >  should contain "nor-jedec" for any SPI NOR flash that can be
> > >  identified by the JEDEC READ ID opcode (0x95)."
> > 
> > I don't really like above. It suggests using manufacturer,name
> > always/in most cases. This is what we want to avoid. Let's use
> > "nor-jedec" wherever possible (when dealing with JEDEC compatible
> > flashes).
> 
> The compatible property is a list. We should have "nor-jedec" in the
> list, but a more specific string should come first. If we don't
> recognise the more specific string, we'll still recognise nor-jedec, if
> we do recognise it then we can do more advacned things with the HW (or
> work around errata).

Right, I think this is fair, and that was really the intention, even if
it wasn't communicated well.

I *do* want to communicate that (where applicable) "nor-jedec" must be
included, since that would eliminate the need for maintaining a stable
list of bindings for >95% of flash. But the 'manufacturer,model' option
is still a good safeguard, and it would definitely come first in the
compatible list.

I'll reword this so the more specific option comes first, but still make
it clear that "nor-jedec" is strongly suggested.

Brian
--
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 1/2] Documentation: devicetree: m25p80: add "nor-jedec" binding

[Mark Rutland]

On Wed, Mar 11, 2015 at 09:57:25PM +0000, Brian Norris wrote:
> Almost all flash that are "compatible" with m25p80 support the JEDEC
> READ ID opcode (0x95), 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.

We really should not be referring to C files for the binding. The right
fix is to define the list in the binding document.

Mark.

> 
> Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/mtd/m25p80.txt | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/m25p80.txt b/Documentation/devicetree/bindings/mtd/m25p80.txt
> index 4611aa83531b..1b2997d4cee4 100644
> --- a/Documentation/devicetree/bindings/mtd/m25p80.txt
> +++ b/Documentation/devicetree/bindings/mtd/m25p80.txt
> @@ -3,9 +3,12 @@
>  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
> +- compatible : Should be "nor-jedec" for any SPI NOR flash that can be
> +               identified by the JEDEC READ ID opcode (0x95).
> +               Additionally, 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
> -               "spi_nor_ids" table in drivers/mtd/spi-nor/spi-nor.c for the list
> +               "m25p_ids" table in drivers/mtd/devices/m25p80.c for the list
>                 of supported chips.
>  - 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
> 
--
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 1/2] Documentation: devicetree: m25p80: add "nor-jedec" binding

[Brian Norris]

On Thu, Mar 12, 2015 at 10:58:24AM +0000, Mark Rutland wrote:
> On Wed, Mar 11, 2015 at 09:57:25PM +0000, Brian Norris wrote:
> > Almost all flash that are "compatible" with m25p80 support the JEDEC
> > READ ID opcode (0x95), 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.
> 
> We really should not be referring to C files for the binding. The right
> fix is to define the list in the binding document.

Yes, and that is an eventual goal I suppose, but the current list is
excessive and is most likely not currently relied on by any one. So I
don't just want to C&P the entire list into this binding immediately.

I guess my plan looks like this:

1. add "nor-jedec" binding, to provide lowest common denominator binding
(this series)

2. stop adding to the m25p_ids[] table unless necessary (enabled by this
series)

3. gauge whether we can remove certain entries from m25p_ids[] (e.g., if
they were only used in platform_data, not DT; or if they were very
recently added just to synchronize with spi-nor.c)

4. once m25p_ids[] contains a reasonable set, maintain it in the binding
doc, like we really should

I don't feel like step 4 is ready yet.

Is that a reasonable plan in your eyes?

Brian
--
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 1/2] Documentation: devicetree: m25p80: add "nor-jedec" binding

[Rafał Miłecki]

On 20 March 2015 at 20:12, Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Yes, and that is an eventual goal I suppose, but the current list is
> excessive and is most likely not currently relied on by any one. So I
> don't just want to C&P the entire list into this binding immediately.
>
> I guess my plan looks like this:
>
> 1. add "nor-jedec" binding, to provide lowest common denominator binding
> (this series)
>
> 2. stop adding to the m25p_ids[] table unless necessary (enabled by this
> series)
>
> 3. gauge whether we can remove certain entries from m25p_ids[] (e.g., if
> they were only used in platform_data, not DT; or if they were very
> recently added just to synchronize with spi-nor.c)

Why we can't remove (slowly) all entries from m25p_ids that don't need
any extra handling?

If we'll have DT with
"spansion,m25p80", "nor-jedec"
and then m25p80.c will handle both: "m25p80" and "nor-jedec" without
any difference, what's the point of keeping "m25p80" entry?
--
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 1/2] Documentation: devicetree: m25p80: add "nor-jedec" binding

[Brian Norris]

On Sun, Mar 22, 2015 at 12:30:05AM +0100, Rafał Miłecki wrote:
> On 20 March 2015 at 20:12, Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > Yes, and that is an eventual goal I suppose, but the current list is
> > excessive and is most likely not currently relied on by any one. So I
> > don't just want to C&P the entire list into this binding immediately.
> >
> > I guess my plan looks like this:
> >
> > 1. add "nor-jedec" binding, to provide lowest common denominator binding
> > (this series)
> >
> > 2. stop adding to the m25p_ids[] table unless necessary (enabled by this
> > series)
> >
> > 3. gauge whether we can remove certain entries from m25p_ids[] (e.g., if
> > they were only used in platform_data, not DT; or if they were very
> > recently added just to synchronize with spi-nor.c)
> 
> Why we can't remove (slowly) all entries from m25p_ids that don't need
> any extra handling?
> 
> If we'll have DT with
> "spansion,m25p80", "nor-jedec"
> and then m25p80.c will handle both: "m25p80" and "nor-jedec" without
> any difference, what's the point of keeping "m25p80" entry?

ABI stability. A lot of DTBs might be using m25p80 already, and they
aren't supposed to have to update just because SW decided to drop them.
But see [1] for the official word on ABI stability. I think that leaves
room for removing most/all of these eventually.

Brian

[1] Documentation/devicetree/bindings/ABI.txt
--
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