[PATCH 0/3] mtd: m25p80: fix some module and documentation issues

[PATCH 0/3] mtd: m25p80: fix some module and documentation issues

[Brian Norris]

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 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 3/3] doc: dt: mtd: stop referring to driver code for spi-nor IDs

[Brian Norris]

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] doc: dt: mtd: stop referring to driver code for spi-nor IDs

[Rob Herring]

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>

Thanks for the clean-up.

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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-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 3/3] doc: dt: mtd: stop referring to driver code for spi-nor IDs

[Javier Martinez Canillas]

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.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 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 3/3] doc: dt: mtd: stop referring to driver code for spi-nor IDs

[Brian Norris]

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 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 3/3] doc: dt: mtd: stop referring to driver code for spi-nor IDs

[Javier Martinez Canillas]

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-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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 0/3] mtd: m25p80: fix some module and documentation issues

[Brian Norris]

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