lkml.org 
[lkml]   [2015]   [Nov]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/3] doc: dt: mtd: stop referring to driver code for spi-nor IDs
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


\
 
 \ /
  Last update: 2015-11-18 21:01    [W:0.093 / U:6.504 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site