diff mbox

[v4,1/2] mfd: devicetree: add bindings for Atmel Flexcom

Message ID 337ab4936bd6ec2f24c42e2f8f363df21f491a02.1434967670.git.cyrille.pitchen@atmel.com
State Superseded, archived
Headers show

Commit Message

Cyrille Pitchen June 22, 2015, 10:12 a.m. UTC
This patch documents the DT bindings for the Atmel Flexcom which will be
introduced by sama5d2x SoCs. These bindings will be used by the actual Flexcom
driver to be sent in another patch.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 .../devicetree/bindings/mfd/atmel-flexcom.txt      | 58 ++++++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/atmel-flexcom.txt

Comments

Lee Jones July 21, 2015, 9:09 a.m. UTC | #1
On Mon, 22 Jun 2015, Cyrille Pitchen wrote:

> This patch documents the DT bindings for the Atmel Flexcom which will be
> introduced by sama5d2x SoCs. These bindings will be used by the actual Flexcom
> driver to be sent in another patch.
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
>  .../devicetree/bindings/mfd/atmel-flexcom.txt      | 58 ++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt b/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
> new file mode 100644
> index 0000000..38f0e43
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
> @@ -0,0 +1,58 @@
> +* Device tree bindings for Atmel Flexcom (Flexible Serial Communication Unit)
> +
> +The Atmel Flexcom is just a wrapper which embeds a SPI controller, an I2C
> +controller and an USART. Only one function can be used at a time and is chosen
> +at boot time according to the device tree.
> +
> +Required properties:
> +- compatible: "atmel,sama5d2-flexcom"
> +- reg: shall be the offset/length value for Flexcom dedicated I/O registers
> +  (without USART, TWI or SPI registers).
> +- clocks: shall be the Flexcom peripheral clock from PMC.
> +- #address-cells: should be <1>
> +- #size-cells: should be <1>
> +- ranges: one range for the full I/O register region (including USART, TWI and
> +  SPI registers)
> +- atmel,flexcom-mode: shall be a string among { "spi", "usart", "i2c", "twi" }.
> +  "i2c" and "twi" are synonymous.

Please use a numerical value instead.  You can then define it in
include/dt-bindings/*

This is only required if you supply more than one child node.  Is that
the plan?  I ask because you only have one child node in the example
below.

This document is pretty hard to read.  Please line everything up
nicely, so it's easy on the eye:

- compatible: 	      "atmel,sama5d2-flexcom"
- reg: 		      shall be the offset/length value for Flexcom dedicated I/O registers
  		      (without USART, TWI or SPI registers).
- clocks: 	      shall be the Flexcom peripheral clock from PMC.
- #address-cells:     should be <1>
- #size-cells: 	      should be <1>
- ranges: 	      one range for the full I/O register region (including USART, TWI and
  		      SPI registers)
- atmel,flexcom-mode: shall be a string among { "spi", "usart", "i2c", "twi" }.
  		      "i2c" and "twi" are synonymous.

> +Required child:
> +a child device of type matching the "atmel,flexcom-mode" property.
> +The reg property of this child should be:
> +- <0x200 0x200> for USART
> +- <0x400 0x200> for SPI
> +- <0x600 0x200> for I2C
> +
> +The phandle provided by the clocks property of the child is the same as one for
> +the Flexcom parent.
> +Other properties remain unchanged. See documentation of the respective device.

Please point to the other documentation.

../uart/<blah>
../spi/<blah>
../i2c/<blah>

> +Example:
> +
> +flx0: flexcom@f8034000 {

What references this node?  If nothing, then you can remove the label.

> +	compatible = "atmel,sama5d2-flexcom";
> +	reg = <0xf8034000 0x200>;
> +	clocks = <&flx0_clk>;
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	ranges = <0x0 0xf8034000 0x800>;
> +	atmel,flexcom-mode = "spi";
> +
> +	spi@f8034400 {
> +		compatible = "atmel,at91rm9200-spi";
> +		reg = <0x400 0x200>;
> +		interrupts = <19 IRQ_TYPE_LEVEL_HIGH 7>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_flx0_ioset1>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		clocks = <&flx0_clk>;
> +		clock-names = "spi_clk";
> +		atmel,fifo-size = <32>;
> +
> +		mtd_dataflash@0 {
> +			compatible = "atmel,at25f512b";
> +			reg = <0>;
> +			spi-max-frequency = <20000000>;
> +		};
> +	};
> +};
Cyrille Pitchen July 21, 2015, 2:15 p.m. UTC | #2
Hi Lee,

Le 21/07/2015 11:09, Lee Jones a écrit :
> On Mon, 22 Jun 2015, Cyrille Pitchen wrote:
...
>> +- atmel,flexcom-mode: shall be a string among { "spi", "usart", "i2c", "twi" }.
>> +  "i2c" and "twi" are synonymous.
> 
> Please use a numerical value instead.  You can then define it in
> include/dt-bindings/*
> 
> This is only required if you supply more than one child node.  Is that
> the plan?  I ask because you only have one child node in the example
> below.
> 
The "atmel,flexcom-mode" property is always required. The reset value of the
Operating Mode bit field inside the Flexcom Mode Register is 0 for "no serial
device". So none of the 3 embedded serial devices is clocked and all of their
registers are inaccessible (read as zero).
Before using any of the 3 serial devices, the Operating Mode must be set
accordingly by updating the Mode Register. Once done, only the selected serial
device is clocked. Also the external I/O lines of the Flexcom are muxed to
reach the selected serial device.

The muxing is given by the following table:

Flexcom pin    USART pin    SPI pin    I2C pin
        IO0          TXD       MOSI        TWD
        IO1          RXD       MISO       TWCK
        IO2          SCK       SPCK          -
        IO3          CTS      NPCS0          - 
        IO4          RTS      NPCS1          -


Bus conflicting configuration:
 ______________________________________
|               Flexcom                |
|  _____         _____         _____   |
| |     |__CLK__| SPI |__I/O__|     |  |    ___________       ___________
| |     |       |_____|       |     |  |   | SPI Slave |     | I2C Slave |
| | CLK |                     | I/O |  |   |___________|     |___________|
|_| MUX |        _____        | MUX |__|_________|________________|
| |     |__CLK__| I2C |__I/O__|     |  |             shared bus
| |_____|       |_____|       |_____|  |
|______________________________________|


For instance, it is very unlikely to connect both I2C and SPI slaves to
the very same Flexcom because I/O lines are shared. The operating mode is
selected once for all during the boot and will never change during runtime.
That's why the Flexcom DT node should have exactly one child.

>> +Example:
>> +
>> +flx0: flexcom@f8034000 {
> 
> What references this node?  If nothing, then you can remove the label.
OK, I will remove the label in the next series.
> 
>> +	compatible = "atmel,sama5d2-flexcom";
>> +	reg = <0xf8034000 0x200>;
>> +	clocks = <&flx0_clk>;
>> +	#address-cells = <1>;
>> +	#size-cells = <1>;
>> +	ranges = <0x0 0xf8034000 0x800>;
>> +	atmel,flexcom-mode = "spi";
>> +
>> +	spi@f8034400 {
>> +		compatible = "atmel,at91rm9200-spi";
>> +		reg = <0x400 0x200>;
>> +		interrupts = <19 IRQ_TYPE_LEVEL_HIGH 7>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_flx0_ioset1>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		clocks = <&flx0_clk>;
>> +		clock-names = "spi_clk";
>> +		atmel,fifo-size = <32>;
>> +
>> +		mtd_dataflash@0 {
>> +			compatible = "atmel,at25f512b";
>> +			reg = <0>;
>> +			spi-max-frequency = <20000000>;
>> +		};
>> +	};
>> +};
> 

Thanks for your review :)

Best Regards,

Cyrille
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lee Jones July 21, 2015, 3:10 p.m. UTC | #3
On Tue, 21 Jul 2015, Cyrille Pitchen wrote:

> Hi Lee,
> 
> Le 21/07/2015 11:09, Lee Jones a écrit :
> > On Mon, 22 Jun 2015, Cyrille Pitchen wrote:
> ...
> >> +- atmel,flexcom-mode: shall be a string among { "spi", "usart", "i2c", "twi" }.
> >> +  "i2c" and "twi" are synonymous.
> > 
> > Please use a numerical value instead.  You can then define it in
> > include/dt-bindings/*
> > 
> > This is only required if you supply more than one child node.  Is that
> > the plan?  I ask because you only have one child node in the example
> > below.
> > 
> The "atmel,flexcom-mode" property is always required. The reset value of the
> Operating Mode bit field inside the Flexcom Mode Register is 0 for "no serial
> device". So none of the 3 embedded serial devices is clocked and all of their
> registers are inaccessible (read as zero).
> Before using any of the 3 serial devices, the Operating Mode must be set
> accordingly by updating the Mode Register. Once done, only the selected serial
> device is clocked. Also the external I/O lines of the Flexcom are muxed to
> reach the selected serial device.
> 
> The muxing is given by the following table:
> 
> Flexcom pin    USART pin    SPI pin    I2C pin
>         IO0          TXD       MOSI        TWD
>         IO1          RXD       MISO       TWCK
>         IO2          SCK       SPCK          -
>         IO3          CTS      NPCS0          - 
>         IO4          RTS      NPCS1          -
> 
> 
> Bus conflicting configuration:
>  ______________________________________
> |               Flexcom                |
> |  _____         _____         _____   |
> | |     |__CLK__| SPI |__I/O__|     |  |    ___________       ___________
> | |     |       |_____|       |     |  |   | SPI Slave |     | I2C Slave |
> | | CLK |                     | I/O |  |   |___________|     |___________|
> |_| MUX |        _____        | MUX |__|_________|________________|
> | |     |__CLK__| I2C |__I/O__|     |  |             shared bus
> | |_____|       |_____|       |_____|  |
> |______________________________________|
> 
> 
> For instance, it is very unlikely to connect both I2C and SPI slaves to
> the very same Flexcom because I/O lines are shared. The operating mode is
> selected once for all during the boot and will never change during runtime.
> That's why the Flexcom DT node should have exactly one child.

You have contradicted your self here.  Firstly you say that the
"atmel,flexcom-mode" property is always required, then you say that
the DT node should only ever have one child.  If the latter is true,
then you can infer which mode Flexcom needs to be in by which node is
present.

> >> +Example:
> >> +
> >> +flx0: flexcom@f8034000 {
> > 
> > What references this node?  If nothing, then you can remove the label.
> OK, I will remove the label in the next series.
> > 
> >> +	compatible = "atmel,sama5d2-flexcom";
> >> +	reg = <0xf8034000 0x200>;
> >> +	clocks = <&flx0_clk>;
> >> +	#address-cells = <1>;
> >> +	#size-cells = <1>;
> >> +	ranges = <0x0 0xf8034000 0x800>;
> >> +	atmel,flexcom-mode = "spi";
> >> +
> >> +	spi@f8034400 {
> >> +		compatible = "atmel,at91rm9200-spi";
> >> +		reg = <0x400 0x200>;
> >> +		interrupts = <19 IRQ_TYPE_LEVEL_HIGH 7>;
> >> +		pinctrl-names = "default";
> >> +		pinctrl-0 = <&pinctrl_flx0_ioset1>;
> >> +		#address-cells = <1>;
> >> +		#size-cells = <0>;
> >> +		clocks = <&flx0_clk>;
> >> +		clock-names = "spi_clk";
> >> +		atmel,fifo-size = <32>;
> >> +
> >> +		mtd_dataflash@0 {
> >> +			compatible = "atmel,at25f512b";
> >> +			reg = <0>;
> >> +			spi-max-frequency = <20000000>;
> >> +		};
> >> +	};
> >> +};
> > 
> 
> Thanks for your review :)
> 
> Best Regards,
> 
> Cyrille
Cyrille Pitchen July 22, 2015, 10:23 a.m. UTC | #4
Le 21/07/2015 17:10, Lee Jones a écrit :
> On Tue, 21 Jul 2015, Cyrille Pitchen wrote:
> 
>> Hi Lee,
>>
>> Le 21/07/2015 11:09, Lee Jones a écrit :
>>> On Mon, 22 Jun 2015, Cyrille Pitchen wrote:
>> ...
>>>> +- atmel,flexcom-mode: shall be a string among { "spi", "usart", "i2c", "twi" }.
>>>> +  "i2c" and "twi" are synonymous.
>>>
>>> Please use a numerical value instead.  You can then define it in
>>> include/dt-bindings/*
>>>
>>> This is only required if you supply more than one child node.  Is that
>>> the plan?  I ask because you only have one child node in the example
>>> below.
>>>
>> The "atmel,flexcom-mode" property is always required. The reset value of the
>> Operating Mode bit field inside the Flexcom Mode Register is 0 for "no serial
>> device". So none of the 3 embedded serial devices is clocked and all of their
>> registers are inaccessible (read as zero).
>> Before using any of the 3 serial devices, the Operating Mode must be set
>> accordingly by updating the Mode Register. Once done, only the selected serial
>> device is clocked. Also the external I/O lines of the Flexcom are muxed to
>> reach the selected serial device.
>>
>> The muxing is given by the following table:
>>
>> Flexcom pin    USART pin    SPI pin    I2C pin
>>         IO0          TXD       MOSI        TWD
>>         IO1          RXD       MISO       TWCK
>>         IO2          SCK       SPCK          -
>>         IO3          CTS      NPCS0          - 
>>         IO4          RTS      NPCS1          -
>>
>>
>> Bus conflicting configuration:
>>  ______________________________________
>> |               Flexcom                |
>> |  _____         _____         _____   |
>> | |     |__CLK__| SPI |__I/O__|     |  |    ___________       ___________
>> | |     |       |_____|       |     |  |   | SPI Slave |     | I2C Slave |
>> | | CLK |                     | I/O |  |   |___________|     |___________|
>> |_| MUX |        _____        | MUX |__|_________|________________|
>> | |     |__CLK__| I2C |__I/O__|     |  |             shared bus
>> | |_____|       |_____|       |_____|  |
>> |______________________________________|
>>
>>
>> For instance, it is very unlikely to connect both I2C and SPI slaves to
>> the very same Flexcom because I/O lines are shared. The operating mode is
>> selected once for all during the boot and will never change during runtime.
>> That's why the Flexcom DT node should have exactly one child.
> 
> You have contradicted your self here.  Firstly you say that the
> "atmel,flexcom-mode" property is always required, then you say that
> the DT node should only ever have one child.  If the latter is true,
> then you can infer which mode Flexcom needs to be in by which node is
> present.
> 
OK, you're right. So on the next series (v6), I will simply remove the
"atmel,flexcom-mode" property from the DT bindings. Then, the driver will
look for the first available child node which "compatible" property contains
one of the substrings "-usart", "-spi" or "-i2c", setting the Flexcom operating
mode accordingly. This child node should be unique.

>>>> +Example:
>>>> +
>>>> +flx0: flexcom@f8034000 {
>>>
>>> What references this node?  If nothing, then you can remove the label.
>> OK, I will remove the label in the next series.
>>>
>>>> +	compatible = "atmel,sama5d2-flexcom";
>>>> +	reg = <0xf8034000 0x200>;
>>>> +	clocks = <&flx0_clk>;
>>>> +	#address-cells = <1>;
>>>> +	#size-cells = <1>;
>>>> +	ranges = <0x0 0xf8034000 0x800>;
>>>> +	atmel,flexcom-mode = "spi";
>>>> +
>>>> +	spi@f8034400 {
>>>> +		compatible = "atmel,at91rm9200-spi";
>>>> +		reg = <0x400 0x200>;
>>>> +		interrupts = <19 IRQ_TYPE_LEVEL_HIGH 7>;
>>>> +		pinctrl-names = "default";
>>>> +		pinctrl-0 = <&pinctrl_flx0_ioset1>;
>>>> +		#address-cells = <1>;
>>>> +		#size-cells = <0>;
>>>> +		clocks = <&flx0_clk>;
>>>> +		clock-names = "spi_clk";
>>>> +		atmel,fifo-size = <32>;
>>>> +
>>>> +		mtd_dataflash@0 {
>>>> +			compatible = "atmel,at25f512b";
>>>> +			reg = <0>;
>>>> +			spi-max-frequency = <20000000>;
>>>> +		};
>>>> +	};
>>>> +};
>>>
>>
>> Thanks for your review :)
>>
>> Best Regards,
>>
>> Cyrille
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt b/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
new file mode 100644
index 0000000..38f0e43
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
@@ -0,0 +1,58 @@ 
+* Device tree bindings for Atmel Flexcom (Flexible Serial Communication Unit)
+
+The Atmel Flexcom is just a wrapper which embeds a SPI controller, an I2C
+controller and an USART. Only one function can be used at a time and is chosen
+at boot time according to the device tree.
+
+Required properties:
+- compatible: "atmel,sama5d2-flexcom"
+- reg: shall be the offset/length value for Flexcom dedicated I/O registers
+  (without USART, TWI or SPI registers).
+- clocks: shall be the Flexcom peripheral clock from PMC.
+- #address-cells: should be <1>
+- #size-cells: should be <1>
+- ranges: one range for the full I/O register region (including USART, TWI and
+  SPI registers)
+- atmel,flexcom-mode: shall be a string among { "spi", "usart", "i2c", "twi" }.
+  "i2c" and "twi" are synonymous.
+
+Required child:
+a child device of type matching the "atmel,flexcom-mode" property.
+The reg property of this child should be:
+- <0x200 0x200> for USART
+- <0x400 0x200> for SPI
+- <0x600 0x200> for I2C
+The phandle provided by the clocks property of the child is the same as one for
+the Flexcom parent.
+Other properties remain unchanged. See documentation of the respective device.
+
+Example:
+
+flx0: flexcom@f8034000 {
+	compatible = "atmel,sama5d2-flexcom";
+	reg = <0xf8034000 0x200>;
+	clocks = <&flx0_clk>;
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges = <0x0 0xf8034000 0x800>;
+	atmel,flexcom-mode = "spi";
+
+	spi@f8034400 {
+		compatible = "atmel,at91rm9200-spi";
+		reg = <0x400 0x200>;
+		interrupts = <19 IRQ_TYPE_LEVEL_HIGH 7>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_flx0_ioset1>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		clocks = <&flx0_clk>;
+		clock-names = "spi_clk";
+		atmel,fifo-size = <32>;
+
+		mtd_dataflash@0 {
+			compatible = "atmel,at25f512b";
+			reg = <0>;
+			spi-max-frequency = <20000000>;
+		};
+	};
+};