| Message ID | ce7c16faafaa24f5bb711115c0eea1f8992d08c3.1433850255.git.cyrille.pitchen@atmel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 2c01a3d6b3cf0aaf39b55318fd986b1bd0b98168 |
| Headers | show |
Le 09/06/2015 13:53, Cyrille Pitchen a écrit : > - add new property "atmel,fifo-size" > - change "cs-gpios" to optional for SPI controller version >= 2. > > Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com> > --- > Documentation/devicetree/bindings/spi/spi_atmel.txt | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/spi/spi_atmel.txt b/Documentation/devicetree/bindings/spi/spi_atmel.txt > index 4f8184d..fb588b3 100644 > --- a/Documentation/devicetree/bindings/spi/spi_atmel.txt > +++ b/Documentation/devicetree/bindings/spi/spi_atmel.txt > @@ -4,11 +4,16 @@ Required properties: > - compatible : should be "atmel,at91rm9200-spi". > - reg: Address and length of the register set for the device > - interrupts: Should contain spi interrupt > -- cs-gpios: chipselects > +- cs-gpios: chipselects (optional for SPI controller version >= 2 with the > + Chip Select Active After Transfer feature). > - clock-names: tuple listing input clock names. > Required elements: "spi_clk" > - clocks: phandles to input clocks. > > +Optional properties: > +- atmel,fifo-size: maximum number of data the RX and TX FIFOs can store for FIFO > + capable SPI controllers. > + > Example: > > spi1: spi@fffcc000 { > @@ -20,6 +25,7 @@ spi1: spi@fffcc000 { > clocks = <&spi1_clk>; > clock-names = "spi_clk"; > cs-gpios = <&pioB 3 0>; > + atmel,fifo-size = <32>; > status = "okay"; > > mmc-slot@0 { >
On Tue, Jun 09, 2015 at 01:53:53PM +0200, Cyrille Pitchen wrote:
> - add new property "atmel,fifo-size"
Why is this a property and not something we know from the IP version?
Le 09/06/2015 19:25, Mark Brown a écrit : > On Tue, Jun 09, 2015 at 01:53:53PM +0200, Cyrille Pitchen wrote: >> - add new property "atmel,fifo-size" > > Why is this a property and not something we know from the IP version? > Hi Mark, Please be aware that the VERSION register can not be used to guess the size of FIFOs. Indeed, for a given hardware version, the SPI controller can be integrated on Atmel SoCs with different FIFO sizes. Also the "atmel,fifo-size" property is optional as older SPI controllers don't embed FIFO at all. Besides, the FIFO size can not be read or guessed from other registers: When designing the FIFO feature, no dedicated registers were added to store this size. Unused spaces in the I/O register range are limited and better reserved for future usages. Instead, the FIFO size of each peripheral is documented in the programmer datasheet. Finally, on a given SoC, there can be several instances of the SPI controller with different FIFO sizes. This explain why we'd rather use a dedicated DT property than use the "compatible" property. I hope these pieces of information will help to clarify this point. Of course, we are open to other suggestions. Best Regards, Cyrille -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 11, 2015 at 06:37:51PM +0200, Cyrille Pitchen wrote: > Le 09/06/2015 19:25, Mark Brown a écrit : > > On Tue, Jun 09, 2015 at 01:53:53PM +0200, Cyrille Pitchen wrote: > >> - add new property "atmel,fifo-size" > > Why is this a property and not something we know from the IP version? > Please be aware that the VERSION register can not be used to guess the > size of FIFOs. Indeed, for a given hardware version, the SPI controller > can be integrated on Atmel SoCs with different FIFO sizes. Also the > "atmel,fifo-size" property is optional as older SPI controllers don't > embed FIFO at all. ... > Finally, on a given SoC, there can be several instances of the SPI > controller with different FIFO sizes. This explain why we'd rather use a > dedicated DT property than use the "compatible" property. > I hope these pieces of information will help to clarify this point. > Of course, we are open to other suggestions. Ugh, what a wonderfully consistent hardware design :( Please make it clear in the documentation what is going on here, this looks like an obvious bug in the DT binding - a very common pattern for bugs is to do version quirks like this as properties.
diff --git a/Documentation/devicetree/bindings/spi/spi_atmel.txt b/Documentation/devicetree/bindings/spi/spi_atmel.txt index 4f8184d..fb588b3 100644 --- a/Documentation/devicetree/bindings/spi/spi_atmel.txt +++ b/Documentation/devicetree/bindings/spi/spi_atmel.txt @@ -4,11 +4,16 @@ Required properties: - compatible : should be "atmel,at91rm9200-spi". - reg: Address and length of the register set for the device - interrupts: Should contain spi interrupt -- cs-gpios: chipselects +- cs-gpios: chipselects (optional for SPI controller version >= 2 with the + Chip Select Active After Transfer feature). - clock-names: tuple listing input clock names. Required elements: "spi_clk" - clocks: phandles to input clocks. +Optional properties: +- atmel,fifo-size: maximum number of data the RX and TX FIFOs can store for FIFO + capable SPI controllers. + Example: spi1: spi@fffcc000 { @@ -20,6 +25,7 @@ spi1: spi@fffcc000 { clocks = <&spi1_clk>; clock-names = "spi_clk"; cs-gpios = <&pioB 3 0>; + atmel,fifo-size = <32>; status = "okay"; mmc-slot@0 {
- add new property "atmel,fifo-size" - change "cs-gpios" to optional for SPI controller version >= 2. Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com> --- Documentation/devicetree/bindings/spi/spi_atmel.txt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)