[PATCH v4 0/2] mfd: flexcom: add a driver for Flexcom

[PATCH v4 0/2] mfd: flexcom: add a driver for Flexcom

[Cyrille Pitchen]

ChangeLog

v4:
- check clk_prepare_enable() return code in atmel_flexcom_probe()
- add a commit message to the DT binding patch

v3:
- remove MODULE_ALIAS()
- add Acked-by from Boris Brezillon and Alexandre Belloni

v2:
- enhance the documentation of DT bindings and change the way the "ranges"
  property is used.
- replace __raw_readl() and __raw_writel() by readl() and writel().
- change the module license to "GPL" for v2 or later
- print the selected flexcom mode after the hardware version

v1:
This series of patches a support to the Atmel Flexcom, a wrapper which
integrates an USART, a SPI controller and a TWI controller. Only one
peripheral can be used at a time. The active function is selected though
the Flexcom Mode Register.

Cyrille Pitchen (2):
  mfd: devicetree: add bindings for Atmel Flexcom
  mfd: flexcom: add a driver for Atmel Flexible Serial Communication
    Unit

 .../devicetree/bindings/mfd/atmel-flexcom.txt      | 58 +++++++++++++
 drivers/mfd/Kconfig                                | 11 +++
 drivers/mfd/Makefile                               |  1 +
 drivers/mfd/atmel-flexcom.c                        | 98 ++++++++++++++++++++++
 4 files changed, 168 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
 create mode 100644 drivers/mfd/atmel-flexcom.c

-- 
1.8.2.2

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

[Cyrille Pitchen]

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.
+
+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 at 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 at 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 at 0 {
+			compatible = "atmel,at25f512b";
+			reg = <0>;
+			spi-max-frequency = <20000000>;
+		};
+	};
+};
-- 
1.8.2.2

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

[Lee Jones]

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 at 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 at 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 at 0 {
> +			compatible = "atmel,at25f512b";
> +			reg = <0>;
> +			spi-max-frequency = <20000000>;
> +		};
> +	};
> +};

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[Cyrille Pitchen]

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 at 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 at 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 at 0 {
>> +			compatible = "atmel,at25f512b";
>> +			reg = <0>;
>> +			spi-max-frequency = <20000000>;
>> +		};
>> +	};
>> +};
> 

Thanks for your review :)

Best Regards,

Cyrille

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

[Lee Jones]

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 at 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 at 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 at 0 {
> >> +			compatible = "atmel,at25f512b";
> >> +			reg = <0>;
> >> +			spi-max-frequency = <20000000>;
> >> +		};
> >> +	};
> >> +};
> > 
> 
> Thanks for your review :)
> 
> Best Regards,
> 
> Cyrille

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[Cyrille Pitchen]

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 at 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 at 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 at 0 {
>>>> +			compatible = "atmel,at25f512b";
>>>> +			reg = <0>;
>>>> +			spi-max-frequency = <20000000>;
>>>> +		};
>>>> +	};
>>>> +};
>>>
>>
>> Thanks for your review :)
>>
>> Best Regards,
>>
>> Cyrille
> 

[PATCH v4 2/2] mfd: flexcom: add a driver for Atmel Flexible Serial Communication Unit

[Cyrille Pitchen]

This driver supports the new Atmel Flexcom. The Flexcom is a wrapper which
integrates one SPI controller, one I2C controller and one USART. Only one
function can be enabled at a time. This driver selects the function once
for all, when the Flexcom is probed, according to the value of the new
"atmel,flexcom-mode" device tree property.

This driver has chosen to present the Flexcom to the system as a MFD so
the implementation is seamless for the existing Atmel SPI, I2C and USART
drivers.

Also the Flexcom embeds FIFOs: the latest patches of the SPI, I2C and
USART drivers take advantage of this new feature.

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>
---
 drivers/mfd/Kconfig         | 11 +++++
 drivers/mfd/Makefile        |  1 +
 drivers/mfd/atmel-flexcom.c | 98 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 110 insertions(+)
 create mode 100644 drivers/mfd/atmel-flexcom.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index d5ad04d..9b33ad0 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -59,6 +59,17 @@ config MFD_AAT2870_CORE
 	  additional drivers must be enabled in order to use the
 	  functionality of the device.
 
+config MFD_ATMEL_FLEXCOM
+	tristate "Atmel Flexcom (Flexible Serial Communication Unit)"
+	select MFD_CORE
+	depends on OF
+	help
+	  Select this to get support for Atmel Flexcom. This is a wrapper
+	  which embeds a SPI controller, a I2C controller and a USART. Only
+	  one function can be used at a time. The choice is done at boot time
+	  by the probe function of this MFD driver according to a device tree
+	  property.
+
 config MFD_ATMEL_HLCDC
 	tristate "Atmel HLCDC (High-end LCD Controller)"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 0e5cfeb..c666bf5 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -160,6 +160,7 @@ obj-$(CONFIG_MFD_SPMI_PMIC)	+= qcom-spmi-pmic.o
 obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
 obj-$(CONFIG_MFD_TPS65090)	+= tps65090.o
 obj-$(CONFIG_MFD_AAT2870_CORE)	+= aat2870-core.o
+obj-$(CONFIG_MFD_ATMEL_FLEXCOM)	+= atmel-flexcom.o
 obj-$(CONFIG_MFD_ATMEL_HLCDC)	+= atmel-hlcdc.o
 obj-$(CONFIG_MFD_INTEL_MSIC)	+= intel_msic.o
 obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
diff --git a/drivers/mfd/atmel-flexcom.c b/drivers/mfd/atmel-flexcom.c
new file mode 100644
index 0000000..7e7d4b8
--- /dev/null
+++ b/drivers/mfd/atmel-flexcom.c
@@ -0,0 +1,98 @@
+/*
+ * Driver for Atmel Flexcom
+ *
+ * Copyright (C) 2015 Atmel Corporation
+ *
+ * Author: Cyrille Pitchen <cyrille.pitchen@atmel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+
+#define FX_MR		0x0
+#define FX_RHR		0x10
+#define FX_THR		0x20
+#define FX_VERSION	0xfc
+
+#define FX_MR_NO_COM	0
+#define FX_MR_USART	1
+#define FX_MR_SPI	2
+#define FX_MR_TWI	3
+
+
+static int atmel_flexcom_probe(struct platform_device *pdev)
+{
+	struct clk *clk;
+	struct resource *res;
+	unsigned char __iomem *map;
+	unsigned int version, mr;
+	const char *mode;
+	int err;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	map = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(map))
+		return PTR_ERR(map);
+
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	err = of_property_read_string(pdev->dev.of_node,
+				      "atmel,flexcom-mode", &mode);
+	if (err)
+		return err;
+
+	if (!strcmp(mode, "usart"))
+		mr = FX_MR_USART;
+	else if (!strcmp(mode, "spi"))
+		mr = FX_MR_SPI;
+	else if (!strcmp(mode, "twi") || !strcmp(mode, "i2c"))
+		mr = FX_MR_TWI;
+	else
+		return -EINVAL;
+
+	err = clk_prepare_enable(clk);
+	if (err)
+		return err;
+	version = readl(map + FX_VERSION);
+	writel(mr, map + FX_MR);
+	clk_disable_unprepare(clk);
+
+	dev_info(&pdev->dev, "version: %#x, mode: %s\n", version, mode);
+
+	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+}
+
+static const struct of_device_id atmel_flexcom_of_match[] = {
+	{ .compatible = "atmel,sama5d2-flexcom" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, atmel_flexcom_of_match);
+
+static struct platform_driver atmel_flexcom_driver = {
+	.driver		= {
+		.name	= "atmel_flexcom",
+		.of_match_table = atmel_flexcom_of_match,
+	},
+	.probe		= atmel_flexcom_probe,
+};
+
+module_platform_driver(atmel_flexcom_driver);
+
+MODULE_AUTHOR("Cyrille Pitchen <cyrille.pitchen@atmel.com>");
+MODULE_DESCRIPTION("Atmel Flexcom MFD driver");
+MODULE_LICENSE("GPL");
-- 
1.8.2.2

[PATCH v4 2/2] mfd: flexcom: add a driver for Atmel Flexible Serial Communication Unit

[Lee Jones]

On Mon, 22 Jun 2015, Cyrille Pitchen wrote:

> This driver supports the new Atmel Flexcom. The Flexcom is a wrapper which
> integrates one SPI controller, one I2C controller and one USART. Only one
> function can be enabled at a time. This driver selects the function once
> for all, when the Flexcom is probed, according to the value of the new
> "atmel,flexcom-mode" device tree property.
> 
> This driver has chosen to present the Flexcom to the system as a MFD so
> the implementation is seamless for the existing Atmel SPI, I2C and USART
> drivers.

No need for this driver.

Just create your 3 platform drivers and get them to read the mode at
probe time.  If the mode isn't the one pertaining to itself return
-ENODEV and the Device Driver API will attempt the probe the next
device.  Ensure all of the devices have the same compatible string
i.e. atmel,sama5d2-flexcom.

> Also the Flexcom embeds FIFOs: the latest patches of the SPI, I2C and
> USART drivers take advantage of this new feature.
> 
> 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>
> ---
>  drivers/mfd/Kconfig         | 11 +++++
>  drivers/mfd/Makefile        |  1 +
>  drivers/mfd/atmel-flexcom.c | 98 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 110 insertions(+)
>  create mode 100644 drivers/mfd/atmel-flexcom.c
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index d5ad04d..9b33ad0 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -59,6 +59,17 @@ config MFD_AAT2870_CORE
>  	  additional drivers must be enabled in order to use the
>  	  functionality of the device.
>  
> +config MFD_ATMEL_FLEXCOM
> +	tristate "Atmel Flexcom (Flexible Serial Communication Unit)"
> +	select MFD_CORE
> +	depends on OF
> +	help
> +	  Select this to get support for Atmel Flexcom. This is a wrapper
> +	  which embeds a SPI controller, a I2C controller and a USART. Only
> +	  one function can be used at a time. The choice is done at boot time
> +	  by the probe function of this MFD driver according to a device tree
> +	  property.
> +
>  config MFD_ATMEL_HLCDC
>  	tristate "Atmel HLCDC (High-end LCD Controller)"
>  	select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 0e5cfeb..c666bf5 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -160,6 +160,7 @@ obj-$(CONFIG_MFD_SPMI_PMIC)	+= qcom-spmi-pmic.o
>  obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
>  obj-$(CONFIG_MFD_TPS65090)	+= tps65090.o
>  obj-$(CONFIG_MFD_AAT2870_CORE)	+= aat2870-core.o
> +obj-$(CONFIG_MFD_ATMEL_FLEXCOM)	+= atmel-flexcom.o
>  obj-$(CONFIG_MFD_ATMEL_HLCDC)	+= atmel-hlcdc.o
>  obj-$(CONFIG_MFD_INTEL_MSIC)	+= intel_msic.o
>  obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
> diff --git a/drivers/mfd/atmel-flexcom.c b/drivers/mfd/atmel-flexcom.c
> new file mode 100644
> index 0000000..7e7d4b8
> --- /dev/null
> +++ b/drivers/mfd/atmel-flexcom.c
> @@ -0,0 +1,98 @@
> +/*
> + * Driver for Atmel Flexcom
> + *
> + * Copyright (C) 2015 Atmel Corporation
> + *
> + * Author: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/clk.h>
> +
> +#define FX_MR		0x0
> +#define FX_RHR		0x10
> +#define FX_THR		0x20
> +#define FX_VERSION	0xfc
> +
> +#define FX_MR_NO_COM	0
> +#define FX_MR_USART	1
> +#define FX_MR_SPI	2
> +#define FX_MR_TWI	3
> +
> +
> +static int atmel_flexcom_probe(struct platform_device *pdev)
> +{
> +	struct clk *clk;
> +	struct resource *res;
> +	unsigned char __iomem *map;
> +	unsigned int version, mr;
> +	const char *mode;
> +	int err;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	map = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(map))
> +		return PTR_ERR(map);
> +
> +	clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	err = of_property_read_string(pdev->dev.of_node,
> +				      "atmel,flexcom-mode", &mode);
> +	if (err)
> +		return err;
> +
> +	if (!strcmp(mode, "usart"))
> +		mr = FX_MR_USART;
> +	else if (!strcmp(mode, "spi"))
> +		mr = FX_MR_SPI;
> +	else if (!strcmp(mode, "twi") || !strcmp(mode, "i2c"))
> +		mr = FX_MR_TWI;
> +	else
> +		return -EINVAL;
> +
> +	err = clk_prepare_enable(clk);
> +	if (err)
> +		return err;
> +	version = readl(map + FX_VERSION);
> +	writel(mr, map + FX_MR);
> +	clk_disable_unprepare(clk);
> +
> +	dev_info(&pdev->dev, "version: %#x, mode: %s\n", version, mode);
> +
> +	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
> +}
> +
> +static const struct of_device_id atmel_flexcom_of_match[] = {
> +	{ .compatible = "atmel,sama5d2-flexcom" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, atmel_flexcom_of_match);
> +
> +static struct platform_driver atmel_flexcom_driver = {
> +	.driver		= {
> +		.name	= "atmel_flexcom",
> +		.of_match_table = atmel_flexcom_of_match,
> +	},
> +	.probe		= atmel_flexcom_probe,
> +};
> +
> +module_platform_driver(atmel_flexcom_driver);
> +
> +MODULE_AUTHOR("Cyrille Pitchen <cyrille.pitchen@atmel.com>");
> +MODULE_DESCRIPTION("Atmel Flexcom MFD driver");
> +MODULE_LICENSE("GPL");

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v4 2/2] mfd: flexcom: add a driver for Atmel Flexible Serial Communication Unit

[Cyrille Pitchen]

Hi lee,

Le 24/06/2015 16:24, Lee Jones a ?crit :
> On Mon, 22 Jun 2015, Cyrille Pitchen wrote:
> 
>> This driver supports the new Atmel Flexcom. The Flexcom is a wrapper which
>> integrates one SPI controller, one I2C controller and one USART. Only one
>> function can be enabled at a time. This driver selects the function once
>> for all, when the Flexcom is probed, according to the value of the new
>> "atmel,flexcom-mode" device tree property.
>>
>> This driver has chosen to present the Flexcom to the system as a MFD so
>> the implementation is seamless for the existing Atmel SPI, I2C and USART
>> drivers.
> 
> No need for this driver.
> 
> Just create your 3 platform drivers and get them to read the mode at
> probe time.  If the mode isn't the one pertaining to itself return
> -ENODEV and the Device Driver API will attempt the probe the next
> device.  Ensure all of the devices have the same compatible string
> i.e. atmel,sama5d2-flexcom.
> 

The main idea of this patch was to hide the Flexcom overlay from the 3 serial
drivers. The Flexcom is a kind of muxer which selects one of the 3 IPs: it 
doesn't change the internal of the IPs. For instance, on sama5d2x SoCs,
some SPI controlers are still standalone whereas others are wrapped by a
Flexcom but all these SPI controllers share the same IP version.

Also, the Flexcom has its own I/O range. Especially, the Flexcom Mode Register
which selects the active peripheral is outside the I/O ranges of the SPI, I2C
or USART IPs.

The active peripheral must be selected first then its driver can access its I/O
registers but registers of the other inactive peripherals are still unavailable
and would be read as zero since their peripherals are still disabled.
That's why the very first version of the driver presented the Flexcom as a
clock because most peripherals can't be used before enabling their clock.
However the Flexcom is not an actual clock so I was told that it would be
better to introduce the Flexcom as an MFD.

The idea is to separate the serial controller drivers and the Flexcom driver
the same way as a clock driver and a pin controller driver are separated from
the driver of a peripheral which uses them.


Best Regards,

Cyrille

[PATCH v4 2/2] mfd: flexcom: add a driver for Atmel Flexible Serial Communication Unit

[Cyrille Pitchen]

Hi all,

based on my previous mail, do someone have any idea on how to add support to
the Flexcom in the kernel ? We would like to manage the Flexcom separately so
we avoid patching 3 existing drivers.

Indeed, their associated devices have no need to know whether they're wrapped
or not by a Flexcom: the Flexcom itself has no functional impact on the serial
controllers it contains.

Also, hiding the additional layer of the Flexcom from the USART, SPI and I2C
drivers would ease their maintenance: it keeps the design of each IP
independent from the others so it would be more easy to add support to future
features.


Best Regards,

Cyrille


Le 25/06/2015 12:11, Cyrille Pitchen a ?crit :
> Hi lee,
> 
> Le 24/06/2015 16:24, Lee Jones a ?crit :
>> On Mon, 22 Jun 2015, Cyrille Pitchen wrote:
>>
>>> This driver supports the new Atmel Flexcom. The Flexcom is a wrapper which
>>> integrates one SPI controller, one I2C controller and one USART. Only one
>>> function can be enabled at a time. This driver selects the function once
>>> for all, when the Flexcom is probed, according to the value of the new
>>> "atmel,flexcom-mode" device tree property.
>>>
>>> This driver has chosen to present the Flexcom to the system as a MFD so
>>> the implementation is seamless for the existing Atmel SPI, I2C and USART
>>> drivers.
>>
>> No need for this driver.
>>
>> Just create your 3 platform drivers and get them to read the mode at
>> probe time.  If the mode isn't the one pertaining to itself return
>> -ENODEV and the Device Driver API will attempt the probe the next
>> device.  Ensure all of the devices have the same compatible string
>> i.e. atmel,sama5d2-flexcom.
>>
> 
> The main idea of this patch was to hide the Flexcom overlay from the 3 serial
> drivers. The Flexcom is a kind of muxer which selects one of the 3 IPs: it 
> doesn't change the internal of the IPs. For instance, on sama5d2x SoCs,
> some SPI controlers are still standalone whereas others are wrapped by a
> Flexcom but all these SPI controllers share the same IP version.
> 
> Also, the Flexcom has its own I/O range. Especially, the Flexcom Mode Register
> which selects the active peripheral is outside the I/O ranges of the SPI, I2C
> or USART IPs.
> 
> The active peripheral must be selected first then its driver can access its I/O
> registers but registers of the other inactive peripherals are still unavailable
> and would be read as zero since their peripherals are still disabled.
> That's why the very first version of the driver presented the Flexcom as a
> clock because most peripherals can't be used before enabling their clock.
> However the Flexcom is not an actual clock so I was told that it would be
> better to introduce the Flexcom as an MFD.
> 
> The idea is to separate the serial controller drivers and the Flexcom driver
> the same way as a clock driver and a pin controller driver are separated from
> the driver of a peripheral which uses them.
> 
> 
> Best Regards,
> 
> Cyrille
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH v4 2/2] mfd: flexcom: add a driver for Atmel Flexible Serial Communication Unit

[Lee Jones]

On Mon, 22 Jun 2015, Cyrille Pitchen wrote:

> This driver supports the new Atmel Flexcom. The Flexcom is a wrapper which
> integrates one SPI controller, one I2C controller and one USART. Only one
> function can be enabled at a time. This driver selects the function once
> for all, when the Flexcom is probed, according to the value of the new
> "atmel,flexcom-mode" device tree property.
> 
> This driver has chosen to present the Flexcom to the system as a MFD so
> the implementation is seamless for the existing Atmel SPI, I2C and USART
> drivers.
> 
> Also the Flexcom embeds FIFOs: the latest patches of the SPI, I2C and
> USART drivers take advantage of this new feature.
> 
> 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>
> ---
>  drivers/mfd/Kconfig         | 11 +++++
>  drivers/mfd/Makefile        |  1 +
>  drivers/mfd/atmel-flexcom.c | 98 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 110 insertions(+)
>  create mode 100644 drivers/mfd/atmel-flexcom.c
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index d5ad04d..9b33ad0 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -59,6 +59,17 @@ config MFD_AAT2870_CORE
>  	  additional drivers must be enabled in order to use the
>  	  functionality of the device.
>  
> +config MFD_ATMEL_FLEXCOM
> +	tristate "Atmel Flexcom (Flexible Serial Communication Unit)"
> +	select MFD_CORE
> +	depends on OF
> +	help
> +	  Select this to get support for Atmel Flexcom. This is a wrapper
> +	  which embeds a SPI controller, a I2C controller and a USART. Only
> +	  one function can be used at a time. The choice is done at boot time
> +	  by the probe function of this MFD driver according to a device tree
> +	  property.
> +
>  config MFD_ATMEL_HLCDC
>  	tristate "Atmel HLCDC (High-end LCD Controller)"
>  	select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 0e5cfeb..c666bf5 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -160,6 +160,7 @@ obj-$(CONFIG_MFD_SPMI_PMIC)	+= qcom-spmi-pmic.o
>  obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
>  obj-$(CONFIG_MFD_TPS65090)	+= tps65090.o
>  obj-$(CONFIG_MFD_AAT2870_CORE)	+= aat2870-core.o
> +obj-$(CONFIG_MFD_ATMEL_FLEXCOM)	+= atmel-flexcom.o
>  obj-$(CONFIG_MFD_ATMEL_HLCDC)	+= atmel-hlcdc.o
>  obj-$(CONFIG_MFD_INTEL_MSIC)	+= intel_msic.o
>  obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
> diff --git a/drivers/mfd/atmel-flexcom.c b/drivers/mfd/atmel-flexcom.c
> new file mode 100644
> index 0000000..7e7d4b8
> --- /dev/null
> +++ b/drivers/mfd/atmel-flexcom.c
> @@ -0,0 +1,98 @@
> +/*
> + * Driver for Atmel Flexcom
> + *
> + * Copyright (C) 2015 Atmel Corporation
> + *
> + * Author: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/clk.h>
> +
> +#define FX_MR		0x0
> +#define FX_RHR		0x10
> +#define FX_THR		0x20
> +#define FX_VERSION	0xfc
> +
> +#define FX_MR_NO_COM	0
> +#define FX_MR_USART	1
> +#define FX_MR_SPI	2
> +#define FX_MR_TWI	3
> +
> +
> +static int atmel_flexcom_probe(struct platform_device *pdev)
> +{
> +	struct clk *clk;
> +	struct resource *res;
> +	unsigned char __iomem *map;

Would prefer:
        void __iomem *base;

> +	unsigned int version, mr;

'mr' is not a good variable name.

> +	const char *mode;
> +	int err;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	map = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(map))
> +		return PTR_ERR(map);
> +
> +	clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	err = of_property_read_string(pdev->dev.of_node,
> +				      "atmel,flexcom-mode", &mode);
> +	if (err)
> +		return err;
> +
> +	if (!strcmp(mode, "usart"))
> +		mr = FX_MR_USART;
> +	else if (!strcmp(mode, "spi"))
> +		mr = FX_MR_SPI;
> +	else if (!strcmp(mode, "twi") || !strcmp(mode, "i2c"))
> +		mr = FX_MR_TWI;
> +	else
> +		return -EINVAL;

Just use a numerical constant in the DT, then you can

of_property_read_u32(np, "atmel,flexcom-mode", &mr);

Although, hopefully you'll rename 'mr' to be something more
forthcoming.

> +	err = clk_prepare_enable(clk);
> +	if (err)
> +		return err;

'\n' here.

> +	version = readl(map + FX_VERSION);
> +	writel(mr, map + FX_MR);

'\n' here.

> +	clk_disable_unprepare(clk);
> +
> +	dev_info(&pdev->dev, "version: %#x, mode: %s\n", version, mode);

How important is it to clog up the bootlog and slow down boot with
this stuff?

> +	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
> +}
> +
> +static const struct of_device_id atmel_flexcom_of_match[] = {
> +	{ .compatible = "atmel,sama5d2-flexcom" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, atmel_flexcom_of_match);
> +
> +static struct platform_driver atmel_flexcom_driver = {
> +	.driver		= {
> +		.name	= "atmel_flexcom",
> +		.of_match_table = atmel_flexcom_of_match,
> +	},
> +	.probe		= atmel_flexcom_probe,
> +};
> +
> +module_platform_driver(atmel_flexcom_driver);
> +
> +MODULE_AUTHOR("Cyrille Pitchen <cyrille.pitchen@atmel.com>");
> +MODULE_DESCRIPTION("Atmel Flexcom MFD driver");
> +MODULE_LICENSE("GPL");

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog