devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/2] mfd: flexcom: add a driver for Flexcom
@ 2015-07-23 16:42 Cyrille Pitchen
       [not found] ` <cover.1437669004.git.cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Cyrille Pitchen @ 2015-07-23 16:42 UTC (permalink / raw)
  To: nicolas.ferre, boris.brezillon, alexandre.belloni, sameo,
	lee.jones
  Cc: mark.rutland, devicetree, pawel.moll, ijc+devicetree,
	linux-kernel, robh+dt, galak, Cyrille Pitchen, linux-arm-kernel

ChangeLog

v7:
- read the operating mode from the very first u32 of the reg property from
  the first available child node (should be unique).
- update the DT bindings documentation accordingly.

v6:
- select the operating mode according to the "compatible" DT property of
  the first available child node (should be unique).
- remove the "atmel,flexcom-mode" DT property so the need of a header file
  defining macros for the possible values of this deprecated property.

v5:
- create a header file containing macros used by DT bindings
- use numeric constants instead of strings to select the Flexcom mode
- change the license to "GPL v2"
- update the DT binding documentation to make it more readable and add
  references to USART, SPI and I2C DT binding documentations. remove the
  useless label in the Example section.
- change the register prefix from FX_ to FLEX_ to match the Flexcom
  programmer datasheet.
- rename some variables to make them more understandable.

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: atmel-flexcom: add a driver for Atmel Flexible Serial
    Communication Unit

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

-- 
1.8.2.2

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v7 1/2] mfd: devicetree: add bindings for Atmel Flexcom
       [not found] ` <cover.1437669004.git.cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
@ 2015-07-23 16:42   ` Cyrille Pitchen
  2015-07-23 19:48     ` Boris Brezillon
  2015-07-23 16:42   ` [PATCH v7 2/2] mfd: atmel-flexcom: add a driver for Atmel Flexible Serial Communication Unit Cyrille Pitchen
  1 sibling, 1 reply; 10+ messages in thread
From: Cyrille Pitchen @ 2015-07-23 16:42 UTC (permalink / raw)
  To: nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w,
	boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	sameo-VuQAYsv1563Yd54FQh9/CA, lee.jones-QSEj5FYQhm4dnm+yROfE0A
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, devicetree-u79uwXL29TY76Z2rM5mHXA,
	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-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/mfd/atmel-flexcom.txt      | 68 ++++++++++++++++++++++
 1 file changed, 68 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 000000000000..a63226b7a9cb
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
@@ -0,0 +1,68 @@
+* 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:		Should be "atmel,sama5d2-flexcom"
+- reg:			Should be the pair (offset, size) for the Flexcom
+			dedicated I/O registers (without USART, TWI or SPI
+			registers).
+- clocks:		Should be the Flexcom peripheral clock from PMC.
+- #address-cells:	Should be <2>
+- #size-cells:		Should be <1>
+- ranges:		Should be a list of ranges.
+			One range per peripheral wrapped by the Flexcom. So each
+			range is a triplet (child_addr, parent_addr, size). The
+			first u32 of "child_addr" is the value to be set in the
+			Operating Mode bitfield of the Flexcom Mode Register.
+			Then "parent_addr" stores the base address of the
+			corresponding peripheral in the system memory. Finally,
+			"size" if the size of the memory region of this
+			peripheral.
+
+Required child:
+A single available child for the serial controller to enable.
+
+Required properties of this child:
+- reg:			Should be a pair (child_addr, size) with child_addr
+			matching one of the parent ranges.
+- clocks:		Should be the very same phandle as for the parent's one.
+
+Other properties remain unchanged. See documentation of the respective device:
+- ../serial/atmel-usart.txt
+- ../spi/spi_atmel.txt
+- ../i2c/i2c-at91.txt
+
+Example:
+
+flexcom@f8034000 {
+	compatible = "atmel,sama5d2-flexcom";
+	reg = <0xf8034000 0x200>;
+	clocks = <&flx0_clk>;
+	#address-cells = <2>;
+	#size-cells = <1>;
+	ranges = <1 0 0xf8034200 0x200		/* opmode 1: USART */
+		  2 0 0xf8034400 0x200		/* opmode 2: SPI */
+		  3 0 0xf8034600 0x200>;	/* opmode 3: I2C */
+
+	spi@f8034400 {
+		compatible = "atmel,at91rm9200-spi";
+		reg = <2 0 0x200>;
+		interrupts = <19 IRQ_TYPE_LEVEL_HIGH 7>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_flx0_default>;
+		#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>;
+		};
+	};
+};
-- 
1.8.2.2

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v7 2/2] mfd: atmel-flexcom: add a driver for Atmel Flexible Serial Communication Unit
       [not found] ` <cover.1437669004.git.cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
  2015-07-23 16:42   ` [PATCH v7 1/2] mfd: devicetree: add bindings for Atmel Flexcom Cyrille Pitchen
@ 2015-07-23 16:42   ` Cyrille Pitchen
       [not found]     ` <55921f08b2d5d14256141edbe12287fba55860c4.1437669004.git.cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
  1 sibling, 1 reply; 10+ messages in thread
From: Cyrille Pitchen @ 2015-07-23 16:42 UTC (permalink / raw)
  To: nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w,
	boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	sameo-VuQAYsv1563Yd54FQh9/CA, lee.jones-QSEj5FYQhm4dnm+yROfE0A
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, devicetree-u79uwXL29TY76Z2rM5mHXA,
	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, using the "reg" property of the first
(should be unique) available DT child node.

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-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
---
 drivers/mfd/Kconfig         |  11 +++++
 drivers/mfd/Makefile        |   1 +
 drivers/mfd/atmel-flexcom.c | 113 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 125 insertions(+)
 create mode 100644 drivers/mfd/atmel-flexcom.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 653815950aa2..2c75472c679c 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -60,6 +60,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 ea40e076cb61..0705eb2d873d 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 000000000000..0d06b70696b0
--- /dev/null
+++ b/drivers/mfd/atmel-flexcom.c
@@ -0,0 +1,113 @@
+/*
+ * Driver for Atmel Flexcom
+ *
+ * Copyright (C) 2015 Atmel Corporation
+ *
+ * Author: Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#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>
+
+/* I/O register offsets */
+#define FLEX_MR		0x0	/* Mode Register */
+#define FLEX_VERSION	0xfc	/* Version Register */
+
+/* Mode Register bit fields */
+#define FLEX_MR_OPMODE_MASK	0x3
+#define FLEX_MR_OPMODE_NO_COM	0x0
+#define FLEX_MR_OPMODE_USART	0x1
+#define FLEX_MR_OPMODE_SPI	0x2
+#define FLEX_MR_OPMODE_TWI	0x3
+
+
+static int atmel_flexcom_probe(struct platform_device *pdev)
+{
+	struct device_node *child, *np = pdev->dev.of_node;
+	struct clk *clk;
+	struct resource *res;
+	void __iomem *base;
+	u32 opmode;
+	int err;
+
+	child = of_get_next_available_child(np, NULL);
+	if (!child)
+		return -ENODEV;
+
+	/*
+	 * The Operating Mode is stored into the first u32 of the reg property
+	 * of the child.
+	 */
+	err = of_property_read_u32_index(child, "reg", 0, &opmode);
+	of_node_put(child);
+	if (err)
+		return -EINVAL;
+
+	if ((opmode == FLEX_MR_OPMODE_NO_COM) ||
+	    (opmode & ~FLEX_MR_OPMODE_MASK))
+		return -EINVAL;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	err = clk_prepare_enable(clk);
+	if (err)
+		return err;
+
+	/*
+	 * Set the Operating Mode in the Mode Register: only the selected device
+	 * is clocked. Hence, registers of the other serial devices remain
+	 * inaccessible and are read as zero. Also the external I/O lines of the
+	 * Flexcom are muxed to reach the selected device.
+	 */
+	writel(opmode, base + FLEX_MR);
+
+	clk_disable_unprepare(clk);
+
+	return of_platform_populate(np, 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 = {
+	.probe	= atmel_flexcom_probe,
+	.driver	= {
+		.name		= "atmel_flexcom",
+		.of_match_table	= atmel_flexcom_of_match,
+	},
+};
+
+module_platform_driver(atmel_flexcom_driver);
+
+MODULE_AUTHOR("Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>");
+MODULE_DESCRIPTION("Atmel Flexcom MFD driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.2.2

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v7 1/2] mfd: devicetree: add bindings for Atmel Flexcom
  2015-07-23 16:42   ` [PATCH v7 1/2] mfd: devicetree: add bindings for Atmel Flexcom Cyrille Pitchen
@ 2015-07-23 19:48     ` Boris Brezillon
  2015-07-24  8:39       ` Nicolas Ferre
  0 siblings, 1 reply; 10+ messages in thread
From: Boris Brezillon @ 2015-07-23 19:48 UTC (permalink / raw)
  To: Cyrille Pitchen
  Cc: nicolas.ferre, alexandre.belloni, sameo, lee.jones,
	linux-arm-kernel, linux-kernel, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, devicetree

On Thu, 23 Jul 2015 18:42:55 +0200
Cyrille Pitchen <cyrille.pitchen@atmel.com> 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>
> ---
>  .../devicetree/bindings/mfd/atmel-flexcom.txt      | 68 ++++++++++++++++++++++
>  1 file changed, 68 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 000000000000..a63226b7a9cb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
> @@ -0,0 +1,68 @@
> +* 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:		Should be "atmel,sama5d2-flexcom"
> +- reg:			Should be the pair (offset, size) for the Flexcom
> +			dedicated I/O registers (without USART, TWI or SPI
> +			registers).
> +- clocks:		Should be the Flexcom peripheral clock from PMC.
> +- #address-cells:	Should be <2>
> +- #size-cells:		Should be <1>
> +- ranges:		Should be a list of ranges.
> +			One range per peripheral wrapped by the Flexcom. So each
> +			range is a triplet (child_addr, parent_addr, size). The
> +			first u32 of "child_addr" is the value to be set in the
> +			Operating Mode bitfield of the Flexcom Mode Register.
> +			Then "parent_addr" stores the base address of the
> +			corresponding peripheral in the system memory. Finally,
> +			"size" if the size of the memory region of this
> +			peripheral.
> +
> +Required child:
> +A single available child for the serial controller to enable.
> +
> +Required properties of this child:
> +- reg:			Should be a pair (child_addr, size) with child_addr
> +			matching one of the parent ranges.
> +- clocks:		Should be the very same phandle as for the parent's one.
> +
> +Other properties remain unchanged. See documentation of the respective device:
> +- ../serial/atmel-usart.txt
> +- ../spi/spi_atmel.txt
> +- ../i2c/i2c-at91.txt
> +
> +Example:
> +
> +flexcom@f8034000 {
> +	compatible = "atmel,sama5d2-flexcom";
> +	reg = <0xf8034000 0x200>;
> +	clocks = <&flx0_clk>;
> +	#address-cells = <2>;
> +	#size-cells = <1>;
> +	ranges = <1 0 0xf8034200 0x200		/* opmode 1: USART */
> +		  2 0 0xf8034400 0x200		/* opmode 2: SPI */
> +		  3 0 0xf8034600 0x200>;	/* opmode 3: I2C */
> +
> +	spi@f8034400 {

Should be:

	spi@2,0 {

> +		compatible = "atmel,at91rm9200-spi";
> +		reg = <2 0 0x200>;
> +		interrupts = <19 IRQ_TYPE_LEVEL_HIGH 7>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_flx0_default>;
> +		#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>;
> +		};
> +	};
> +};



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v7 1/2] mfd: devicetree: add bindings for Atmel Flexcom
  2015-07-23 19:48     ` Boris Brezillon
@ 2015-07-24  8:39       ` Nicolas Ferre
       [not found]         ` <55B1F9B7.2080206-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Nicolas Ferre @ 2015-07-24  8:39 UTC (permalink / raw)
  To: Boris Brezillon, Cyrille Pitchen, lee.jones
  Cc: alexandre.belloni, sameo, linux-arm-kernel, linux-kernel, robh+dt,
	pawel.moll, mark.rutland, ijc+devicetree, galak, devicetree

Le 23/07/2015 21:48, Boris Brezillon a écrit :
> On Thu, 23 Jul 2015 18:42:55 +0200
> Cyrille Pitchen <cyrille.pitchen@atmel.com> 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>
>> ---
>>  .../devicetree/bindings/mfd/atmel-flexcom.txt      | 68 ++++++++++++++++++++++
>>  1 file changed, 68 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 000000000000..a63226b7a9cb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
>> @@ -0,0 +1,68 @@
>> +* 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:		Should be "atmel,sama5d2-flexcom"
>> +- reg:			Should be the pair (offset, size) for the Flexcom
>> +			dedicated I/O registers (without USART, TWI or SPI
>> +			registers).
>> +- clocks:		Should be the Flexcom peripheral clock from PMC.
>> +- #address-cells:	Should be <2>
>> +- #size-cells:		Should be <1>
>> +- ranges:		Should be a list of ranges.
>> +			One range per peripheral wrapped by the Flexcom. So each
>> +			range is a triplet (child_addr, parent_addr, size). The
>> +			first u32 of "child_addr" is the value to be set in the
>> +			Operating Mode bitfield of the Flexcom Mode Register.
>> +			Then "parent_addr" stores the base address of the
>> +			corresponding peripheral in the system memory. Finally,
>> +			"size" if the size of the memory region of this
>> +			peripheral.
>> +
>> +Required child:
>> +A single available child for the serial controller to enable.
>> +
>> +Required properties of this child:
>> +- reg:			Should be a pair (child_addr, size) with child_addr
>> +			matching one of the parent ranges.
>> +- clocks:		Should be the very same phandle as for the parent's one.
>> +
>> +Other properties remain unchanged. See documentation of the respective device:
>> +- ../serial/atmel-usart.txt
>> +- ../spi/spi_atmel.txt
>> +- ../i2c/i2c-at91.txt
>> +
>> +Example:
>> +
>> +flexcom@f8034000 {
>> +	compatible = "atmel,sama5d2-flexcom";
>> +	reg = <0xf8034000 0x200>;
>> +	clocks = <&flx0_clk>;
>> +	#address-cells = <2>;
>> +	#size-cells = <1>;
>> +	ranges = <1 0 0xf8034200 0x200		/* opmode 1: USART */
>> +		  2 0 0xf8034400 0x200		/* opmode 2: SPI */
>> +		  3 0 0xf8034600 0x200>;	/* opmode 3: I2C */

Yes, the opmode(s) are exactly the values described in the product
datasheet.

>> +
>> +	spi@f8034400 {
> 
> Should be:
> 
> 	spi@2,0 {

Yep!

I was silently following the discussion in the background and I admit
that this approach is pretty elegant.

So, if this approach is validated you can also add my:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>


>> +		compatible = "atmel,at91rm9200-spi";
>> +		reg = <2 0 0x200>;
>> +		interrupts = <19 IRQ_TYPE_LEVEL_HIGH 7>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_flx0_default>;
>> +		#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>;
>> +		};
>> +	};
>> +};
> 
> 
> 


-- 
Nicolas Ferre

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v7 2/2] mfd: atmel-flexcom: add a driver for Atmel Flexible Serial Communication Unit
       [not found]     ` <55921f08b2d5d14256141edbe12287fba55860c4.1437669004.git.cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
@ 2015-07-24  8:42       ` Nicolas Ferre
  2015-07-24  8:43       ` Lee Jones
  1 sibling, 0 replies; 10+ messages in thread
From: Nicolas Ferre @ 2015-07-24  8:42 UTC (permalink / raw)
  To: Cyrille Pitchen, lee.jones-QSEj5FYQhm4dnm+yROfE0A
  Cc: boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	sameo-VuQAYsv1563Yd54FQh9/CA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, devicetree-u79uwXL29TY76Z2rM5mHXA

Le 23/07/2015 18:42, Cyrille Pitchen a écrit :
> 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, using the "reg" property of the first
> (should be unique) available DT child node.
> 
> 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.

Acked-by: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
Thank you.

Bye,


> Signed-off-by: Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/mfd/Kconfig         |  11 +++++
>  drivers/mfd/Makefile        |   1 +
>  drivers/mfd/atmel-flexcom.c | 113 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 125 insertions(+)
>  create mode 100644 drivers/mfd/atmel-flexcom.c
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 653815950aa2..2c75472c679c 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -60,6 +60,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 ea40e076cb61..0705eb2d873d 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 000000000000..0d06b70696b0
> --- /dev/null
> +++ b/drivers/mfd/atmel-flexcom.c
> @@ -0,0 +1,113 @@
> +/*
> + * Driver for Atmel Flexcom
> + *
> + * Copyright (C) 2015 Atmel Corporation
> + *
> + * Author: Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#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>
> +
> +/* I/O register offsets */
> +#define FLEX_MR		0x0	/* Mode Register */
> +#define FLEX_VERSION	0xfc	/* Version Register */
> +
> +/* Mode Register bit fields */
> +#define FLEX_MR_OPMODE_MASK	0x3
> +#define FLEX_MR_OPMODE_NO_COM	0x0
> +#define FLEX_MR_OPMODE_USART	0x1
> +#define FLEX_MR_OPMODE_SPI	0x2
> +#define FLEX_MR_OPMODE_TWI	0x3
> +
> +
> +static int atmel_flexcom_probe(struct platform_device *pdev)
> +{
> +	struct device_node *child, *np = pdev->dev.of_node;
> +	struct clk *clk;
> +	struct resource *res;
> +	void __iomem *base;
> +	u32 opmode;
> +	int err;
> +
> +	child = of_get_next_available_child(np, NULL);
> +	if (!child)
> +		return -ENODEV;
> +
> +	/*
> +	 * The Operating Mode is stored into the first u32 of the reg property
> +	 * of the child.
> +	 */
> +	err = of_property_read_u32_index(child, "reg", 0, &opmode);
> +	of_node_put(child);
> +	if (err)
> +		return -EINVAL;
> +
> +	if ((opmode == FLEX_MR_OPMODE_NO_COM) ||
> +	    (opmode & ~FLEX_MR_OPMODE_MASK))
> +		return -EINVAL;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	err = clk_prepare_enable(clk);
> +	if (err)
> +		return err;
> +
> +	/*
> +	 * Set the Operating Mode in the Mode Register: only the selected device
> +	 * is clocked. Hence, registers of the other serial devices remain
> +	 * inaccessible and are read as zero. Also the external I/O lines of the
> +	 * Flexcom are muxed to reach the selected device.
> +	 */
> +	writel(opmode, base + FLEX_MR);
> +
> +	clk_disable_unprepare(clk);
> +
> +	return of_platform_populate(np, 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 = {
> +	.probe	= atmel_flexcom_probe,
> +	.driver	= {
> +		.name		= "atmel_flexcom",
> +		.of_match_table	= atmel_flexcom_of_match,
> +	},
> +};
> +
> +module_platform_driver(atmel_flexcom_driver);
> +
> +MODULE_AUTHOR("Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>");
> +MODULE_DESCRIPTION("Atmel Flexcom MFD driver");
> +MODULE_LICENSE("GPL v2");
> 


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v7 2/2] mfd: atmel-flexcom: add a driver for Atmel Flexible Serial Communication Unit
       [not found]     ` <55921f08b2d5d14256141edbe12287fba55860c4.1437669004.git.cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
  2015-07-24  8:42       ` Nicolas Ferre
@ 2015-07-24  8:43       ` Lee Jones
  2015-07-24  8:47         ` Nicolas Ferre
  1 sibling, 1 reply; 10+ messages in thread
From: Lee Jones @ 2015-07-24  8:43 UTC (permalink / raw)
  To: Cyrille Pitchen
  Cc: nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w,
	boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	sameo-VuQAYsv1563Yd54FQh9/CA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, devicetree-u79uwXL29TY76Z2rM5mHXA

On Thu, 23 Jul 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, using the "reg" property of the first
> (should be unique) available DT child node.
> 
> 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-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/mfd/Kconfig         |  11 +++++
>  drivers/mfd/Makefile        |   1 +
>  drivers/mfd/atmel-flexcom.c | 113 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 125 insertions(+)
>  create mode 100644 drivers/mfd/atmel-flexcom.c
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 653815950aa2..2c75472c679c 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -60,6 +60,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.

Please grep for QCOM_GSBI in drivers/soc/qcom/*

Similar right?

Perhaps this should live in drivers/soc/atmel/* instead?

>  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 ea40e076cb61..0705eb2d873d 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 000000000000..0d06b70696b0
> --- /dev/null
> +++ b/drivers/mfd/atmel-flexcom.c
> @@ -0,0 +1,113 @@
> +/*
> + * Driver for Atmel Flexcom
> + *
> + * Copyright (C) 2015 Atmel Corporation
> + *
> + * Author: Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#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>
> +
> +/* I/O register offsets */
> +#define FLEX_MR		0x0	/* Mode Register */
> +#define FLEX_VERSION	0xfc	/* Version Register */
> +
> +/* Mode Register bit fields */
> +#define FLEX_MR_OPMODE_MASK	0x3
> +#define FLEX_MR_OPMODE_NO_COM	0x0
> +#define FLEX_MR_OPMODE_USART	0x1
> +#define FLEX_MR_OPMODE_SPI	0x2
> +#define FLEX_MR_OPMODE_TWI	0x3
> +
> +
> +static int atmel_flexcom_probe(struct platform_device *pdev)
> +{
> +	struct device_node *child, *np = pdev->dev.of_node;
> +	struct clk *clk;
> +	struct resource *res;
> +	void __iomem *base;
> +	u32 opmode;
> +	int err;
> +
> +	child = of_get_next_available_child(np, NULL);
> +	if (!child)
> +		return -ENODEV;
> +
> +	/*
> +	 * The Operating Mode is stored into the first u32 of the reg property
> +	 * of the child.
> +	 */
> +	err = of_property_read_u32_index(child, "reg", 0, &opmode);
> +	of_node_put(child);

Don't think you need to of_node_put() after a u32 read.

> +	if (err)
> +		return -EINVAL;

Why are you making up your own return value.

Just return err.

> +	if ((opmode == FLEX_MR_OPMODE_NO_COM) ||
> +	    (opmode & ~FLEX_MR_OPMODE_MASK))
> +		return -EINVAL;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	err = clk_prepare_enable(clk);
> +	if (err)
> +		return err;
> +
> +	/*
> +	 * Set the Operating Mode in the Mode Register: only the selected device
> +	 * is clocked. Hence, registers of the other serial devices remain
> +	 * inaccessible and are read as zero. Also the external I/O lines of the
> +	 * Flexcom are muxed to reach the selected device.
> +	 */
> +	writel(opmode, base + FLEX_MR);
> +
> +	clk_disable_unprepare(clk);
> +
> +	return of_platform_populate(np, 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 = {
> +	.probe	= atmel_flexcom_probe,
> +	.driver	= {
> +		.name		= "atmel_flexcom",
> +		.of_match_table	= atmel_flexcom_of_match,
> +	},
> +};
> +
> +module_platform_driver(atmel_flexcom_driver);
> +
> +MODULE_AUTHOR("Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>");
> +MODULE_DESCRIPTION("Atmel Flexcom MFD driver");
> +MODULE_LICENSE("GPL v2");

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v7 2/2] mfd: atmel-flexcom: add a driver for Atmel Flexible Serial Communication Unit
  2015-07-24  8:43       ` Lee Jones
@ 2015-07-24  8:47         ` Nicolas Ferre
  0 siblings, 0 replies; 10+ messages in thread
From: Nicolas Ferre @ 2015-07-24  8:47 UTC (permalink / raw)
  To: Lee Jones, Cyrille Pitchen
  Cc: boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	sameo-VuQAYsv1563Yd54FQh9/CA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, devicetree-u79uwXL29TY76Z2rM5mHXA

Le 24/07/2015 10:43, Lee Jones a écrit :
> On Thu, 23 Jul 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, using the "reg" property of the first
>> (should be unique) available DT child node.
>>
>> 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-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>> ---
>>  drivers/mfd/Kconfig         |  11 +++++
>>  drivers/mfd/Makefile        |   1 +
>>  drivers/mfd/atmel-flexcom.c | 113 ++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 125 insertions(+)
>>  create mode 100644 drivers/mfd/atmel-flexcom.c
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index 653815950aa2..2c75472c679c 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -60,6 +60,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.
> 
> Please grep for QCOM_GSBI in drivers/soc/qcom/*
> 
> Similar right?
> 
> Perhaps this should live in drivers/soc/atmel/* instead?
> 
>>  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 ea40e076cb61..0705eb2d873d 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 000000000000..0d06b70696b0
>> --- /dev/null
>> +++ b/drivers/mfd/atmel-flexcom.c
>> @@ -0,0 +1,113 @@
>> +/*
>> + * Driver for Atmel Flexcom
>> + *
>> + * Copyright (C) 2015 Atmel Corporation
>> + *
>> + * Author: Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#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>
>> +
>> +/* I/O register offsets */
>> +#define FLEX_MR		0x0	/* Mode Register */
>> +#define FLEX_VERSION	0xfc	/* Version Register */
>> +
>> +/* Mode Register bit fields */
>> +#define FLEX_MR_OPMODE_MASK	0x3
>> +#define FLEX_MR_OPMODE_NO_COM	0x0
>> +#define FLEX_MR_OPMODE_USART	0x1
>> +#define FLEX_MR_OPMODE_SPI	0x2
>> +#define FLEX_MR_OPMODE_TWI	0x3
>> +
>> +
>> +static int atmel_flexcom_probe(struct platform_device *pdev)
>> +{
>> +	struct device_node *child, *np = pdev->dev.of_node;
>> +	struct clk *clk;
>> +	struct resource *res;
>> +	void __iomem *base;
>> +	u32 opmode;
>> +	int err;
>> +
>> +	child = of_get_next_available_child(np, NULL);
>> +	if (!child)
>> +		return -ENODEV;
>> +
>> +	/*
>> +	 * The Operating Mode is stored into the first u32 of the reg property
>> +	 * of the child.
>> +	 */
>> +	err = of_property_read_u32_index(child, "reg", 0, &opmode);
>> +	of_node_put(child);
> 
> Don't think you need to of_node_put() after a u32 read.

Well, it's after an of_get_next_available_child() which is similar to
of_get_next_child(). It is said to release the node by calling
of_node_put()... So I suspect it's necessary.

Bye,


> 
>> +	if (err)
>> +		return -EINVAL;
> 
> Why are you making up your own return value.
> 
> Just return err.
> 
>> +	if ((opmode == FLEX_MR_OPMODE_NO_COM) ||
>> +	    (opmode & ~FLEX_MR_OPMODE_MASK))
>> +		return -EINVAL;
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	base = devm_ioremap_resource(&pdev->dev, res);
>> +	if (IS_ERR(base))
>> +		return PTR_ERR(base);
>> +
>> +	clk = devm_clk_get(&pdev->dev, NULL);
>> +	if (IS_ERR(clk))
>> +		return PTR_ERR(clk);
>> +
>> +	err = clk_prepare_enable(clk);
>> +	if (err)
>> +		return err;
>> +
>> +	/*
>> +	 * Set the Operating Mode in the Mode Register: only the selected device
>> +	 * is clocked. Hence, registers of the other serial devices remain
>> +	 * inaccessible and are read as zero. Also the external I/O lines of the
>> +	 * Flexcom are muxed to reach the selected device.
>> +	 */
>> +	writel(opmode, base + FLEX_MR);
>> +
>> +	clk_disable_unprepare(clk);
>> +
>> +	return of_platform_populate(np, 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 = {
>> +	.probe	= atmel_flexcom_probe,
>> +	.driver	= {
>> +		.name		= "atmel_flexcom",
>> +		.of_match_table	= atmel_flexcom_of_match,
>> +	},
>> +};
>> +
>> +module_platform_driver(atmel_flexcom_driver);
>> +
>> +MODULE_AUTHOR("Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>");
>> +MODULE_DESCRIPTION("Atmel Flexcom MFD driver");
>> +MODULE_LICENSE("GPL v2");
> 


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v7 1/2] mfd: devicetree: add bindings for Atmel Flexcom
       [not found]         ` <55B1F9B7.2080206-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
@ 2015-07-24 14:56           ` Lee Jones
  2015-07-24 15:07             ` Lee Jones
  0 siblings, 1 reply; 10+ messages in thread
From: Lee Jones @ 2015-07-24 14:56 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: Boris Brezillon, Cyrille Pitchen,
	alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	sameo-VuQAYsv1563Yd54FQh9/CA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, devicetree-u79uwXL29TY76Z2rM5mHXA

On Fri, 24 Jul 2015, Nicolas Ferre wrote:

> Le 23/07/2015 21:48, Boris Brezillon a écrit :
> > On Thu, 23 Jul 2015 18:42:55 +0200
> > Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org> 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-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> >> ---
> >>  .../devicetree/bindings/mfd/atmel-flexcom.txt      | 68 ++++++++++++++++++++++
> >>  1 file changed, 68 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 000000000000..a63226b7a9cb
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
> >> @@ -0,0 +1,68 @@
> >> +* 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:		Should be "atmel,sama5d2-flexcom"
> >> +- reg:			Should be the pair (offset, size) for the Flexcom
> >> +			dedicated I/O registers (without USART, TWI or SPI
> >> +			registers).
> >> +- clocks:		Should be the Flexcom peripheral clock from PMC.
> >> +- #address-cells:	Should be <2>
> >> +- #size-cells:		Should be <1>
> >> +- ranges:		Should be a list of ranges.
> >> +			One range per peripheral wrapped by the Flexcom. So each
> >> +			range is a triplet (child_addr, parent_addr, size). The
> >> +			first u32 of "child_addr" is the value to be set in the
> >> +			Operating Mode bitfield of the Flexcom Mode Register.
> >> +			Then "parent_addr" stores the base address of the
> >> +			corresponding peripheral in the system memory. Finally,
> >> +			"size" if the size of the memory region of this
> >> +			peripheral.
> >> +
> >> +Required child:
> >> +A single available child for the serial controller to enable.
> >> +
> >> +Required properties of this child:
> >> +- reg:			Should be a pair (child_addr, size) with child_addr
> >> +			matching one of the parent ranges.
> >> +- clocks:		Should be the very same phandle as for the parent's one.
> >> +
> >> +Other properties remain unchanged. See documentation of the respective device:
> >> +- ../serial/atmel-usart.txt
> >> +- ../spi/spi_atmel.txt
> >> +- ../i2c/i2c-at91.txt
> >> +
> >> +Example:
> >> +
> >> +flexcom@f8034000 {
> >> +	compatible = "atmel,sama5d2-flexcom";
> >> +	reg = <0xf8034000 0x200>;
> >> +	clocks = <&flx0_clk>;
> >> +	#address-cells = <2>;
> >> +	#size-cells = <1>;
> >> +	ranges = <1 0 0xf8034200 0x200		/* opmode 1: USART */
> >> +		  2 0 0xf8034400 0x200		/* opmode 2: SPI */
> >> +		  3 0 0xf8034600 0x200>;	/* opmode 3: I2C */
> 
> Yes, the opmode(s) are exactly the values described in the product
> datasheet.
> 
> >> +
> >> +	spi@f8034400 {
> > 
> > Should be:
> > 
> > 	spi@2,0 {
> 
> Yep!
> 
> I was silently following the discussion in the background and I admit
> that this approach is pretty elegant.
> 
> So, if this approach is validated you can also add my:
> Acked-by: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>

Please see my reply to v6 for the usual way to achieve this.

> >> +		compatible = "atmel,at91rm9200-spi";
> >> +		reg = <2 0 0x200>;
> >> +		interrupts = <19 IRQ_TYPE_LEVEL_HIGH 7>;
> >> +		pinctrl-names = "default";
> >> +		pinctrl-0 = <&pinctrl_flx0_default>;
> >> +		#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>;
> >> +		};
> >> +	};
> >> +};
> > 
> > 
> > 
> 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v7 1/2] mfd: devicetree: add bindings for Atmel Flexcom
  2015-07-24 14:56           ` Lee Jones
@ 2015-07-24 15:07             ` Lee Jones
  0 siblings, 0 replies; 10+ messages in thread
From: Lee Jones @ 2015-07-24 15:07 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: Boris Brezillon, Cyrille Pitchen, alexandre.belloni, sameo,
	linux-arm-kernel, linux-kernel, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, devicetree

On Fri, 24 Jul 2015, Lee Jones wrote:

> On Fri, 24 Jul 2015, Nicolas Ferre wrote:
> 
> > Le 23/07/2015 21:48, Boris Brezillon a écrit :
> > > On Thu, 23 Jul 2015 18:42:55 +0200
> > > Cyrille Pitchen <cyrille.pitchen@atmel.com> 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>
> > >> ---
> > >>  .../devicetree/bindings/mfd/atmel-flexcom.txt      | 68 ++++++++++++++++++++++
> > >>  1 file changed, 68 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 000000000000..a63226b7a9cb
> > >> --- /dev/null
> > >> +++ b/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
> > >> @@ -0,0 +1,68 @@
> > >> +* 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:		Should be "atmel,sama5d2-flexcom"
> > >> +- reg:			Should be the pair (offset, size) for the Flexcom
> > >> +			dedicated I/O registers (without USART, TWI or SPI
> > >> +			registers).
> > >> +- clocks:		Should be the Flexcom peripheral clock from PMC.
> > >> +- #address-cells:	Should be <2>
> > >> +- #size-cells:		Should be <1>
> > >> +- ranges:		Should be a list of ranges.
> > >> +			One range per peripheral wrapped by the Flexcom. So each
> > >> +			range is a triplet (child_addr, parent_addr, size). The
> > >> +			first u32 of "child_addr" is the value to be set in the
> > >> +			Operating Mode bitfield of the Flexcom Mode Register.
> > >> +			Then "parent_addr" stores the base address of the
> > >> +			corresponding peripheral in the system memory. Finally,
> > >> +			"size" if the size of the memory region of this
> > >> +			peripheral.
> > >> +
> > >> +Required child:
> > >> +A single available child for the serial controller to enable.
> > >> +
> > >> +Required properties of this child:
> > >> +- reg:			Should be a pair (child_addr, size) with child_addr
> > >> +			matching one of the parent ranges.
> > >> +- clocks:		Should be the very same phandle as for the parent's one.
> > >> +
> > >> +Other properties remain unchanged. See documentation of the respective device:
> > >> +- ../serial/atmel-usart.txt
> > >> +- ../spi/spi_atmel.txt
> > >> +- ../i2c/i2c-at91.txt
> > >> +
> > >> +Example:
> > >> +
> > >> +flexcom@f8034000 {
> > >> +	compatible = "atmel,sama5d2-flexcom";
> > >> +	reg = <0xf8034000 0x200>;
> > >> +	clocks = <&flx0_clk>;
> > >> +	#address-cells = <2>;
> > >> +	#size-cells = <1>;
> > >> +	ranges = <1 0 0xf8034200 0x200		/* opmode 1: USART */
> > >> +		  2 0 0xf8034400 0x200		/* opmode 2: SPI */
> > >> +		  3 0 0xf8034600 0x200>;	/* opmode 3: I2C */
> > 
> > Yes, the opmode(s) are exactly the values described in the product
> > datasheet.
> > 
> > >> +
> > >> +	spi@f8034400 {
> > > 
> > > Should be:
> > > 
> > > 	spi@2,0 {
> > 
> > Yep!
> > 
> > I was silently following the discussion in the background and I admit
> > that this approach is pretty elegant.
> > 
> > So, if this approach is validated you can also add my:
> > Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> 
> Please see my reply to v6 for the usual way to achieve this.

Ah, actually scrap that.  Looks like I was thinking you were doing
something different.  Well I don't know enough about the advanced use
of the ranges property to comment on this.  I need to call for some
assistance from the DT chaps.

> > >> +		compatible = "atmel,at91rm9200-spi";
> > >> +		reg = <2 0 0x200>;
> > >> +		interrupts = <19 IRQ_TYPE_LEVEL_HIGH 7>;
> > >> +		pinctrl-names = "default";
> > >> +		pinctrl-0 = <&pinctrl_flx0_default>;
> > >> +		#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>;
> > >> +		};
> > >> +	};
> > >> +};
> > > 
> > > 
> > > 
> > 
> > 
> 

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-07-24 15:07 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-23 16:42 [PATCH v7 0/2] mfd: flexcom: add a driver for Flexcom Cyrille Pitchen
     [not found] ` <cover.1437669004.git.cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
2015-07-23 16:42   ` [PATCH v7 1/2] mfd: devicetree: add bindings for Atmel Flexcom Cyrille Pitchen
2015-07-23 19:48     ` Boris Brezillon
2015-07-24  8:39       ` Nicolas Ferre
     [not found]         ` <55B1F9B7.2080206-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
2015-07-24 14:56           ` Lee Jones
2015-07-24 15:07             ` Lee Jones
2015-07-23 16:42   ` [PATCH v7 2/2] mfd: atmel-flexcom: add a driver for Atmel Flexible Serial Communication Unit Cyrille Pitchen
     [not found]     ` <55921f08b2d5d14256141edbe12287fba55860c4.1437669004.git.cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
2015-07-24  8:42       ` Nicolas Ferre
2015-07-24  8:43       ` Lee Jones
2015-07-24  8:47         ` Nicolas Ferre

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).