linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] Add STiH407 SoC and reference board support
@ 2014-03-07 14:40 Maxime COQUELIN
  2014-03-07 14:40 ` [PATCH v3 1/5] ARM: STi: Add STiH407 SoC support Maxime COQUELIN
                   ` (4 more replies)
  0 siblings, 5 replies; 21+ messages in thread
From: Maxime COQUELIN @ 2014-03-07 14:40 UTC (permalink / raw)
  To: Rob Landley, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Russell King, Srinivas Kandagatla, Stuart Menefy,
	Linus Walleij, Giuseppe Cavallaro, Maxime Coquelin, linux-doc,
	linux-kernel, devicetree, linux-arm-kernel, kernel
  Cc: lee.jones

This series adds basic support to the STMicroelectronics STiH407 SoC and its
B2120 reference board. The STiH407 is a dual-core ARM Cortex-A9 CPU aimed at
STB market.

Changes since v2:
-----------------
    - Reordered the pinctrl patches
    - Moved stih407_flashdata to stih407 pinctrl patch

Changes since v1:
-----------------
	- Changed patch 2 commit title
	- Rebased pinctrl patches to linux-pinctrl/devel
	- Rebased ARM patches to arm_soc/for-next

Giuseppe Cavallaro (2):
  pinctrl: st: Enhance the controller to manage unavailable registers
  pinctrl: st: add pinctrl support for the STiH407 SoC

Maxime Coquelin (3):
  ARM: STi: Add STiH407 SoC support
  ARM: dts: Add STiH407 SoC support
  ARM: dts: STiH407: Add B2120 board support

 Documentation/arm/sti/stih407-overview.txt    |  18 +
 Documentation/devicetree/bindings/arm/sti.txt |  15 +
 arch/arm/boot/dts/Makefile                    |   3 +-
 arch/arm/boot/dts/stih407-b2120.dts           |  78 ++++
 arch/arm/boot/dts/stih407-clock.dtsi          |  41 ++
 arch/arm/boot/dts/stih407-pinctrl.dtsi        | 618 ++++++++++++++++++++++++++
 arch/arm/boot/dts/stih407.dtsi                | 250 +++++++++++
 arch/arm/mach-sti/Kconfig                     |   9 +
 arch/arm/mach-sti/board-dt.c                  |   1 +
 drivers/pinctrl/pinctrl-st.c                  | 123 +++--
 10 files changed, 1110 insertions(+), 46 deletions(-)
 create mode 100644 Documentation/arm/sti/stih407-overview.txt
 create mode 100644 Documentation/devicetree/bindings/arm/sti.txt
 create mode 100644 arch/arm/boot/dts/stih407-b2120.dts
 create mode 100644 arch/arm/boot/dts/stih407-clock.dtsi
 create mode 100644 arch/arm/boot/dts/stih407-pinctrl.dtsi
 create mode 100644 arch/arm/boot/dts/stih407.dtsi

-- 
1.9.0


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

* [PATCH v3 1/5] ARM: STi: Add STiH407 SoC support
  2014-03-07 14:40 [PATCH v3 0/5] Add STiH407 SoC and reference board support Maxime COQUELIN
@ 2014-03-07 14:40 ` Maxime COQUELIN
  2014-03-10 12:34   ` Lee Jones
  2014-03-07 14:40 ` [PATCH v3 2/5] pinctrl: st: Enhance the controller to manage unavailable registers Maxime COQUELIN
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 21+ messages in thread
From: Maxime COQUELIN @ 2014-03-07 14:40 UTC (permalink / raw)
  To: Rob Landley, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Russell King, Srinivas Kandagatla, Stuart Menefy,
	Linus Walleij, Giuseppe Cavallaro, Maxime Coquelin, linux-doc,
	linux-kernel, devicetree, linux-arm-kernel, kernel
  Cc: lee.jones

This patch adds support to STiH407 SoC.

Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 Documentation/arm/sti/stih407-overview.txt    | 18 ++++++++++++++++++
 Documentation/devicetree/bindings/arm/sti.txt | 15 +++++++++++++++
 arch/arm/mach-sti/Kconfig                     |  9 +++++++++
 arch/arm/mach-sti/board-dt.c                  |  1 +
 4 files changed, 43 insertions(+)
 create mode 100644 Documentation/arm/sti/stih407-overview.txt
 create mode 100644 Documentation/devicetree/bindings/arm/sti.txt

diff --git a/Documentation/arm/sti/stih407-overview.txt b/Documentation/arm/sti/stih407-overview.txt
new file mode 100644
index 0000000..3343f32
--- /dev/null
+++ b/Documentation/arm/sti/stih407-overview.txt
@@ -0,0 +1,18 @@
+			STiH407 Overview
+			================
+
+Introduction
+------------
+
+    The STiH407 is the new generation of SoC for Multi-HD, AVC set-top boxes
+    and server/connected client application for satellite, cable, terrestrial
+    and IP-STB markets.
+
+    Features
+    - ARM Cortex-A9 1.5 GHz dual core CPU (28nm)
+    - SATA2, USB 3.0, PCIe, Gbit Ethernet
+
+  Document Author
+  ---------------
+
+  Maxime Coquelin <maxime.coquelin@st.com>, (c) 2014 ST Microelectronics
diff --git a/Documentation/devicetree/bindings/arm/sti.txt b/Documentation/devicetree/bindings/arm/sti.txt
new file mode 100644
index 0000000..92f16c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/sti.txt
@@ -0,0 +1,15 @@
+ST STi Platforms Device Tree Bindings
+---------------------------------------
+
+Boards with the ST STiH415 SoC shall have the following properties:
+Required root node property:
+compatible = "st,stih415";
+
+Boards with the ST STiH416 SoC shall have the following properties:
+Required root node property:
+compatible = "st,stih416";
+
+Boards with the ST STiH407 SoC shall have the following properties:
+Required root node property:
+compatible = "st,stih407";
+
diff --git a/arch/arm/mach-sti/Kconfig b/arch/arm/mach-sti/Kconfig
index d2c13ba..c0a3f53 100644
--- a/arch/arm/mach-sti/Kconfig
+++ b/arch/arm/mach-sti/Kconfig
@@ -39,4 +39,13 @@ config SOC_STIH416
 	  and other digital audio/video applications using Flattened Device
 	  Trees.
 
+config SOC_STIH407
+	bool "STiH407 STMicroelectronics Consumer Electronics family"
+	default y
+	help
+	  This enables support for STMicroelectronics Digital Consumer
+	  Electronics family StiH407 parts, primarily targeted at set-top-box
+	  and other digital audio/video applications using Flattened Device
+	  Trees.
+
 endif
diff --git a/arch/arm/mach-sti/board-dt.c b/arch/arm/mach-sti/board-dt.c
index 1217fb5..df731f2 100644
--- a/arch/arm/mach-sti/board-dt.c
+++ b/arch/arm/mach-sti/board-dt.c
@@ -36,6 +36,7 @@ static void __init stih41x_machine_init(void)
 static const char *stih41x_dt_match[] __initdata = {
 	"st,stih415",
 	"st,stih416",
+	"st,stih407",
 	NULL
 };
 
-- 
1.9.0


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

* [PATCH v3 2/5] pinctrl: st: Enhance the controller to manage unavailable registers
  2014-03-07 14:40 [PATCH v3 0/5] Add STiH407 SoC and reference board support Maxime COQUELIN
  2014-03-07 14:40 ` [PATCH v3 1/5] ARM: STi: Add STiH407 SoC support Maxime COQUELIN
@ 2014-03-07 14:40 ` Maxime COQUELIN
  2014-03-07 15:37   ` srinivas kandagatla
  2014-03-10  9:17   ` Lee Jones
  2014-03-07 14:40 ` [PATCH v3 3/5] pinctrl: st: add pinctrl support for the STiH407 SoC Maxime COQUELIN
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 21+ messages in thread
From: Maxime COQUELIN @ 2014-03-07 14:40 UTC (permalink / raw)
  To: Rob Landley, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Russell King, Srinivas Kandagatla, Stuart Menefy,
	Linus Walleij, Giuseppe Cavallaro, Maxime Coquelin, linux-doc,
	linux-kernel, devicetree, linux-arm-kernel, kernel
  Cc: lee.jones

From: Giuseppe Cavallaro <peppe.cavallaro@st.com>

This patch adds a new logic inside the st pinctrl to manage
an unsupported scenario: some sysconfig are not available!

This is the case of STiH407 where, although documented, the
following registers from SYSCFG_FLASH have been removed from the SoC.

SYSTEM_CONFIG3040
   Output Enable pad control for all PIO Alternate Functions
and
SYSTEM_ CONFIG3050
   Pull Up pad control for all PIO Alternate Functions

Without managing this condition an imprecise external abort
will be detect.

To do this the patch also reviews the st_parse_syscfgs
and other routines to manipulate the registers only if
actually available.
In any case, for example the st_parse_syscfgs detected
an error condition but no action was made in the
st_pctl_probe_dt.

Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/pinctrl/pinctrl-st.c | 106 +++++++++++++++++++++++++------------------
 1 file changed, 61 insertions(+), 45 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
index 9fb66aa..1721611 100644
--- a/drivers/pinctrl/pinctrl-st.c
+++ b/drivers/pinctrl/pinctrl-st.c
@@ -410,25 +410,27 @@ static void st_pinconf_set_config(struct st_pio_control *pc,
 	unsigned int oe_value, pu_value, od_value;
 	unsigned long mask = BIT(pin);
 
-	regmap_field_read(output_enable, &oe_value);
-	regmap_field_read(pull_up, &pu_value);
-	regmap_field_read(open_drain, &od_value);
-
-	/* Clear old values */
-	oe_value &= ~mask;
-	pu_value &= ~mask;
-	od_value &= ~mask;
-
-	if (config & ST_PINCONF_OE)
-		oe_value |= mask;
-	if (config & ST_PINCONF_PU)
-		pu_value |= mask;
-	if (config & ST_PINCONF_OD)
-		od_value |= mask;
-
-	regmap_field_write(output_enable, oe_value);
-	regmap_field_write(pull_up, pu_value);
-	regmap_field_write(open_drain, od_value);
+	if (output_enable) {
+		regmap_field_read(output_enable, &oe_value);
+		oe_value &= ~mask;
+		if (config & ST_PINCONF_OE)
+			oe_value |= mask;
+		regmap_field_write(output_enable, oe_value);
+	}
+	if (pull_up) {
+		regmap_field_read(pull_up, &pu_value);
+		pu_value &= ~mask;
+		if (config & ST_PINCONF_PU)
+			pu_value |= mask;
+		regmap_field_write(pull_up, pu_value);
+	}
+	if (open_drain) {
+		regmap_field_read(open_drain, &od_value);
+		od_value &= ~mask;
+		if (config & ST_PINCONF_OD)
+			od_value |= mask;
+		regmap_field_write(open_drain, od_value);
+	}
 }
 
 static void st_pctl_set_function(struct st_pio_control *pc,
@@ -439,6 +441,9 @@ static void st_pctl_set_function(struct st_pio_control *pc,
 	int pin = st_gpio_pin(pin_id);
 	int offset = pin * 4;
 
+	if (!alt)
+		return;
+
 	regmap_field_read(alt, &val);
 	val &= ~(0xf << offset);
 	val |= function << offset;
@@ -571,22 +576,28 @@ static void st_pinconf_set_retime_dedicated(struct st_pinctrl *info,
 	regmap_field_write(rt_d->rt[pin], retime_config);
 }
 
-static void st_pinconf_get_direction(struct st_pio_control *pc,
-	int pin, unsigned long *config)
+static void st_pinconf_get_direction(struct st_pio_control *pc, int pin,
+				     unsigned long *config)
 {
 	unsigned int oe_value, pu_value, od_value;
 
-	regmap_field_read(pc->oe, &oe_value);
-	regmap_field_read(pc->pu, &pu_value);
-	regmap_field_read(pc->od, &od_value);
+	if (pc->oe) {
+		regmap_field_read(pc->oe, &oe_value);
+		if (oe_value & BIT(pin))
+			ST_PINCONF_PACK_OE(*config);
+	}
 
-	if (oe_value & BIT(pin))
-		ST_PINCONF_PACK_OE(*config);
-	if (pu_value & BIT(pin))
-		ST_PINCONF_PACK_PU(*config);
-	if (od_value & BIT(pin))
-		ST_PINCONF_PACK_OD(*config);
+	if (pc->pu) {
+		regmap_field_read(pc->pu, &pu_value);
+		if (pu_value & BIT(pin))
+			ST_PINCONF_PACK_PU(*config);
+	}
 
+	if (pc->od) {
+		regmap_field_read(pc->od, &od_value);
+		if (od_value & BIT(pin))
+			ST_PINCONF_PACK_OD(*config);
+	}
 }
 
 static int st_pinconf_get_retime_packed(struct st_pinctrl *info,
@@ -1105,8 +1116,21 @@ static int st_pctl_dt_setup_retime(struct st_pinctrl *info,
 	return -EINVAL;
 }
 
-static int st_parse_syscfgs(struct st_pinctrl *info,
-		int bank, struct device_node *np)
+
+static struct regmap_field *st_pc_get_value(struct device *dev,
+					    struct regmap *regmap, int bank,
+					    int data, int lsb, int msb)
+{
+	struct reg_field reg = REG_FIELD((data + bank) * 4, lsb, msb);
+
+	if (data < 0)
+		return NULL;
+
+	return devm_regmap_field_alloc(dev, regmap, reg);
+}
+
+static void st_parse_syscfgs(struct st_pinctrl *info, int bank,
+			     struct device_node *np)
 {
 	const struct st_pctl_data *data = info->data;
 	/**
@@ -1116,29 +1140,21 @@ static int st_parse_syscfgs(struct st_pinctrl *info,
 	 */
 	int lsb = (bank%4) * ST_GPIO_PINS_PER_BANK;
 	int msb = lsb + ST_GPIO_PINS_PER_BANK - 1;
-	struct reg_field alt_reg = REG_FIELD((data->alt + bank) * 4, 0, 31);
-	struct reg_field oe_reg = REG_FIELD((data->oe + bank/4) * 4, lsb, msb);
-	struct reg_field pu_reg = REG_FIELD((data->pu + bank/4) * 4, lsb, msb);
-	struct reg_field od_reg = REG_FIELD((data->od + bank/4) * 4, lsb, msb);
 	struct st_pio_control *pc = &info->banks[bank].pc;
 	struct device *dev = info->dev;
 	struct regmap *regmap  = info->regmap;
 
-	pc->alt = devm_regmap_field_alloc(dev, regmap, alt_reg);
-	pc->oe = devm_regmap_field_alloc(dev, regmap, oe_reg);
-	pc->pu = devm_regmap_field_alloc(dev, regmap, pu_reg);
-	pc->od = devm_regmap_field_alloc(dev, regmap, od_reg);
-
-	if (IS_ERR(pc->alt) || IS_ERR(pc->oe) ||
-			IS_ERR(pc->pu) || IS_ERR(pc->od))
-		return -EINVAL;
+	pc->alt = st_pc_get_value(dev, regmap, bank, data->alt, 0, 31);
+	pc->oe = st_pc_get_value(dev, regmap, bank/4, data->oe, lsb, msb);
+	pc->pu = st_pc_get_value(dev, regmap, bank/4, data->pu, lsb, msb);
+	pc->od = st_pc_get_value(dev, regmap, bank/4, data->od, lsb, msb);
 
 	/* retime avaiable for all pins by default */
 	pc->rt_pin_mask = 0xff;
 	of_property_read_u32(np, "st,retime-pin-mask", &pc->rt_pin_mask);
 	st_pctl_dt_setup_retime(info, bank, pc);
 
-	return 0;
+	return;
 }
 
 /*
-- 
1.9.0


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

* [PATCH v3 3/5] pinctrl: st: add pinctrl support for the STiH407 SoC
  2014-03-07 14:40 [PATCH v3 0/5] Add STiH407 SoC and reference board support Maxime COQUELIN
  2014-03-07 14:40 ` [PATCH v3 1/5] ARM: STi: Add STiH407 SoC support Maxime COQUELIN
  2014-03-07 14:40 ` [PATCH v3 2/5] pinctrl: st: Enhance the controller to manage unavailable registers Maxime COQUELIN
@ 2014-03-07 14:40 ` Maxime COQUELIN
  2014-03-07 15:36   ` srinivas kandagatla
  2014-03-10 14:44   ` Lee Jones
  2014-03-07 14:40 ` [PATCH v3 4/5] ARM: dts: Add STiH407 SoC support Maxime COQUELIN
  2014-03-07 14:40 ` [PATCH v3 5/5] ARM: dts: STiH407: Add B2120 board support Maxime COQUELIN
  4 siblings, 2 replies; 21+ messages in thread
From: Maxime COQUELIN @ 2014-03-07 14:40 UTC (permalink / raw)
  To: Rob Landley, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Russell King, Srinivas Kandagatla, Stuart Menefy,
	Linus Walleij, Giuseppe Cavallaro, Maxime Coquelin, linux-doc,
	linux-kernel, devicetree, linux-arm-kernel, kernel
  Cc: lee.jones

From: Giuseppe Cavallaro <peppe.cavallaro@st.com>

This patch adds the initial support for pinctrl based on H407 SoC.

Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/pinctrl/pinctrl-st.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
index 1721611..d1886b4 100644
--- a/drivers/pinctrl/pinctrl-st.c
+++ b/drivers/pinctrl/pinctrl-st.c
@@ -390,6 +390,19 @@ static const struct st_pctl_data  stih416_data = {
 	.alt = 0, .oe = 40, .pu = 50, .od = 60, .rt = 100,
 };
 
+static const struct st_pctl_data  stih407_flashdata = {
+	.rt_style	= st_retime_style_none,
+	.input_delays	= stih416_delays,
+	.ninput_delays	= 14,
+	.output_delays	= stih416_delays,
+	.noutput_delays = 14,
+	.alt = 0,
+	.oe = -1, /* Not Available */
+	.pu = -1, /* Not Available */
+	.od = 60,
+	.rt = 100,
+};
+
 /* Low level functions.. */
 static inline int st_gpio_bank(int gpio)
 {
@@ -1596,6 +1609,10 @@ static struct of_device_id st_pctl_of_match[] = {
 	{ .compatible = "st,stih416-rear-pinctrl", .data = &stih416_data},
 	{ .compatible = "st,stih416-fvdp-fe-pinctrl", .data = &stih416_data},
 	{ .compatible = "st,stih416-fvdp-lite-pinctrl", .data = &stih416_data},
+	{ .compatible = "st,stih407-sbc-pinctrl", .data = &stih416_data},
+	{ .compatible = "st,stih407-front-pinctrl", .data = &stih416_data},
+	{ .compatible = "st,stih407-rear-pinctrl", .data = &stih416_data},
+	{ .compatible = "st,stih407-flash-pinctrl", .data = &stih407_flashdata},
 	{ /* sentinel */ }
 };
 
-- 
1.9.0


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

* [PATCH v3 4/5] ARM: dts: Add STiH407 SoC support
  2014-03-07 14:40 [PATCH v3 0/5] Add STiH407 SoC and reference board support Maxime COQUELIN
                   ` (2 preceding siblings ...)
  2014-03-07 14:40 ` [PATCH v3 3/5] pinctrl: st: add pinctrl support for the STiH407 SoC Maxime COQUELIN
@ 2014-03-07 14:40 ` Maxime COQUELIN
  2014-03-10 12:28   ` Lee Jones
  2014-03-07 14:40 ` [PATCH v3 5/5] ARM: dts: STiH407: Add B2120 board support Maxime COQUELIN
  4 siblings, 1 reply; 21+ messages in thread
From: Maxime COQUELIN @ 2014-03-07 14:40 UTC (permalink / raw)
  To: Rob Landley, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Russell King, Srinivas Kandagatla, Stuart Menefy,
	Linus Walleij, Giuseppe Cavallaro, Maxime Coquelin, linux-doc,
	linux-kernel, devicetree, linux-arm-kernel, kernel
  Cc: lee.jones

The STiH407 is advanced multi-HD AVC processor with 3D graphics acceleration
and 1.5-GHz ARM Cortex-A9 SMP CPU.

Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 arch/arm/boot/dts/stih407-clock.dtsi   |  41 +++
 arch/arm/boot/dts/stih407-pinctrl.dtsi | 618 +++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/stih407.dtsi         | 250 +++++++++++++
 3 files changed, 909 insertions(+)
 create mode 100644 arch/arm/boot/dts/stih407-clock.dtsi
 create mode 100644 arch/arm/boot/dts/stih407-pinctrl.dtsi
 create mode 100644 arch/arm/boot/dts/stih407.dtsi

diff --git a/arch/arm/boot/dts/stih407-clock.dtsi b/arch/arm/boot/dts/stih407-clock.dtsi
new file mode 100644
index 0000000..f50ac6f
--- /dev/null
+++ b/arch/arm/boot/dts/stih407-clock.dtsi
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2013 STMicroelectronics R&D Limited
+ * <stlinux-devel@stlinux.com>
+ *
+ * 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.
+ */
+/ {
+	clocks {
+		/*
+		 * Fixed 30MHz oscillator inputs to SoC
+		 */
+		CLK_SYSIN: CLK_SYSIN {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <30000000>;
+			clock-output-names = "CLK_SYSIN";
+		};
+
+		/*
+		 * ARM Peripheral clock for timers
+		 */
+		arm_periph_clk: arm_periph_clk {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <600000000>;
+		};
+
+		/*
+		 * Bootloader initialized system infrastructure clock for
+		 * serial devices.
+		 */
+		CLK_EXT2F_A9: clockgenC0@13 {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <200000000>;
+			clock-output-names = "CLK_S_ICN_REG_0";
+		};
+	};
+};
diff --git a/arch/arm/boot/dts/stih407-pinctrl.dtsi b/arch/arm/boot/dts/stih407-pinctrl.dtsi
new file mode 100644
index 0000000..2d8543e
--- /dev/null
+++ b/arch/arm/boot/dts/stih407-pinctrl.dtsi
@@ -0,0 +1,618 @@
+/*
+ * Copyright (C) 2013 STMicroelectronics Limited.
+ * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
+ *
+ * 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
+ * publishhed by the Free Software Foundation.
+ */
+#include "st-pincfg.h"
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+/ {
+
+	aliases {
+		/* 0-5: PIO_SBC */
+		gpio0 = &PIO0;
+		gpio1 = &PIO1;
+		gpio2 = &PIO2;
+		gpio3 = &PIO3;
+		gpio4 = &PIO4;
+		gpio5 = &PIO5;
+		/* 10-19: PIO_FRONT0 */
+		gpio6 = &PIO10;
+		gpio7 = &PIO11;
+		gpio8 = &PIO12;
+		gpio9 = &PIO13;
+		gpio10 = &PIO14;
+		gpio11 = &PIO15;
+		gpio12 = &PIO16;
+		gpio13 = &PIO17;
+		gpio14 = &PIO18;
+		gpio15 = &PIO19;
+		/* 20: PIO_FRONT1 */
+		gpio16 = &PIO20;
+		/* 30-35: PIO_REAR */
+		gpio17 = &PIO30;
+		gpio18 = &PIO31;
+		gpio19 = &PIO32;
+		gpio20 = &PIO33;
+		gpio21 = &PIO34;
+		gpio22 = &PIO35;
+		/* 40-42: PIO_FLASH */
+		gpio23 = &PIO40;
+		gpio24 = &PIO41;
+		gpio25 = &PIO42;
+	};
+
+	soc {
+		pin-controller-sbc {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			compatible = "st,stih407-sbc-pinctrl";
+			st,syscfg = <&syscfg_sbc>;
+			reg = <0x0961f080 0x4>;
+			reg-names = "irqmux";
+			interrupts = <GIC_SPI 188 IRQ_TYPE_NONE>;
+			interrupts-names = "irqmux";
+			ranges = <0 0x09610000 0x6000>;
+
+			PIO0: gpio@09610000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x0 0x100>;
+				st,bank-name = "PIO0";
+			};
+			PIO1: gpio@09611000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x1000 0x100>;
+				st,bank-name = "PIO1";
+			};
+			PIO2: gpio@09612000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x2000 0x100>;
+				st,bank-name = "PIO2";
+			};
+			PIO3: gpio@09613000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x3000 0x100>;
+				st,bank-name = "PIO3";
+			};
+			PIO4: gpio@09614000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x4000 0x100>;
+				st,bank-name = "PIO4";
+			};
+
+			PIO5: gpio@09615000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x5000 0x100>;
+				st,bank-name = "PIO5";
+			};
+
+			rc {
+				pinctrl_ir: ir0 {
+					st,pins {
+						ir = <&PIO4 0 ALT2 IN>;
+					};
+				};
+			};
+
+			/* SBC_ASC0 - UART10 */
+			sbc_serial0 {
+				pinctrl_sbc_serial0: sbc_serial0-0 {
+					st,pins {
+						tx = <&PIO3 4 ALT1 OUT>;
+						rx = <&PIO3 5 ALT1 IN>;
+					};
+				};
+			};
+			/* SBC_ASC1 - UART11 */
+			sbc_serial1 {
+				pinctrl_sbc_serial1: sbc_serial1-0 {
+					st,pins {
+						tx = <&PIO2 6 ALT3 OUT>;
+						rx = <&PIO2 7 ALT3 IN>;
+					};
+				};
+			};
+
+			i2c10 {
+				pinctrl_i2c10_default: i2c10-default {
+					st,pins {
+						sda = <&PIO4 6 ALT1 BIDIR>;
+						scl = <&PIO4 5 ALT1 BIDIR>;
+					};
+				};
+			};
+
+			i2c11 {
+				pinctrl_i2c11_default: i2c11-default {
+					st,pins {
+						sda = <&PIO5 1 ALT1 BIDIR>;
+						scl = <&PIO5 0 ALT1 BIDIR>;
+					};
+				};
+			};
+
+			keyscan {
+				pinctrl_keyscan: keyscan {
+					st,pins {
+						keyin0 = <&PIO4 0 ALT6 IN>;
+						keyin1 = <&PIO4 5 ALT4 IN>;
+						keyin2 = <&PIO0 4 ALT2 IN>;
+						keyin3 = <&PIO2 6 ALT2 IN>;
+
+						keyout0 = <&PIO4 6 ALT4 OUT>;
+						keyout1 = <&PIO1 7 ALT2 OUT>;
+						keyout2 = <&PIO0 6 ALT2 OUT>;
+						keyout3 = <&PIO2 7 ALT2 OUT>;
+					};
+				};
+			};
+
+			gmac1 {
+				/*
+				  Almost all the boards based on STiH407 SoC have an embedded
+				  switch where the mdio/mdc have been used for managing the SMI
+				  iface via I2C. For this reason these lines can be allocated
+				  by using dedicated configuration (in case of there will be a
+				  standard PHY transceiver on-board).
+				*/
+				pinctrl_rgmii1: rgmii1-0 {
+					st,pins {
+
+						txd0 = <&PIO0 0 ALT1 OUT DE_IO 0 CLK_A>;
+						txd1 = <&PIO0 1 ALT1 OUT DE_IO 0 CLK_A>;
+						txd2 = <&PIO0 2 ALT1 OUT DE_IO 0 CLK_A>;
+						txd3 = <&PIO0 3 ALT1 OUT DE_IO 0 CLK_A>;
+						txen = <&PIO0 5 ALT1 OUT DE_IO 0 CLK_A>;
+						txclk = <&PIO0 6 ALT1 IN NICLK 0 CLK_A>;
+						rxd0 = <&PIO1 4 ALT1 IN DE_IO 0 CLK_A>;
+						rxd1 = <&PIO1 5 ALT1 IN DE_IO 0 CLK_A>;
+						rxd2 = <&PIO1 6 ALT1 IN DE_IO 0 CLK_A>;
+						rxd3 = <&PIO1 7 ALT1 IN DE_IO 0 CLK_A>;
+						rxdv = <&PIO2 0 ALT1 IN DE_IO 0 CLK_A>;
+						rxclk = <&PIO2 2 ALT1 IN NICLK 500 CLK_A>;
+						clk125 = <&PIO3 7 ALT4 IN NICLK 0 CLK_A>;
+						phyclk = <&PIO2 3 ALT4 OUT NICLK 1750 CLK_B>;
+					};
+				};
+
+				pinctrl_rgmii1_mdio: rgmii1-mdio {
+					st,pins {
+						mdio = <&PIO1 0 ALT1 OUT BYPASS 0>;
+						mdc = <&PIO1 1 ALT1 OUT NICLK 0 CLK_A>;
+						mdint = <&PIO1 3 ALT1 IN BYPASS 0>;
+					};
+				};
+
+				pinctrl_mii1: mii1 {
+					st,pins {
+						txd0 = <&PIO0 0 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
+						txd1 = <&PIO0 1 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
+						txd2 = <&PIO0 2 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
+						txd3 = <&PIO0 3 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
+						txer = <&PIO0 4 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
+						txen = <&PIO0 5 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
+						txclk = <&PIO0 6 ALT1 IN NICLK 0 CLK_A>;
+						col = <&PIO0 7 ALT1 IN BYPASS 1000>;
+
+						mdio = <&PIO1 0 ALT1 OUT BYPASS 1500>;
+						mdc = <&PIO1 1 ALT1 OUT NICLK 0 CLK_A>;
+						crs = <&PIO1 2 ALT1 IN BYPASS 1000>;
+						mdint = <&PIO1 3 ALT1 IN BYPASS 0>;
+						rxd0 = <&PIO1 4 ALT1 IN SE_NICLK_IO 0 CLK_A>;
+						rxd1 = <&PIO1 5 ALT1 IN SE_NICLK_IO 0 CLK_A>;
+						rxd2 = <&PIO1 6 ALT1 IN SE_NICLK_IO 0 CLK_A>;
+						rxd3 = <&PIO1 7 ALT1 IN SE_NICLK_IO 0 CLK_A>;
+
+						rxdv = <&PIO2 0 ALT1 IN SE_NICLK_IO 0 CLK_A>;
+						rx_er = <&PIO2 1 ALT1 IN SE_NICLK_IO 0 CLK_A>;
+						rxclk = <&PIO2 2 ALT1 IN NICLK 0 CLK_A>;
+						phyclk = <&PIO2 3 ALT1 OUT NICLK 0 CLK_A>;
+					};
+				};
+
+			};
+
+			pwm1 {
+				pinctrl_pwm1_chan0_default: pwm1-0-default {
+					st,pins {
+						pwm-out = <&PIO3 0 ALT1 OUT>;
+					};
+				};
+				pinctrl_pwm1_chan1_default: pwm1-1-default {
+					st,pins {
+						pwm-out = <&PIO4 4 ALT1 OUT>;
+					};
+				};
+				pinctrl_pwm1_chan2_default: pwm1-2-default {
+					st,pins {
+						pwm-out = <&PIO4 6 ALT3 OUT>;
+					};
+				};
+				pinctrl_pwm1_chan3_default: pwm1-3-default {
+					st,pins {
+						pwm-out = <&PIO4 7 ALT3 OUT>;
+					};
+				};
+			};
+
+		};
+
+		pin-controller-front0 {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			compatible = "st,stih407-front-pinctrl";
+			st,syscfg = <&syscfg_front>;
+			reg = <0x0920f080 0x4>;
+			reg-names = "irqmux";
+			interrupts = <GIC_SPI 189 IRQ_TYPE_NONE>;
+			interrupts-names = "irqmux";
+			ranges = <0 0x09200000 0x10000>;
+
+			PIO10: PIO@09200000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x0 0x100>;
+				st,bank-name = "PIO10";
+			};
+			PIO11: PIO@09201000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x1000 0x100>;
+				st,bank-name = "PIO11";
+			};
+			PIO12: PIO@09202000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x2000 0x100>;
+				st,bank-name = "PIO12";
+			};
+			PIO13: PIO@09203000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x3000 0x100>;
+				st,bank-name = "PIO13";
+			};
+			PIO14: PIO@09204000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x4000 0x100>;
+				st,bank-name = "PIO14";
+			};
+			PIO15: PIO@09205000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x5000 0x100>;
+				st,bank-name = "PIO15";
+			};
+			PIO16: PIO@09206000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x6000 0x100>;
+				st,bank-name = "PIO16";
+			};
+			PIO17: PIO@09207000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x7000 0x100>;
+				st,bank-name = "PIO17";
+			};
+			PIO18: PIO@09208000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x8000 0x100>;
+				st,bank-name = "PIO18";
+			};
+			PIO19: PIO@09209000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x9000 0x100>;
+				st,bank-name = "PIO19";
+			};
+
+			/* Comms */
+			serial0 {
+				pinctrl_serial0: serial0-0 {
+					st,pins {
+						tx = <&PIO17 0 ALT1 OUT>;
+						rx = <&PIO17 1 ALT1 IN>;
+					};
+				};
+			};
+
+			serial1 {
+				pinctrl_serial1: serial1-0 {
+					st,pins {
+						tx = <&PIO16 0 ALT1 OUT>;
+						rx = <&PIO16 1 ALT1 IN>;
+					};
+				};
+			};
+
+			serial2 {
+				pinctrl_serial2: serial2-0 {
+					st,pins {
+						tx = <&PIO15 0 ALT1 OUT>;
+						rx = <&PIO15 1 ALT1 IN>;
+					};
+				};
+			};
+
+			mmc1 {
+				pinctrl_sd1: sd1-0 {
+					st,pins {
+						sd_clk = <&PIO19 3 ALT5 BIDIR NICLK 0 CLK_B>;
+						sd_cmd = <&PIO19 2 ALT5 BIDIR_PU BYPASS 0>;
+						sd_dat0 = <&PIO19 4 ALT5 BIDIR_PU BYPASS 0>;
+						sd_dat1 = <&PIO19 5 ALT5 BIDIR_PU BYPASS 0>;
+						sd_dat2 = <&PIO19 6 ALT5 BIDIR_PU BYPASS 0>;
+						sd_dat3 = <&PIO19 7 ALT5 BIDIR_PU BYPASS 0>;
+						sd_led = <&PIO16 6 ALT6 OUT>;
+						sd_pwren = <&PIO16 7 ALT6 OUT>;
+						sd_cd = <&PIO19 0 ALT6 IN>;
+						sd_wp = <&PIO19 1 ALT6 IN>;
+					};
+				};
+			};
+
+
+			i2c0 {
+				pinctrl_i2c0_default: i2c0-default {
+					st,pins {
+						sda = <&PIO10 6 ALT2 BIDIR>;
+						scl = <&PIO10 5 ALT2 BIDIR>;
+					};
+				};
+			};
+
+			i2c1 {
+				pinctrl_i2c1_default: i2c1-default {
+					st,pins {
+						sda = <&PIO11 1 ALT2 BIDIR>;
+						scl = <&PIO11 0 ALT2 BIDIR>;
+					};
+				};
+			};
+
+			i2c2 {
+				pinctrl_i2c2_default: i2c2-default {
+					st,pins {
+						sda = <&PIO15 6 ALT2 BIDIR>;
+						scl = <&PIO15 5 ALT2 BIDIR>;
+					};
+				};
+			};
+
+			i2c3 {
+				pinctrl_i2c3_default: i2c3-default {
+					st,pins {
+						sda = <&PIO18 6 ALT1 BIDIR>;
+						scl = <&PIO18 5 ALT1 BIDIR>;
+					};
+				};
+			};
+
+			spi0 {
+				pinctrl_spi0_default: spi0-default {
+					st,pins {
+						mtsr = <&PIO12 6 ALT2 BIDIR>;
+						mrst = <&PIO12 7 ALT2 BIDIR>;
+						scl = <&PIO12 5 ALT2 BIDIR>;
+					};
+				};
+			};
+		};
+
+		pin-controller-front1 {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			compatible = "st,stih407-front-pinctrl";
+			st,syscfg = <&syscfg_front>;
+			reg = <0x0921f080 0x4>;
+			reg-names = "irqmux";
+			interrupts = <GIC_SPI 190 IRQ_TYPE_NONE>;
+			interrupts-names = "irqmux";
+			ranges = <0 0x09210000 0x10000>;
+
+			PIO20: PIO@09210000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x0 0x100>;
+				st,bank-name = "PIO20";
+			};
+		};
+
+		pin-controller-rear {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			compatible = "st,stih407-rear-pinctrl";
+			st,syscfg = <&syscfg_rear>;
+			reg = <0x0922f080 0x4>;
+			reg-names = "irqmux";
+			interrupts = <GIC_SPI 191 IRQ_TYPE_NONE>;
+			interrupts-names = "irqmux";
+			ranges = <0 0x09220000 0x6000>;
+
+			PIO30: gpio@09220000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x0 0x100>;
+				st,bank-name = "PIO30";
+			};
+			PIO31: gpio@09221000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x1000 0x100>;
+				st,bank-name = "PIO31";
+			};
+			PIO32: gpio@09222000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x2000 0x100>;
+				st,bank-name = "PIO32";
+			};
+			PIO33: gpio@09223000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x3000 0x100>;
+				st,bank-name = "PIO33";
+			};
+			PIO34: gpio@09224000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x4000 0x100>;
+				st,bank-name = "PIO34";
+			};
+			PIO35: gpio@09225000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x5000 0x100>;
+				st,bank-name = "PIO35";
+			};
+
+			i2c4 {
+				pinctrl_i2c4_default: i2c4-default {
+					st,pins {
+						sda = <&PIO30 1 ALT1 BIDIR>;
+						scl = <&PIO30 0 ALT1 BIDIR>;
+					};
+				};
+			};
+
+			i2c5 {
+				pinctrl_i2c5_default: i2c5-default {
+					st,pins {
+						sda = <&PIO34 4 ALT1 BIDIR>;
+						scl = <&PIO34 3 ALT1 BIDIR>;
+					};
+				};
+			};
+
+			usb3 {
+				pinctrl_usb3: usb3-2 {
+					st,pins {
+						usb-oc-detect = <&PIO35 4 ALT1 IN>;
+						usb-pwr-enable = <&PIO35 5 ALT1 OUT>;
+						usb-vbus-valid = <&PIO35 6 ALT1 IN>;
+					};
+				};
+			};
+
+			pwm0 {
+				pinctrl_pwm0_chan0_default: pwm0-0-default {
+					st,pins {
+						pwm-out = <&PIO31 1 ALT1 OUT>;
+					};
+				};
+			};
+
+		};
+
+		pin-controller-flash {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			compatible = "st,stih407-flash-pinctrl";
+			st,syscfg = <&syscfg_flash>;
+			reg = <0x0923f080 0x4>;
+			reg-names = "irqmux";
+			interrupts = <GIC_SPI 192 IRQ_TYPE_NONE>;
+			interrupts-names = "irqmux";
+			ranges = <0 0x09230000 0x3000>;
+
+			PIO40: gpio@09230000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0 0x100>;
+				st,bank-name = "PIO40";
+			};
+			PIO41: gpio@09231000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x1000 0x100>;
+				st,bank-name = "PIO41";
+			};
+			PIO42: gpio@09232000 {
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				reg = <0x2000 0x100>;
+				st,bank-name = "PIO42";
+			};
+
+			mmc0 {
+				pinctrl_mmc0: mmc0-0 {
+					st,pins {
+						emmc_clk = <&PIO40 6 ALT1 BIDIR>;
+						emmc_cmd = <&PIO40 7 ALT1 BIDIR_PU>;
+						emmc_d0 = <&PIO41 0 ALT1 BIDIR_PU>;
+						emmc_d1 = <&PIO41 1 ALT1 BIDIR_PU>;
+						emmc_d2 = <&PIO41 2 ALT1 BIDIR_PU>;
+						emmc_d3 = <&PIO41 3 ALT1 BIDIR_PU>;
+						emmc_d4 = <&PIO41 4 ALT1 BIDIR_PU>;
+						emmc_d5 = <&PIO41 5 ALT1 BIDIR_PU>;
+						emmc_d6 = <&PIO41 6 ALT1 BIDIR_PU>;
+						emmc_d7 = <&PIO41 7 ALT1 BIDIR_PU>;
+					};
+				};
+			};
+		};
+	};
+};
diff --git a/arch/arm/boot/dts/stih407.dtsi b/arch/arm/boot/dts/stih407.dtsi
new file mode 100644
index 0000000..2a0566b
--- /dev/null
+++ b/arch/arm/boot/dts/stih407.dtsi
@@ -0,0 +1,250 @@
+/*
+ * Copyright (C) 2013 STMicroelectronics Limited.
+ * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
+ *
+ * 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
+ * publishhed by the Free Software Foundation.
+ */
+#include "stih407-clock.dtsi"
+#include "stih407-pinctrl.dtsi"
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a9";
+			reg = <0>;
+		};
+		cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a9";
+			reg = <1>;
+		};
+	};
+
+	intc: interrupt-controller@08761000 {
+		compatible = "arm,cortex-a9-gic";
+		#interrupt-cells = <3>;
+		interrupt-controller;
+		reg = <0x08761000 0x1000>, <0x08760100 0x100>;
+	};
+
+	scu@08760000 {
+		compatible = "arm,cortex-a9-scu";
+		reg = <0x08760000 0x1000>;
+	};
+
+	timer@08760200 {
+		interrupt-parent = <&intc>;
+		compatible = "arm,cortex-a9-global-timer";
+		reg = <0x08760200 0x100>;
+		interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&arm_periph_clk>;
+	};
+
+	L2: cache-controller {
+		compatible = "arm,pl310-cache";
+		reg = <0x08762000 0x1000>;
+		arm,data-latency = <3 3 3>;
+		arm,tag-latency = <2 2 2>;
+		cache-unified;
+		cache-level = <2>;
+	};
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		interrupt-parent = <&intc>;
+		ranges;
+		compatible = "simple-bus";
+
+		syscfg_sbc:sbc-syscfg@9620000{
+			compatible = "st,stih407-sbc-syscfg", "syscon";
+			reg = <0x9620000 0x1000>;
+		};
+
+		syscfg_front:front-syscfg@9280000{
+			compatible = "st,stih407-front-syscfg", "syscon";
+			reg = <0x9280000 0x1000>;
+		};
+
+		syscfg_rear:rear-syscfg@9290000{
+			compatible = "st,stih407-rear-syscfg", "syscon";
+			reg = <0x9290000 0x1000>;
+		};
+
+		syscfg_flash:flash-syscfg@92a0000{
+			compatible = "st,stih407-flash-syscfg", "syscon";
+			reg = <0x92a0000 0x1000>;
+		};
+
+		syscfg_sbc_reg:fvdp-lite-syscfg@9600000{
+			compatible = "st,stih407-sbc-reg-syscfg", "syscon";
+			reg = <0x9600000 0x1000>;
+		};
+
+		syscfg_core:core-syscfg@92b0000{
+			compatible = "st,stih407-core-syscfg", "syscon";
+			reg = <0x92b0000 0x1000>;
+		};
+
+		syscfg_lpm:lpm-syscfg@94b5100{
+			compatible = "st,stih407-lpm-syscfg", "syscon";
+			reg = <0x94b5100 0x1000>;
+		};
+
+		serial@9830000{
+			compatible = "st,asc";
+			status = "disabled";
+			reg = <0x9830000 0x2c>;
+			interrupts = <GIC_SPI 122 IRQ_TYPE_NONE>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_serial0>;
+			clocks = <&CLK_EXT2F_A9>;
+		};
+
+		serial@9831000{
+			compatible = "st,asc";
+			status = "disabled";
+			reg = <0x9831000 0x2c>;
+			interrupts = <GIC_SPI 123 IRQ_TYPE_NONE>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_serial1>;
+			clocks = <&CLK_EXT2F_A9>;
+		};
+
+		serial@9832000{
+			compatible = "st,asc";
+			status = "disabled";
+			reg = <0x9832000 0x2c>;
+			interrupts = <GIC_SPI 124 IRQ_TYPE_NONE>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_serial2>;
+			clocks = <&CLK_EXT2F_A9>;
+		};
+
+		/* SBC_ASC0 - UART10 */
+		sbc_serial0: serial@9530000 {
+			compatible = "st,asc";
+			status = "disabled";
+			reg = <0x9530000 0x2c>;
+			interrupts = <GIC_SPI 138 IRQ_TYPE_NONE>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_sbc_serial0>;
+			clocks = <&CLK_SYSIN>;
+		};
+
+		serial@9531000 {
+			compatible = "st,asc";
+			status = "disabled";
+			reg = <0x9531000 0x2c>;
+			interrupts = <GIC_SPI 139 IRQ_TYPE_NONE>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_sbc_serial1>;
+			clocks = <&CLK_SYSIN>;
+		};
+
+		i2c@9840000 {
+			compatible = "st,comms-ssc4-i2c";
+			status = "disabled";
+			interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
+			reg = <0x9840000 0x110>;
+			clocks = <&CLK_EXT2F_A9>;
+			clock-names = "ssc";
+			clock-frequency = <400000>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_i2c0_default>;
+		};
+
+		i2c@9841000 {
+			compatible = "st,comms-ssc4-i2c";
+			status = "disabled";
+			reg = <0x9841000 0x110>;
+			interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&CLK_EXT2F_A9>;
+			clock-names = "ssc";
+			clock-frequency = <400000>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_i2c1_default>;
+		};
+
+		i2c@9842000 {
+			compatible = "st,comms-ssc4-i2c";
+			status = "disabled";
+			reg = <0x9842000 0x110>;
+			interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&CLK_EXT2F_A9>;
+			clock-names = "ssc";
+			clock-frequency = <400000>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_i2c2_default>;
+		};
+
+		i2c@9843000 {
+			compatible = "st,comms-ssc4-i2c";
+			status = "disabled";
+			reg = <0x9843000 0x110>;
+			interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&CLK_EXT2F_A9>;
+			clock-names = "ssc";
+			clock-frequency = <400000>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_i2c3_default>;
+		};
+
+		i2c@9844000 {
+			compatible = "st,comms-ssc4-i2c";
+			status = "disabled";
+			reg = <0x9844000 0x110>;
+			interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&CLK_EXT2F_A9>;
+			clock-names = "ssc";
+			clock-frequency = <400000>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_i2c4_default>;
+		};
+
+		i2c@9845000 {
+			compatible = "st,comms-ssc4-i2c";
+			status = "disabled";
+			reg = <0x9845000 0x110>;
+			interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&CLK_EXT2F_A9>;
+			clock-names = "ssc";
+			clock-frequency = <400000>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_i2c5_default>;
+		};
+
+
+		/* SSCs on SBC */
+		i2c@9540000 {
+			compatible = "st,comms-ssc4-i2c";
+			status = "disabled";
+			reg = <0x9540000 0x110>;
+			interrupts = <GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&CLK_SYSIN>;
+			clock-names = "ssc";
+			clock-frequency = <400000>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_i2c10_default>;
+		};
+
+		i2c@9541000 {
+			compatible = "st,comms-ssc4-i2c";
+			status = "disabled";
+			reg = <0x9541000 0x110>;
+			interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&CLK_SYSIN>;
+			clock-names = "ssc";
+			clock-frequency = <400000>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_i2c11_default>;
+		};
+	};
+};
-- 
1.9.0


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

* [PATCH v3 5/5] ARM: dts: STiH407: Add B2120 board support
  2014-03-07 14:40 [PATCH v3 0/5] Add STiH407 SoC and reference board support Maxime COQUELIN
                   ` (3 preceding siblings ...)
  2014-03-07 14:40 ` [PATCH v3 4/5] ARM: dts: Add STiH407 SoC support Maxime COQUELIN
@ 2014-03-07 14:40 ` Maxime COQUELIN
  2014-03-10 12:37   ` Lee Jones
  4 siblings, 1 reply; 21+ messages in thread
From: Maxime COQUELIN @ 2014-03-07 14:40 UTC (permalink / raw)
  To: Rob Landley, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Russell King, Srinivas Kandagatla, Stuart Menefy,
	Linus Walleij, Giuseppe Cavallaro, Maxime Coquelin, linux-doc,
	linux-kernel, devicetree, linux-arm-kernel, kernel
  Cc: lee.jones

B2120 HDK is the reference board for STiH407 SoC.
It has the following characteristics:
 - 1GB DDR3
 - 8GB eMMC / SD-Card slot
 - 32MB NOR Flash
 - 1 x Gbit Ethernet
 - 1 x USB 3.0 port
 - 1 x Mini-PCIe
 - 1 x SATA
 - 1 x HDMI output
 - 1 x HDMI input
 - 1 x SPDIF

This patch only introduces basic functionnalities, such as I2C and UART.

Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 arch/arm/boot/dts/Makefile          |  3 +-
 arch/arm/boot/dts/stih407-b2120.dts | 78 +++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/stih407-b2120.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index d109908..c8ad5f5 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -310,7 +310,8 @@ dtb-$(CONFIG_ARCH_SPEAR6XX)+= spear600-evb.dtb
 dtb-$(CONFIG_ARCH_STI)+= stih415-b2000.dtb \
 	stih416-b2000.dtb \
 	stih415-b2020.dtb \
-	stih416-b2020.dtb
+	stih416-b2020.dtb \
+	stih407-b2120.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += \
 	sun4i-a10-a1000.dtb \
 	sun4i-a10-cubieboard.dtb \
diff --git a/arch/arm/boot/dts/stih407-b2120.dts b/arch/arm/boot/dts/stih407-b2120.dts
new file mode 100644
index 0000000..9c97da4
--- /dev/null
+++ b/arch/arm/boot/dts/stih407-b2120.dts
@@ -0,0 +1,78 @@
+/*
+ * Copyright (C) 2014 STMicroelectronics (R&D) Limited.
+ * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
+ *
+ * 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.
+ */
+/dts-v1/;
+#include "stih407.dtsi"
+/ {
+	model = "STiH407 B2120";
+	compatible = "st,stih407", "st,stih407-b2120";
+
+	chosen {
+		bootargs = "console=ttyAS0,115200";
+		linux,stdout-path = &sbc_serial0;
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <0x40000000 0x80000000>;
+	};
+
+	aliases {
+		ttyAS0 = &sbc_serial0;
+	};
+
+	soc {
+		sbc_serial0: serial@9530000 {
+			status = "okay";
+		};
+
+		leds {
+			compatible = "gpio-leds";
+			red {
+				#gpio-cells = <2>;
+				label = "Front Panel LED";
+				gpios = <&PIO4 1 0>;
+				linux,default-trigger = "heartbeat";
+			};
+			green {
+				#gpio-cells = <2>;
+				gpios = <&PIO1 3 0>;
+				default-state = "off";
+			};
+		};
+
+		i2c@9842000 {
+			status = "okay";
+		};
+
+		i2c@9843000 {
+			status = "okay";
+		};
+
+		i2c@9844000 {
+			status = "okay";
+		};
+
+		i2c@9845000 {
+			status = "okay";
+		};
+
+		i2c@9540000 {
+			status = "okay";
+		};
+
+		/* SSC11 to HDMI */
+		i2c@9541000 {
+			status = "okay";
+			/* HDMI V1.3a supports Standard mode only */
+			clock-frequency = <100000>;
+			st,i2c-min-scl-pulse-width-us = <0>;
+			st,i2c-min-sda-pulse-width-us = <5>;
+		};
+	};
+};
-- 
1.9.0


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

* Re: [PATCH v3 3/5] pinctrl: st: add pinctrl support for the STiH407 SoC
  2014-03-07 14:40 ` [PATCH v3 3/5] pinctrl: st: add pinctrl support for the STiH407 SoC Maxime COQUELIN
@ 2014-03-07 15:36   ` srinivas kandagatla
  2014-03-10 14:44   ` Lee Jones
  1 sibling, 0 replies; 21+ messages in thread
From: srinivas kandagatla @ 2014-03-07 15:36 UTC (permalink / raw)
  To: Maxime COQUELIN, Rob Landley, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Russell King,
	Stuart Menefy, Linus Walleij, Giuseppe Cavallaro, linux-doc,
	linux-kernel, devicetree, linux-arm-kernel, kernel
  Cc: lee.jones

On 07/03/14 14:40, Maxime COQUELIN wrote:
> From: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> 
> This patch adds the initial support for pinctrl based on H407 SoC.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> ---
Acked-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>


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

* Re: [PATCH v3 2/5] pinctrl: st: Enhance the controller to manage unavailable registers
  2014-03-07 14:40 ` [PATCH v3 2/5] pinctrl: st: Enhance the controller to manage unavailable registers Maxime COQUELIN
@ 2014-03-07 15:37   ` srinivas kandagatla
  2014-03-10  9:17   ` Lee Jones
  1 sibling, 0 replies; 21+ messages in thread
From: srinivas kandagatla @ 2014-03-07 15:37 UTC (permalink / raw)
  To: Maxime COQUELIN, Rob Landley, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Russell King,
	Stuart Menefy, Linus Walleij, Giuseppe Cavallaro, linux-doc,
	linux-kernel, devicetree, linux-arm-kernel, kernel
  Cc: lee.jones

On 07/03/14 14:40, Maxime COQUELIN wrote:
> From: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> 
> This patch adds a new logic inside the st pinctrl to manage
> an unsupported scenario: some sysconfig are not available!
> 
> This is the case of STiH407 where, although documented, the
> following registers from SYSCFG_FLASH have been removed from the SoC.
> 
> SYSTEM_CONFIG3040
>    Output Enable pad control for all PIO Alternate Functions
> and
> SYSTEM_ CONFIG3050
>    Pull Up pad control for all PIO Alternate Functions
> 
> Without managing this condition an imprecise external abort
> will be detect.
> 
> To do this the patch also reviews the st_parse_syscfgs
> and other routines to manipulate the registers only if
> actually available.
> In any case, for example the st_parse_syscfgs detected
> an error condition but no action was made in the
> st_pctl_probe_dt.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Acked-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>


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

* Re: [PATCH v3 2/5] pinctrl: st: Enhance the controller to manage unavailable registers
  2014-03-07 14:40 ` [PATCH v3 2/5] pinctrl: st: Enhance the controller to manage unavailable registers Maxime COQUELIN
  2014-03-07 15:37   ` srinivas kandagatla
@ 2014-03-10  9:17   ` Lee Jones
  2014-03-11  8:18     ` Maxime Coquelin
  1 sibling, 1 reply; 21+ messages in thread
From: Lee Jones @ 2014-03-10  9:17 UTC (permalink / raw)
  To: Maxime COQUELIN
  Cc: Rob Landley, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Russell King, Srinivas Kandagatla, Stuart Menefy,
	Linus Walleij, Giuseppe Cavallaro, linux-doc, linux-kernel,
	devicetree, linux-arm-kernel, kernel

> From: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> 
> This patch adds a new logic inside the st pinctrl to manage
> an unsupported scenario: some sysconfig are not available!
> 
> This is the case of STiH407 where, although documented, the
> following registers from SYSCFG_FLASH have been removed from the SoC.
> 
> SYSTEM_CONFIG3040
>    Output Enable pad control for all PIO Alternate Functions
> and
> SYSTEM_ CONFIG3050
>    Pull Up pad control for all PIO Alternate Functions
> 
> Without managing this condition an imprecise external abort
> will be detect.
> 
> To do this the patch also reviews the st_parse_syscfgs
> and other routines to manipulate the registers only if
> actually available.
> In any case, for example the st_parse_syscfgs detected
> an error condition but no action was made in the
> st_pctl_probe_dt.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>

These two SOBs need reordering.

> ---
>  drivers/pinctrl/pinctrl-st.c | 106 +++++++++++++++++++++++++------------------
>  1 file changed, 61 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
> index 9fb66aa..1721611 100644
> --- a/drivers/pinctrl/pinctrl-st.c
> +++ b/drivers/pinctrl/pinctrl-st.c
> @@ -410,25 +410,27 @@ static void st_pinconf_set_config(struct st_pio_control *pc,
>  	unsigned int oe_value, pu_value, od_value;
>  	unsigned long mask = BIT(pin);
>  
> -	regmap_field_read(output_enable, &oe_value);
> -	regmap_field_read(pull_up, &pu_value);
> -	regmap_field_read(open_drain, &od_value);
> -
> -	/* Clear old values */
> -	oe_value &= ~mask;
> -	pu_value &= ~mask;
> -	od_value &= ~mask;
> -
> -	if (config & ST_PINCONF_OE)
> -		oe_value |= mask;
> -	if (config & ST_PINCONF_PU)
> -		pu_value |= mask;
> -	if (config & ST_PINCONF_OD)
> -		od_value |= mask;
> -
> -	regmap_field_write(output_enable, oe_value);
> -	regmap_field_write(pull_up, pu_value);
> -	regmap_field_write(open_drain, od_value);
> +	if (output_enable) {
> +		regmap_field_read(output_enable, &oe_value);
> +		oe_value &= ~mask;
> +		if (config & ST_PINCONF_OE)
> +			oe_value |= mask;
> +		regmap_field_write(output_enable, oe_value);
> +	}
> +	if (pull_up) {
> +		regmap_field_read(pull_up, &pu_value);
> +		pu_value &= ~mask;
> +		if (config & ST_PINCONF_PU)
> +			pu_value |= mask;
> +		regmap_field_write(pull_up, pu_value);
> +	}
> +	if (open_drain) {
> +		regmap_field_read(open_drain, &od_value);
> +		od_value &= ~mask;
> +		if (config & ST_PINCONF_OD)
> +			od_value |= mask;
> +		regmap_field_write(open_drain, od_value);
> +	}

Nice change.

Nit: For consistency with the changes below, please consider placing
new lines between the 3 outer checks.

>  }
>  

<snip>

> -static void st_pinconf_get_direction(struct st_pio_control *pc,
> -	int pin, unsigned long *config)
> +static void st_pinconf_get_direction(struct st_pio_control *pc, int pin,
> +				     unsigned long *config)

Unrelated change?

>  {
>  	unsigned int oe_value, pu_value, od_value;

Is it worth checking for (!config) here?

> -	regmap_field_read(pc->oe, &oe_value);
> -	regmap_field_read(pc->pu, &pu_value);
> -	regmap_field_read(pc->od, &od_value);
> +	if (pc->oe) {
> +		regmap_field_read(pc->oe, &oe_value);
> +		if (oe_value & BIT(pin))
> +			ST_PINCONF_PACK_OE(*config);
> +	}
>  
> -	if (oe_value & BIT(pin))
> -		ST_PINCONF_PACK_OE(*config);
> -	if (pu_value & BIT(pin))
> -		ST_PINCONF_PACK_PU(*config);
> -	if (od_value & BIT(pin))
> -		ST_PINCONF_PACK_OD(*config);
> +	if (pc->pu) {
> +		regmap_field_read(pc->pu, &pu_value);
> +		if (pu_value & BIT(pin))
> +			ST_PINCONF_PACK_PU(*config);
> +	}
>  
> +	if (pc->od) {
> +		regmap_field_read(pc->od, &od_value);
> +		if (od_value & BIT(pin))
> +			ST_PINCONF_PACK_OD(*config);
> +	}
>  }

Nice.

>  static int st_pinconf_get_retime_packed(struct st_pinctrl *info,
> @@ -1105,8 +1116,21 @@ static int st_pctl_dt_setup_retime(struct st_pinctrl *info,
>  	return -EINVAL;
>  }
>  
> -static int st_parse_syscfgs(struct st_pinctrl *info,
> -		int bank, struct device_node *np)
> +
> +static struct regmap_field *st_pc_get_value(struct device *dev,
> +					    struct regmap *regmap, int bank,
> +					    int data, int lsb, int msb)
> +{
> +	struct reg_field reg = REG_FIELD((data + bank) * 4, lsb, msb);
> +
> +	if (data < 0)
> +		return NULL;

What happens is data < 0 and it's used in REG_FIELD?

Would it make more sense to make this check before calling REG_FIELD?

> +	return devm_regmap_field_alloc(dev, regmap, reg);
> +}
> +
> +static void st_parse_syscfgs(struct st_pinctrl *info, int bank,
> +			     struct device_node *np)
>  {
>  	const struct st_pctl_data *data = info->data;
>  	/**
> @@ -1116,29 +1140,21 @@ static int st_parse_syscfgs(struct st_pinctrl *info,
>  	 */
>  	int lsb = (bank%4) * ST_GPIO_PINS_PER_BANK;
>  	int msb = lsb + ST_GPIO_PINS_PER_BANK - 1;
> -	struct reg_field alt_reg = REG_FIELD((data->alt + bank) * 4, 0, 31);
> -	struct reg_field oe_reg = REG_FIELD((data->oe + bank/4) * 4, lsb, msb);
> -	struct reg_field pu_reg = REG_FIELD((data->pu + bank/4) * 4, lsb, msb);
> -	struct reg_field od_reg = REG_FIELD((data->od + bank/4) * 4, lsb, msb);
>  	struct st_pio_control *pc = &info->banks[bank].pc;
>  	struct device *dev = info->dev;
>  	struct regmap *regmap  = info->regmap;
>  
> -	pc->alt = devm_regmap_field_alloc(dev, regmap, alt_reg);
> -	pc->oe = devm_regmap_field_alloc(dev, regmap, oe_reg);
> -	pc->pu = devm_regmap_field_alloc(dev, regmap, pu_reg);
> -	pc->od = devm_regmap_field_alloc(dev, regmap, od_reg);
> -
> -	if (IS_ERR(pc->alt) || IS_ERR(pc->oe) ||
> -			IS_ERR(pc->pu) || IS_ERR(pc->od))
> -		return -EINVAL;
> +	pc->alt = st_pc_get_value(dev, regmap, bank, data->alt, 0, 31);
> +	pc->oe = st_pc_get_value(dev, regmap, bank/4, data->oe, lsb, msb);
> +	pc->pu = st_pc_get_value(dev, regmap, bank/4, data->pu, lsb, msb);
> +	pc->od = st_pc_get_value(dev, regmap, bank/4, data->od, lsb, msb);
>  
>  	/* retime avaiable for all pins by default */
>  	pc->rt_pin_mask = 0xff;
>  	of_property_read_u32(np, "st,retime-pin-mask", &pc->rt_pin_mask);
>  	st_pctl_dt_setup_retime(info, bank, pc);
>  
> -	return 0;
> +	return;
>  }
>  
>  /*

-- 
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] 21+ messages in thread

* Re: [PATCH v3 4/5] ARM: dts: Add STiH407 SoC support
  2014-03-07 14:40 ` [PATCH v3 4/5] ARM: dts: Add STiH407 SoC support Maxime COQUELIN
@ 2014-03-10 12:28   ` Lee Jones
  2014-03-10 16:16     ` Lee Jones
  2014-03-11 10:09     ` Maxime Coquelin
  0 siblings, 2 replies; 21+ messages in thread
From: Lee Jones @ 2014-03-10 12:28 UTC (permalink / raw)
  To: Maxime COQUELIN
  Cc: Rob Landley, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Russell King, Srinivas Kandagatla, Stuart Menefy,
	Linus Walleij, Giuseppe Cavallaro, linux-doc, linux-kernel,
	devicetree, linux-arm-kernel, kernel

On Fri, 07 Mar 2014, Maxime COQUELIN wrote:

> The STiH407 is advanced multi-HD AVC processor with 3D graphics acceleration
> and 1.5-GHz ARM Cortex-A9 SMP CPU.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> ---
>  arch/arm/boot/dts/stih407-clock.dtsi   |  41 +++
>  arch/arm/boot/dts/stih407-pinctrl.dtsi | 618 +++++++++++++++++++++++++++++++++
>  arch/arm/boot/dts/stih407.dtsi         | 250 +++++++++++++
>  3 files changed, 909 insertions(+)
>  create mode 100644 arch/arm/boot/dts/stih407-clock.dtsi
>  create mode 100644 arch/arm/boot/dts/stih407-pinctrl.dtsi
>  create mode 100644 arch/arm/boot/dts/stih407.dtsi
> 
> diff --git a/arch/arm/boot/dts/stih407-clock.dtsi b/arch/arm/boot/dts/stih407-clock.dtsi
> new file mode 100644
> index 0000000..f50ac6f
> --- /dev/null
> +++ b/arch/arm/boot/dts/stih407-clock.dtsi
> @@ -0,0 +1,41 @@

Going to gloss over this, as you and Srini are the platform experts.

> +/*
> + * Copyright (C) 2013 STMicroelectronics R&D Limited

s/2013/2014

> + * <stlinux-devel@stlinux.com>

Might consider submitting a MAINTAINERS entry for all of ST's DTS(I)
files and removing this from them. Only if this ML is pretty
stable/constant of course.

> + * 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.
> + */
> +/ {
> +	clocks {
> +		/*
> +		 * Fixed 30MHz oscillator inputs to SoC
> +		 */
> +		CLK_SYSIN: CLK_SYSIN {
> +			#clock-cells = <0>;
> +			compatible = "fixed-clock";
> +			clock-frequency = <30000000>;
> +			clock-output-names = "CLK_SYSIN";
> +		};
> +
> +		/*
> +		 * ARM Peripheral clock for timers
> +		 */
> +		arm_periph_clk: arm_periph_clk {
> +			#clock-cells = <0>;
> +			compatible = "fixed-clock";
> +			clock-frequency = <600000000>;
> +		};
> +
> +		/*
> +		 * Bootloader initialized system infrastructure clock for
> +		 * serial devices.
> +		 */
> +		CLK_EXT2F_A9: clockgenC0@13 {

s/@/@0x

> +			#clock-cells = <0>;
> +			compatible = "fixed-clock";
> +			clock-frequency = <200000000>;
> +			clock-output-names = "CLK_S_ICN_REG_0";
> +		};
> +	};
> +};
> diff --git a/arch/arm/boot/dts/stih407-pinctrl.dtsi b/arch/arm/boot/dts/stih407-pinctrl.dtsi
> new file mode 100644
> index 0000000..2d8543e
> --- /dev/null
> +++ b/arch/arm/boot/dts/stih407-pinctrl.dtsi
> @@ -0,0 +1,618 @@
> +/*
> + * Copyright (C) 2013 STMicroelectronics Limited.
> + * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> + *
> + * 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
> + * publishhed by the Free Software Foundation.
> + */
> +#include "st-pincfg.h"
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +/ {
> +
> +	aliases {
> +		/* 0-5: PIO_SBC */
> +		gpio0 = &PIO0;
> +		gpio1 = &PIO1;
> +		gpio2 = &PIO2;
> +		gpio3 = &PIO3;
> +		gpio4 = &PIO4;
> +		gpio5 = &PIO5;
> +		/* 10-19: PIO_FRONT0 */
> +		gpio6 = &PIO10;
> +		gpio7 = &PIO11;
> +		gpio8 = &PIO12;
> +		gpio9 = &PIO13;
> +		gpio10 = &PIO14;
> +		gpio11 = &PIO15;
> +		gpio12 = &PIO16;
> +		gpio13 = &PIO17;
> +		gpio14 = &PIO18;
> +		gpio15 = &PIO19;
> +		/* 20: PIO_FRONT1 */
> +		gpio16 = &PIO20;
> +		/* 30-35: PIO_REAR */
> +		gpio17 = &PIO30;
> +		gpio18 = &PIO31;
> +		gpio19 = &PIO32;
> +		gpio20 = &PIO33;
> +		gpio21 = &PIO34;
> +		gpio22 = &PIO35;
> +		/* 40-42: PIO_FLASH */
> +		gpio23 = &PIO40;
> +		gpio24 = &PIO41;
> +		gpio25 = &PIO42;
> +	};
> +
> +	soc {
> +		pin-controller-sbc {
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			compatible = "st,stih407-sbc-pinctrl";
> +			st,syscfg = <&syscfg_sbc>;
> +			reg = <0x0961f080 0x4>;
> +			reg-names = "irqmux";
> +			interrupts = <GIC_SPI 188 IRQ_TYPE_NONE>;
> +			interrupts-names = "irqmux";
> +			ranges = <0 0x09610000 0x6000>;
> +
> +			PIO0: gpio@09610000 {

s/@/@0x

I won't mention this particular point again, but it needs to be
propagated throughout the patch.

> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x0 0x100>;
> +				st,bank-name = "PIO0";
> +			};
> +			PIO1: gpio@09611000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x1000 0x100>;

Are there really these big gaps in the register space i.e 0x1100 -> 0x2000 etc?

> +				st,bank-name = "PIO1";
> +			};
> +			PIO2: gpio@09612000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x2000 0x100>;
> +				st,bank-name = "PIO2";
> +			};
> +			PIO3: gpio@09613000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x3000 0x100>;
> +				st,bank-name = "PIO3";
> +			};
> +			PIO4: gpio@09614000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x4000 0x100>;
> +				st,bank-name = "PIO4";
> +			};
> +
> +			PIO5: gpio@09615000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x5000 0x100>;
> +				st,bank-name = "PIO5";
> +			};
> +
> +			rc {
> +				pinctrl_ir: ir0 {
> +					st,pins {
> +						ir = <&PIO4 0 ALT2 IN>;
> +					};
> +				};
> +			};
> +
> +			/* SBC_ASC0 - UART10 */
> +			sbc_serial0 {
> +				pinctrl_sbc_serial0: sbc_serial0-0 {
> +					st,pins {
> +						tx = <&PIO3 4 ALT1 OUT>;
> +						rx = <&PIO3 5 ALT1 IN>;
> +					};
> +				};
> +			};
> +			/* SBC_ASC1 - UART11 */
> +			sbc_serial1 {
> +				pinctrl_sbc_serial1: sbc_serial1-0 {
> +					st,pins {
> +						tx = <&PIO2 6 ALT3 OUT>;
> +						rx = <&PIO2 7 ALT3 IN>;
> +					};
> +				};
> +			};
> +
> +			i2c10 {
> +				pinctrl_i2c10_default: i2c10-default {
> +					st,pins {
> +						sda = <&PIO4 6 ALT1 BIDIR>;
> +						scl = <&PIO4 5 ALT1 BIDIR>;
> +					};
> +				};
> +			};
> +
> +			i2c11 {
> +				pinctrl_i2c11_default: i2c11-default {
> +					st,pins {
> +						sda = <&PIO5 1 ALT1 BIDIR>;
> +						scl = <&PIO5 0 ALT1 BIDIR>;
> +					};
> +				};
> +			};
> +
> +			keyscan {
> +				pinctrl_keyscan: keyscan {
> +					st,pins {
> +						keyin0 = <&PIO4 0 ALT6 IN>;
> +						keyin1 = <&PIO4 5 ALT4 IN>;
> +						keyin2 = <&PIO0 4 ALT2 IN>;
> +						keyin3 = <&PIO2 6 ALT2 IN>;
> +
> +						keyout0 = <&PIO4 6 ALT4 OUT>;
> +						keyout1 = <&PIO1 7 ALT2 OUT>;
> +						keyout2 = <&PIO0 6 ALT2 OUT>;
> +						keyout3 = <&PIO2 7 ALT2 OUT>;
> +					};
> +				};
> +			};
> +
> +			gmac1 {
> +				/*
> +				  Almost all the boards based on STiH407 SoC have an embedded
> +				  switch where the mdio/mdc have been used for managing the SMI
> +				  iface via I2C. For this reason these lines can be allocated
> +				  by using dedicated configuration (in case of there will be a
> +				  standard PHY transceiver on-board).
> +				*/

Non-standard multi-line comment. Please fill in the remaining '*'s.

> +				pinctrl_rgmii1: rgmii1-0 {
> +					st,pins {
> +
> +						txd0 = <&PIO0 0 ALT1 OUT DE_IO 0 CLK_A>;
> +						txd1 = <&PIO0 1 ALT1 OUT DE_IO 0 CLK_A>;
> +						txd2 = <&PIO0 2 ALT1 OUT DE_IO 0 CLK_A>;
> +						txd3 = <&PIO0 3 ALT1 OUT DE_IO 0 CLK_A>;
> +						txen = <&PIO0 5 ALT1 OUT DE_IO 0 CLK_A>;
> +						txclk = <&PIO0 6 ALT1 IN NICLK 0 CLK_A>;
> +						rxd0 = <&PIO1 4 ALT1 IN DE_IO 0 CLK_A>;
> +						rxd1 = <&PIO1 5 ALT1 IN DE_IO 0 CLK_A>;
> +						rxd2 = <&PIO1 6 ALT1 IN DE_IO 0 CLK_A>;
> +						rxd3 = <&PIO1 7 ALT1 IN DE_IO 0 CLK_A>;
> +						rxdv = <&PIO2 0 ALT1 IN DE_IO 0 CLK_A>;
> +						rxclk = <&PIO2 2 ALT1 IN NICLK 500 CLK_A>;
> +						clk125 = <&PIO3 7 ALT4 IN NICLK 0 CLK_A>;
> +						phyclk = <&PIO2 3 ALT4 OUT NICLK 1750 CLK_B>;
> +					};
> +				};
> +
> +				pinctrl_rgmii1_mdio: rgmii1-mdio {
> +					st,pins {
> +						mdio = <&PIO1 0 ALT1 OUT BYPASS 0>;
> +						mdc = <&PIO1 1 ALT1 OUT NICLK 0 CLK_A>;
> +						mdint = <&PIO1 3 ALT1 IN BYPASS 0>;
> +					};
> +				};
> +
> +				pinctrl_mii1: mii1 {
> +					st,pins {
> +						txd0 = <&PIO0 0 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
> +						txd1 = <&PIO0 1 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
> +						txd2 = <&PIO0 2 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
> +						txd3 = <&PIO0 3 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
> +						txer = <&PIO0 4 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
> +						txen = <&PIO0 5 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
> +						txclk = <&PIO0 6 ALT1 IN NICLK 0 CLK_A>;
> +						col = <&PIO0 7 ALT1 IN BYPASS 1000>;
> +
> +						mdio = <&PIO1 0 ALT1 OUT BYPASS 1500>;
> +						mdc = <&PIO1 1 ALT1 OUT NICLK 0 CLK_A>;
> +						crs = <&PIO1 2 ALT1 IN BYPASS 1000>;
> +						mdint = <&PIO1 3 ALT1 IN BYPASS 0>;
> +						rxd0 = <&PIO1 4 ALT1 IN SE_NICLK_IO 0 CLK_A>;
> +						rxd1 = <&PIO1 5 ALT1 IN SE_NICLK_IO 0 CLK_A>;
> +						rxd2 = <&PIO1 6 ALT1 IN SE_NICLK_IO 0 CLK_A>;
> +						rxd3 = <&PIO1 7 ALT1 IN SE_NICLK_IO 0 CLK_A>;
> +
> +						rxdv = <&PIO2 0 ALT1 IN SE_NICLK_IO 0 CLK_A>;
> +						rx_er = <&PIO2 1 ALT1 IN SE_NICLK_IO 0 CLK_A>;
> +						rxclk = <&PIO2 2 ALT1 IN NICLK 0 CLK_A>;
> +						phyclk = <&PIO2 3 ALT1 OUT NICLK 0 CLK_A>;
> +					};
> +				};
> +
> +			};

Superflous new line.

> +			pwm1 {
> +				pinctrl_pwm1_chan0_default: pwm1-0-default {
> +					st,pins {
> +						pwm-out = <&PIO3 0 ALT1 OUT>;
> +					};
> +				};
> +				pinctrl_pwm1_chan1_default: pwm1-1-default {
> +					st,pins {
> +						pwm-out = <&PIO4 4 ALT1 OUT>;
> +					};
> +				};
> +				pinctrl_pwm1_chan2_default: pwm1-2-default {
> +					st,pins {
> +						pwm-out = <&PIO4 6 ALT3 OUT>;
> +					};
> +				};
> +				pinctrl_pwm1_chan3_default: pwm1-3-default {
> +					st,pins {
> +						pwm-out = <&PIO4 7 ALT3 OUT>;
> +					};
> +				};
> +			};
> +
> +		};
> +
> +		pin-controller-front0 {
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			compatible = "st,stih407-front-pinctrl";
> +			st,syscfg = <&syscfg_front>;
> +			reg = <0x0920f080 0x4>;
> +			reg-names = "irqmux";
> +			interrupts = <GIC_SPI 189 IRQ_TYPE_NONE>;
> +			interrupts-names = "irqmux";
> +			ranges = <0 0x09200000 0x10000>;
> +
> +			PIO10: PIO@09200000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x0 0x100>;
> +				st,bank-name = "PIO10";
> +			};
> +			PIO11: PIO@09201000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x1000 0x100>;
> +				st,bank-name = "PIO11";
> +			};
> +			PIO12: PIO@09202000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x2000 0x100>;
> +				st,bank-name = "PIO12";
> +			};
> +			PIO13: PIO@09203000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x3000 0x100>;
> +				st,bank-name = "PIO13";
> +			};
> +			PIO14: PIO@09204000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x4000 0x100>;
> +				st,bank-name = "PIO14";
> +			};
> +			PIO15: PIO@09205000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x5000 0x100>;
> +				st,bank-name = "PIO15";
> +			};
> +			PIO16: PIO@09206000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x6000 0x100>;
> +				st,bank-name = "PIO16";
> +			};
> +			PIO17: PIO@09207000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x7000 0x100>;
> +				st,bank-name = "PIO17";
> +			};
> +			PIO18: PIO@09208000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x8000 0x100>;
> +				st,bank-name = "PIO18";
> +			};
> +			PIO19: PIO@09209000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x9000 0x100>;
> +				st,bank-name = "PIO19";
> +			};
> +
> +			/* Comms */
> +			serial0 {
> +				pinctrl_serial0: serial0-0 {
> +					st,pins {
> +						tx = <&PIO17 0 ALT1 OUT>;
> +						rx = <&PIO17 1 ALT1 IN>;
> +					};
> +				};
> +			};
> +
> +			serial1 {
> +				pinctrl_serial1: serial1-0 {
> +					st,pins {
> +						tx = <&PIO16 0 ALT1 OUT>;
> +						rx = <&PIO16 1 ALT1 IN>;
> +					};
> +				};
> +			};
> +
> +			serial2 {
> +				pinctrl_serial2: serial2-0 {
> +					st,pins {
> +						tx = <&PIO15 0 ALT1 OUT>;
> +						rx = <&PIO15 1 ALT1 IN>;
> +					};
> +				};
> +			};
> +
> +			mmc1 {
> +				pinctrl_sd1: sd1-0 {
> +					st,pins {
> +						sd_clk = <&PIO19 3 ALT5 BIDIR NICLK 0 CLK_B>;
> +						sd_cmd = <&PIO19 2 ALT5 BIDIR_PU BYPASS 0>;
> +						sd_dat0 = <&PIO19 4 ALT5 BIDIR_PU BYPASS 0>;
> +						sd_dat1 = <&PIO19 5 ALT5 BIDIR_PU BYPASS 0>;
> +						sd_dat2 = <&PIO19 6 ALT5 BIDIR_PU BYPASS 0>;
> +						sd_dat3 = <&PIO19 7 ALT5 BIDIR_PU BYPASS 0>;
> +						sd_led = <&PIO16 6 ALT6 OUT>;
> +						sd_pwren = <&PIO16 7 ALT6 OUT>;
> +						sd_cd = <&PIO19 0 ALT6 IN>;
> +						sd_wp = <&PIO19 1 ALT6 IN>;
> +					};
> +				};
> +			};
> +
> +
> +			i2c0 {
> +				pinctrl_i2c0_default: i2c0-default {
> +					st,pins {
> +						sda = <&PIO10 6 ALT2 BIDIR>;
> +						scl = <&PIO10 5 ALT2 BIDIR>;
> +					};
> +				};
> +			};
> +
> +			i2c1 {
> +				pinctrl_i2c1_default: i2c1-default {
> +					st,pins {
> +						sda = <&PIO11 1 ALT2 BIDIR>;
> +						scl = <&PIO11 0 ALT2 BIDIR>;
> +					};
> +				};
> +			};
> +
> +			i2c2 {
> +				pinctrl_i2c2_default: i2c2-default {
> +					st,pins {
> +						sda = <&PIO15 6 ALT2 BIDIR>;
> +						scl = <&PIO15 5 ALT2 BIDIR>;
> +					};
> +				};
> +			};
> +
> +			i2c3 {
> +				pinctrl_i2c3_default: i2c3-default {
> +					st,pins {
> +						sda = <&PIO18 6 ALT1 BIDIR>;
> +						scl = <&PIO18 5 ALT1 BIDIR>;
> +					};
> +				};
> +			};
> +
> +			spi0 {
> +				pinctrl_spi0_default: spi0-default {
> +					st,pins {
> +						mtsr = <&PIO12 6 ALT2 BIDIR>;
> +						mrst = <&PIO12 7 ALT2 BIDIR>;
> +						scl = <&PIO12 5 ALT2 BIDIR>;
> +					};
> +				};
> +			};
> +		};
> +
> +		pin-controller-front1 {
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			compatible = "st,stih407-front-pinctrl";
> +			st,syscfg = <&syscfg_front>;
> +			reg = <0x0921f080 0x4>;
> +			reg-names = "irqmux";
> +			interrupts = <GIC_SPI 190 IRQ_TYPE_NONE>;
> +			interrupts-names = "irqmux";
> +			ranges = <0 0x09210000 0x10000>;
> +
> +			PIO20: PIO@09210000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x0 0x100>;
> +				st,bank-name = "PIO20";
> +			};
> +		};
> +
> +		pin-controller-rear {
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			compatible = "st,stih407-rear-pinctrl";
> +			st,syscfg = <&syscfg_rear>;
> +			reg = <0x0922f080 0x4>;
> +			reg-names = "irqmux";
> +			interrupts = <GIC_SPI 191 IRQ_TYPE_NONE>;
> +			interrupts-names = "irqmux";
> +			ranges = <0 0x09220000 0x6000>;
> +
> +			PIO30: gpio@09220000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x0 0x100>;
> +				st,bank-name = "PIO30";
> +			};
> +			PIO31: gpio@09221000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x1000 0x100>;
> +				st,bank-name = "PIO31";
> +			};
> +			PIO32: gpio@09222000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x2000 0x100>;
> +				st,bank-name = "PIO32";
> +			};
> +			PIO33: gpio@09223000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x3000 0x100>;
> +				st,bank-name = "PIO33";
> +			};
> +			PIO34: gpio@09224000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x4000 0x100>;
> +				st,bank-name = "PIO34";
> +			};
> +			PIO35: gpio@09225000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x5000 0x100>;
> +				st,bank-name = "PIO35";
> +			};
> +
> +			i2c4 {
> +				pinctrl_i2c4_default: i2c4-default {
> +					st,pins {
> +						sda = <&PIO30 1 ALT1 BIDIR>;
> +						scl = <&PIO30 0 ALT1 BIDIR>;
> +					};
> +				};
> +			};
> +
> +			i2c5 {
> +				pinctrl_i2c5_default: i2c5-default {
> +					st,pins {
> +						sda = <&PIO34 4 ALT1 BIDIR>;
> +						scl = <&PIO34 3 ALT1 BIDIR>;
> +					};
> +				};
> +			};
> +
> +			usb3 {
> +				pinctrl_usb3: usb3-2 {
> +					st,pins {
> +						usb-oc-detect = <&PIO35 4 ALT1 IN>;
> +						usb-pwr-enable = <&PIO35 5 ALT1 OUT>;
> +						usb-vbus-valid = <&PIO35 6 ALT1 IN>;
> +					};
> +				};
> +			};
> +
> +			pwm0 {
> +				pinctrl_pwm0_chan0_default: pwm0-0-default {
> +					st,pins {
> +						pwm-out = <&PIO31 1 ALT1 OUT>;
> +					};
> +				};
> +			};
> +
> +		};

Extra new line above.

> +		pin-controller-flash {
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			compatible = "st,stih407-flash-pinctrl";
> +			st,syscfg = <&syscfg_flash>;
> +			reg = <0x0923f080 0x4>;
> +			reg-names = "irqmux";
> +			interrupts = <GIC_SPI 192 IRQ_TYPE_NONE>;
> +			interrupts-names = "irqmux";
> +			ranges = <0 0x09230000 0x3000>;
> +
> +			PIO40: gpio@09230000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0 0x100>;
> +				st,bank-name = "PIO40";
> +			};
> +			PIO41: gpio@09231000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x1000 0x100>;
> +				st,bank-name = "PIO41";
> +			};
> +			PIO42: gpio@09232000 {
> +				gpio-controller;
> +				#gpio-cells = <1>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				reg = <0x2000 0x100>;
> +				st,bank-name = "PIO42";
> +			};
> +
> +			mmc0 {
> +				pinctrl_mmc0: mmc0-0 {
> +					st,pins {
> +						emmc_clk = <&PIO40 6 ALT1 BIDIR>;
> +						emmc_cmd = <&PIO40 7 ALT1 BIDIR_PU>;
> +						emmc_d0 = <&PIO41 0 ALT1 BIDIR_PU>;
> +						emmc_d1 = <&PIO41 1 ALT1 BIDIR_PU>;
> +						emmc_d2 = <&PIO41 2 ALT1 BIDIR_PU>;
> +						emmc_d3 = <&PIO41 3 ALT1 BIDIR_PU>;
> +						emmc_d4 = <&PIO41 4 ALT1 BIDIR_PU>;
> +						emmc_d5 = <&PIO41 5 ALT1 BIDIR_PU>;
> +						emmc_d6 = <&PIO41 6 ALT1 BIDIR_PU>;
> +						emmc_d7 = <&PIO41 7 ALT1 BIDIR_PU>;
> +					};
> +				};
> +			};
> +		};
> +	};
> +};
> diff --git a/arch/arm/boot/dts/stih407.dtsi b/arch/arm/boot/dts/stih407.dtsi
> new file mode 100644
> index 0000000..2a0566b
> --- /dev/null
> +++ b/arch/arm/boot/dts/stih407.dtsi
> @@ -0,0 +1,250 @@
> +/*
> + * Copyright (C) 2013 STMicroelectronics Limited.
> + * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> + *
> + * 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
> + * publishhed by the Free Software Foundation.
> + */
> +#include "stih407-clock.dtsi"
> +#include "stih407-pinctrl.dtsi"
> +/ {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a9";
> +			reg = <0>;
> +		};
> +		cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a9";
> +			reg = <1>;
> +		};
> +	};
> +
> +	intc: interrupt-controller@08761000 {
> +		compatible = "arm,cortex-a9-gic";
> +		#interrupt-cells = <3>;
> +		interrupt-controller;
> +		reg = <0x08761000 0x1000>, <0x08760100 0x100>;
> +	};
> +
> +	scu@08760000 {
> +		compatible = "arm,cortex-a9-scu";
> +		reg = <0x08760000 0x1000>;
> +	};
> +
> +	timer@08760200 {
> +		interrupt-parent = <&intc>;
> +		compatible = "arm,cortex-a9-global-timer";
> +		reg = <0x08760200 0x100>;
> +		interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&arm_periph_clk>;
> +	};
> +
> +	L2: cache-controller {
> +		compatible = "arm,pl310-cache";
> +		reg = <0x08762000 0x1000>;
> +		arm,data-latency = <3 3 3>;
> +		arm,tag-latency = <2 2 2>;
> +		cache-unified;
> +		cache-level = <2>;
> +	};
> +
> +	soc {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		interrupt-parent = <&intc>;
> +		ranges;
> +		compatible = "simple-bus";
> +
> +		syscfg_sbc:sbc-syscfg@9620000{

Space after colon and another after the address.

Same for all of the nodes below and throughout the patch.

> +			compatible = "st,stih407-sbc-syscfg", "syscon";
> +			reg = <0x9620000 0x1000>;
> +		};
> +
> +		syscfg_front:front-syscfg@9280000{
> +			compatible = "st,stih407-front-syscfg", "syscon";
> +			reg = <0x9280000 0x1000>;
> +		};
> +
> +		syscfg_rear:rear-syscfg@9290000{
> +			compatible = "st,stih407-rear-syscfg", "syscon";
> +			reg = <0x9290000 0x1000>;
> +		};
> +
> +		syscfg_flash:flash-syscfg@92a0000{
> +			compatible = "st,stih407-flash-syscfg", "syscon";
> +			reg = <0x92a0000 0x1000>;
> +		};
> +
> +		syscfg_sbc_reg:fvdp-lite-syscfg@9600000{
> +			compatible = "st,stih407-sbc-reg-syscfg", "syscon";
> +			reg = <0x9600000 0x1000>;
> +		};
> +
> +		syscfg_core:core-syscfg@92b0000{
> +			compatible = "st,stih407-core-syscfg", "syscon";
> +			reg = <0x92b0000 0x1000>;
> +		};
> +
> +		syscfg_lpm:lpm-syscfg@94b5100{
> +			compatible = "st,stih407-lpm-syscfg", "syscon";
> +			reg = <0x94b5100 0x1000>;
> +		};
> +
> +		serial@9830000{
> +			compatible = "st,asc";
> +			status = "disabled";

This might just be a personal thing, but I prefer to see the status at
the base of the node with a new line above it, as it is only relevant
to the node rather than the device.

> +			reg = <0x9830000 0x2c>;
> +			interrupts = <GIC_SPI 122 IRQ_TYPE_NONE>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_serial0>;
> +			clocks = <&CLK_EXT2F_A9>;
> +		};

Like this:
	serial@9830000{
		compatible = "st,asc";
		reg = <0x9830000 0x2c>;
		interrupts = <GIC_SPI 122 IRQ_TYPE_NONE>;
		pinctrl-names = "default";
		pinctrl-0 = <&pinctrl_serial0>;
		clocks = <&CLK_EXT2F_A9>;

		status = "disabled";
	};

> +
> +		serial@9831000{
> +			compatible = "st,asc";
> +			status = "disabled";
> +			reg = <0x9831000 0x2c>;
> +			interrupts = <GIC_SPI 123 IRQ_TYPE_NONE>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_serial1>;
> +			clocks = <&CLK_EXT2F_A9>;
> +		};
> +
> +		serial@9832000{
> +			compatible = "st,asc";
> +			status = "disabled";
> +			reg = <0x9832000 0x2c>;
> +			interrupts = <GIC_SPI 124 IRQ_TYPE_NONE>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_serial2>;
> +			clocks = <&CLK_EXT2F_A9>;
> +		};
> +
> +		/* SBC_ASC0 - UART10 */
> +		sbc_serial0: serial@9530000 {
> +			compatible = "st,asc";
> +			status = "disabled";
> +			reg = <0x9530000 0x2c>;
> +			interrupts = <GIC_SPI 138 IRQ_TYPE_NONE>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_sbc_serial0>;
> +			clocks = <&CLK_SYSIN>;
> +		};
> +
> +		serial@9531000 {
> +			compatible = "st,asc";
> +			status = "disabled";
> +			reg = <0x9531000 0x2c>;
> +			interrupts = <GIC_SPI 139 IRQ_TYPE_NONE>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_sbc_serial1>;
> +			clocks = <&CLK_SYSIN>;
> +		};
> +
> +		i2c@9840000 {
> +			compatible = "st,comms-ssc4-i2c";
> +			status = "disabled";
> +			interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
> +			reg = <0x9840000 0x110>;
> +			clocks = <&CLK_EXT2F_A9>;
> +			clock-names = "ssc";
> +			clock-frequency = <400000>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_i2c0_default>;
> +		};
> +
> +		i2c@9841000 {
> +			compatible = "st,comms-ssc4-i2c";
> +			status = "disabled";
> +			reg = <0x9841000 0x110>;
> +			interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&CLK_EXT2F_A9>;
> +			clock-names = "ssc";
> +			clock-frequency = <400000>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_i2c1_default>;
> +		};
> +
> +		i2c@9842000 {
> +			compatible = "st,comms-ssc4-i2c";
> +			status = "disabled";
> +			reg = <0x9842000 0x110>;
> +			interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&CLK_EXT2F_A9>;
> +			clock-names = "ssc";
> +			clock-frequency = <400000>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_i2c2_default>;
> +		};
> +
> +		i2c@9843000 {
> +			compatible = "st,comms-ssc4-i2c";
> +			status = "disabled";
> +			reg = <0x9843000 0x110>;
> +			interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&CLK_EXT2F_A9>;
> +			clock-names = "ssc";
> +			clock-frequency = <400000>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_i2c3_default>;
> +		};
> +
> +		i2c@9844000 {
> +			compatible = "st,comms-ssc4-i2c";
> +			status = "disabled";
> +			reg = <0x9844000 0x110>;
> +			interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&CLK_EXT2F_A9>;
> +			clock-names = "ssc";
> +			clock-frequency = <400000>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_i2c4_default>;
> +		};
> +
> +		i2c@9845000 {
> +			compatible = "st,comms-ssc4-i2c";
> +			status = "disabled";
> +			reg = <0x9845000 0x110>;
> +			interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&CLK_EXT2F_A9>;
> +			clock-names = "ssc";
> +			clock-frequency = <400000>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_i2c5_default>;
> +		};
> +
> +
> +		/* SSCs on SBC */
> +		i2c@9540000 {
> +			compatible = "st,comms-ssc4-i2c";
> +			status = "disabled";
> +			reg = <0x9540000 0x110>;
> +			interrupts = <GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&CLK_SYSIN>;
> +			clock-names = "ssc";
> +			clock-frequency = <400000>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_i2c10_default>;
> +		};
> +
> +		i2c@9541000 {
> +			compatible = "st,comms-ssc4-i2c";
> +			status = "disabled";
> +			reg = <0x9541000 0x110>;
> +			interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&CLK_SYSIN>;
> +			clock-names = "ssc";
> +			clock-frequency = <400000>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_i2c11_default>;
> +		};
> +	};
> +};

Not sure if this is Git playing around again, but the };s look
misaligned in the submission.

Wow, this is good work and a lot of code!

Once my comments have been rectified, feel free to add my:
  Acked-by: Lee Jones <lee.jones@linaro.org> 

-- 
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] 21+ messages in thread

* Re: [PATCH v3 1/5] ARM: STi: Add STiH407 SoC support
  2014-03-07 14:40 ` [PATCH v3 1/5] ARM: STi: Add STiH407 SoC support Maxime COQUELIN
@ 2014-03-10 12:34   ` Lee Jones
  2014-03-11 13:56     ` Maxime Coquelin
  0 siblings, 1 reply; 21+ messages in thread
From: Lee Jones @ 2014-03-10 12:34 UTC (permalink / raw)
  To: Maxime COQUELIN
  Cc: Rob Landley, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Russell King, Srinivas Kandagatla, Stuart Menefy,
	Linus Walleij, Giuseppe Cavallaro, linux-doc, linux-kernel,
	devicetree, linux-arm-kernel, kernel

> This patch adds support to STiH407 SoC.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> ---
>  Documentation/arm/sti/stih407-overview.txt    | 18 ++++++++++++++++++
>  Documentation/devicetree/bindings/arm/sti.txt | 15 +++++++++++++++
>  arch/arm/mach-sti/Kconfig                     |  9 +++++++++
>  arch/arm/mach-sti/board-dt.c                  |  1 +
>  4 files changed, 43 insertions(+)
>  create mode 100644 Documentation/arm/sti/stih407-overview.txt
>  create mode 100644 Documentation/devicetree/bindings/arm/sti.txt

<snip>

> +config SOC_STIH407
> +	bool "STiH407 STMicroelectronics Consumer Electronics family"
> +	default y
> +	help
> +	  This enables support for STMicroelectronics Digital Consumer
> +	  Electronics family StiH407 parts, primarily targeted at set-top-box
> +	  and other digital audio/video applications using Flattened Device
> +	  Trees.
> +

Are these configs actually used for something?

<snip>

-- 
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] 21+ messages in thread

* Re: [PATCH v3 5/5] ARM: dts: STiH407: Add B2120 board support
  2014-03-07 14:40 ` [PATCH v3 5/5] ARM: dts: STiH407: Add B2120 board support Maxime COQUELIN
@ 2014-03-10 12:37   ` Lee Jones
  0 siblings, 0 replies; 21+ messages in thread
From: Lee Jones @ 2014-03-10 12:37 UTC (permalink / raw)
  To: Maxime COQUELIN
  Cc: Rob Landley, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Russell King, Srinivas Kandagatla, Stuart Menefy,
	Linus Walleij, Giuseppe Cavallaro, linux-doc, linux-kernel,
	devicetree, linux-arm-kernel, kernel

> B2120 HDK is the reference board for STiH407 SoC.
> It has the following characteristics:
>  - 1GB DDR3
>  - 8GB eMMC / SD-Card slot
>  - 32MB NOR Flash
>  - 1 x Gbit Ethernet
>  - 1 x USB 3.0 port
>  - 1 x Mini-PCIe
>  - 1 x SATA
>  - 1 x HDMI output
>  - 1 x HDMI input
>  - 1 x SPDIF
> 
> This patch only introduces basic functionnalities, such as I2C and UART.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> ---
>  arch/arm/boot/dts/Makefile          |  3 +-
>  arch/arm/boot/dts/stih407-b2120.dts | 78 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 80 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boot/dts/stih407-b2120.dts

Patch looks pretty inert:
  Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
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] 21+ messages in thread

* Re: [PATCH v3 3/5] pinctrl: st: add pinctrl support for the STiH407 SoC
  2014-03-07 14:40 ` [PATCH v3 3/5] pinctrl: st: add pinctrl support for the STiH407 SoC Maxime COQUELIN
  2014-03-07 15:36   ` srinivas kandagatla
@ 2014-03-10 14:44   ` Lee Jones
  2014-03-11  8:52     ` Maxime Coquelin
  1 sibling, 1 reply; 21+ messages in thread
From: Lee Jones @ 2014-03-10 14:44 UTC (permalink / raw)
  To: Maxime COQUELIN
  Cc: Rob Landley, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Russell King, Srinivas Kandagatla, Stuart Menefy,
	Linus Walleij, Giuseppe Cavallaro, linux-doc, linux-kernel,
	devicetree, linux-arm-kernel, kernel

> From: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> 
> This patch adds the initial support for pinctrl based on H407 SoC.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>

SOBs should be in order of submission flow and author's should only be
kept if the patch is unchanged. If it's been adapted you will require
a re-review and they Acked-by or Reviewed-by tag.

> ---
>  drivers/pinctrl/pinctrl-st.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
> index 1721611..d1886b4 100644
> --- a/drivers/pinctrl/pinctrl-st.c
> +++ b/drivers/pinctrl/pinctrl-st.c
> @@ -390,6 +390,19 @@ static const struct st_pctl_data  stih416_data = {
>  	.alt = 0, .oe = 40, .pu = 50, .od = 60, .rt = 100,
>  };
>  
> +static const struct st_pctl_data  stih407_flashdata = {

Nit: Double space.

> +	.rt_style	= st_retime_style_none,
> +	.input_delays	= stih416_delays,
> +	.ninput_delays	= 14,

ARRAY_SIZE(stih416_delays)

> +	.output_delays	= stih416_delays,
> +	.noutput_delays = 14,

ARRAY_SIZE(stih416_delays)

> +	.alt = 0,
> +	.oe = -1, /* Not Available */
> +	.pu = -1, /* Not Available */
> +	.od = 60,
> +	.rt = 100,
> +};
> +
>  /* Low level functions.. */
>  static inline int st_gpio_bank(int gpio)
>  {
> @@ -1596,6 +1609,10 @@ static struct of_device_id st_pctl_of_match[] = {
>  	{ .compatible = "st,stih416-rear-pinctrl", .data = &stih416_data},
>  	{ .compatible = "st,stih416-fvdp-fe-pinctrl", .data = &stih416_data},
>  	{ .compatible = "st,stih416-fvdp-lite-pinctrl", .data = &stih416_data},
> +	{ .compatible = "st,stih407-sbc-pinctrl", .data = &stih416_data},
> +	{ .compatible = "st,stih407-front-pinctrl", .data = &stih416_data},
> +	{ .compatible = "st,stih407-rear-pinctrl", .data = &stih416_data},
> +	{ .compatible = "st,stih407-flash-pinctrl", .data = &stih407_flashdata},
>  	{ /* sentinel */ }
>  };

After the small changes above, feel free to add my:
  Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
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] 21+ messages in thread

* Re: [PATCH v3 4/5] ARM: dts: Add STiH407 SoC support
  2014-03-10 12:28   ` Lee Jones
@ 2014-03-10 16:16     ` Lee Jones
  2014-03-11 10:09     ` Maxime Coquelin
  1 sibling, 0 replies; 21+ messages in thread
From: Lee Jones @ 2014-03-10 16:16 UTC (permalink / raw)
  To: Maxime COQUELIN
  Cc: Rob Landley, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Russell King, Srinivas Kandagatla, Stuart Menefy,
	Linus Walleij, Giuseppe Cavallaro, linux-doc, linux-kernel,
	devicetree, linux-arm-kernel, kernel

> > +		CLK_EXT2F_A9: clockgenC0@13 {
> 
> s/@/@0x

<snip>

> > +			PIO0: gpio@09610000 {
> 
> s/@/@0x
> 
> I won't mention this particular point again, but it needs to be
> propagated throughout the patch.

Whoops, I'm actually completely wrong on this point! There is to be
_no_ 0x prefix prior to the address, as per the following commits: 

`git log --author="Lee Jones" --grep="0x" mainline/master -- arch/arm/boot/dts/`

-- 
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] 21+ messages in thread

* Re: [PATCH v3 2/5] pinctrl: st: Enhance the controller to manage unavailable registers
  2014-03-10  9:17   ` Lee Jones
@ 2014-03-11  8:18     ` Maxime Coquelin
  2014-03-11 10:55       ` Maxime Coquelin
  2014-03-11 11:12       ` Lee Jones
  0 siblings, 2 replies; 21+ messages in thread
From: Maxime Coquelin @ 2014-03-11  8:18 UTC (permalink / raw)
  To: Lee Jones
  Cc: Rob Landley, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Russell King, Srinivas Kandagatla, Stuart Menefy,
	Linus Walleij, Giuseppe Cavallaro, linux-doc, linux-kernel,
	devicetree, linux-arm-kernel, kernel



On 03/10/2014 10:17 AM, Lee Jones wrote:
>> From: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>>
>> This patch adds a new logic inside the st pinctrl to manage
>> an unsupported scenario: some sysconfig are not available!
>>
>> This is the case of STiH407 where, although documented, the
>> following registers from SYSCFG_FLASH have been removed from the SoC.
>>
>> SYSTEM_CONFIG3040
>>     Output Enable pad control for all PIO Alternate Functions
>> and
>> SYSTEM_ CONFIG3050
>>     Pull Up pad control for all PIO Alternate Functions
>>
>> Without managing this condition an imprecise external abort
>> will be detect.
>>
>> To do this the patch also reviews the st_parse_syscfgs
>> and other routines to manipulate the registers only if
>> actually available.
>> In any case, for example the st_parse_syscfgs detected
>> an error condition but no action was made in the
>> st_pctl_probe_dt.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
>> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>
> These two SOBs need reordering.
Right,
this will be changed here and everywhere else in the series.

>
>> ---
>>   drivers/pinctrl/pinctrl-st.c | 106 +++++++++++++++++++++++++------------------
>>   1 file changed, 61 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
>> index 9fb66aa..1721611 100644
>> --- a/drivers/pinctrl/pinctrl-st.c
>> +++ b/drivers/pinctrl/pinctrl-st.c
>> @@ -410,25 +410,27 @@ static void st_pinconf_set_config(struct st_pio_control *pc,
>>   	unsigned int oe_value, pu_value, od_value;
>>   	unsigned long mask = BIT(pin);
>>
>> -	regmap_field_read(output_enable, &oe_value);
>> -	regmap_field_read(pull_up, &pu_value);
>> -	regmap_field_read(open_drain, &od_value);
>> -
>> -	/* Clear old values */
>> -	oe_value &= ~mask;
>> -	pu_value &= ~mask;
>> -	od_value &= ~mask;
>> -
>> -	if (config & ST_PINCONF_OE)
>> -		oe_value |= mask;
>> -	if (config & ST_PINCONF_PU)
>> -		pu_value |= mask;
>> -	if (config & ST_PINCONF_OD)
>> -		od_value |= mask;
>> -
>> -	regmap_field_write(output_enable, oe_value);
>> -	regmap_field_write(pull_up, pu_value);
>> -	regmap_field_write(open_drain, od_value);
>> +	if (output_enable) {
>> +		regmap_field_read(output_enable, &oe_value);
>> +		oe_value &= ~mask;
>> +		if (config & ST_PINCONF_OE)
>> +			oe_value |= mask;
>> +		regmap_field_write(output_enable, oe_value);
>> +	}
>> +	if (pull_up) {
>> +		regmap_field_read(pull_up, &pu_value);
>> +		pu_value &= ~mask;
>> +		if (config & ST_PINCONF_PU)
>> +			pu_value |= mask;
>> +		regmap_field_write(pull_up, pu_value);
>> +	}
>> +	if (open_drain) {
>> +		regmap_field_read(open_drain, &od_value);
>> +		od_value &= ~mask;
>> +		if (config & ST_PINCONF_OD)
>> +			od_value |= mask;
>> +		regmap_field_write(open_drain, od_value);
>> +	}
>
> Nice change.
>
> Nit: For consistency with the changes below, please consider placing
> new lines between the 3 outer checks.
Done

>
>>   }
>>
>
> <snip>
>
>> -static void st_pinconf_get_direction(struct st_pio_control *pc,
>> -	int pin, unsigned long *config)
>> +static void st_pinconf_get_direction(struct st_pio_control *pc, int pin,
>> +				     unsigned long *config)
>
> Unrelated change?
Yes this is unrelated.
I removed this change. It will be sent later with maybe other cosmetic 
changes.

>
>>   {
>>   	unsigned int oe_value, pu_value, od_value;
>
> Is it worth checking for (!config) here?

That would be better indeed.
But since that lack of safety check was already present before this 
change, it should be handled in a separate patch.

I'll add this in my todo list.

>
>> -	regmap_field_read(pc->oe, &oe_value);
>> -	regmap_field_read(pc->pu, &pu_value);
>> -	regmap_field_read(pc->od, &od_value);
>> +	if (pc->oe) {
>> +		regmap_field_read(pc->oe, &oe_value);
>> +		if (oe_value & BIT(pin))
>> +			ST_PINCONF_PACK_OE(*config);
>> +	}
>>
>> -	if (oe_value & BIT(pin))
>> -		ST_PINCONF_PACK_OE(*config);
>> -	if (pu_value & BIT(pin))
>> -		ST_PINCONF_PACK_PU(*config);
>> -	if (od_value & BIT(pin))
>> -		ST_PINCONF_PACK_OD(*config);
>> +	if (pc->pu) {
>> +		regmap_field_read(pc->pu, &pu_value);
>> +		if (pu_value & BIT(pin))
>> +			ST_PINCONF_PACK_PU(*config);
>> +	}
>>
>> +	if (pc->od) {
>> +		regmap_field_read(pc->od, &od_value);
>> +		if (od_value & BIT(pin))
>> +			ST_PINCONF_PACK_OD(*config);
>> +	}
>>   }
>
> Nice.
>
>>   static int st_pinconf_get_retime_packed(struct st_pinctrl *info,
>> @@ -1105,8 +1116,21 @@ static int st_pctl_dt_setup_retime(struct st_pinctrl *info,
>>   	return -EINVAL;
>>   }
>>
>> -static int st_parse_syscfgs(struct st_pinctrl *info,
>> -		int bank, struct device_node *np)
>> +
>> +static struct regmap_field *st_pc_get_value(struct device *dev,
>> +					    struct regmap *regmap, int bank,
>> +					    int data, int lsb, int msb)
>> +{
>> +	struct reg_field reg = REG_FIELD((data + bank) * 4, lsb, msb);
>> +
>> +	if (data < 0)
>> +		return NULL;
>
> What happens is data < 0 and it's used in REG_FIELD?
Nothing bad, but I agree this is not crystal clear.

>
> Would it make more sense to make this check before calling REG_FIELD?
Yes, it will be done in the v4.

<snip>

Thanks,
Maxime

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

* Re: [PATCH v3 3/5] pinctrl: st: add pinctrl support for the STiH407 SoC
  2014-03-10 14:44   ` Lee Jones
@ 2014-03-11  8:52     ` Maxime Coquelin
  0 siblings, 0 replies; 21+ messages in thread
From: Maxime Coquelin @ 2014-03-11  8:52 UTC (permalink / raw)
  To: Lee Jones
  Cc: Rob Landley, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Russell King, Srinivas Kandagatla, Stuart Menefy,
	Linus Walleij, Giuseppe Cavallaro, linux-doc, linux-kernel,
	devicetree, linux-arm-kernel, kernel



On 03/10/2014 03:44 PM, Lee Jones wrote:
>> From: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>>
>> This patch adds the initial support for pinctrl based on H407 SoC.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
>> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>
> SOBs should be in order of submission flow and author's should only be
> kept if the patch is unchanged. If it's been adapted you will require
> a re-review and they Acked-by or Reviewed-by tag.
>
>> ---
>>   drivers/pinctrl/pinctrl-st.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
>> index 1721611..d1886b4 100644
>> --- a/drivers/pinctrl/pinctrl-st.c
>> +++ b/drivers/pinctrl/pinctrl-st.c
>> @@ -390,6 +390,19 @@ static const struct st_pctl_data  stih416_data = {
>>   	.alt = 0, .oe = 40, .pu = 50, .od = 60, .rt = 100,
>>   };
>>
>> +static const struct st_pctl_data  stih407_flashdata = {
>
> Nit: Double space.
Fixed in upcoming v4.

>
>> +	.rt_style	= st_retime_style_none,
>> +	.input_delays	= stih416_delays,
>> +	.ninput_delays	= 14,
>
> ARRAY_SIZE(stih416_delays)
>
>> +	.output_delays	= stih416_delays,
>> +	.noutput_delays = 14,
>
> ARRAY_SIZE(stih416_delays)
Fixed too.

V4 will contain one more patch to fix this also in stih416_data.

>
>> +	.alt = 0,
>> +	.oe = -1, /* Not Available */
>> +	.pu = -1, /* Not Available */
>> +	.od = 60,
>> +	.rt = 100,
>> +};
>> +
>>   /* Low level functions.. */
>>   static inline int st_gpio_bank(int gpio)
>>   {
>> @@ -1596,6 +1609,10 @@ static struct of_device_id st_pctl_of_match[] = {
>>   	{ .compatible = "st,stih416-rear-pinctrl", .data = &stih416_data},
>>   	{ .compatible = "st,stih416-fvdp-fe-pinctrl", .data = &stih416_data},
>>   	{ .compatible = "st,stih416-fvdp-lite-pinctrl", .data = &stih416_data},
>> +	{ .compatible = "st,stih407-sbc-pinctrl", .data = &stih416_data},
>> +	{ .compatible = "st,stih407-front-pinctrl", .data = &stih416_data},
>> +	{ .compatible = "st,stih407-rear-pinctrl", .data = &stih416_data},
>> +	{ .compatible = "st,stih407-flash-pinctrl", .data = &stih407_flashdata},
>>   	{ /* sentinel */ }
>>   };
>
> After the small changes above, feel free to add my:
>    Acked-by: Lee Jones <lee.jones@linaro.org>

Added.

Thanks,
Maxime
>

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

* Re: [PATCH v3 4/5] ARM: dts: Add STiH407 SoC support
  2014-03-10 12:28   ` Lee Jones
  2014-03-10 16:16     ` Lee Jones
@ 2014-03-11 10:09     ` Maxime Coquelin
  1 sibling, 0 replies; 21+ messages in thread
From: Maxime Coquelin @ 2014-03-11 10:09 UTC (permalink / raw)
  To: Lee Jones
  Cc: Rob Landley, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Russell King, Srinivas Kandagatla, Stuart Menefy,
	Linus Walleij, Giuseppe Cavallaro, linux-doc, linux-kernel,
	devicetree, linux-arm-kernel, kernel



On 03/10/2014 01:28 PM, Lee Jones wrote:
> On Fri, 07 Mar 2014, Maxime COQUELIN wrote:
>
>> The STiH407 is advanced multi-HD AVC processor with 3D graphics acceleration
>> and 1.5-GHz ARM Cortex-A9 SMP CPU.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
>> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>> ---
>>   arch/arm/boot/dts/stih407-clock.dtsi   |  41 +++
>>   arch/arm/boot/dts/stih407-pinctrl.dtsi | 618 +++++++++++++++++++++++++++++++++
>>   arch/arm/boot/dts/stih407.dtsi         | 250 +++++++++++++
>>   3 files changed, 909 insertions(+)
>>   create mode 100644 arch/arm/boot/dts/stih407-clock.dtsi
>>   create mode 100644 arch/arm/boot/dts/stih407-pinctrl.dtsi
>>   create mode 100644 arch/arm/boot/dts/stih407.dtsi
>>
>> diff --git a/arch/arm/boot/dts/stih407-clock.dtsi b/arch/arm/boot/dts/stih407-clock.dtsi
>> new file mode 100644
>> index 0000000..f50ac6f
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/stih407-clock.dtsi
>> @@ -0,0 +1,41 @@
>
> Going to gloss over this, as you and Srini are the platform experts.
>
>> +/*
>> + * Copyright (C) 2013 STMicroelectronics R&D Limited
>
> s/2013/2014
>
>> + * <stlinux-devel@stlinux.com>
>
> Might consider submitting a MAINTAINERS entry for all of ST's DTS(I)
> files and removing this from them. Only if this ML is pretty
> stable/constant of course.

I prefer removing this ML.
The right one is kernel@stlinux.com as specified in the MAINTAINERS file.

ST's DTS files are being added to MAINTAINERS file with this patch from 
Srini: http://marc.info/?l=linux-arm-kernel&m=139384666716614&w=2


>

<snip>

>> +			PIO1: gpio@09611000 {
>> +				gpio-controller;
>> +				#gpio-cells = <1>;
>> +				interrupt-controller;
>> +				#interrupt-cells = <2>;
>> +				reg = <0x1000 0x100>;
>
> Are there really these big gaps in the register space i.e 0x1100 -> 0x2000 etc?

Yes.

>

<snip>

>> +
>> +			gmac1 {
>> +				/*
>> +				  Almost all the boards based on STiH407 SoC have an embedded
>> +				  switch where the mdio/mdc have been used for managing the SMI
>> +				  iface via I2C. For this reason these lines can be allocated
>> +				  by using dedicated configuration (in case of there will be a
>> +				  standard PHY transceiver on-board).
>> +				*/
>
> Non-standard multi-line comment. Please fill in the remaining '*'s.
Right, this will be fixed in v4.

>
>> +				pinctrl_rgmii1: rgmii1-0 {
>> +					st,pins {
>> +
>> +						txd0 = <&PIO0 0 ALT1 OUT DE_IO 0 CLK_A>;
>> +						txd1 = <&PIO0 1 ALT1 OUT DE_IO 0 CLK_A>;
>> +						txd2 = <&PIO0 2 ALT1 OUT DE_IO 0 CLK_A>;
>> +						txd3 = <&PIO0 3 ALT1 OUT DE_IO 0 CLK_A>;
>> +						txen = <&PIO0 5 ALT1 OUT DE_IO 0 CLK_A>;
>> +						txclk = <&PIO0 6 ALT1 IN NICLK 0 CLK_A>;
>> +						rxd0 = <&PIO1 4 ALT1 IN DE_IO 0 CLK_A>;
>> +						rxd1 = <&PIO1 5 ALT1 IN DE_IO 0 CLK_A>;
>> +						rxd2 = <&PIO1 6 ALT1 IN DE_IO 0 CLK_A>;
>> +						rxd3 = <&PIO1 7 ALT1 IN DE_IO 0 CLK_A>;
>> +						rxdv = <&PIO2 0 ALT1 IN DE_IO 0 CLK_A>;
>> +						rxclk = <&PIO2 2 ALT1 IN NICLK 500 CLK_A>;
>> +						clk125 = <&PIO3 7 ALT4 IN NICLK 0 CLK_A>;
>> +						phyclk = <&PIO2 3 ALT4 OUT NICLK 1750 CLK_B>;
>> +					};
>> +				};
>> +
>> +				pinctrl_rgmii1_mdio: rgmii1-mdio {
>> +					st,pins {
>> +						mdio = <&PIO1 0 ALT1 OUT BYPASS 0>;
>> +						mdc = <&PIO1 1 ALT1 OUT NICLK 0 CLK_A>;
>> +						mdint = <&PIO1 3 ALT1 IN BYPASS 0>;
>> +					};
>> +				};
>> +
>> +				pinctrl_mii1: mii1 {
>> +					st,pins {
>> +						txd0 = <&PIO0 0 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
>> +						txd1 = <&PIO0 1 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
>> +						txd2 = <&PIO0 2 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
>> +						txd3 = <&PIO0 3 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
>> +						txer = <&PIO0 4 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
>> +						txen = <&PIO0 5 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
>> +						txclk = <&PIO0 6 ALT1 IN NICLK 0 CLK_A>;
>> +						col = <&PIO0 7 ALT1 IN BYPASS 1000>;
>> +
>> +						mdio = <&PIO1 0 ALT1 OUT BYPASS 1500>;
>> +						mdc = <&PIO1 1 ALT1 OUT NICLK 0 CLK_A>;
>> +						crs = <&PIO1 2 ALT1 IN BYPASS 1000>;
>> +						mdint = <&PIO1 3 ALT1 IN BYPASS 0>;
>> +						rxd0 = <&PIO1 4 ALT1 IN SE_NICLK_IO 0 CLK_A>;
>> +						rxd1 = <&PIO1 5 ALT1 IN SE_NICLK_IO 0 CLK_A>;
>> +						rxd2 = <&PIO1 6 ALT1 IN SE_NICLK_IO 0 CLK_A>;
>> +						rxd3 = <&PIO1 7 ALT1 IN SE_NICLK_IO 0 CLK_A>;
>> +
>> +						rxdv = <&PIO2 0 ALT1 IN SE_NICLK_IO 0 CLK_A>;
>> +						rx_er = <&PIO2 1 ALT1 IN SE_NICLK_IO 0 CLK_A>;
>> +						rxclk = <&PIO2 2 ALT1 IN NICLK 0 CLK_A>;
>> +						phyclk = <&PIO2 3 ALT1 OUT NICLK 0 CLK_A>;
>> +					};
>> +				};
>> +
>> +			};
>
> Superflous new line.

Fixed.

>

<snip>

>> +
>> +			pwm0 {
>> +				pinctrl_pwm0_chan0_default: pwm0-0-default {
>> +					st,pins {
>> +						pwm-out = <&PIO31 1 ALT1 OUT>;
>> +					};
>> +				};
>> +			};
>> +
>> +		};
>
> Extra new line above.
Fixed.

>
<snip>

>> diff --git a/arch/arm/boot/dts/stih407.dtsi b/arch/arm/boot/dts/stih407.dtsi
>> new file mode 100644
>> index 0000000..2a0566b
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/stih407.dtsi
>> @@ -0,0 +1,250 @@
>> +/*
>> + * Copyright (C) 2013 STMicroelectronics Limited.
>> + * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>> + *
>> + * 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
>> + * publishhed by the Free Software Foundation.
>> + */
>> +#include "stih407-clock.dtsi"
>> +#include "stih407-pinctrl.dtsi"
>> +/ {
>> +	#address-cells = <1>;
>> +	#size-cells = <1>;
>> +
>> +	cpus {
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		cpu@0 {
>> +			device_type = "cpu";
>> +			compatible = "arm,cortex-a9";
>> +			reg = <0>;
>> +		};
>> +		cpu@1 {
>> +			device_type = "cpu";
>> +			compatible = "arm,cortex-a9";
>> +			reg = <1>;
>> +		};
>> +	};
>> +
>> +	intc: interrupt-controller@08761000 {
>> +		compatible = "arm,cortex-a9-gic";
>> +		#interrupt-cells = <3>;
>> +		interrupt-controller;
>> +		reg = <0x08761000 0x1000>, <0x08760100 0x100>;
>> +	};
>> +
>> +	scu@08760000 {
>> +		compatible = "arm,cortex-a9-scu";
>> +		reg = <0x08760000 0x1000>;
>> +	};
>> +
>> +	timer@08760200 {
>> +		interrupt-parent = <&intc>;
>> +		compatible = "arm,cortex-a9-global-timer";
>> +		reg = <0x08760200 0x100>;
>> +		interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
>> +		clocks = <&arm_periph_clk>;
>> +	};
>> +
>> +	L2: cache-controller {
>> +		compatible = "arm,pl310-cache";
>> +		reg = <0x08762000 0x1000>;
>> +		arm,data-latency = <3 3 3>;
>> +		arm,tag-latency = <2 2 2>;
>> +		cache-unified;
>> +		cache-level = <2>;
>> +	};
>> +
>> +	soc {
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		interrupt-parent = <&intc>;
>> +		ranges;
>> +		compatible = "simple-bus";
>> +
>> +		syscfg_sbc:sbc-syscfg@9620000{
>
> Space after colon and another after the address.
>
> Same for all of the nodes below and throughout the patch.

Fixed here and evrywhere in the file.


<snip>

>> +		serial@9830000{
>> +			compatible = "st,asc";
>> +			status = "disabled";
>
> This might just be a personal thing, but I prefer to see the status at
> the base of the node with a new line above it, as it is only relevant
> to the node rather than the device.
>
>> +			reg = <0x9830000 0x2c>;
>> +			interrupts = <GIC_SPI 122 IRQ_TYPE_NONE>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_serial0>;
>> +			clocks = <&CLK_EXT2F_A9>;
>> +		};
>
> Like this:
> 	serial@9830000{
> 		compatible = "st,asc";
> 		reg = <0x9830000 0x2c>;
> 		interrupts = <GIC_SPI 122 IRQ_TYPE_NONE>;
> 		pinctrl-names = "default";
> 		pinctrl-0 = <&pinctrl_serial0>;
> 		clocks = <&CLK_EXT2F_A9>;
>
> 		status = "disabled";
> 	};

Okay. I made the change for all the nodes.

>

<snip>

>
> Not sure if this is Git playing around again, but the };s look
> misaligned in the submission.
>
> Wow, this is good work and a lot of code!
>
> Once my comments have been rectified, feel free to add my:
>    Acked-by: Lee Jones <lee.jones@linaro.org>

Thanks for the review Lee.
I added your Ack.

Regards,
Maxime
>

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

* Re: [PATCH v3 2/5] pinctrl: st: Enhance the controller to manage unavailable registers
  2014-03-11  8:18     ` Maxime Coquelin
@ 2014-03-11 10:55       ` Maxime Coquelin
  2014-03-11 11:13         ` Lee Jones
  2014-03-11 11:12       ` Lee Jones
  1 sibling, 1 reply; 21+ messages in thread
From: Maxime Coquelin @ 2014-03-11 10:55 UTC (permalink / raw)
  To: Lee Jones
  Cc: Mark Rutland, devicetree, Russell King, kernel, Pawel Moll,
	Ian Campbell, Linus Walleij, linux-doc, linux-kernel,
	Rob Herring, Rob Landley, Kumar Gala, linux-arm-kernel



On 03/11/2014 09:18 AM, Maxime Coquelin wrote:
>
>
> On 03/10/2014 10:17 AM, Lee Jones wrote:
>>> From: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>>>
>>> This patch adds a new logic inside the st pinctrl to manage
>>> an unsupported scenario: some sysconfig are not available!
>>>
>>> This is the case of STiH407 where, although documented, the
>>> following registers from SYSCFG_FLASH have been removed from the SoC.
>>>
>>> SYSTEM_CONFIG3040
>>>     Output Enable pad control for all PIO Alternate Functions
>>> and
>>> SYSTEM_ CONFIG3050
>>>     Pull Up pad control for all PIO Alternate Functions
>>>
>>> Without managing this condition an imprecise external abort
>>> will be detect.
>>>
>>> To do this the patch also reviews the st_parse_syscfgs
>>> and other routines to manipulate the registers only if
>>> actually available.
>>> In any case, for example the st_parse_syscfgs detected
>>> an error condition but no action was made in the
>>> st_pctl_probe_dt.
>>>
>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
>>> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>>
>> These two SOBs need reordering.
> Right,
> this will be changed here and everywhere else in the series.
>
>>
>>> ---
>>>   drivers/pinctrl/pinctrl-st.c | 106
>>> +++++++++++++++++++++++++------------------
>>>   1 file changed, 61 insertions(+), 45 deletions(-)
>>>
>>> diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
>>> index 9fb66aa..1721611 100644
>>> --- a/drivers/pinctrl/pinctrl-st.c
>>> +++ b/drivers/pinctrl/pinctrl-st.c

<snip>

>>>
>>> -static int st_parse_syscfgs(struct st_pinctrl *info,
>>> -        int bank, struct device_node *np)
>>> +
>>> +static struct regmap_field *st_pc_get_value(struct device *dev,
>>> +                        struct regmap *regmap, int bank,
>>> +                        int data, int lsb, int msb)
>>> +{
>>> +    struct reg_field reg = REG_FIELD((data + bank) * 4, lsb, msb);
>>> +
>>> +    if (data < 0)
>>> +        return NULL;
>>
>> What happens is data < 0 and it's used in REG_FIELD?
> Nothing bad, but I agree this is not crystal clear.
>
>>
>> Would it make more sense to make this check before calling REG_FIELD?
> Yes, it will be done in the v4.

Finally, I have to keep it as it was if I want this patch to compile.

>
> <snip>
>
> Thanks,
> Maxime
>
> _______________________________________________
> Kernel mailing list
> Kernel@stlinux.com
> http://www.stlinux.com/mailman/listinfo/kernel

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

* Re: [PATCH v3 2/5] pinctrl: st: Enhance the controller to manage unavailable registers
  2014-03-11  8:18     ` Maxime Coquelin
  2014-03-11 10:55       ` Maxime Coquelin
@ 2014-03-11 11:12       ` Lee Jones
  1 sibling, 0 replies; 21+ messages in thread
From: Lee Jones @ 2014-03-11 11:12 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: Rob Landley, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Russell King, Srinivas Kandagatla, Stuart Menefy,
	Linus Walleij, Giuseppe Cavallaro, linux-doc, linux-kernel,
	devicetree, linux-arm-kernel, kernel

> >>From: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> >>
> >>This patch adds a new logic inside the st pinctrl to manage
> >>an unsupported scenario: some sysconfig are not available!
> >>
> >>This is the case of STiH407 where, although documented, the
> >>following registers from SYSCFG_FLASH have been removed from the SoC.
> >>
> >>SYSTEM_CONFIG3040
> >>    Output Enable pad control for all PIO Alternate Functions
> >>and
> >>SYSTEM_ CONFIG3050
> >>    Pull Up pad control for all PIO Alternate Functions
> >>
> >>Without managing this condition an imprecise external abort
> >>will be detect.
> >>
> >>To do this the patch also reviews the st_parse_syscfgs
> >>and other routines to manipulate the registers only if
> >>actually available.
> >>In any case, for example the st_parse_syscfgs detected
> >>an error condition but no action was made in the
> >>st_pctl_probe_dt.
> >>
> >>Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
> >>Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>

No problem with any of your comments. Once the fixups have been made,
feel free to apply my:

  Acked-by: Lee Jones <lee.jones@linaro.org>  

-- 
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] 21+ messages in thread

* Re: [PATCH v3 2/5] pinctrl: st: Enhance the controller to manage unavailable registers
  2014-03-11 10:55       ` Maxime Coquelin
@ 2014-03-11 11:13         ` Lee Jones
  0 siblings, 0 replies; 21+ messages in thread
From: Lee Jones @ 2014-03-11 11:13 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: Mark Rutland, devicetree, Russell King, kernel, Pawel Moll,
	Ian Campbell, Linus Walleij, linux-doc, linux-kernel,
	Rob Herring, Rob Landley, Kumar Gala, linux-arm-kernel

> >>>From: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> >>>
> >>>This patch adds a new logic inside the st pinctrl to manage
> >>>an unsupported scenario: some sysconfig are not available!
> >>>
> >>>This is the case of STiH407 where, although documented, the
> >>>following registers from SYSCFG_FLASH have been removed from the SoC.
> >>>
> >>>SYSTEM_CONFIG3040
> >>>    Output Enable pad control for all PIO Alternate Functions
> >>>and
> >>>SYSTEM_ CONFIG3050
> >>>    Pull Up pad control for all PIO Alternate Functions
> >>>
> >>>Without managing this condition an imprecise external abort
> >>>will be detect.
> >>>
> >>>To do this the patch also reviews the st_parse_syscfgs
> >>>and other routines to manipulate the registers only if
> >>>actually available.
> >>>In any case, for example the st_parse_syscfgs detected
> >>>an error condition but no action was made in the
> >>>st_pctl_probe_dt.
> >>>
> >>>Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
> >>>Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> >>>+
> >>>+static struct regmap_field *st_pc_get_value(struct device *dev,
> >>>+                        struct regmap *regmap, int bank,
> >>>+                        int data, int lsb, int msb)
> >>>+{
> >>>+    struct reg_field reg = REG_FIELD((data + bank) * 4, lsb, msb);
> >>>+
> >>>+    if (data < 0)
> >>>+        return NULL;
> >>
> >>What happens is data < 0 and it's used in REG_FIELD?
> >Nothing bad, but I agree this is not crystal clear.
> >
> >>Would it make more sense to make this check before calling REG_FIELD?
> >Yes, it will be done in the v4.
> 
> Finally, I have to keep it as it was if I want this patch to compile.

Ah, why's that?

-- 
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] 21+ messages in thread

* Re: [PATCH v3 1/5] ARM: STi: Add STiH407 SoC support
  2014-03-10 12:34   ` Lee Jones
@ 2014-03-11 13:56     ` Maxime Coquelin
  0 siblings, 0 replies; 21+ messages in thread
From: Maxime Coquelin @ 2014-03-11 13:56 UTC (permalink / raw)
  To: Lee Jones
  Cc: Mark Rutland, devicetree, Russell King, kernel, Pawel Moll,
	Ian Campbell, Linus Walleij, Srinivas Kandagatla, linux-doc,
	linux-kernel, Stuart Menefy, Rob Herring, Rob Landley,
	Kumar Gala, Giuseppe Cavallaro, linux-arm-kernel

Sorry, I missed this one.

On 03/10/2014 01:34 PM, Lee Jones wrote:
>> This patch adds support to STiH407 SoC.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
>> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>> ---
>>   Documentation/arm/sti/stih407-overview.txt    | 18 ++++++++++++++++++
>>   Documentation/devicetree/bindings/arm/sti.txt | 15 +++++++++++++++
>>   arch/arm/mach-sti/Kconfig                     |  9 +++++++++
>>   arch/arm/mach-sti/board-dt.c                  |  1 +
>>   4 files changed, 43 insertions(+)
>>   create mode 100644 Documentation/arm/sti/stih407-overview.txt
>>   create mode 100644 Documentation/devicetree/bindings/arm/sti.txt
>
> <snip>
>
>> +config SOC_STIH407
>> +	bool "STiH407 STMicroelectronics Consumer Electronics family"
>> +	default y
>> +	help
>> +	  This enables support for STMicroelectronics Digital Consumer
>> +	  Electronics family StiH407 parts, primarily targeted at set-top-box
>> +	  and other digital audio/video applications using Flattened Device
>> +	  Trees.
>> +
>
> Are these configs actually used for something?
Not currently, but it is going to happen (for reset controller).

I will add it only when I'll send the reset controller driver for STiH407.

Regards,
Maxime

>
> <snip>
>

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

end of thread, other threads:[~2014-03-11 13:57 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-07 14:40 [PATCH v3 0/5] Add STiH407 SoC and reference board support Maxime COQUELIN
2014-03-07 14:40 ` [PATCH v3 1/5] ARM: STi: Add STiH407 SoC support Maxime COQUELIN
2014-03-10 12:34   ` Lee Jones
2014-03-11 13:56     ` Maxime Coquelin
2014-03-07 14:40 ` [PATCH v3 2/5] pinctrl: st: Enhance the controller to manage unavailable registers Maxime COQUELIN
2014-03-07 15:37   ` srinivas kandagatla
2014-03-10  9:17   ` Lee Jones
2014-03-11  8:18     ` Maxime Coquelin
2014-03-11 10:55       ` Maxime Coquelin
2014-03-11 11:13         ` Lee Jones
2014-03-11 11:12       ` Lee Jones
2014-03-07 14:40 ` [PATCH v3 3/5] pinctrl: st: add pinctrl support for the STiH407 SoC Maxime COQUELIN
2014-03-07 15:36   ` srinivas kandagatla
2014-03-10 14:44   ` Lee Jones
2014-03-11  8:52     ` Maxime Coquelin
2014-03-07 14:40 ` [PATCH v3 4/5] ARM: dts: Add STiH407 SoC support Maxime COQUELIN
2014-03-10 12:28   ` Lee Jones
2014-03-10 16:16     ` Lee Jones
2014-03-11 10:09     ` Maxime Coquelin
2014-03-07 14:40 ` [PATCH v3 5/5] ARM: dts: STiH407: Add B2120 board support Maxime COQUELIN
2014-03-10 12:37   ` Lee Jones

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).