Re: [PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

Re: [PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Alexander Shiyan]

Wed, 13 Aug 2014 19:16:22 +0300 от Grygorii Strashko <grygorii.strashko@ti.com>:
> Add Keystone 2 DSP GPIO nodes.
> DSP GPIO banks 0-7 correspond to DSP0-DSP7
> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  arch/arm/boot/dts/k2hk.dtsi |   56 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/k2hk.dtsi b/arch/arm/boot/dts/k2hk.dtsi
> index 321ba2f..009e180 100644
> --- a/arch/arm/boot/dts/k2hk.dtsi
> +++ b/arch/arm/boot/dts/k2hk.dtsi
> @@ -50,5 +50,61 @@
>  			#interrupt-cells = <1>;
>  			ti,syscon-dev = <&devctrl 0x2a0>;
>  		};
> +
> +		dspgpio0: keystone_dsp_gpio@02620240 {
> +			compatible = "ti,keystone-mctrl-gpio";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			gpio,syscon-dev = <&devctrl 0x240>;
> +		};
> +
> +		dspgpio1: keystone_dsp_gpio@2620244 {
> +			compatible = "ti,keystone-mctrl-gpio";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			gpio,syscon-dev = <&devctrl 0x244>;
> +		};
...
> +		dspgpio7: keystone_dsp_gpio@262025C {
> +			compatible = "ti,keystone-mctrl-gpio";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			gpio,syscon-dev = <&devctrl 0x25c>;
> +		};

So, devctrl is a syscon device and this DTS introduce several
identical GPIO descriptions?

On my opinion this should be placed in the gpio-syscon.c,
where you can add support for ti,keystone-dsp0{..7}-gpio.
Such change will avoid parts 2 and 3 of this patch.

static const struct syscon_gpio_data ti_keystone_dsp0_gpio = {
  .compatible = "ti,keystone-syscon",
  .dat_bit_offset = 0x240 * 8,
  ...
  .set = etc...
};

---

Re: [PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Alexander Shiyan]

Wed, 13 Aug 2014 19:16:22 +0300 от Grygorii Strashko <grygorii.strashko@ti.com>:
> Add Keystone 2 DSP GPIO nodes.
> DSP GPIO banks 0-7 correspond to DSP0-DSP7
> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  arch/arm/boot/dts/k2hk.dtsi |   56 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/k2hk.dtsi b/arch/arm/boot/dts/k2hk.dtsi
> index 321ba2f..009e180 100644
> --- a/arch/arm/boot/dts/k2hk.dtsi
> +++ b/arch/arm/boot/dts/k2hk.dtsi
> @@ -50,5 +50,61 @@
>  			#interrupt-cells = <1>;
>  			ti,syscon-dev = <&devctrl 0x2a0>;
>  		};
> +
> +		dspgpio0: keystone_dsp_gpio@02620240 {
> +			compatible = "ti,keystone-mctrl-gpio";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			gpio,syscon-dev = <&devctrl 0x240>;
> +		};
> +
> +		dspgpio1: keystone_dsp_gpio@2620244 {
> +			compatible = "ti,keystone-mctrl-gpio";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			gpio,syscon-dev = <&devctrl 0x244>;
> +		};
...
> +		dspgpio7: keystone_dsp_gpio@262025C {
> +			compatible = "ti,keystone-mctrl-gpio";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			gpio,syscon-dev = <&devctrl 0x25c>;
> +		};

So, devctrl is a syscon device and this DTS introduce several
identical GPIO descriptions?

On my opinion this should be placed in the gpio-syscon.c,
where you can add support for ti,keystone-dsp0{..7}-gpio.
Such change will avoid parts 2 and 3 of this patch.

static const struct syscon_gpio_data ti_keystone_dsp0_gpio = {
  .compatible = "ti,keystone-syscon",
  .dat_bit_offset = 0x240 * 8,
  ...
  .set = etc...
};

---

Re: [PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Alexander Shiyan]

Wed, 13 Aug 2014 19:16:22 +0300 от Grygorii Strashko <grygorii.strashko@ti.com>:
> Add Keystone 2 DSP GPIO nodes.
> DSP GPIO banks 0-7 correspond to DSP0-DSP7
> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  arch/arm/boot/dts/k2hk.dtsi |   56 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/k2hk.dtsi b/arch/arm/boot/dts/k2hk.dtsi
> index 321ba2f..009e180 100644
> --- a/arch/arm/boot/dts/k2hk.dtsi
> +++ b/arch/arm/boot/dts/k2hk.dtsi
> @@ -50,5 +50,61 @@
>  			#interrupt-cells = <1>;
>  			ti,syscon-dev = <&devctrl 0x2a0>;
>  		};
> +
> +		dspgpio0: keystone_dsp_gpio@02620240 {
> +			compatible = "ti,keystone-mctrl-gpio";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			gpio,syscon-dev = <&devctrl 0x240>;
> +		};
> +
> +		dspgpio1: keystone_dsp_gpio@2620244 {
> +			compatible = "ti,keystone-mctrl-gpio";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			gpio,syscon-dev = <&devctrl 0x244>;
> +		};
...
> +		dspgpio7: keystone_dsp_gpio@262025C {
> +			compatible = "ti,keystone-mctrl-gpio";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			gpio,syscon-dev = <&devctrl 0x25c>;
> +		};

So, devctrl is a syscon device and this DTS introduce several
identical GPIO descriptions?

On my opinion this should be placed in the gpio-syscon.c,
where you can add support for ti,keystone-dsp0{..7}-gpio.
Such change will avoid parts 2 and 3 of this patch.

static const struct syscon_gpio_data ti_keystone_dsp0_gpio = {
  .compatible = "ti,keystone-syscon",
  .dat_bit_offset = 0x240 * 8,
  ...
  .set = etc...
};

---

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Alexander Shiyan]

Wed, 13 Aug 2014 19:16:22 +0300 ?? Grygorii Strashko <grygorii.strashko@ti.com>:
> Add Keystone 2 DSP GPIO nodes.
> DSP GPIO banks 0-7 correspond to DSP0-DSP7
> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  arch/arm/boot/dts/k2hk.dtsi |   56 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/k2hk.dtsi b/arch/arm/boot/dts/k2hk.dtsi
> index 321ba2f..009e180 100644
> --- a/arch/arm/boot/dts/k2hk.dtsi
> +++ b/arch/arm/boot/dts/k2hk.dtsi
> @@ -50,5 +50,61 @@
>  			#interrupt-cells = <1>;
>  			ti,syscon-dev = <&devctrl 0x2a0>;
>  		};
> +
> +		dspgpio0: keystone_dsp_gpio at 02620240 {
> +			compatible = "ti,keystone-mctrl-gpio";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			gpio,syscon-dev = <&devctrl 0x240>;
> +		};
> +
> +		dspgpio1: keystone_dsp_gpio at 2620244 {
> +			compatible = "ti,keystone-mctrl-gpio";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			gpio,syscon-dev = <&devctrl 0x244>;
> +		};
...
> +		dspgpio7: keystone_dsp_gpio at 262025C {
> +			compatible = "ti,keystone-mctrl-gpio";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			gpio,syscon-dev = <&devctrl 0x25c>;
> +		};

So, devctrl is a syscon device and this DTS introduce several
identical GPIO descriptions?

On my opinion this should be placed in the gpio-syscon.c,
where you can add support for ti,keystone-dsp0{..7}-gpio.
Such change will avoid parts 2 and 3 of this patch.

static const struct syscon_gpio_data ti_keystone_dsp0_gpio = {
  .compatible = "ti,keystone-syscon",
  .dat_bit_offset = 0x240 * 8,
  ...
  .set = etc...
};

---

[PATCH 0/4] gpio: syscon: reuse for keystone 2 socs

[Grygorii Strashko]

Hi,

This series intended to integrate Keystone 2 DSP GPIO controller functionality into
gpio-syscon driver (drivers/gpio/gpio-syscon.c) as requested by Linus Walleij.

On Keystone SOCs, ARM host can send interrupts to DSP cores using the
DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
each DSP core. This is one of the component used by the IPC mechanism used
on Keystone SOCs.

Keystone 2 DSP GPIO controller has specific features:
- each GPIO can be configured only as output pin;
- setting GPIO value to 1 causes IRQ generation on target DSP core;
- reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
  pending.

The gpio-syscon driver was need to be updated to satisfy Keystone 2 SoC
requirements:
1) special sequence of operations need to be used to assign output GPIO value.
   As result, first patch introduces SoC specific callback .set() to configure
   output GPIO value.

2) - eight (8) GPIO banks should be supported, but current gpio-syscon driver
   supports only one.
   - there can be more than on syscon devices per SoC and GPIO specific registers 
   can be placed any where. More over, number of syscon devices and their
   configuration can vary even between two versions of the same SoC. 
   As result, second patch introduces new DT property for gpio-syscon devices:
 	gpio,syscon-dev = <&syscon_dev data_reg_offset [direction_reg_offset]>;
   which allows to specify syscon node and data/direction registers offsets in DT.

Also, patch 4 added to illustrate DSP GPIO configuration in DT used by Keystone 2.

Related sicussions:
 [1] https://lkml.org/lkml/2014/7/16/170
 [2] https://lkml.org/lkml/2014/7/23/352

Grygorii Strashko (4):
  gpio: syscon: add soc specific callback to assign output value
  gpio: syscon: retrive syscon node and regs offsets from dt
  gpio: syscon: reuse for keystone 2 socs
  ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

 .../bindings/gpio/gpio-mctrl-keystone.txt          |   39 ++++++++
 arch/arm/boot/dts/k2hk.dtsi                        |   56 +++++++++++
 drivers/gpio/gpio-syscon.c                         |   99 ++++++++++++++++++--
 3 files changed, 186 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt

-- 
1.7.9.5

[PATCH 0/4] gpio: syscon: reuse for keystone 2 socs

[Grygorii Strashko]

Hi,

This series intended to integrate Keystone 2 DSP GPIO controller functionality into
gpio-syscon driver (drivers/gpio/gpio-syscon.c) as requested by Linus Walleij.

On Keystone SOCs, ARM host can send interrupts to DSP cores using the
DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
each DSP core. This is one of the component used by the IPC mechanism used
on Keystone SOCs.

Keystone 2 DSP GPIO controller has specific features:
- each GPIO can be configured only as output pin;
- setting GPIO value to 1 causes IRQ generation on target DSP core;
- reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
  pending.

The gpio-syscon driver was need to be updated to satisfy Keystone 2 SoC
requirements:
1) special sequence of operations need to be used to assign output GPIO value.
   As result, first patch introduces SoC specific callback .set() to configure
   output GPIO value.

2) - eight (8) GPIO banks should be supported, but current gpio-syscon driver
   supports only one.
   - there can be more than on syscon devices per SoC and GPIO specific registers 
   can be placed any where. More over, number of syscon devices and their
   configuration can vary even between two versions of the same SoC. 
   As result, second patch introduces new DT property for gpio-syscon devices:
 	gpio,syscon-dev = <&syscon_dev data_reg_offset [direction_reg_offset]>;
   which allows to specify syscon node and data/direction registers offsets in DT.

Also, patch 4 added to illustrate DSP GPIO configuration in DT used by Keystone 2.

Related sicussions:
 [1] https://lkml.org/lkml/2014/7/16/170
 [2] https://lkml.org/lkml/2014/7/23/352

Grygorii Strashko (4):
  gpio: syscon: add soc specific callback to assign output value
  gpio: syscon: retrive syscon node and regs offsets from dt
  gpio: syscon: reuse for keystone 2 socs
  ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

 .../bindings/gpio/gpio-mctrl-keystone.txt          |   39 ++++++++
 arch/arm/boot/dts/k2hk.dtsi                        |   56 +++++++++++
 drivers/gpio/gpio-syscon.c                         |   99 ++++++++++++++++++--
 3 files changed, 186 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt

-- 
1.7.9.5

[PATCH 1/4] gpio: syscon: add soc specific callback to assign output value

[Grygorii Strashko]

Some SoCs (like Keystone) may require to perform special
sequence of operations to assign output GPIO value, so default
implementation of .set() callback from gpio-syscon driver
can't be used.

Hence, add optional, SoC specific callback to assign output
gpio value.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/gpio/gpio-syscon.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
index 30884fb..03b4699 100644
--- a/drivers/gpio/gpio-syscon.c
+++ b/drivers/gpio/gpio-syscon.c
@@ -37,6 +37,8 @@
  * dat_bit_offset:	Offset (in bits) to the first GPIO bit.
  * dir_bit_offset:	Optional offset (in bits) to the first bit to switch
  *			GPIO direction (Used with GPIO_SYSCON_FEAT_DIR flag).
+ * set:		HW specific callback to assigns output value
+ *			for signal "offset"
  */
 
 struct syscon_gpio_data {
@@ -45,6 +47,8 @@ struct syscon_gpio_data {
 	unsigned int	bit_count;
 	unsigned int	dat_bit_offset;
 	unsigned int	dir_bit_offset;
+	void		(*set)(struct gpio_chip *chip,
+			       unsigned offset, int value);
 };
 
 struct syscon_gpio_priv {
@@ -77,6 +81,11 @@ static void syscon_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
 	struct syscon_gpio_priv *priv = to_syscon_gpio(chip);
 	unsigned int offs = priv->data->dat_bit_offset + offset;
 
+	if (priv->data->set) {
+		priv->data->set(chip, offset, val);
+		return;
+	}
+
 	regmap_update_bits(priv->syscon,
 			   (offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE,
 			   BIT(offs % SYSCON_REG_BITS),
-- 
1.7.9.5

[PATCH 1/4] gpio: syscon: add soc specific callback to assign output value

[Grygorii Strashko]

Some SoCs (like Keystone) may require to perform special
sequence of operations to assign output GPIO value, so default
implementation of .set() callback from gpio-syscon driver
can't be used.

Hence, add optional, SoC specific callback to assign output
gpio value.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/gpio/gpio-syscon.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
index 30884fb..03b4699 100644
--- a/drivers/gpio/gpio-syscon.c
+++ b/drivers/gpio/gpio-syscon.c
@@ -37,6 +37,8 @@
  * dat_bit_offset:	Offset (in bits) to the first GPIO bit.
  * dir_bit_offset:	Optional offset (in bits) to the first bit to switch
  *			GPIO direction (Used with GPIO_SYSCON_FEAT_DIR flag).
+ * set:		HW specific callback to assigns output value
+ *			for signal "offset"
  */
 
 struct syscon_gpio_data {
@@ -45,6 +47,8 @@ struct syscon_gpio_data {
 	unsigned int	bit_count;
 	unsigned int	dat_bit_offset;
 	unsigned int	dir_bit_offset;
+	void		(*set)(struct gpio_chip *chip,
+			       unsigned offset, int value);
 };
 
 struct syscon_gpio_priv {
@@ -77,6 +81,11 @@ static void syscon_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
 	struct syscon_gpio_priv *priv = to_syscon_gpio(chip);
 	unsigned int offs = priv->data->dat_bit_offset + offset;
 
+	if (priv->data->set) {
+		priv->data->set(chip, offset, val);
+		return;
+	}
+
 	regmap_update_bits(priv->syscon,
 			   (offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE,
 			   BIT(offs % SYSCON_REG_BITS),
-- 
1.7.9.5

[PATCH 2/4] gpio: syscon: retrive syscon node and regs offsets from dt

[Grygorii Strashko]

This patch adds handling of new "gpio,syscon-dev" DT property,
which allows to specify syscon node and data/direction registers
offsets in DT.

"gpio,syscon-dev" has following format:
	gpio,syscon-dev = <&syscon_dev data_reg_offset [direction_reg_offset]>;

where
 - syscon_dev - phandle on syscon node
 - data_reg_offset - offset of data register (in bytes)
 - direction_reg_offset - offset of dirrection register (optional, in bytes)

for example:
	gpio,syscon-dev = <&devctrl 0x254>;

In such way, the support of multiple Syscon GPIO devices is added.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/gpio/gpio-syscon.c |   51 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 43 insertions(+), 8 deletions(-)

diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
index 03b4699..05d528f 100644
--- a/drivers/gpio/gpio-syscon.c
+++ b/drivers/gpio/gpio-syscon.c
@@ -55,6 +55,8 @@ struct syscon_gpio_priv {
 	struct gpio_chip		chip;
 	struct regmap			*syscon;
 	const struct syscon_gpio_data	*data;
+	u32				dreg_offset;
+	u32				dir_reg_offset;
 };
 
 static inline struct syscon_gpio_priv *to_syscon_gpio(struct gpio_chip *chip)
@@ -65,9 +67,11 @@ static inline struct syscon_gpio_priv *to_syscon_gpio(struct gpio_chip *chip)
 static int syscon_gpio_get(struct gpio_chip *chip, unsigned offset)
 {
 	struct syscon_gpio_priv *priv = to_syscon_gpio(chip);
-	unsigned int val, offs = priv->data->dat_bit_offset + offset;
+	unsigned int val, offs;
 	int ret;
 
+	offs = priv->dreg_offset + priv->data->dat_bit_offset + offset;
+
 	ret = regmap_read(priv->syscon,
 			  (offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE, &val);
 	if (ret)
@@ -79,7 +83,9 @@ static int syscon_gpio_get(struct gpio_chip *chip, unsigned offset)
 static void syscon_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
 {
 	struct syscon_gpio_priv *priv = to_syscon_gpio(chip);
-	unsigned int offs = priv->data->dat_bit_offset + offset;
+	unsigned int offs;
+
+	offs = priv->dreg_offset + priv->data->dat_bit_offset + offset;
 
 	if (priv->data->set) {
 		priv->data->set(chip, offset, val);
@@ -97,7 +103,10 @@ static int syscon_gpio_dir_in(struct gpio_chip *chip, unsigned offset)
 	struct syscon_gpio_priv *priv = to_syscon_gpio(chip);
 
 	if (priv->data->flags & GPIO_SYSCON_FEAT_DIR) {
-		unsigned int offs = priv->data->dir_bit_offset + offset;
+		unsigned int offs;
+
+		offs = priv->dir_reg_offset +
+		       priv->data->dir_bit_offset + offset;
 
 		regmap_update_bits(priv->syscon,
 				   (offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE,
@@ -112,7 +121,10 @@ static int syscon_gpio_dir_out(struct gpio_chip *chip, unsigned offset, int val)
 	struct syscon_gpio_priv *priv = to_syscon_gpio(chip);
 
 	if (priv->data->flags & GPIO_SYSCON_FEAT_DIR) {
-		unsigned int offs = priv->data->dir_bit_offset + offset;
+		unsigned int offs;
+
+		offs = priv->dir_reg_offset +
+		       priv->data->dir_bit_offset + offset;
 
 		regmap_update_bits(priv->syscon,
 				   (offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE,
@@ -147,6 +159,8 @@ static int syscon_gpio_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	const struct of_device_id *of_id = of_match_device(syscon_gpio_ids, dev);
 	struct syscon_gpio_priv *priv;
+	struct device_node *np = dev->of_node;
+	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -154,10 +168,31 @@ static int syscon_gpio_probe(struct platform_device *pdev)
 
 	priv->data = of_id->data;
 
-	priv->syscon =
-		syscon_regmap_lookup_by_compatible(priv->data->compatible);
-	if (IS_ERR(priv->syscon))
-		return PTR_ERR(priv->syscon);
+	if (priv->data->compatible) {
+		priv->syscon = syscon_regmap_lookup_by_compatible(
+					priv->data->compatible);
+		if (IS_ERR(priv->syscon))
+			return PTR_ERR(priv->syscon);
+	} else {
+		priv->syscon =
+			syscon_regmap_lookup_by_phandle(np, "gpio,syscon-dev");
+		if (IS_ERR(priv->syscon))
+			return PTR_ERR(priv->syscon);
+
+		ret = of_property_read_u32_index(np, "gpio,syscon-dev", 1,
+						 &priv->dreg_offset);
+		if (ret)
+			dev_err(dev, "can't read the data register offset!\n");
+
+		priv->dreg_offset <<= 3;
+
+		ret = of_property_read_u32_index(np, "gpio,syscon-dev", 2,
+						 &priv->dir_reg_offset);
+		if (ret)
+			dev_err(dev, "can't read the dir register offset!\n");
+
+		priv->dir_reg_offset <<= 3;
+	}
 
 	priv->chip.dev = dev;
 	priv->chip.owner = THIS_MODULE;
-- 
1.7.9.5

[PATCH 2/4] gpio: syscon: retrive syscon node and regs offsets from dt

[Grygorii Strashko]

This patch adds handling of new "gpio,syscon-dev" DT property,
which allows to specify syscon node and data/direction registers
offsets in DT.

"gpio,syscon-dev" has following format:
	gpio,syscon-dev = <&syscon_dev data_reg_offset [direction_reg_offset]>;

where
 - syscon_dev - phandle on syscon node
 - data_reg_offset - offset of data register (in bytes)
 - direction_reg_offset - offset of dirrection register (optional, in bytes)

for example:
	gpio,syscon-dev = <&devctrl 0x254>;

In such way, the support of multiple Syscon GPIO devices is added.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/gpio/gpio-syscon.c |   51 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 43 insertions(+), 8 deletions(-)

diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
index 03b4699..05d528f 100644
--- a/drivers/gpio/gpio-syscon.c
+++ b/drivers/gpio/gpio-syscon.c
@@ -55,6 +55,8 @@ struct syscon_gpio_priv {
 	struct gpio_chip		chip;
 	struct regmap			*syscon;
 	const struct syscon_gpio_data	*data;
+	u32				dreg_offset;
+	u32				dir_reg_offset;
 };
 
 static inline struct syscon_gpio_priv *to_syscon_gpio(struct gpio_chip *chip)
@@ -65,9 +67,11 @@ static inline struct syscon_gpio_priv *to_syscon_gpio(struct gpio_chip *chip)
 static int syscon_gpio_get(struct gpio_chip *chip, unsigned offset)
 {
 	struct syscon_gpio_priv *priv = to_syscon_gpio(chip);
-	unsigned int val, offs = priv->data->dat_bit_offset + offset;
+	unsigned int val, offs;
 	int ret;
 
+	offs = priv->dreg_offset + priv->data->dat_bit_offset + offset;
+
 	ret = regmap_read(priv->syscon,
 			  (offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE, &val);
 	if (ret)
@@ -79,7 +83,9 @@ static int syscon_gpio_get(struct gpio_chip *chip, unsigned offset)
 static void syscon_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
 {
 	struct syscon_gpio_priv *priv = to_syscon_gpio(chip);
-	unsigned int offs = priv->data->dat_bit_offset + offset;
+	unsigned int offs;
+
+	offs = priv->dreg_offset + priv->data->dat_bit_offset + offset;
 
 	if (priv->data->set) {
 		priv->data->set(chip, offset, val);
@@ -97,7 +103,10 @@ static int syscon_gpio_dir_in(struct gpio_chip *chip, unsigned offset)
 	struct syscon_gpio_priv *priv = to_syscon_gpio(chip);
 
 	if (priv->data->flags & GPIO_SYSCON_FEAT_DIR) {
-		unsigned int offs = priv->data->dir_bit_offset + offset;
+		unsigned int offs;
+
+		offs = priv->dir_reg_offset +
+		       priv->data->dir_bit_offset + offset;
 
 		regmap_update_bits(priv->syscon,
 				   (offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE,
@@ -112,7 +121,10 @@ static int syscon_gpio_dir_out(struct gpio_chip *chip, unsigned offset, int val)
 	struct syscon_gpio_priv *priv = to_syscon_gpio(chip);
 
 	if (priv->data->flags & GPIO_SYSCON_FEAT_DIR) {
-		unsigned int offs = priv->data->dir_bit_offset + offset;
+		unsigned int offs;
+
+		offs = priv->dir_reg_offset +
+		       priv->data->dir_bit_offset + offset;
 
 		regmap_update_bits(priv->syscon,
 				   (offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE,
@@ -147,6 +159,8 @@ static int syscon_gpio_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	const struct of_device_id *of_id = of_match_device(syscon_gpio_ids, dev);
 	struct syscon_gpio_priv *priv;
+	struct device_node *np = dev->of_node;
+	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -154,10 +168,31 @@ static int syscon_gpio_probe(struct platform_device *pdev)
 
 	priv->data = of_id->data;
 
-	priv->syscon =
-		syscon_regmap_lookup_by_compatible(priv->data->compatible);
-	if (IS_ERR(priv->syscon))
-		return PTR_ERR(priv->syscon);
+	if (priv->data->compatible) {
+		priv->syscon = syscon_regmap_lookup_by_compatible(
+					priv->data->compatible);
+		if (IS_ERR(priv->syscon))
+			return PTR_ERR(priv->syscon);
+	} else {
+		priv->syscon =
+			syscon_regmap_lookup_by_phandle(np, "gpio,syscon-dev");
+		if (IS_ERR(priv->syscon))
+			return PTR_ERR(priv->syscon);
+
+		ret = of_property_read_u32_index(np, "gpio,syscon-dev", 1,
+						 &priv->dreg_offset);
+		if (ret)
+			dev_err(dev, "can't read the data register offset!\n");
+
+		priv->dreg_offset <<= 3;
+
+		ret = of_property_read_u32_index(np, "gpio,syscon-dev", 2,
+						 &priv->dir_reg_offset);
+		if (ret)
+			dev_err(dev, "can't read the dir register offset!\n");
+
+		priv->dir_reg_offset <<= 3;
+	}
 
 	priv->chip.dev = dev;
 	priv->chip.owner = THIS_MODULE;
-- 
1.7.9.5

[PATCH 3/4] gpio: syscon: reuse for keystone 2 socs

[Grygorii Strashko]

On Keystone SOCs, ARM host can send interrupts to DSP cores using the
DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
each DSP core. This is one of the component used by the IPC mechanism used
on Keystone SOCs.

Keystone 2 DSP GPIO controller has specific features:
- each GPIO can be configured only as output pin;
- setting GPIO value to 1 causes IRQ generation on target DSP core;
- reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
  pending.

This patch updates gpio-syscon driver to be reused by Keystone 2 SoCs,
because the Keystone 2 DSP GPIO controller is controlled through Syscon
devices and, as requested by Linus Walleij, such kind of GPIO controllers
should be integrated with drivers/gpio/gpio-syscon.c driver.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 .../bindings/gpio/gpio-mctrl-keystone.txt          |   39 ++++++++++++++++++++
 drivers/gpio/gpio-syscon.c                         |   39 ++++++++++++++++++++
 2 files changed, 78 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt b/Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt
new file mode 100644
index 0000000..0687f5e
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt
@@ -0,0 +1,39 @@
+Keystone 2 DSP MCTRL GPIO controller bindings
+
+HOST OS userland running on ARM can send interrupts to DSP cores using
+the DSP GPIO controller IP. It provides 28 IRQ signals per each DSP core.
+This is one of the component used by the IPC mechanism used on Keystone SOCs.
+
+For example TCI6638K2K SoC has 8 DSP GPIO controllers:
+ - 8 for C66x CorePacx CPUs 0-7
+
+Keystone 2 DSP GPIO controller has specific features:
+- each GPIO can be configured only as output pin;
+- setting GPIO value to 1 causes IRQ generation on target DSP core;
+- reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
+  pending.
+
+Required Properties:
+- compatible: should be "ti,keystone-mctrl-gpio"
+- ti,syscon-dev: phandle/offset pair. The phandle to syscon used to
+  access device state control registers and the offset of device's specific
+  registers within device state control registers range.
+- gpio-controller: Marks the device node as a gpio controller.
+- #gpio-cells: Should be 2.
+
+Please refer to gpio.txt in this directory for details of the common GPIO
+bindings used by client devices.
+
+Example:
+	dspgpio0: keystone_dsp_gpio@02620240 {
+		compatible = "ti,keystone-mctrl-gpio";
+		ti,syscon-dev = <&devctrl 0x240>;
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
+
+	dsp0: dsp0 {
+		compatible = "linux,rproc-user";
+		...
+		kick-gpio = <&dspgpio0 27>;
+	};
diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
index 05d528f..8a7f715 100644
--- a/drivers/gpio/gpio-syscon.c
+++ b/drivers/gpio/gpio-syscon.c
@@ -145,11 +145,50 @@ static const struct syscon_gpio_data clps711x_mctrl_gpio = {
 	.dat_bit_offset	= 0x40 * 8 + 8,
 };
 
+#ifdef CONFIG_ARCH_KEYSTONE
+#define KEYSTONE_LOCK_BIT BIT(0)
+
+static void keystone_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
+{
+	struct syscon_gpio_priv *priv = to_syscon_gpio(chip);
+	unsigned int offs;
+	int ret;
+
+	offs = priv->dreg_offset + priv->data->dat_bit_offset + offset;
+
+	if (!val)
+		return;
+
+	ret = regmap_update_bits(
+			priv->syscon,
+			(offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE,
+			BIT(offs % SYSCON_REG_BITS) | KEYSTONE_LOCK_BIT,
+			BIT(offs % SYSCON_REG_BITS) | KEYSTONE_LOCK_BIT);
+	if (ret < 0)
+		dev_err(chip->dev, "gpio write failed ret(%d)\n", ret);
+}
+
+static const struct syscon_gpio_data keystone_mctrl_gpio = {
+	/* ARM Keystone 2 */
+	.compatible	= NULL,
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= 28,
+	.dat_bit_offset	= 4,
+	.set		= keystone_gpio_set,
+};
+#endif
+
 static const struct of_device_id syscon_gpio_ids[] = {
 	{
 		.compatible	= "cirrus,clps711x-mctrl-gpio",
 		.data		= &clps711x_mctrl_gpio,
 	},
+#ifdef CONFIG_ARCH_KEYSTONE
+	{
+		.compatible	= "ti,keystone-mctrl-gpio",
+		.data		= &keystone_mctrl_gpio,
+	},
+#endif
 	{ }
 };
 MODULE_DEVICE_TABLE(of, syscon_gpio_ids);
-- 
1.7.9.5

[PATCH 3/4] gpio: syscon: reuse for keystone 2 socs

[Grygorii Strashko]

On Keystone SOCs, ARM host can send interrupts to DSP cores using the
DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
each DSP core. This is one of the component used by the IPC mechanism used
on Keystone SOCs.

Keystone 2 DSP GPIO controller has specific features:
- each GPIO can be configured only as output pin;
- setting GPIO value to 1 causes IRQ generation on target DSP core;
- reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
  pending.

This patch updates gpio-syscon driver to be reused by Keystone 2 SoCs,
because the Keystone 2 DSP GPIO controller is controlled through Syscon
devices and, as requested by Linus Walleij, such kind of GPIO controllers
should be integrated with drivers/gpio/gpio-syscon.c driver.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 .../bindings/gpio/gpio-mctrl-keystone.txt          |   39 ++++++++++++++++++++
 drivers/gpio/gpio-syscon.c                         |   39 ++++++++++++++++++++
 2 files changed, 78 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt b/Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt
new file mode 100644
index 0000000..0687f5e
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt
@@ -0,0 +1,39 @@
+Keystone 2 DSP MCTRL GPIO controller bindings
+
+HOST OS userland running on ARM can send interrupts to DSP cores using
+the DSP GPIO controller IP. It provides 28 IRQ signals per each DSP core.
+This is one of the component used by the IPC mechanism used on Keystone SOCs.
+
+For example TCI6638K2K SoC has 8 DSP GPIO controllers:
+ - 8 for C66x CorePacx CPUs 0-7
+
+Keystone 2 DSP GPIO controller has specific features:
+- each GPIO can be configured only as output pin;
+- setting GPIO value to 1 causes IRQ generation on target DSP core;
+- reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
+  pending.
+
+Required Properties:
+- compatible: should be "ti,keystone-mctrl-gpio"
+- ti,syscon-dev: phandle/offset pair. The phandle to syscon used to
+  access device state control registers and the offset of device's specific
+  registers within device state control registers range.
+- gpio-controller: Marks the device node as a gpio controller.
+- #gpio-cells: Should be 2.
+
+Please refer to gpio.txt in this directory for details of the common GPIO
+bindings used by client devices.
+
+Example:
+	dspgpio0: keystone_dsp_gpio at 02620240 {
+		compatible = "ti,keystone-mctrl-gpio";
+		ti,syscon-dev = <&devctrl 0x240>;
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
+
+	dsp0: dsp0 {
+		compatible = "linux,rproc-user";
+		...
+		kick-gpio = <&dspgpio0 27>;
+	};
diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
index 05d528f..8a7f715 100644
--- a/drivers/gpio/gpio-syscon.c
+++ b/drivers/gpio/gpio-syscon.c
@@ -145,11 +145,50 @@ static const struct syscon_gpio_data clps711x_mctrl_gpio = {
 	.dat_bit_offset	= 0x40 * 8 + 8,
 };
 
+#ifdef CONFIG_ARCH_KEYSTONE
+#define KEYSTONE_LOCK_BIT BIT(0)
+
+static void keystone_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
+{
+	struct syscon_gpio_priv *priv = to_syscon_gpio(chip);
+	unsigned int offs;
+	int ret;
+
+	offs = priv->dreg_offset + priv->data->dat_bit_offset + offset;
+
+	if (!val)
+		return;
+
+	ret = regmap_update_bits(
+			priv->syscon,
+			(offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE,
+			BIT(offs % SYSCON_REG_BITS) | KEYSTONE_LOCK_BIT,
+			BIT(offs % SYSCON_REG_BITS) | KEYSTONE_LOCK_BIT);
+	if (ret < 0)
+		dev_err(chip->dev, "gpio write failed ret(%d)\n", ret);
+}
+
+static const struct syscon_gpio_data keystone_mctrl_gpio = {
+	/* ARM Keystone 2 */
+	.compatible	= NULL,
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= 28,
+	.dat_bit_offset	= 4,
+	.set		= keystone_gpio_set,
+};
+#endif
+
 static const struct of_device_id syscon_gpio_ids[] = {
 	{
 		.compatible	= "cirrus,clps711x-mctrl-gpio",
 		.data		= &clps711x_mctrl_gpio,
 	},
+#ifdef CONFIG_ARCH_KEYSTONE
+	{
+		.compatible	= "ti,keystone-mctrl-gpio",
+		.data		= &keystone_mctrl_gpio,
+	},
+#endif
 	{ }
 };
 MODULE_DEVICE_TABLE(of, syscon_gpio_ids);
-- 
1.7.9.5

[PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Grygorii Strashko]

Add Keystone 2 DSP GPIO nodes.
DSP GPIO banks 0-7 correspond to DSP0-DSP7

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/boot/dts/k2hk.dtsi |   56 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/arch/arm/boot/dts/k2hk.dtsi b/arch/arm/boot/dts/k2hk.dtsi
index 321ba2f..009e180 100644
--- a/arch/arm/boot/dts/k2hk.dtsi
+++ b/arch/arm/boot/dts/k2hk.dtsi
@@ -50,5 +50,61 @@
 			#interrupt-cells = <1>;
 			ti,syscon-dev = <&devctrl 0x2a0>;
 		};
+
+		dspgpio0: keystone_dsp_gpio@02620240 {
+			compatible = "ti,keystone-mctrl-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio,syscon-dev = <&devctrl 0x240>;
+		};
+
+		dspgpio1: keystone_dsp_gpio@2620244 {
+			compatible = "ti,keystone-mctrl-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio,syscon-dev = <&devctrl 0x244>;
+		};
+
+		dspgpio2: keystone_dsp_gpio@2620248 {
+			compatible = "ti,keystone-mctrl-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio,syscon-dev = <&devctrl 0x248>;
+		};
+
+		dspgpio3: keystone_dsp_gpio@262024c {
+			compatible = "ti,keystone-mctrl-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio,syscon-dev = <&devctrl 0x24c>;
+		};
+
+		dspgpio4: keystone_dsp_gpio@2620250 {
+			compatible = "ti,keystone-mctrl-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio,syscon-dev = <&devctrl 0x250>;
+		};
+
+		dspgpio5: keystone_dsp_gpio@2620254 {
+			compatible = "ti,keystone-mctrl-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio,syscon-dev = <&devctrl 0x254>;
+		};
+
+		dspgpio6: keystone_dsp_gpio@2620258 {
+			compatible = "ti,keystone-mctrl-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio,syscon-dev = <&devctrl 0x258>;
+		};
+
+		dspgpio7: keystone_dsp_gpio@262025C {
+			compatible = "ti,keystone-mctrl-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio,syscon-dev = <&devctrl 0x25c>;
+		};
 	};
 };
-- 
1.7.9.5

[PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Grygorii Strashko]

Add Keystone 2 DSP GPIO nodes.
DSP GPIO banks 0-7 correspond to DSP0-DSP7

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/boot/dts/k2hk.dtsi |   56 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/arch/arm/boot/dts/k2hk.dtsi b/arch/arm/boot/dts/k2hk.dtsi
index 321ba2f..009e180 100644
--- a/arch/arm/boot/dts/k2hk.dtsi
+++ b/arch/arm/boot/dts/k2hk.dtsi
@@ -50,5 +50,61 @@
 			#interrupt-cells = <1>;
 			ti,syscon-dev = <&devctrl 0x2a0>;
 		};
+
+		dspgpio0: keystone_dsp_gpio at 02620240 {
+			compatible = "ti,keystone-mctrl-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio,syscon-dev = <&devctrl 0x240>;
+		};
+
+		dspgpio1: keystone_dsp_gpio at 2620244 {
+			compatible = "ti,keystone-mctrl-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio,syscon-dev = <&devctrl 0x244>;
+		};
+
+		dspgpio2: keystone_dsp_gpio at 2620248 {
+			compatible = "ti,keystone-mctrl-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio,syscon-dev = <&devctrl 0x248>;
+		};
+
+		dspgpio3: keystone_dsp_gpio at 262024c {
+			compatible = "ti,keystone-mctrl-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio,syscon-dev = <&devctrl 0x24c>;
+		};
+
+		dspgpio4: keystone_dsp_gpio at 2620250 {
+			compatible = "ti,keystone-mctrl-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio,syscon-dev = <&devctrl 0x250>;
+		};
+
+		dspgpio5: keystone_dsp_gpio at 2620254 {
+			compatible = "ti,keystone-mctrl-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio,syscon-dev = <&devctrl 0x254>;
+		};
+
+		dspgpio6: keystone_dsp_gpio at 2620258 {
+			compatible = "ti,keystone-mctrl-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio,syscon-dev = <&devctrl 0x258>;
+		};
+
+		dspgpio7: keystone_dsp_gpio at 262025C {
+			compatible = "ti,keystone-mctrl-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio,syscon-dev = <&devctrl 0x25c>;
+		};
 	};
 };
-- 
1.7.9.5

Re: [PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Alexander Shiyan]

Thu, 14 Aug 2014 15:13:39 +0300 от Grygorii Strashko <grygorii.strashko@ti.com>:
> Hi Alexander,
> 
> On 08/13/2014 07:06 PM, Alexander Shiyan wrote:
> > Wed, 13 Aug 2014 19:16:22 +0300 от Grygorii Strashko <grygorii.strashko@ti.com>:
> >> Add Keystone 2 DSP GPIO nodes.
> >> DSP GPIO banks 0-7 correspond to DSP0-DSP7
> >>
> >> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> >> ---
> >>   arch/arm/boot/dts/k2hk.dtsi |   56 +++++++++++++++++++++++++++++++++++++++++++
> >>   1 file changed, 56 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/k2hk.dtsi b/arch/arm/boot/dts/k2hk.dtsi
> >> index 321ba2f..009e180 100644
> >> --- a/arch/arm/boot/dts/k2hk.dtsi
> >> +++ b/arch/arm/boot/dts/k2hk.dtsi
> >> @@ -50,5 +50,61 @@
> >>   			#interrupt-cells = <1>;
> >>   			ti,syscon-dev = <&devctrl 0x2a0>;
> >>   		};
> >> +
> >> +		dspgpio0: keystone_dsp_gpio@02620240 {
> >> +			compatible = "ti,keystone-mctrl-gpio";
> >> +			gpio-controller;
> >> +			#gpio-cells = <2>;
> >> +			gpio,syscon-dev = <&devctrl 0x240>;
> >> +		};
> >> +
> >> +		dspgpio1: keystone_dsp_gpio@2620244 {
> >> +			compatible = "ti,keystone-mctrl-gpio";
> >> +			gpio-controller;
> >> +			#gpio-cells = <2>;
> >> +			gpio,syscon-dev = <&devctrl 0x244>;
> >> +		};
> > ...
> >> +		dspgpio7: keystone_dsp_gpio@262025C {
> >> +			compatible = "ti,keystone-mctrl-gpio";
> >> +			gpio-controller;
> >> +			#gpio-cells = <2>;
> >> +			gpio,syscon-dev = <&devctrl 0x25c>;
> >> +		};
> > 
> > So, devctrl is a syscon device and this DTS introduce several
> > identical GPIO descriptions?
> > 
> > On my opinion this should be placed in the gpio-syscon.c,
> > where you can add support for ti,keystone-dsp0{..7}-gpio.
> > Such change will avoid parts 2 and 3 of this patch.
> > 
> > static const struct syscon_gpio_data ti_keystone_dsp0_gpio = {
> >    .compatible = "ti,keystone-syscon",
> >    .dat_bit_offset = 0x240 * 8,
> >    ...
> >    .set = etc...
> > };
> 
> So, if I understand you right, you propose to add 8 additional compatible
> strings just to encode different register offsets. Is it?
> 1) add "ti,keystone-mctrl-gpio0"..."ti,keystone-mctrl-gpio7"

Yes, but replace "mctrl" with "dsp" since mctrl is not applicable here.

> 2) add 8 structures keystone_mctrl_gpio0..keystone_mctrl_gpio7 in gpio-syscon.c
>    (which will have only different values of field - .dat_bit_offset = 0x2yy * 8,)
> 
> 3) add 8 additional items in array syscon_gpio_ids
> 	{
> 		.compatible	= "ti,keystone-mctrl-gpio0",
> 		.data		= &keystone_mctrl_gpio0,
> 	}, ...
> 
> I can do it if you strictly insist, BUT I don't like it :(
> - just imagine how your driver will look look like if 5 or 6 SoCs will re-use it ;)
> - as I mentioned in cover letter and commit messages even each SoC revision can have
>   different Syscon implementation with different registers offsets and with different
>   number of Syscon register ranges (for example for Keystone 2 we already have two
>   Syscon devices defined in DT).

The initial version of this driver contains addresses and offsets in, but this approach has
been criticized by DT maintainers.

"different Syscon with different registers" - is OK, since we use compatible string in definition.
If you have two identical syscon, just append an unique additional string and use it in this driver.

> Now we already have 3 Keystone 2 SoC revisions (K2HK, K2L, K2E)  and there are no guarantee
> that the next revision k2X will have the same register offset for GPIO0.

Then use more exact string for syscon-gpio, like ti,keystone-k2hk-dsp-gpio0.

---

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Alexander Shiyan]

Thu, 14 Aug 2014 15:13:39 +0300 ?? Grygorii Strashko <grygorii.strashko@ti.com>:
> Hi Alexander,
> 
> On 08/13/2014 07:06 PM, Alexander Shiyan wrote:
> > Wed, 13 Aug 2014 19:16:22 +0300 ?? Grygorii Strashko <grygorii.strashko@ti.com>:
> >> Add Keystone 2 DSP GPIO nodes.
> >> DSP GPIO banks 0-7 correspond to DSP0-DSP7
> >>
> >> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> >> ---
> >>   arch/arm/boot/dts/k2hk.dtsi |   56 +++++++++++++++++++++++++++++++++++++++++++
> >>   1 file changed, 56 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/k2hk.dtsi b/arch/arm/boot/dts/k2hk.dtsi
> >> index 321ba2f..009e180 100644
> >> --- a/arch/arm/boot/dts/k2hk.dtsi
> >> +++ b/arch/arm/boot/dts/k2hk.dtsi
> >> @@ -50,5 +50,61 @@
> >>   			#interrupt-cells = <1>;
> >>   			ti,syscon-dev = <&devctrl 0x2a0>;
> >>   		};
> >> +
> >> +		dspgpio0: keystone_dsp_gpio at 02620240 {
> >> +			compatible = "ti,keystone-mctrl-gpio";
> >> +			gpio-controller;
> >> +			#gpio-cells = <2>;
> >> +			gpio,syscon-dev = <&devctrl 0x240>;
> >> +		};
> >> +
> >> +		dspgpio1: keystone_dsp_gpio at 2620244 {
> >> +			compatible = "ti,keystone-mctrl-gpio";
> >> +			gpio-controller;
> >> +			#gpio-cells = <2>;
> >> +			gpio,syscon-dev = <&devctrl 0x244>;
> >> +		};
> > ...
> >> +		dspgpio7: keystone_dsp_gpio at 262025C {
> >> +			compatible = "ti,keystone-mctrl-gpio";
> >> +			gpio-controller;
> >> +			#gpio-cells = <2>;
> >> +			gpio,syscon-dev = <&devctrl 0x25c>;
> >> +		};
> > 
> > So, devctrl is a syscon device and this DTS introduce several
> > identical GPIO descriptions?
> > 
> > On my opinion this should be placed in the gpio-syscon.c,
> > where you can add support for ti,keystone-dsp0{..7}-gpio.
> > Such change will avoid parts 2 and 3 of this patch.
> > 
> > static const struct syscon_gpio_data ti_keystone_dsp0_gpio = {
> >    .compatible = "ti,keystone-syscon",
> >    .dat_bit_offset = 0x240 * 8,
> >    ...
> >    .set = etc...
> > };
> 
> So, if I understand you right, you propose to add 8 additional compatible
> strings just to encode different register offsets. Is it?
> 1) add "ti,keystone-mctrl-gpio0"..."ti,keystone-mctrl-gpio7"

Yes, but replace "mctrl" with "dsp" since mctrl is not applicable here.

> 2) add 8 structures keystone_mctrl_gpio0..keystone_mctrl_gpio7 in gpio-syscon.c
>    (which will have only different values of field - .dat_bit_offset = 0x2yy * 8,)
> 
> 3) add 8 additional items in array syscon_gpio_ids
> 	{
> 		.compatible	= "ti,keystone-mctrl-gpio0",
> 		.data		= &keystone_mctrl_gpio0,
> 	}, ...
> 
> I can do it if you strictly insist, BUT I don't like it :(
> - just imagine how your driver will look look like if 5 or 6 SoCs will re-use it ;)
> - as I mentioned in cover letter and commit messages even each SoC revision can have
>   different Syscon implementation with different registers offsets and with different
>   number of Syscon register ranges (for example for Keystone 2 we already have two
>   Syscon devices defined in DT).

The initial version of this driver contains addresses and offsets in, but this approach has
been criticized by DT maintainers.

"different Syscon with different registers" - is OK, since we use compatible string in definition.
If you have two identical syscon, just append an unique additional string and use it in this driver.

> Now we already have 3 Keystone 2 SoC revisions (K2HK, K2L, K2E)  and there are no guarantee
> that the next revision k2X will have the same register offset for GPIO0.

Then use more exact string for syscon-gpio, like ti,keystone-k2hk-dsp-gpio0.

---

Re: [PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Grygorii Strashko]

Hi Alexander,

On 08/13/2014 07:06 PM, Alexander Shiyan wrote:
> Wed, 13 Aug 2014 19:16:22 +0300 от Grygorii Strashko <grygorii.strashko@ti.com>:
>> Add Keystone 2 DSP GPIO nodes.
>> DSP GPIO banks 0-7 correspond to DSP0-DSP7
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>   arch/arm/boot/dts/k2hk.dtsi |   56 +++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 56 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/k2hk.dtsi b/arch/arm/boot/dts/k2hk.dtsi
>> index 321ba2f..009e180 100644
>> --- a/arch/arm/boot/dts/k2hk.dtsi
>> +++ b/arch/arm/boot/dts/k2hk.dtsi
>> @@ -50,5 +50,61 @@
>>   			#interrupt-cells = <1>;
>>   			ti,syscon-dev = <&devctrl 0x2a0>;
>>   		};
>> +
>> +		dspgpio0: keystone_dsp_gpio@02620240 {
>> +			compatible = "ti,keystone-mctrl-gpio";
>> +			gpio-controller;
>> +			#gpio-cells = <2>;
>> +			gpio,syscon-dev = <&devctrl 0x240>;
>> +		};
>> +
>> +		dspgpio1: keystone_dsp_gpio@2620244 {
>> +			compatible = "ti,keystone-mctrl-gpio";
>> +			gpio-controller;
>> +			#gpio-cells = <2>;
>> +			gpio,syscon-dev = <&devctrl 0x244>;
>> +		};
> ...
>> +		dspgpio7: keystone_dsp_gpio@262025C {
>> +			compatible = "ti,keystone-mctrl-gpio";
>> +			gpio-controller;
>> +			#gpio-cells = <2>;
>> +			gpio,syscon-dev = <&devctrl 0x25c>;
>> +		};
> 
> So, devctrl is a syscon device and this DTS introduce several
> identical GPIO descriptions?
> 
> On my opinion this should be placed in the gpio-syscon.c,
> where you can add support for ti,keystone-dsp0{..7}-gpio.
> Such change will avoid parts 2 and 3 of this patch.
> 
> static const struct syscon_gpio_data ti_keystone_dsp0_gpio = {
>    .compatible = "ti,keystone-syscon",
>    .dat_bit_offset = 0x240 * 8,
>    ...
>    .set = etc...
> };

So, if I understand you right, you propose to add 8 additional compatible
strings just to encode different register offsets. Is it?
1) add "ti,keystone-mctrl-gpio0"..."ti,keystone-mctrl-gpio7"

2) add 8 structures keystone_mctrl_gpio0..keystone_mctrl_gpio7 in gpio-syscon.c
   (which will have only different values of field - .dat_bit_offset = 0x2yy * 8,)

3) add 8 additional items in array syscon_gpio_ids
	{
		.compatible	= "ti,keystone-mctrl-gpio0",
		.data		= &keystone_mctrl_gpio0,
	}, ...

I can do it if you strictly insist, BUT I don't like it :(
- just imagine how your driver will look look like if 5 or 6 SoCs will re-use it ;)
- as I mentioned in cover letter and commit messages even each SoC revision can have
  different Syscon implementation with different registers offsets and with different
  number of Syscon register ranges (for example for Keystone 2 we already have two
  Syscon devices defined in DT).

Now we already have 3 Keystone 2 SoC revisions (K2HK, K2L, K2E)  and there are no guarantee
that the next revision k2X will have the same register offset for GPIO0.

Best regards,
-grygorii

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

[PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Grygorii Strashko]

Hi Alexander,

On 08/13/2014 07:06 PM, Alexander Shiyan wrote:
> Wed, 13 Aug 2014 19:16:22 +0300 ?? Grygorii Strashko <grygorii.strashko@ti.com>:
>> Add Keystone 2 DSP GPIO nodes.
>> DSP GPIO banks 0-7 correspond to DSP0-DSP7
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>   arch/arm/boot/dts/k2hk.dtsi |   56 +++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 56 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/k2hk.dtsi b/arch/arm/boot/dts/k2hk.dtsi
>> index 321ba2f..009e180 100644
>> --- a/arch/arm/boot/dts/k2hk.dtsi
>> +++ b/arch/arm/boot/dts/k2hk.dtsi
>> @@ -50,5 +50,61 @@
>>   			#interrupt-cells = <1>;
>>   			ti,syscon-dev = <&devctrl 0x2a0>;
>>   		};
>> +
>> +		dspgpio0: keystone_dsp_gpio at 02620240 {
>> +			compatible = "ti,keystone-mctrl-gpio";
>> +			gpio-controller;
>> +			#gpio-cells = <2>;
>> +			gpio,syscon-dev = <&devctrl 0x240>;
>> +		};
>> +
>> +		dspgpio1: keystone_dsp_gpio at 2620244 {
>> +			compatible = "ti,keystone-mctrl-gpio";
>> +			gpio-controller;
>> +			#gpio-cells = <2>;
>> +			gpio,syscon-dev = <&devctrl 0x244>;
>> +		};
> ...
>> +		dspgpio7: keystone_dsp_gpio at 262025C {
>> +			compatible = "ti,keystone-mctrl-gpio";
>> +			gpio-controller;
>> +			#gpio-cells = <2>;
>> +			gpio,syscon-dev = <&devctrl 0x25c>;
>> +		};
> 
> So, devctrl is a syscon device and this DTS introduce several
> identical GPIO descriptions?
> 
> On my opinion this should be placed in the gpio-syscon.c,
> where you can add support for ti,keystone-dsp0{..7}-gpio.
> Such change will avoid parts 2 and 3 of this patch.
> 
> static const struct syscon_gpio_data ti_keystone_dsp0_gpio = {
>    .compatible = "ti,keystone-syscon",
>    .dat_bit_offset = 0x240 * 8,
>    ...
>    .set = etc...
> };

So, if I understand you right, you propose to add 8 additional compatible
strings just to encode different register offsets. Is it?
1) add "ti,keystone-mctrl-gpio0"..."ti,keystone-mctrl-gpio7"

2) add 8 structures keystone_mctrl_gpio0..keystone_mctrl_gpio7 in gpio-syscon.c
   (which will have only different values of field - .dat_bit_offset = 0x2yy * 8,)

3) add 8 additional items in array syscon_gpio_ids
	{
		.compatible	= "ti,keystone-mctrl-gpio0",
		.data		= &keystone_mctrl_gpio0,
	}, ...

I can do it if you strictly insist, BUT I don't like it :(
- just imagine how your driver will look look like if 5 or 6 SoCs will re-use it ;)
- as I mentioned in cover letter and commit messages even each SoC revision can have
  different Syscon implementation with different registers offsets and with different
  number of Syscon register ranges (for example for Keystone 2 we already have two
  Syscon devices defined in DT).

Now we already have 3 Keystone 2 SoC revisions (K2HK, K2L, K2E)  and there are no guarantee
that the next revision k2X will have the same register offset for GPIO0.

Best regards,
-grygorii

Re: [PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Alexander Shiyan]

Thu, 14 Aug 2014 18:57:08 +0300 от Grygorii Strashko <grygorii.strashko@ti.com>:
...
> >>>> +		dspgpio7: keystone_dsp_gpio@262025C {
> >>>> +			compatible = "ti,keystone-mctrl-gpio";
> >>>> +			gpio-controller;
> >>>> +			#gpio-cells = <2>;
> >>>> +			gpio,syscon-dev = <&devctrl 0x25c>;
> >>>> +		};
> >>>
> >>> So, devctrl is a syscon device and this DTS introduce several
> >>> identical GPIO descriptions?
> >>>
> >>> On my opinion this should be placed in the gpio-syscon.c,
> >>> where you can add support for ti,keystone-dsp0{..7}-gpio.
> >>> Such change will avoid parts 2 and 3 of this patch.
> >>>
> >>> static const struct syscon_gpio_data ti_keystone_dsp0_gpio = {
> >>>     .compatible = "ti,keystone-syscon",
> >>>     .dat_bit_offset = 0x240 * 8,
> >>>     ...
> >>>     .set = etc...
> >>> };
> >>
> >> So, if I understand you right, you propose to add 8 additional compatible
> >> strings just to encode different register offsets. Is it?
> >> 1) add "ti,keystone-mctrl-gpio0"..."ti,keystone-mctrl-gpio7"
> > 
> > Yes, but replace "mctrl" with "dsp" since mctrl is not applicable here.
> > 
> >> 2) add 8 structures keystone_mctrl_gpio0..keystone_mctrl_gpio7 in gpio-syscon.c
> >>     (which will have only different values of field - .dat_bit_offset = 0x2yy * 8,)
> >>
> >> 3) add 8 additional items in array syscon_gpio_ids
> >> 	{
> >> 		.compatible	= "ti,keystone-mctrl-gpio0",
> >> 		.data		= &keystone_mctrl_gpio0,
> >> 	}, ...
> >>
> >> I can do it if you strictly insist, BUT I don't like it :(
> >> - just imagine how your driver will look look like if 5 or 6 SoCs will re-use it ;)
> >> - as I mentioned in cover letter and commit messages even each SoC revision can have
> >>    different Syscon implementation with different registers offsets and with different
> >>    number of Syscon register ranges (for example for Keystone 2 we already have two
> >>    Syscon devices defined in DT).
> > 
> > The initial version of this driver contains addresses and offsets in, but this approach has
> > been criticized by DT maintainers.
> 
> Could you provide link on this thread^, pls?

Here is a link to first version:
https://www.mail-archive.com/linux-gpio@vger.kernel.org/msg01616.html

Unfortunately, I lost thread for DT-related stuffs.

---

Re: [PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Alexander Shiyan]

Thu, 14 Aug 2014 18:57:08 +0300 ?? Grygorii Strashko <grygorii.strashko@ti.com>:
...
> >>>> +		dspgpio7: keystone_dsp_gpio at 262025C {
> >>>> +			compatible = "ti,keystone-mctrl-gpio";
> >>>> +			gpio-controller;
> >>>> +			#gpio-cells = <2>;
> >>>> +			gpio,syscon-dev = <&devctrl 0x25c>;
> >>>> +		};
> >>>
> >>> So, devctrl is a syscon device and this DTS introduce several
> >>> identical GPIO descriptions?
> >>>
> >>> On my opinion this should be placed in the gpio-syscon.c,
> >>> where you can add support for ti,keystone-dsp0{..7}-gpio.
> >>> Such change will avoid parts 2 and 3 of this patch.
> >>>
> >>> static const struct syscon_gpio_data ti_keystone_dsp0_gpio = {
> >>>     .compatible = "ti,keystone-syscon",
> >>>     .dat_bit_offset = 0x240 * 8,
> >>>     ...
> >>>     .set = etc...
> >>> };
> >>
> >> So, if I understand you right, you propose to add 8 additional compatible
> >> strings just to encode different register offsets. Is it?
> >> 1) add "ti,keystone-mctrl-gpio0"..."ti,keystone-mctrl-gpio7"
> > 
> > Yes, but replace "mctrl" with "dsp" since mctrl is not applicable here.
> > 
> >> 2) add 8 structures keystone_mctrl_gpio0..keystone_mctrl_gpio7 in gpio-syscon.c
> >>     (which will have only different values of field - .dat_bit_offset = 0x2yy * 8,)
> >>
> >> 3) add 8 additional items in array syscon_gpio_ids
> >> 	{
> >> 		.compatible	= "ti,keystone-mctrl-gpio0",
> >> 		.data		= &keystone_mctrl_gpio0,
> >> 	}, ...
> >>
> >> I can do it if you strictly insist, BUT I don't like it :(
> >> - just imagine how your driver will look look like if 5 or 6 SoCs will re-use it ;)
> >> - as I mentioned in cover letter and commit messages even each SoC revision can have
> >>    different Syscon implementation with different registers offsets and with different
> >>    number of Syscon register ranges (for example for Keystone 2 we already have two
> >>    Syscon devices defined in DT).
> > 
> > The initial version of this driver contains addresses and offsets in, but this approach has
> > been criticized by DT maintainers.
> 
> Could you provide link on this thread^, pls?

Here is a link to first version:
https://www.mail-archive.com/linux-gpio at vger.kernel.org/msg01616.html

Unfortunately, I lost thread for DT-related stuffs.

---

Re: [PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Grygorii Strashko]

On 08/14/2014 03:12 PM, Alexander Shiyan wrote:
> Thu, 14 Aug 2014 15:13:39 +0300 от Grygorii Strashko <grygorii.strashko@ti.com>:
>> Hi Alexander,
>>
>> On 08/13/2014 07:06 PM, Alexander Shiyan wrote:
>>> Wed, 13 Aug 2014 19:16:22 +0300 от Grygorii Strashko <grygorii.strashko@ti.com>:
>>>> Add Keystone 2 DSP GPIO nodes.
>>>> DSP GPIO banks 0-7 correspond to DSP0-DSP7
>>>>
>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>>> ---
>>>>    arch/arm/boot/dts/k2hk.dtsi |   56 +++++++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 56 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/k2hk.dtsi b/arch/arm/boot/dts/k2hk.dtsi
>>>> index 321ba2f..009e180 100644
>>>> --- a/arch/arm/boot/dts/k2hk.dtsi
>>>> +++ b/arch/arm/boot/dts/k2hk.dtsi
>>>> @@ -50,5 +50,61 @@
>>>>    			#interrupt-cells = <1>;
>>>>    			ti,syscon-dev = <&devctrl 0x2a0>;
>>>>    		};
>>>> +
>>>> +		dspgpio0: keystone_dsp_gpio@02620240 {
>>>> +			compatible = "ti,keystone-mctrl-gpio";
>>>> +			gpio-controller;
>>>> +			#gpio-cells = <2>;
>>>> +			gpio,syscon-dev = <&devctrl 0x240>;
>>>> +		};
>>>> +
>>>> +		dspgpio1: keystone_dsp_gpio@2620244 {
>>>> +			compatible = "ti,keystone-mctrl-gpio";
>>>> +			gpio-controller;
>>>> +			#gpio-cells = <2>;
>>>> +			gpio,syscon-dev = <&devctrl 0x244>;
>>>> +		};
>>> ...
>>>> +		dspgpio7: keystone_dsp_gpio@262025C {
>>>> +			compatible = "ti,keystone-mctrl-gpio";
>>>> +			gpio-controller;
>>>> +			#gpio-cells = <2>;
>>>> +			gpio,syscon-dev = <&devctrl 0x25c>;
>>>> +		};
>>>
>>> So, devctrl is a syscon device and this DTS introduce several
>>> identical GPIO descriptions?
>>>
>>> On my opinion this should be placed in the gpio-syscon.c,
>>> where you can add support for ti,keystone-dsp0{..7}-gpio.
>>> Such change will avoid parts 2 and 3 of this patch.
>>>
>>> static const struct syscon_gpio_data ti_keystone_dsp0_gpio = {
>>>     .compatible = "ti,keystone-syscon",
>>>     .dat_bit_offset = 0x240 * 8,
>>>     ...
>>>     .set = etc...
>>> };
>>
>> So, if I understand you right, you propose to add 8 additional compatible
>> strings just to encode different register offsets. Is it?
>> 1) add "ti,keystone-mctrl-gpio0"..."ti,keystone-mctrl-gpio7"
> 
> Yes, but replace "mctrl" with "dsp" since mctrl is not applicable here.
> 
>> 2) add 8 structures keystone_mctrl_gpio0..keystone_mctrl_gpio7 in gpio-syscon.c
>>     (which will have only different values of field - .dat_bit_offset = 0x2yy * 8,)
>>
>> 3) add 8 additional items in array syscon_gpio_ids
>> 	{
>> 		.compatible	= "ti,keystone-mctrl-gpio0",
>> 		.data		= &keystone_mctrl_gpio0,
>> 	}, ...
>>
>> I can do it if you strictly insist, BUT I don't like it :(
>> - just imagine how your driver will look look like if 5 or 6 SoCs will re-use it ;)
>> - as I mentioned in cover letter and commit messages even each SoC revision can have
>>    different Syscon implementation with different registers offsets and with different
>>    number of Syscon register ranges (for example for Keystone 2 we already have two
>>    Syscon devices defined in DT).
> 
> The initial version of this driver contains addresses and offsets in, but this approach has
> been criticized by DT maintainers.

Could you provide link on this thread^, pls?

Curious, it looks like Rob Herring has just proposed to encode offsets & bits in DT:
"Re: [PATCH 2/6] leds: add device tree bindings for syscon LEDs"
http://www.spinics.net/lists/arm-kernel/msg354182.html

> 
> "different Syscon with different registers" - is OK, since we use compatible string in definition.
> If you have two identical syscon, just append an unique additional string and use it in this driver.
> 
>> Now we already have 3 Keystone 2 SoC revisions (K2HK, K2L, K2E)  and there are no guarantee
>> that the next revision k2X will have the same register offset for GPIO0.
> 
> Then use more exact string for syscon-gpio, like ti,keystone-k2hk-dsp-gpio0.

Nothing to say :) - DT is funny thing. It has such good feature as phandle, but
people continue hard-coding relation between HW blocks in code :(

Any way, Thanks for your comments. I'll wait a bit then update and resend.


Best regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Grygorii Strashko]

On 08/14/2014 03:12 PM, Alexander Shiyan wrote:
> Thu, 14 Aug 2014 15:13:39 +0300 ?? Grygorii Strashko <grygorii.strashko@ti.com>:
>> Hi Alexander,
>>
>> On 08/13/2014 07:06 PM, Alexander Shiyan wrote:
>>> Wed, 13 Aug 2014 19:16:22 +0300 ?? Grygorii Strashko <grygorii.strashko@ti.com>:
>>>> Add Keystone 2 DSP GPIO nodes.
>>>> DSP GPIO banks 0-7 correspond to DSP0-DSP7
>>>>
>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>>> ---
>>>>    arch/arm/boot/dts/k2hk.dtsi |   56 +++++++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 56 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/k2hk.dtsi b/arch/arm/boot/dts/k2hk.dtsi
>>>> index 321ba2f..009e180 100644
>>>> --- a/arch/arm/boot/dts/k2hk.dtsi
>>>> +++ b/arch/arm/boot/dts/k2hk.dtsi
>>>> @@ -50,5 +50,61 @@
>>>>    			#interrupt-cells = <1>;
>>>>    			ti,syscon-dev = <&devctrl 0x2a0>;
>>>>    		};
>>>> +
>>>> +		dspgpio0: keystone_dsp_gpio at 02620240 {
>>>> +			compatible = "ti,keystone-mctrl-gpio";
>>>> +			gpio-controller;
>>>> +			#gpio-cells = <2>;
>>>> +			gpio,syscon-dev = <&devctrl 0x240>;
>>>> +		};
>>>> +
>>>> +		dspgpio1: keystone_dsp_gpio at 2620244 {
>>>> +			compatible = "ti,keystone-mctrl-gpio";
>>>> +			gpio-controller;
>>>> +			#gpio-cells = <2>;
>>>> +			gpio,syscon-dev = <&devctrl 0x244>;
>>>> +		};
>>> ...
>>>> +		dspgpio7: keystone_dsp_gpio at 262025C {
>>>> +			compatible = "ti,keystone-mctrl-gpio";
>>>> +			gpio-controller;
>>>> +			#gpio-cells = <2>;
>>>> +			gpio,syscon-dev = <&devctrl 0x25c>;
>>>> +		};
>>>
>>> So, devctrl is a syscon device and this DTS introduce several
>>> identical GPIO descriptions?
>>>
>>> On my opinion this should be placed in the gpio-syscon.c,
>>> where you can add support for ti,keystone-dsp0{..7}-gpio.
>>> Such change will avoid parts 2 and 3 of this patch.
>>>
>>> static const struct syscon_gpio_data ti_keystone_dsp0_gpio = {
>>>     .compatible = "ti,keystone-syscon",
>>>     .dat_bit_offset = 0x240 * 8,
>>>     ...
>>>     .set = etc...
>>> };
>>
>> So, if I understand you right, you propose to add 8 additional compatible
>> strings just to encode different register offsets. Is it?
>> 1) add "ti,keystone-mctrl-gpio0"..."ti,keystone-mctrl-gpio7"
> 
> Yes, but replace "mctrl" with "dsp" since mctrl is not applicable here.
> 
>> 2) add 8 structures keystone_mctrl_gpio0..keystone_mctrl_gpio7 in gpio-syscon.c
>>     (which will have only different values of field - .dat_bit_offset = 0x2yy * 8,)
>>
>> 3) add 8 additional items in array syscon_gpio_ids
>> 	{
>> 		.compatible	= "ti,keystone-mctrl-gpio0",
>> 		.data		= &keystone_mctrl_gpio0,
>> 	}, ...
>>
>> I can do it if you strictly insist, BUT I don't like it :(
>> - just imagine how your driver will look look like if 5 or 6 SoCs will re-use it ;)
>> - as I mentioned in cover letter and commit messages even each SoC revision can have
>>    different Syscon implementation with different registers offsets and with different
>>    number of Syscon register ranges (for example for Keystone 2 we already have two
>>    Syscon devices defined in DT).
> 
> The initial version of this driver contains addresses and offsets in, but this approach has
> been criticized by DT maintainers.

Could you provide link on this thread^, pls?

Curious, it looks like Rob Herring has just proposed to encode offsets & bits in DT:
"Re: [PATCH 2/6] leds: add device tree bindings for syscon LEDs"
http://www.spinics.net/lists/arm-kernel/msg354182.html

> 
> "different Syscon with different registers" - is OK, since we use compatible string in definition.
> If you have two identical syscon, just append an unique additional string and use it in this driver.
> 
>> Now we already have 3 Keystone 2 SoC revisions (K2HK, K2L, K2E)  and there are no guarantee
>> that the next revision k2X will have the same register offset for GPIO0.
> 
> Then use more exact string for syscon-gpio, like ti,keystone-k2hk-dsp-gpio0.

Nothing to say :) - DT is funny thing. It has such good feature as phandle, but
people continue hard-coding relation between HW blocks in code :(

Any way, Thanks for your comments. I'll wait a bit then update and resend.


Best regards,
-grygorii

Re: [PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Grygorii Strashko]

On 08/14/2014 06:26 PM, Alexander Shiyan wrote:
> Thu, 14 Aug 2014 18:57:08 +0300 от Grygorii Strashko <grygorii.strashko@ti.com>:
> ...
>>>>>> +		dspgpio7: keystone_dsp_gpio@262025C {
>>>>>> +			compatible = "ti,keystone-mctrl-gpio";
>>>>>> +			gpio-controller;
>>>>>> +			#gpio-cells = <2>;
>>>>>> +			gpio,syscon-dev = <&devctrl 0x25c>;
>>>>>> +		};
>>>>>
>>>>> So, devctrl is a syscon device and this DTS introduce several
>>>>> identical GPIO descriptions?
>>>>>
>>>>> On my opinion this should be placed in the gpio-syscon.c,
>>>>> where you can add support for ti,keystone-dsp0{..7}-gpio.
>>>>> Such change will avoid parts 2 and 3 of this patch.
>>>>>
>>>>> static const struct syscon_gpio_data ti_keystone_dsp0_gpio = {
>>>>>      .compatible = "ti,keystone-syscon",
>>>>>      .dat_bit_offset = 0x240 * 8,
>>>>>      ...
>>>>>      .set = etc...
>>>>> };
>>>>
>>>> So, if I understand you right, you propose to add 8 additional compatible
>>>> strings just to encode different register offsets. Is it?
>>>> 1) add "ti,keystone-mctrl-gpio0"..."ti,keystone-mctrl-gpio7"
>>>
>>> Yes, but replace "mctrl" with "dsp" since mctrl is not applicable here.
>>>
>>>> 2) add 8 structures keystone_mctrl_gpio0..keystone_mctrl_gpio7 in gpio-syscon.c
>>>>      (which will have only different values of field - .dat_bit_offset = 0x2yy * 8,)
>>>>
>>>> 3) add 8 additional items in array syscon_gpio_ids
>>>> 	{
>>>> 		.compatible	= "ti,keystone-mctrl-gpio0",
>>>> 		.data		= &keystone_mctrl_gpio0,
>>>> 	}, ...
>>>>
>>>> I can do it if you strictly insist, BUT I don't like it :(
>>>> - just imagine how your driver will look look like if 5 or 6 SoCs will re-use it ;)
>>>> - as I mentioned in cover letter and commit messages even each SoC revision can have
>>>>     different Syscon implementation with different registers offsets and with different
>>>>     number of Syscon register ranges (for example for Keystone 2 we already have two
>>>>     Syscon devices defined in DT).
>>>
>>> The initial version of this driver contains addresses and offsets in, but this approach has
>>> been criticized by DT maintainers.
>>
>> Could you provide link on this thread^, pls?
> 
> Here is a link to first version:
> https://www.mail-archive.com/linux-gpio@vger.kernel.org/msg01616.html

10x

> 
> Unfortunately, I lost thread for DT-related stuffs.
> 

Oh, I've just checked ARM dts directory and found that 
constructions like this:
	vendor,syscon = <&syscon_phandle>; /// or even this vendor,syscon = <&syscon_phandle 0x60 2>;
are widely used as of now.
Also, According to the bindings doc, the Syscon is not a Linux specific definition (mfd/syscon.txt).

Regards,
-grygorii

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

[PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Grygorii Strashko]

On 08/14/2014 06:26 PM, Alexander Shiyan wrote:
> Thu, 14 Aug 2014 18:57:08 +0300 ?? Grygorii Strashko <grygorii.strashko@ti.com>:
> ...
>>>>>> +		dspgpio7: keystone_dsp_gpio at 262025C {
>>>>>> +			compatible = "ti,keystone-mctrl-gpio";
>>>>>> +			gpio-controller;
>>>>>> +			#gpio-cells = <2>;
>>>>>> +			gpio,syscon-dev = <&devctrl 0x25c>;
>>>>>> +		};
>>>>>
>>>>> So, devctrl is a syscon device and this DTS introduce several
>>>>> identical GPIO descriptions?
>>>>>
>>>>> On my opinion this should be placed in the gpio-syscon.c,
>>>>> where you can add support for ti,keystone-dsp0{..7}-gpio.
>>>>> Such change will avoid parts 2 and 3 of this patch.
>>>>>
>>>>> static const struct syscon_gpio_data ti_keystone_dsp0_gpio = {
>>>>>      .compatible = "ti,keystone-syscon",
>>>>>      .dat_bit_offset = 0x240 * 8,
>>>>>      ...
>>>>>      .set = etc...
>>>>> };
>>>>
>>>> So, if I understand you right, you propose to add 8 additional compatible
>>>> strings just to encode different register offsets. Is it?
>>>> 1) add "ti,keystone-mctrl-gpio0"..."ti,keystone-mctrl-gpio7"
>>>
>>> Yes, but replace "mctrl" with "dsp" since mctrl is not applicable here.
>>>
>>>> 2) add 8 structures keystone_mctrl_gpio0..keystone_mctrl_gpio7 in gpio-syscon.c
>>>>      (which will have only different values of field - .dat_bit_offset = 0x2yy * 8,)
>>>>
>>>> 3) add 8 additional items in array syscon_gpio_ids
>>>> 	{
>>>> 		.compatible	= "ti,keystone-mctrl-gpio0",
>>>> 		.data		= &keystone_mctrl_gpio0,
>>>> 	}, ...
>>>>
>>>> I can do it if you strictly insist, BUT I don't like it :(
>>>> - just imagine how your driver will look look like if 5 or 6 SoCs will re-use it ;)
>>>> - as I mentioned in cover letter and commit messages even each SoC revision can have
>>>>     different Syscon implementation with different registers offsets and with different
>>>>     number of Syscon register ranges (for example for Keystone 2 we already have two
>>>>     Syscon devices defined in DT).
>>>
>>> The initial version of this driver contains addresses and offsets in, but this approach has
>>> been criticized by DT maintainers.
>>
>> Could you provide link on this thread^, pls?
> 
> Here is a link to first version:
> https://www.mail-archive.com/linux-gpio at vger.kernel.org/msg01616.html

10x

> 
> Unfortunately, I lost thread for DT-related stuffs.
> 

Oh, I've just checked ARM dts directory and found that 
constructions like this:
	vendor,syscon = <&syscon_phandle>; /// or even this vendor,syscon = <&syscon_phandle 0x60 2>;
are widely used as of now.
Also, According to the bindings doc, the Syscon is not a Linux specific definition (mfd/syscon.txt).

Regards,
-grygorii

[PATCH v2 0/3] gpio: syscon: reuse for keystone 2 socs

[Grygorii Strashko]

Hi All,

Alexander,

I've updated gpio-syscon as requested in [3].
I still don't like it, but any way I did it :(

Linus,

I'd very appreciated if you can comment on these series.
Personally, I like v1 [3], because this v2 is not elegant and will
require constant code patching in case of adding new SoCs or new SoC's versions.

This series intended to integrate Keystone 2 DSP GPIO controller functionality
into gpio-syscon driver (drivers/gpio/gpio-syscon.c) as requested
by Linus Walleij in [1].

On Keystone SOCs, ARM host can send interrupts to DSP cores using the
DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
each DSP core. This is one of the component used by the IPC mechanism used
on Keystone SOCs.

Keystone 2 DSP GPIO controller has specific features:
- each GPIO can be configured only as output pin;
- setting GPIO value to 1 causes IRQ generation on target DSP core;
- reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
  pending.

The gpio-syscon driver was need to be updated to satisfy Keystone 2 SoC
requirements:
- special sequence of operations need to be used to assign output GPIO value.
   As result, first patch introduces SoC specific callback .set() to configure
   output GPIO value.

Also, patch 3 was added to illustrate DSP GPIO configuration in DT used by Keystone 2.

Related sicussions:
 [1] https://lkml.org/lkml/2014/7/16/170
 [2] https://lkml.org/lkml/2014/7/23/352
 [3] https://www.mail-archive.com/devicetree@vger.kernel.org/msg37863.html

Grygorii Strashko (3):
  gpio: syscon: add soc specific callback to assign output value
  gpio: syscon: reuse for keystone 2 socs
  ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

 .../bindings/gpio/gpio-mctrl-keystone.txt          |   42 ++++++
 arch/arm/boot/dts/k2hk.dtsi                        |   48 +++++++
 drivers/gpio/gpio-syscon.c                         |  140 ++++++++++++++++++++
 3 files changed, 230 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt

-- 
1.7.9.5

[PATCH v2 0/3] gpio: syscon: reuse for keystone 2 socs

[Grygorii Strashko]

Hi All,

Alexander,

I've updated gpio-syscon as requested in [3].
I still don't like it, but any way I did it :(

Linus,

I'd very appreciated if you can comment on these series.
Personally, I like v1 [3], because this v2 is not elegant and will
require constant code patching in case of adding new SoCs or new SoC's versions.

This series intended to integrate Keystone 2 DSP GPIO controller functionality
into gpio-syscon driver (drivers/gpio/gpio-syscon.c) as requested
by Linus Walleij in [1].

On Keystone SOCs, ARM host can send interrupts to DSP cores using the
DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
each DSP core. This is one of the component used by the IPC mechanism used
on Keystone SOCs.

Keystone 2 DSP GPIO controller has specific features:
- each GPIO can be configured only as output pin;
- setting GPIO value to 1 causes IRQ generation on target DSP core;
- reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
  pending.

The gpio-syscon driver was need to be updated to satisfy Keystone 2 SoC
requirements:
- special sequence of operations need to be used to assign output GPIO value.
   As result, first patch introduces SoC specific callback .set() to configure
   output GPIO value.

Also, patch 3 was added to illustrate DSP GPIO configuration in DT used by Keystone 2.

Related sicussions:
 [1] https://lkml.org/lkml/2014/7/16/170
 [2] https://lkml.org/lkml/2014/7/23/352
 [3] https://www.mail-archive.com/devicetree at vger.kernel.org/msg37863.html

Grygorii Strashko (3):
  gpio: syscon: add soc specific callback to assign output value
  gpio: syscon: reuse for keystone 2 socs
  ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

 .../bindings/gpio/gpio-mctrl-keystone.txt          |   42 ++++++
 arch/arm/boot/dts/k2hk.dtsi                        |   48 +++++++
 drivers/gpio/gpio-syscon.c                         |  140 ++++++++++++++++++++
 3 files changed, 230 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt

-- 
1.7.9.5

[PATCH v2 1/3] gpio: syscon: add soc specific callback to assign output value

[Grygorii Strashko]

Some SoCs (like Keystone) may require to perform special
sequence of operations to assign output GPIO value, so default
implementation of .set() callback from gpio-syscon driver
can't be used.

Hence, add optional, SoC specific callback to assign output
gpio value.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/gpio/gpio-syscon.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
index 30884fb..03b4699 100644
--- a/drivers/gpio/gpio-syscon.c
+++ b/drivers/gpio/gpio-syscon.c
@@ -37,6 +37,8 @@
  * dat_bit_offset:	Offset (in bits) to the first GPIO bit.
  * dir_bit_offset:	Optional offset (in bits) to the first bit to switch
  *			GPIO direction (Used with GPIO_SYSCON_FEAT_DIR flag).
+ * set:		HW specific callback to assigns output value
+ *			for signal "offset"
  */
 
 struct syscon_gpio_data {
@@ -45,6 +47,8 @@ struct syscon_gpio_data {
 	unsigned int	bit_count;
 	unsigned int	dat_bit_offset;
 	unsigned int	dir_bit_offset;
+	void		(*set)(struct gpio_chip *chip,
+			       unsigned offset, int value);
 };
 
 struct syscon_gpio_priv {
@@ -77,6 +81,11 @@ static void syscon_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
 	struct syscon_gpio_priv *priv = to_syscon_gpio(chip);
 	unsigned int offs = priv->data->dat_bit_offset + offset;
 
+	if (priv->data->set) {
+		priv->data->set(chip, offset, val);
+		return;
+	}
+
 	regmap_update_bits(priv->syscon,
 			   (offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE,
 			   BIT(offs % SYSCON_REG_BITS),
-- 
1.7.9.5

[PATCH v2 1/3] gpio: syscon: add soc specific callback to assign output value

[Grygorii Strashko]

Some SoCs (like Keystone) may require to perform special
sequence of operations to assign output GPIO value, so default
implementation of .set() callback from gpio-syscon driver
can't be used.

Hence, add optional, SoC specific callback to assign output
gpio value.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/gpio/gpio-syscon.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
index 30884fb..03b4699 100644
--- a/drivers/gpio/gpio-syscon.c
+++ b/drivers/gpio/gpio-syscon.c
@@ -37,6 +37,8 @@
  * dat_bit_offset:	Offset (in bits) to the first GPIO bit.
  * dir_bit_offset:	Optional offset (in bits) to the first bit to switch
  *			GPIO direction (Used with GPIO_SYSCON_FEAT_DIR flag).
+ * set:		HW specific callback to assigns output value
+ *			for signal "offset"
  */
 
 struct syscon_gpio_data {
@@ -45,6 +47,8 @@ struct syscon_gpio_data {
 	unsigned int	bit_count;
 	unsigned int	dat_bit_offset;
 	unsigned int	dir_bit_offset;
+	void		(*set)(struct gpio_chip *chip,
+			       unsigned offset, int value);
 };
 
 struct syscon_gpio_priv {
@@ -77,6 +81,11 @@ static void syscon_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
 	struct syscon_gpio_priv *priv = to_syscon_gpio(chip);
 	unsigned int offs = priv->data->dat_bit_offset + offset;
 
+	if (priv->data->set) {
+		priv->data->set(chip, offset, val);
+		return;
+	}
+
 	regmap_update_bits(priv->syscon,
 			   (offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE,
 			   BIT(offs % SYSCON_REG_BITS),
-- 
1.7.9.5

[PATCH v2 2/3] gpio: syscon: reuse for keystone 2 socs

[Grygorii Strashko]

On Keystone SOCs, ARM host can send interrupts to DSP cores using the
DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
each DSP core. This is one of the component used by the IPC mechanism used
on Keystone SOCs.

Keystone 2 DSP GPIO controller has specific features:
- each GPIO can be configured only as output pin;
- setting GPIO value to 1 causes IRQ generation on target DSP core;
- reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
  pending.

This patch updates gpio-syscon driver to be reused by Keystone 2 SoCs,
because the Keystone 2 DSP GPIO controller is controlled through Syscon
devices and, as requested by Linus Walleij, such kind of GPIO controllers
should be integrated with drivers/gpio/gpio-syscon.c driver.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 .../bindings/gpio/gpio-mctrl-keystone.txt          |   42 +++++++
 drivers/gpio/gpio-syscon.c                         |  131 ++++++++++++++++++++
 2 files changed, 173 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt b/Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt
new file mode 100644
index 0000000..d3858aa
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt
@@ -0,0 +1,42 @@
+Keystone 2 DSP GPIO controller bindings
+
+HOST OS userland running on ARM can send interrupts to DSP cores using
+the DSP GPIO controller IP. It provides 28 IRQ signals per each DSP core.
+This is one of the component used by the IPC mechanism used on Keystone SOCs.
+
+For example TCI6638K2K SoC has 8 DSP GPIO controllers:
+ - 8 for C66x CorePacx CPUs 0-7
+
+Keystone 2 DSP GPIO controller has specific features:
+- each GPIO can be configured only as output pin;
+- setting GPIO value to 1 causes IRQ generation on target DSP core;
+- reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
+  pending.
+
+Required Properties:
+- compatible: should be "ti,keystone-dsp-gpio0" or
+			"ti,keystone-dsp-gpio1" or
+			"ti,keystone-dsp-gpio2" or
+			"ti,keystone-dsp-gpio3" or
+			"ti,keystone-dsp-gpio4" or
+			"ti,keystone-dsp-gpio5" or
+			"ti,keystone-dsp-gpio6" or
+			"ti,keystone-dsp-gpio7" or
+- gpio-controller: Marks the device node as a gpio controller.
+- #gpio-cells: Should be 2.
+
+Please refer to gpio.txt in this directory for details of the common GPIO
+bindings used by client devices.
+
+Example:
+	dspgpio0: keystone_dsp_gpio@02620240 {
+		compatible = "ti,keystone-mctrl-gpio";
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
+
+	dsp0: dsp0 {
+		compatible = "linux,rproc-user";
+		...
+		kick-gpio = <&dspgpio0 27>;
+	};
diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
index 03b4699..0b5f998 100644
--- a/drivers/gpio/gpio-syscon.c
+++ b/drivers/gpio/gpio-syscon.c
@@ -133,11 +133,142 @@ static const struct syscon_gpio_data clps711x_mctrl_gpio = {
 	.dat_bit_offset	= 0x40 * 8 + 8,
 };
 
+#ifdef CONFIG_ARCH_KEYSTONE
+#define KEYSTONE_LOCK_BIT BIT(0)
+
+static void keystone_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
+{
+	struct syscon_gpio_priv *priv = to_syscon_gpio(chip);
+	unsigned int offs;
+	int ret;
+
+	offs = priv->data->dat_bit_offset + offset;
+
+	if (!val)
+		return;
+
+	ret = regmap_update_bits(
+			priv->syscon,
+			(offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE,
+			BIT(offs % SYSCON_REG_BITS) | KEYSTONE_LOCK_BIT,
+			BIT(offs % SYSCON_REG_BITS) | KEYSTONE_LOCK_BIT);
+	if (ret < 0)
+		dev_err(chip->dev, "gpio write failed ret(%d)\n", ret);
+}
+
+static const struct syscon_gpio_data keystone_dsp_gpio0 = {
+	/* ARM Keystone 2 */
+	.compatible	= "ti,keystone-devctrl",
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= 28,
+	.dat_bit_offset	= 0x240 * 8 + 4,
+	.set		= keystone_gpio_set,
+};
+
+static const struct syscon_gpio_data keystone_dsp_gpio1 = {
+	/* ARM Keystone 2 */
+	.compatible	= "ti,keystone-devctrl",
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= 28,
+	.dat_bit_offset	= 0x244 * 8 + 4,
+	.set		= keystone_gpio_set,
+};
+
+static const struct syscon_gpio_data keystone_dsp_gpio2 = {
+	/* ARM Keystone 2 */
+	.compatible	= "ti,keystone-devctrl",
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= 28,
+	.dat_bit_offset	= 0x248 * 8 + 4,
+	.set		= keystone_gpio_set,
+};
+
+static const struct syscon_gpio_data keystone_dsp_gpio3 = {
+	/* ARM Keystone 2 */
+	.compatible	= "ti,keystone-devctrl",
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= 28,
+	.dat_bit_offset	= 0x24c * 8 + 4,
+	.set		= keystone_gpio_set,
+};
+
+static const struct syscon_gpio_data keystone_dsp_gpio4 = {
+	/* ARM Keystone 2 */
+	.compatible	= "ti,keystone-devctrl",
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= 28,
+	.dat_bit_offset	= 0x250 * 8 + 4,
+	.set		= keystone_gpio_set,
+};
+
+static const struct syscon_gpio_data keystone_dsp_gpio5 = {
+	/* ARM Keystone 2 */
+	.compatible	= "ti,keystone-devctrl",
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= 28,
+	.dat_bit_offset	= 0x254 * 8 + 4,
+	.set		= keystone_gpio_set,
+};
+
+static const struct syscon_gpio_data keystone_dsp_gpio6 = {
+	/* ARM Keystone 2 */
+	.compatible	= "ti,keystone-devctrl",
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= 28,
+	.dat_bit_offset	= 0x258 * 8 + 4,
+	.set		= keystone_gpio_set,
+};
+
+static const struct syscon_gpio_data keystone_dsp_gpio7 = {
+	/* ARM Keystone 2 */
+	.compatible	= "ti,keystone-devctrl",
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= 28,
+	.dat_bit_offset	= 0x25c * 8 + 4,
+	.set		= keystone_gpio_set,
+};
+
+#endif
+
 static const struct of_device_id syscon_gpio_ids[] = {
 	{
 		.compatible	= "cirrus,clps711x-mctrl-gpio",
 		.data		= &clps711x_mctrl_gpio,
 	},
+#ifdef CONFIG_ARCH_KEYSTONE
+	{
+		.compatible	= "ti,keystone-dsp-gpio0",
+		.data		= &keystone_dsp_gpio0,
+	},
+	{
+		.compatible	= "ti,keystone-dsp-gpio1",
+		.data		= &keystone_dsp_gpio1,
+	},
+	{
+		.compatible	= "ti,keystone-dsp-gpio2",
+		.data		= &keystone_dsp_gpio2,
+	},
+	{
+		.compatible	= "ti,keystone-dsp-gpio3",
+		.data		= &keystone_dsp_gpio3,
+	},
+	{
+		.compatible	= "ti,keystone-dsp-gpio4",
+		.data		= &keystone_dsp_gpio5,
+	},
+	{
+		.compatible	= "ti,keystone-dsp-gpio5",
+		.data		= &keystone_dsp_gpio6,
+	},
+	{
+		.compatible	= "ti,keystone-dsp-gpio6",
+		.data		= &keystone_dsp_gpio6,
+	},
+	{
+		.compatible	= "ti,keystone-dsp-gpio7",
+		.data		= &keystone_dsp_gpio7,
+	},
+#endif
 	{ }
 };
 MODULE_DEVICE_TABLE(of, syscon_gpio_ids);
-- 
1.7.9.5

[PATCH v2 2/3] gpio: syscon: reuse for keystone 2 socs

[Grygorii Strashko]

On Keystone SOCs, ARM host can send interrupts to DSP cores using the
DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
each DSP core. This is one of the component used by the IPC mechanism used
on Keystone SOCs.

Keystone 2 DSP GPIO controller has specific features:
- each GPIO can be configured only as output pin;
- setting GPIO value to 1 causes IRQ generation on target DSP core;
- reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
  pending.

This patch updates gpio-syscon driver to be reused by Keystone 2 SoCs,
because the Keystone 2 DSP GPIO controller is controlled through Syscon
devices and, as requested by Linus Walleij, such kind of GPIO controllers
should be integrated with drivers/gpio/gpio-syscon.c driver.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 .../bindings/gpio/gpio-mctrl-keystone.txt          |   42 +++++++
 drivers/gpio/gpio-syscon.c                         |  131 ++++++++++++++++++++
 2 files changed, 173 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt b/Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt
new file mode 100644
index 0000000..d3858aa
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt
@@ -0,0 +1,42 @@
+Keystone 2 DSP GPIO controller bindings
+
+HOST OS userland running on ARM can send interrupts to DSP cores using
+the DSP GPIO controller IP. It provides 28 IRQ signals per each DSP core.
+This is one of the component used by the IPC mechanism used on Keystone SOCs.
+
+For example TCI6638K2K SoC has 8 DSP GPIO controllers:
+ - 8 for C66x CorePacx CPUs 0-7
+
+Keystone 2 DSP GPIO controller has specific features:
+- each GPIO can be configured only as output pin;
+- setting GPIO value to 1 causes IRQ generation on target DSP core;
+- reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
+  pending.
+
+Required Properties:
+- compatible: should be "ti,keystone-dsp-gpio0" or
+			"ti,keystone-dsp-gpio1" or
+			"ti,keystone-dsp-gpio2" or
+			"ti,keystone-dsp-gpio3" or
+			"ti,keystone-dsp-gpio4" or
+			"ti,keystone-dsp-gpio5" or
+			"ti,keystone-dsp-gpio6" or
+			"ti,keystone-dsp-gpio7" or
+- gpio-controller: Marks the device node as a gpio controller.
+- #gpio-cells: Should be 2.
+
+Please refer to gpio.txt in this directory for details of the common GPIO
+bindings used by client devices.
+
+Example:
+	dspgpio0: keystone_dsp_gpio at 02620240 {
+		compatible = "ti,keystone-mctrl-gpio";
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
+
+	dsp0: dsp0 {
+		compatible = "linux,rproc-user";
+		...
+		kick-gpio = <&dspgpio0 27>;
+	};
diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
index 03b4699..0b5f998 100644
--- a/drivers/gpio/gpio-syscon.c
+++ b/drivers/gpio/gpio-syscon.c
@@ -133,11 +133,142 @@ static const struct syscon_gpio_data clps711x_mctrl_gpio = {
 	.dat_bit_offset	= 0x40 * 8 + 8,
 };
 
+#ifdef CONFIG_ARCH_KEYSTONE
+#define KEYSTONE_LOCK_BIT BIT(0)
+
+static void keystone_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
+{
+	struct syscon_gpio_priv *priv = to_syscon_gpio(chip);
+	unsigned int offs;
+	int ret;
+
+	offs = priv->data->dat_bit_offset + offset;
+
+	if (!val)
+		return;
+
+	ret = regmap_update_bits(
+			priv->syscon,
+			(offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE,
+			BIT(offs % SYSCON_REG_BITS) | KEYSTONE_LOCK_BIT,
+			BIT(offs % SYSCON_REG_BITS) | KEYSTONE_LOCK_BIT);
+	if (ret < 0)
+		dev_err(chip->dev, "gpio write failed ret(%d)\n", ret);
+}
+
+static const struct syscon_gpio_data keystone_dsp_gpio0 = {
+	/* ARM Keystone 2 */
+	.compatible	= "ti,keystone-devctrl",
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= 28,
+	.dat_bit_offset	= 0x240 * 8 + 4,
+	.set		= keystone_gpio_set,
+};
+
+static const struct syscon_gpio_data keystone_dsp_gpio1 = {
+	/* ARM Keystone 2 */
+	.compatible	= "ti,keystone-devctrl",
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= 28,
+	.dat_bit_offset	= 0x244 * 8 + 4,
+	.set		= keystone_gpio_set,
+};
+
+static const struct syscon_gpio_data keystone_dsp_gpio2 = {
+	/* ARM Keystone 2 */
+	.compatible	= "ti,keystone-devctrl",
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= 28,
+	.dat_bit_offset	= 0x248 * 8 + 4,
+	.set		= keystone_gpio_set,
+};
+
+static const struct syscon_gpio_data keystone_dsp_gpio3 = {
+	/* ARM Keystone 2 */
+	.compatible	= "ti,keystone-devctrl",
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= 28,
+	.dat_bit_offset	= 0x24c * 8 + 4,
+	.set		= keystone_gpio_set,
+};
+
+static const struct syscon_gpio_data keystone_dsp_gpio4 = {
+	/* ARM Keystone 2 */
+	.compatible	= "ti,keystone-devctrl",
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= 28,
+	.dat_bit_offset	= 0x250 * 8 + 4,
+	.set		= keystone_gpio_set,
+};
+
+static const struct syscon_gpio_data keystone_dsp_gpio5 = {
+	/* ARM Keystone 2 */
+	.compatible	= "ti,keystone-devctrl",
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= 28,
+	.dat_bit_offset	= 0x254 * 8 + 4,
+	.set		= keystone_gpio_set,
+};
+
+static const struct syscon_gpio_data keystone_dsp_gpio6 = {
+	/* ARM Keystone 2 */
+	.compatible	= "ti,keystone-devctrl",
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= 28,
+	.dat_bit_offset	= 0x258 * 8 + 4,
+	.set		= keystone_gpio_set,
+};
+
+static const struct syscon_gpio_data keystone_dsp_gpio7 = {
+	/* ARM Keystone 2 */
+	.compatible	= "ti,keystone-devctrl",
+	.flags		= GPIO_SYSCON_FEAT_OUT,
+	.bit_count	= 28,
+	.dat_bit_offset	= 0x25c * 8 + 4,
+	.set		= keystone_gpio_set,
+};
+
+#endif
+
 static const struct of_device_id syscon_gpio_ids[] = {
 	{
 		.compatible	= "cirrus,clps711x-mctrl-gpio",
 		.data		= &clps711x_mctrl_gpio,
 	},
+#ifdef CONFIG_ARCH_KEYSTONE
+	{
+		.compatible	= "ti,keystone-dsp-gpio0",
+		.data		= &keystone_dsp_gpio0,
+	},
+	{
+		.compatible	= "ti,keystone-dsp-gpio1",
+		.data		= &keystone_dsp_gpio1,
+	},
+	{
+		.compatible	= "ti,keystone-dsp-gpio2",
+		.data		= &keystone_dsp_gpio2,
+	},
+	{
+		.compatible	= "ti,keystone-dsp-gpio3",
+		.data		= &keystone_dsp_gpio3,
+	},
+	{
+		.compatible	= "ti,keystone-dsp-gpio4",
+		.data		= &keystone_dsp_gpio5,
+	},
+	{
+		.compatible	= "ti,keystone-dsp-gpio5",
+		.data		= &keystone_dsp_gpio6,
+	},
+	{
+		.compatible	= "ti,keystone-dsp-gpio6",
+		.data		= &keystone_dsp_gpio6,
+	},
+	{
+		.compatible	= "ti,keystone-dsp-gpio7",
+		.data		= &keystone_dsp_gpio7,
+	},
+#endif
 	{ }
 };
 MODULE_DEVICE_TABLE(of, syscon_gpio_ids);
-- 
1.7.9.5

[PATCH v2 3/3] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Grygorii Strashko]

Add Keystone 2 DSP GPIO nodes.
DSP GPIO banks 0-7 correspond to DSP0-DSP7

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/boot/dts/k2hk.dtsi |   48 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/arch/arm/boot/dts/k2hk.dtsi b/arch/arm/boot/dts/k2hk.dtsi
index 321ba2f..84a8887 100644
--- a/arch/arm/boot/dts/k2hk.dtsi
+++ b/arch/arm/boot/dts/k2hk.dtsi
@@ -50,5 +50,53 @@
 			#interrupt-cells = <1>;
 			ti,syscon-dev = <&devctrl 0x2a0>;
 		};
+
+		dspgpio0: keystone_dsp_gpio@02620240 {
+			compatible = "ti,keystone-dsp-gpio0";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		dspgpio1: keystone_dsp_gpio@02620244 {
+			compatible = "ti,keystone-dsp-gpio1";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		dspgpio2: keystone_dsp_gpio@02620248 {
+			compatible = "ti,keystone-dsp-gpio2";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		dspgpio3: keystone_dsp_gpio@0262024c {
+			compatible = "ti,keystone-dsp-gpio3";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		dspgpio4: keystone_dsp_gpio@02620250 {
+			compatible = "ti,keystone-dsp-gpio4";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		dspgpio5: keystone_dsp_gpio@02620254 {
+			compatible = "ti,keystone-dsp-gpio5";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		dspgpio6: keystone_dsp_gpio@02620258 {
+			compatible = "ti,keystone-dsp-gpio6";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		dspgpio7: keystone_dsp_gpio@0262025C {
+			compatible = "ti,keystone-dsp-gpio7";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
 	};
 };
-- 
1.7.9.5

[PATCH v2 3/3] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Grygorii Strashko]

Add Keystone 2 DSP GPIO nodes.
DSP GPIO banks 0-7 correspond to DSP0-DSP7

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/boot/dts/k2hk.dtsi |   48 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/arch/arm/boot/dts/k2hk.dtsi b/arch/arm/boot/dts/k2hk.dtsi
index 321ba2f..84a8887 100644
--- a/arch/arm/boot/dts/k2hk.dtsi
+++ b/arch/arm/boot/dts/k2hk.dtsi
@@ -50,5 +50,53 @@
 			#interrupt-cells = <1>;
 			ti,syscon-dev = <&devctrl 0x2a0>;
 		};
+
+		dspgpio0: keystone_dsp_gpio at 02620240 {
+			compatible = "ti,keystone-dsp-gpio0";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		dspgpio1: keystone_dsp_gpio at 02620244 {
+			compatible = "ti,keystone-dsp-gpio1";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		dspgpio2: keystone_dsp_gpio at 02620248 {
+			compatible = "ti,keystone-dsp-gpio2";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		dspgpio3: keystone_dsp_gpio at 0262024c {
+			compatible = "ti,keystone-dsp-gpio3";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		dspgpio4: keystone_dsp_gpio at 02620250 {
+			compatible = "ti,keystone-dsp-gpio4";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		dspgpio5: keystone_dsp_gpio at 02620254 {
+			compatible = "ti,keystone-dsp-gpio5";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		dspgpio6: keystone_dsp_gpio at 02620258 {
+			compatible = "ti,keystone-dsp-gpio6";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		dspgpio7: keystone_dsp_gpio at 0262025C {
+			compatible = "ti,keystone-dsp-gpio7";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
 	};
 };
-- 
1.7.9.5

Re: [PATCH v2 3/3] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Alexander Shiyan]

Thu, 21 Aug 2014 19:23:23 +0300 от Grygorii Strashko <grygorii.strashko@ti.com>:
> Add Keystone 2 DSP GPIO nodes.
> DSP GPIO banks 0-7 correspond to DSP0-DSP7
> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
...
> +		dspgpio3: keystone_dsp_gpio@0262024c {
> +			compatible = "ti,keystone-dsp-gpio3";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +		};
...
> +		dspgpio7: keystone_dsp_gpio@0262025C {

Use lower case for addresses.

---

Re: [PATCH v2 3/3] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Alexander Shiyan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 599 bytes --]

Thu, 21 Aug 2014 19:23:23 +0300 от Grygorii Strashko <grygorii.strashko@ti.com>:
> Add Keystone 2 DSP GPIO nodes.
> DSP GPIO banks 0-7 correspond to DSP0-DSP7
> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
...
> +		dspgpio3: keystone_dsp_gpio@0262024c {
> +			compatible = "ti,keystone-dsp-gpio3";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +		};
...
> +		dspgpio7: keystone_dsp_gpio@0262025C {

Use lower case for addresses.

---

N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·zøœzÚÞz)í…æèw*\x1fjg¬±¨\x1e¶‰šŽŠÝ¢j.ïÛ°\½½MŽúgjÌæa×\x02››–' ™©Þ¢¸\f¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾\a«‘êçzZ+ƒùšŽŠÝ¢j"ú!¶i

Re: [PATCH v2 3/3] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Alexander Shiyan]

Thu, 21 Aug 2014 19:23:23 +0300 от Grygorii Strashko <grygorii.strashko@ti.com>:
> Add Keystone 2 DSP GPIO nodes.
> DSP GPIO banks 0-7 correspond to DSP0-DSP7
> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
...
> +		dspgpio3: keystone_dsp_gpio@0262024c {
> +			compatible = "ti,keystone-dsp-gpio3";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +		};
...
> +		dspgpio7: keystone_dsp_gpio@0262025C {

Use lower case for addresses.

---

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/3] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes

[Alexander Shiyan]

Thu, 21 Aug 2014 19:23:23 +0300 ?? Grygorii Strashko <grygorii.strashko@ti.com>:
> Add Keystone 2 DSP GPIO nodes.
> DSP GPIO banks 0-7 correspond to DSP0-DSP7
> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
...
> +		dspgpio3: keystone_dsp_gpio at 0262024c {
> +			compatible = "ti,keystone-dsp-gpio3";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +		};
...
> +		dspgpio7: keystone_dsp_gpio at 0262025C {

Use lower case for addresses.

---

Re: [PATCH v2 0/3] gpio: syscon: reuse for keystone 2 socs

[Alexander Shiyan]

Thu, 21 Aug 2014 19:23:20 +0300 от Grygorii Strashko <grygorii.strashko@ti.com>:
> Hi All,
> 
> Alexander,
> 
> I've updated gpio-syscon as requested in [3].
> I still don't like it, but any way I did it :(
> 
> Linus,
> 
> I'd very appreciated if you can comment on these series.
> Personally, I like v1 [3], because this v2 is not elegant and will
> require constant code patching in case of adding new SoCs or new SoC's versions.
> 
> This series intended to integrate Keystone 2 DSP GPIO controller functionality
> into gpio-syscon driver (drivers/gpio/gpio-syscon.c) as requested
> by Linus Walleij in [1].
> 
> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
> each DSP core. This is one of the component used by the IPC mechanism used
> on Keystone SOCs.
> 
> Keystone 2 DSP GPIO controller has specific features:
> - each GPIO can be configured only as output pin;
> - setting GPIO value to 1 causes IRQ generation on target DSP core;
> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>   pending.
> 
> The gpio-syscon driver was need to be updated to satisfy Keystone 2 SoC
> requirements:
> - special sequence of operations need to be used to assign output GPIO value.
>    As result, first patch introduces SoC specific callback .set() to configure
>    output GPIO value.
> 
> Also, patch 3 was added to illustrate DSP GPIO configuration in DT used by Keystone 2.
> 
> Related sicussions:
>  [1] https://lkml.org/lkml/2014/7/16/170
>  [2] https://lkml.org/lkml/2014/7/23/352
>  [3] https://www.mail-archive.com/devicetree@vger.kernel.org/msg37863.html
> 
> Grygorii Strashko (3):
>   gpio: syscon: add soc specific callback to assign output value
>   gpio: syscon: reuse for keystone 2 socs
>   ARM: dts: keystone-k2hk: add dsp gpio controllers nodes
> 
>  .../bindings/gpio/gpio-mctrl-keystone.txt          |   42 ++++++
>  arch/arm/boot/dts/k2hk.dtsi                        |   48 +++++++
>  drivers/gpio/gpio-syscon.c                         |  140 ++++++++++++++++++++
>  3 files changed, 230 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt

1. mctrl -> dsp in filenames
2. mctrl -> dsp in documentation.
3. Here is a more elegant solution for first part.

--- gpio-syscon.c.old	2014-08-19 09:46:09.000000000 +0400
+++ gpio-syscon.c	2014-08-21 20:45:49.357529323 +0400
@@ -111,7 +111,7 @@
 				   BIT(offs % SYSCON_REG_BITS));
 	}
 
-	syscon_gpio_set(chip, offset, val);
+	priv->data->set(chip, offset, val);
 
 	return 0;
 }
@@ -159,7 +159,7 @@
 	if (priv->data->flags & GPIO_SYSCON_FEAT_IN)
 		priv->chip.direction_input = syscon_gpio_dir_in;
 	if (priv->data->flags & GPIO_SYSCON_FEAT_OUT) {
-		priv->chip.set = syscon_gpio_set;
+		priv->chip.set = priv->data->set ? : syscon_gpio_set;
 		priv->chip.direction_output = syscon_gpio_dir_out;
 	}
 

---

Re: [PATCH v2 0/3] gpio: syscon: reuse for keystone 2 socs

[Alexander Shiyan]

Thu, 21 Aug 2014 19:23:20 +0300 от Grygorii Strashko <grygorii.strashko@ti.com>:
> Hi All,
> 
> Alexander,
> 
> I've updated gpio-syscon as requested in [3].
> I still don't like it, but any way I did it :(
> 
> Linus,
> 
> I'd very appreciated if you can comment on these series.
> Personally, I like v1 [3], because this v2 is not elegant and will
> require constant code patching in case of adding new SoCs or new SoC's versions.
> 
> This series intended to integrate Keystone 2 DSP GPIO controller functionality
> into gpio-syscon driver (drivers/gpio/gpio-syscon.c) as requested
> by Linus Walleij in [1].
> 
> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
> each DSP core. This is one of the component used by the IPC mechanism used
> on Keystone SOCs.
> 
> Keystone 2 DSP GPIO controller has specific features:
> - each GPIO can be configured only as output pin;
> - setting GPIO value to 1 causes IRQ generation on target DSP core;
> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>   pending.
> 
> The gpio-syscon driver was need to be updated to satisfy Keystone 2 SoC
> requirements:
> - special sequence of operations need to be used to assign output GPIO value.
>    As result, first patch introduces SoC specific callback .set() to configure
>    output GPIO value.
> 
> Also, patch 3 was added to illustrate DSP GPIO configuration in DT used by Keystone 2.
> 
> Related sicussions:
>  [1] https://lkml.org/lkml/2014/7/16/170
>  [2] https://lkml.org/lkml/2014/7/23/352
>  [3] https://www.mail-archive.com/devicetree@vger.kernel.org/msg37863.html
> 
> Grygorii Strashko (3):
>   gpio: syscon: add soc specific callback to assign output value
>   gpio: syscon: reuse for keystone 2 socs
>   ARM: dts: keystone-k2hk: add dsp gpio controllers nodes
> 
>  .../bindings/gpio/gpio-mctrl-keystone.txt          |   42 ++++++
>  arch/arm/boot/dts/k2hk.dtsi                        |   48 +++++++
>  drivers/gpio/gpio-syscon.c                         |  140 ++++++++++++++++++++
>  3 files changed, 230 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt

1. mctrl -> dsp in filenames
2. mctrl -> dsp in documentation.
3. Here is a more elegant solution for first part.

--- gpio-syscon.c.old	2014-08-19 09:46:09.000000000 +0400
+++ gpio-syscon.c	2014-08-21 20:45:49.357529323 +0400
@@ -111,7 +111,7 @@
 				   BIT(offs % SYSCON_REG_BITS));
 	}
 
-	syscon_gpio_set(chip, offset, val);
+	priv->data->set(chip, offset, val);
 
 	return 0;
 }
@@ -159,7 +159,7 @@
 	if (priv->data->flags & GPIO_SYSCON_FEAT_IN)
 		priv->chip.direction_input = syscon_gpio_dir_in;
 	if (priv->data->flags & GPIO_SYSCON_FEAT_OUT) {
-		priv->chip.set = syscon_gpio_set;
+		priv->chip.set = priv->data->set ? : syscon_gpio_set;
 		priv->chip.direction_output = syscon_gpio_dir_out;
 	}
 

---

Re: [PATCH v2 0/3] gpio: syscon: reuse for keystone 2 socs

[Alexander Shiyan]

Thu, 21 Aug 2014 19:23:20 +0300 от Grygorii Strashko <grygorii.strashko@ti.com>:
> Hi All,
> 
> Alexander,
> 
> I've updated gpio-syscon as requested in [3].
> I still don't like it, but any way I did it :(
> 
> Linus,
> 
> I'd very appreciated if you can comment on these series.
> Personally, I like v1 [3], because this v2 is not elegant and will
> require constant code patching in case of adding new SoCs or new SoC's versions.
> 
> This series intended to integrate Keystone 2 DSP GPIO controller functionality
> into gpio-syscon driver (drivers/gpio/gpio-syscon.c) as requested
> by Linus Walleij in [1].
> 
> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
> each DSP core. This is one of the component used by the IPC mechanism used
> on Keystone SOCs.
> 
> Keystone 2 DSP GPIO controller has specific features:
> - each GPIO can be configured only as output pin;
> - setting GPIO value to 1 causes IRQ generation on target DSP core;
> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>   pending.
> 
> The gpio-syscon driver was need to be updated to satisfy Keystone 2 SoC
> requirements:
> - special sequence of operations need to be used to assign output GPIO value.
>    As result, first patch introduces SoC specific callback .set() to configure
>    output GPIO value.
> 
> Also, patch 3 was added to illustrate DSP GPIO configuration in DT used by Keystone 2.
> 
> Related sicussions:
>  [1] https://lkml.org/lkml/2014/7/16/170
>  [2] https://lkml.org/lkml/2014/7/23/352
>  [3] https://www.mail-archive.com/devicetree@vger.kernel.org/msg37863.html
> 
> Grygorii Strashko (3):
>   gpio: syscon: add soc specific callback to assign output value
>   gpio: syscon: reuse for keystone 2 socs
>   ARM: dts: keystone-k2hk: add dsp gpio controllers nodes
> 
>  .../bindings/gpio/gpio-mctrl-keystone.txt          |   42 ++++++
>  arch/arm/boot/dts/k2hk.dtsi                        |   48 +++++++
>  drivers/gpio/gpio-syscon.c                         |  140 ++++++++++++++++++++
>  3 files changed, 230 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt

1. mctrl -> dsp in filenames
2. mctrl -> dsp in documentation.
3. Here is a more elegant solution for first part.

--- gpio-syscon.c.old	2014-08-19 09:46:09.000000000 +0400
+++ gpio-syscon.c	2014-08-21 20:45:49.357529323 +0400
@@ -111,7 +111,7 @@
 				   BIT(offs % SYSCON_REG_BITS));
 	}
 
-	syscon_gpio_set(chip, offset, val);
+	priv->data->set(chip, offset, val);
 
 	return 0;
 }
@@ -159,7 +159,7 @@
 	if (priv->data->flags & GPIO_SYSCON_FEAT_IN)
 		priv->chip.direction_input = syscon_gpio_dir_in;
 	if (priv->data->flags & GPIO_SYSCON_FEAT_OUT) {
-		priv->chip.set = syscon_gpio_set;
+		priv->chip.set = priv->data->set ? : syscon_gpio_set;
 		priv->chip.direction_output = syscon_gpio_dir_out;
 	}
 

---

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 0/3] gpio: syscon: reuse for keystone 2 socs

[Alexander Shiyan]

Thu, 21 Aug 2014 19:23:20 +0300 ?? Grygorii Strashko <grygorii.strashko@ti.com>:
> Hi All,
> 
> Alexander,
> 
> I've updated gpio-syscon as requested in [3].
> I still don't like it, but any way I did it :(
> 
> Linus,
> 
> I'd very appreciated if you can comment on these series.
> Personally, I like v1 [3], because this v2 is not elegant and will
> require constant code patching in case of adding new SoCs or new SoC's versions.
> 
> This series intended to integrate Keystone 2 DSP GPIO controller functionality
> into gpio-syscon driver (drivers/gpio/gpio-syscon.c) as requested
> by Linus Walleij in [1].
> 
> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
> each DSP core. This is one of the component used by the IPC mechanism used
> on Keystone SOCs.
> 
> Keystone 2 DSP GPIO controller has specific features:
> - each GPIO can be configured only as output pin;
> - setting GPIO value to 1 causes IRQ generation on target DSP core;
> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>   pending.
> 
> The gpio-syscon driver was need to be updated to satisfy Keystone 2 SoC
> requirements:
> - special sequence of operations need to be used to assign output GPIO value.
>    As result, first patch introduces SoC specific callback .set() to configure
>    output GPIO value.
> 
> Also, patch 3 was added to illustrate DSP GPIO configuration in DT used by Keystone 2.
> 
> Related sicussions:
>  [1] https://lkml.org/lkml/2014/7/16/170
>  [2] https://lkml.org/lkml/2014/7/23/352
>  [3] https://www.mail-archive.com/devicetree at vger.kernel.org/msg37863.html
> 
> Grygorii Strashko (3):
>   gpio: syscon: add soc specific callback to assign output value
>   gpio: syscon: reuse for keystone 2 socs
>   ARM: dts: keystone-k2hk: add dsp gpio controllers nodes
> 
>  .../bindings/gpio/gpio-mctrl-keystone.txt          |   42 ++++++
>  arch/arm/boot/dts/k2hk.dtsi                        |   48 +++++++
>  drivers/gpio/gpio-syscon.c                         |  140 ++++++++++++++++++++
>  3 files changed, 230 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt

1. mctrl -> dsp in filenames
2. mctrl -> dsp in documentation.
3. Here is a more elegant solution for first part.

--- gpio-syscon.c.old	2014-08-19 09:46:09.000000000 +0400
+++ gpio-syscon.c	2014-08-21 20:45:49.357529323 +0400
@@ -111,7 +111,7 @@
 				   BIT(offs % SYSCON_REG_BITS));
 	}
 
-	syscon_gpio_set(chip, offset, val);
+	priv->data->set(chip, offset, val);
 
 	return 0;
 }
@@ -159,7 +159,7 @@
 	if (priv->data->flags & GPIO_SYSCON_FEAT_IN)
 		priv->chip.direction_input = syscon_gpio_dir_in;
 	if (priv->data->flags & GPIO_SYSCON_FEAT_OUT) {
-		priv->chip.set = syscon_gpio_set;
+		priv->chip.set = priv->data->set ? : syscon_gpio_set;
 		priv->chip.direction_output = syscon_gpio_dir_out;
 	}
 

---

Re: [PATCH v2 0/3] gpio: syscon: reuse for keystone 2 socs

[Grygorii Strashko]

Hi Alexander, All,

On 08/21/2014 07:51 PM, Alexander Shiyan wrote:
> Thu, 21 Aug 2014 19:23:20 +0300 от Grygorii Strashko <grygorii.strashko@ti.com>:
>> Hi All,
>>
>> Alexander,
>>
>> I've updated gpio-syscon as requested in [3].
>> I still don't like it, but any way I did it :(
>>
>> Linus,
>>
>> I'd very appreciated if you can comment on these series.
>> Personally, I like v1 [3], because this v2 is not elegant and will
>> require constant code patching in case of adding new SoCs or new SoC's versions.
>>
>> This series intended to integrate Keystone 2 DSP GPIO controller functionality
>> into gpio-syscon driver (drivers/gpio/gpio-syscon.c) as requested
>> by Linus Walleij in [1].
>>
>> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
>> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
>> each DSP core. This is one of the component used by the IPC mechanism used
>> on Keystone SOCs.
>>
>> Keystone 2 DSP GPIO controller has specific features:
>> - each GPIO can be configured only as output pin;
>> - setting GPIO value to 1 causes IRQ generation on target DSP core;
>> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>>    pending.
>>
>> The gpio-syscon driver was need to be updated to satisfy Keystone 2 SoC
>> requirements:
>> - special sequence of operations need to be used to assign output GPIO value.
>>     As result, first patch introduces SoC specific callback .set() to configure
>>     output GPIO value.
>>
>> Also, patch 3 was added to illustrate DSP GPIO configuration in DT used by Keystone 2.
>>
>> Related sicussions:
>>   [1] https://lkml.org/lkml/2014/7/16/170
>>   [2] https://lkml.org/lkml/2014/7/23/352
>>   [3] https://www.mail-archive.com/devicetree@vger.kernel.org/msg37863.html
>>
>> Grygorii Strashko (3):
>>    gpio: syscon: add soc specific callback to assign output value
>>    gpio: syscon: reuse for keystone 2 socs
>>    ARM: dts: keystone-k2hk: add dsp gpio controllers nodes
>>
>>   .../bindings/gpio/gpio-mctrl-keystone.txt          |   42 ++++++
>>   arch/arm/boot/dts/k2hk.dtsi                        |   48 +++++++
>>   drivers/gpio/gpio-syscon.c                         |  140 ++++++++++++++++++++
>>   3 files changed, 230 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt
> 
> 1. mctrl -> dsp in filenames
> 2. mctrl -> dsp in documentation.
> 3. Here is a more elegant solution for first part.
> 
> --- gpio-syscon.c.old	2014-08-19 09:46:09.000000000 +0400
> +++ gpio-syscon.c	2014-08-21 20:45:49.357529323 +0400
> @@ -111,7 +111,7 @@
>   				   BIT(offs % SYSCON_REG_BITS));
>   	}
>   
> -	syscon_gpio_set(chip, offset, val);
> +	priv->data->set(chip, offset, val);
>   
>   	return 0;
>   }
> @@ -159,7 +159,7 @@
>   	if (priv->data->flags & GPIO_SYSCON_FEAT_IN)
>   		priv->chip.direction_input = syscon_gpio_dir_in;
>   	if (priv->data->flags & GPIO_SYSCON_FEAT_OUT) {
> -		priv->chip.set = syscon_gpio_set;
> +		priv->chip.set = priv->data->set ? : syscon_gpio_set;
>   		priv->chip.direction_output = syscon_gpio_dir_out;
>   	}

yep. It's better.

Thanks for your comments.
I'll wait few days with hope to get some comments from comunity
(as I'm still thinking v1 is better :) then will update it and re-send.

Regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 0/3] gpio: syscon: reuse for keystone 2 socs

[Grygorii Strashko]

Hi Alexander, All,

On 08/21/2014 07:51 PM, Alexander Shiyan wrote:
> Thu, 21 Aug 2014 19:23:20 +0300 ?? Grygorii Strashko <grygorii.strashko@ti.com>:
>> Hi All,
>>
>> Alexander,
>>
>> I've updated gpio-syscon as requested in [3].
>> I still don't like it, but any way I did it :(
>>
>> Linus,
>>
>> I'd very appreciated if you can comment on these series.
>> Personally, I like v1 [3], because this v2 is not elegant and will
>> require constant code patching in case of adding new SoCs or new SoC's versions.
>>
>> This series intended to integrate Keystone 2 DSP GPIO controller functionality
>> into gpio-syscon driver (drivers/gpio/gpio-syscon.c) as requested
>> by Linus Walleij in [1].
>>
>> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
>> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
>> each DSP core. This is one of the component used by the IPC mechanism used
>> on Keystone SOCs.
>>
>> Keystone 2 DSP GPIO controller has specific features:
>> - each GPIO can be configured only as output pin;
>> - setting GPIO value to 1 causes IRQ generation on target DSP core;
>> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>>    pending.
>>
>> The gpio-syscon driver was need to be updated to satisfy Keystone 2 SoC
>> requirements:
>> - special sequence of operations need to be used to assign output GPIO value.
>>     As result, first patch introduces SoC specific callback .set() to configure
>>     output GPIO value.
>>
>> Also, patch 3 was added to illustrate DSP GPIO configuration in DT used by Keystone 2.
>>
>> Related sicussions:
>>   [1] https://lkml.org/lkml/2014/7/16/170
>>   [2] https://lkml.org/lkml/2014/7/23/352
>>   [3] https://www.mail-archive.com/devicetree at vger.kernel.org/msg37863.html
>>
>> Grygorii Strashko (3):
>>    gpio: syscon: add soc specific callback to assign output value
>>    gpio: syscon: reuse for keystone 2 socs
>>    ARM: dts: keystone-k2hk: add dsp gpio controllers nodes
>>
>>   .../bindings/gpio/gpio-mctrl-keystone.txt          |   42 ++++++
>>   arch/arm/boot/dts/k2hk.dtsi                        |   48 +++++++
>>   drivers/gpio/gpio-syscon.c                         |  140 ++++++++++++++++++++
>>   3 files changed, 230 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mctrl-keystone.txt
> 
> 1. mctrl -> dsp in filenames
> 2. mctrl -> dsp in documentation.
> 3. Here is a more elegant solution for first part.
> 
> --- gpio-syscon.c.old	2014-08-19 09:46:09.000000000 +0400
> +++ gpio-syscon.c	2014-08-21 20:45:49.357529323 +0400
> @@ -111,7 +111,7 @@
>   				   BIT(offs % SYSCON_REG_BITS));
>   	}
>   
> -	syscon_gpio_set(chip, offset, val);
> +	priv->data->set(chip, offset, val);
>   
>   	return 0;
>   }
> @@ -159,7 +159,7 @@
>   	if (priv->data->flags & GPIO_SYSCON_FEAT_IN)
>   		priv->chip.direction_input = syscon_gpio_dir_in;
>   	if (priv->data->flags & GPIO_SYSCON_FEAT_OUT) {
> -		priv->chip.set = syscon_gpio_set;
> +		priv->chip.set = priv->data->set ? : syscon_gpio_set;
>   		priv->chip.direction_output = syscon_gpio_dir_out;
>   	}

yep. It's better.

Thanks for your comments.
I'll wait few days with hope to get some comments from comunity
(as I'm still thinking v1 is better :) then will update it and re-send.

Regards,
-grygorii

Re: [PATCH 3/4] gpio: syscon: reuse for keystone 2 socs

[Linus Walleij]

On Wed, Aug 13, 2014 at 6:16 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:

> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
> each DSP core. This is one of the component used by the IPC mechanism used
> on Keystone SOCs.
>
> Keystone 2 DSP GPIO controller has specific features:
> - each GPIO can be configured only as output pin;
> - setting GPIO value to 1 causes IRQ generation on target DSP core;
> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>   pending.
>
> This patch updates gpio-syscon driver to be reused by Keystone 2 SoCs,
> because the Keystone 2 DSP GPIO controller is controlled through Syscon
> devices and, as requested by Linus Walleij, such kind of GPIO controllers
> should be integrated with drivers/gpio/gpio-syscon.c driver.
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

OK overall I'm getting used to the idea of merging this.

> +#ifdef CONFIG_ARCH_KEYSTONE
(...)
> +#endif

Can you please remove the ifdefs. I don't mind the minimal growth
in footprint, and it makes the code simpler to read.

Yours,
Linus Walleij

[PATCH 3/4] gpio: syscon: reuse for keystone 2 socs

[Linus Walleij]

On Wed, Aug 13, 2014 at 6:16 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:

> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
> each DSP core. This is one of the component used by the IPC mechanism used
> on Keystone SOCs.
>
> Keystone 2 DSP GPIO controller has specific features:
> - each GPIO can be configured only as output pin;
> - setting GPIO value to 1 causes IRQ generation on target DSP core;
> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>   pending.
>
> This patch updates gpio-syscon driver to be reused by Keystone 2 SoCs,
> because the Keystone 2 DSP GPIO controller is controlled through Syscon
> devices and, as requested by Linus Walleij, such kind of GPIO controllers
> should be integrated with drivers/gpio/gpio-syscon.c driver.
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

OK overall I'm getting used to the idea of merging this.

> +#ifdef CONFIG_ARCH_KEYSTONE
(...)
> +#endif

Can you please remove the ifdefs. I don't mind the minimal growth
in footprint, and it makes the code simpler to read.

Yours,
Linus Walleij

Re: [PATCH 1/4] gpio: syscon: add soc specific callback to assign output value

[Linus Walleij]

On Wed, Aug 13, 2014 at 6:16 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:

> Some SoCs (like Keystone) may require to perform special
> sequence of operations to assign output GPIO value, so default
> implementation of .set() callback from gpio-syscon driver
> can't be used.
>
> Hence, add optional, SoC specific callback to assign output
> gpio value.
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

Hm :-/

I didn't realize this wasn't a quite so straight-forward a
syscon GPIO driver.

Now I start to think that it looks kludgy to bolt this onto
the other driver and think we may need to go back to the
other version which puts it as a separate driver. I guess
that is what you refer to as v1?

I have a hard time to make my mind up about these
syscon things, sorry :-(

Now I have to ask you: which way do you prefer to do it,
if you can choose freely? The initial driver or augmenting
the syscon driver (patch v1)?

Yours,
Linus Walleij

[PATCH 1/4] gpio: syscon: add soc specific callback to assign output value

[Linus Walleij]

On Wed, Aug 13, 2014 at 6:16 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:

> Some SoCs (like Keystone) may require to perform special
> sequence of operations to assign output GPIO value, so default
> implementation of .set() callback from gpio-syscon driver
> can't be used.
>
> Hence, add optional, SoC specific callback to assign output
> gpio value.
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

Hm :-/

I didn't realize this wasn't a quite so straight-forward a
syscon GPIO driver.

Now I start to think that it looks kludgy to bolt this onto
the other driver and think we may need to go back to the
other version which puts it as a separate driver. I guess
that is what you refer to as v1?

I have a hard time to make my mind up about these
syscon things, sorry :-(

Now I have to ask you: which way do you prefer to do it,
if you can choose freely? The initial driver or augmenting
the syscon driver (patch v1)?

Yours,
Linus Walleij

Re: [PATCH 1/4] gpio: syscon: add soc specific callback to assign output value

[Grygorii Strashko]

Hi Linus,

On 08/29/2014 09:19 AM, Linus Walleij wrote:
> On Wed, Aug 13, 2014 at 6:16 PM, Grygorii Strashko
> <grygorii.strashko@ti.com> wrote:
> 
>> Some SoCs (like Keystone) may require to perform special
>> sequence of operations to assign output GPIO value, so default
>> implementation of .set() callback from gpio-syscon driver
>> can't be used.
>>
>> Hence, add optional, SoC specific callback to assign output
>> gpio value.
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> 
> Hm :-/
> 
> I didn't realize this wasn't a quite so straight-forward a
> syscon GPIO driver.

Yep. At first glance, everything seemed more simple.

> 
> Now I start to think that it looks kludgy to bolt this onto
> the other driver and think we may need to go back to the
> other version which puts it as a separate driver. I guess
> that is what you refer to as v1?
> 
> I have a hard time to make my mind up about these
> syscon things, sorry :-(
> 
> Now I have to ask you: which way do you prefer to do it,
> if you can choose freely? The initial driver or augmenting
> the syscon driver (patch v1)?

Honestly, I think, It is better to keep Keystone 2 functionality in 
standalone file [1], as it allows to simply manage this code using
build system, avoid ugly #ifdefs in code (as you note on patch 3)
and keep commits history more clear and HW specific (very helpful in 
case of any issues). 

But, as you agree now to take this syscon-based patches, I'll update & re-send them,
applying comments from Alexander to the patch 1 and your comments to the patch 3 :)

Thanks for your comments.

[1] https://lkml.org/lkml/2014/7/23/352

Best regards,
-grygorii

[PATCH 1/4] gpio: syscon: add soc specific callback to assign output value

[Grygorii Strashko]

Hi Linus,

On 08/29/2014 09:19 AM, Linus Walleij wrote:
> On Wed, Aug 13, 2014 at 6:16 PM, Grygorii Strashko
> <grygorii.strashko@ti.com> wrote:
> 
>> Some SoCs (like Keystone) may require to perform special
>> sequence of operations to assign output GPIO value, so default
>> implementation of .set() callback from gpio-syscon driver
>> can't be used.
>>
>> Hence, add optional, SoC specific callback to assign output
>> gpio value.
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> 
> Hm :-/
> 
> I didn't realize this wasn't a quite so straight-forward a
> syscon GPIO driver.

Yep. At first glance, everything seemed more simple.

> 
> Now I start to think that it looks kludgy to bolt this onto
> the other driver and think we may need to go back to the
> other version which puts it as a separate driver. I guess
> that is what you refer to as v1?
> 
> I have a hard time to make my mind up about these
> syscon things, sorry :-(
> 
> Now I have to ask you: which way do you prefer to do it,
> if you can choose freely? The initial driver or augmenting
> the syscon driver (patch v1)?

Honestly, I think, It is better to keep Keystone 2 functionality in 
standalone file [1], as it allows to simply manage this code using
build system, avoid ugly #ifdefs in code (as you note on patch 3)
and keep commits history more clear and HW specific (very helpful in 
case of any issues). 

But, as you agree now to take this syscon-based patches, I'll update & re-send them,
applying comments from Alexander to the patch 1 and your comments to the patch 3 :)

Thanks for your comments.

[1] https://lkml.org/lkml/2014/7/23/352

Best regards,
-grygorii