On Fri, Jun 1, 2018 at 12:09 PM, Boris Brezillon <boris.brezillon@xxxxxxxxxxx> wrote: > Hi Geert, > > On Fri, 1 Jun 2018 16:42:26 +0200 > Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > >> Hi Boris, >> >> I became interested after reading the cover letter... >> >> On Fri, Jun 1, 2018 at 3:13 PM, Boris Brezillon >> <boris.brezillon@xxxxxxxxxxx> wrote: >> > Add bindings for SPI NAND chips. >> > >> > Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxx> >> >> Thanks for your patch! > > And thanks for reviewing it ;-). > >> >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/mtd/spi-nand.txt >> > @@ -0,0 +1,27 @@ >> > +SPI NAND flash >> > + >> > +Required properties: >> > +- compatible: should be "spi-nand" Seems awfully generic if you expect this alone. No chips with quirks yet? Is there a standard for detection like jedec? >> > +- reg: should encode the chip-select line used to access the NAND chip "see spi.txt" should be enough. >> > + >> > +Optional properties >> > +- spi-max-frequency: maximum frequency of the SPI bus the chip can operate at. >> > + This should encode board limitations (i.e. max freq can't >> > + be achieved due to crosstalk on IO lines). >> > + When unspecified, the driver assumes the chip can run at >> > + the max frequency defined in the spec (information >> > + extracted chip detection time). >> >> This is a standard property according to >> Documentation/devicetree/bindings/spi/spi-bus.txt. Can't you just refer >> to that file, or just omit it, as it applies to all SPI slaves anyway? > > The thing is, the maximum frequency supported by a SPI NAND is directly > encoded in the NAND device ID and can be auto-detected. Why should we > define spi-max-frequency in the DT when we can automatically detect > this information? The only reason one might want to override > spi-max-frequency is when the board design impose such restrictions, > hence the precision I give here. This should always be the case. The operating frequency should be min(host max, device max) unless the board has restrictions and needs to set spi-max-frequency (and we really should have used 'bus-frequency' here). No doubt though, that is not what has been done. >> > +- spi-tx-bus-width: The bus width (number of data wires) that is used for MOSI. >> > + Only encodes the board constraints (i.e. when not all IO >> > + signals are routed on the board). Device constraints are >> > + extracted when detecting the chip, and controller >> > + constraints are exposed by the SPI mem controller. If this >> > + property is missing that means no constraint at the board >> > + level. >> > +- spi-rx-bus-width: The bus width (number of data wires) that is used for MISO. >> > + Only encodes the board constraints (i.e. when not all IO >> > + signals are routed on the board). Device constraints are >> > + extracted when detecting the chip, and controller >> > + constraints are exposed by the SPI mem controller. If this >> > + property is missing that means no constraint at the board >> > + level. >> >> This does not match Documentation/devicetree/bindings/spi/spi-bus.txt, >> which says the default is 1. > > Yes, I know. Like frequency, this should have been similar. I imagine for the common case, the number of host and device lines are 1 so it doesn't really apply as this was added later on. Perhaps we can reword the common definition to work for both? Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
