All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level
@ 2014-02-27 20:51 ` Stephen Warren
  0 siblings, 0 replies; 22+ messages in thread
From: Stephen Warren @ 2014-02-27 20:51 UTC (permalink / raw)
  To: Samuel Ortiz, Lee Jones
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA, J Keerthy, Ian Lartey,
	Stephen Warren

From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Some boards or SoCs have an inverter between the PMIC IRQ output pin and
the IRQ controller input signal.

The IRQ specifier in DT is meant to represent the IRQ flags at the input
to the IRQ controller.

The Palmas HW's IRQ output has configurable polarity. Software needs to
know which polarity to choose for the IRQ output. Software may be tempted
to extract the IRQ polarity from the IRQ specifier in order to make this
choice.

That approach works fine if the IRQ signal is routed directly from the
PMIC to the IRQ controller with no intervening logic. However, if the
signal is inverted between the two, this approach gets the wrong answer.

Add an additional optional DT property which indicates that such an
inversion occurs. This allows DT to give complete information about the
desired IRQ output polarity to software.

An alternative would have been to add a new non-optional DT parameter to
indicate the exact desired output polarity. However, this would have been
an incompatible change to the DT binding.

Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
v2: Split V1's patch 1/2 into separate patches 1/3 and 2/3.
---
 Documentation/devicetree/bindings/mfd/palmas.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/palmas.txt b/Documentation/devicetree/bindings/mfd/palmas.txt
index e5f0f8303461..76ec509d5f87 100644
--- a/Documentation/devicetree/bindings/mfd/palmas.txt
+++ b/Documentation/devicetree/bindings/mfd/palmas.txt
@@ -18,6 +18,12 @@ Required properties:
   ti,tps659038
 and also the generic series names
   ti,palmas
+- interrupts : Should contain a single entry for the IRQ output.
+- ti,irq-externally-inverted : If missing, the polarity of the Palmas IRQ
+  output should be set to the opposite of the polarity indicated by the IRQ
+  specifier in the interrupts property. If absent, the polarity should be
+  configured to match. This allows the representation of an inverter between
+  the Palmas IRQ output and the interrupt parent's IRQ input.
 - interrupt-controller : palmas has its own internal IRQs
 - #interrupt-cells : should be set to 2 for IRQ number and flags
   The first cell is the IRQ number.
-- 
1.8.1.5

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

* [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level
@ 2014-02-27 20:51 ` Stephen Warren
  0 siblings, 0 replies; 22+ messages in thread
From: Stephen Warren @ 2014-02-27 20:51 UTC (permalink / raw)
  To: linux-arm-kernel

From: Stephen Warren <swarren@nvidia.com>

Some boards or SoCs have an inverter between the PMIC IRQ output pin and
the IRQ controller input signal.

The IRQ specifier in DT is meant to represent the IRQ flags at the input
to the IRQ controller.

The Palmas HW's IRQ output has configurable polarity. Software needs to
know which polarity to choose for the IRQ output. Software may be tempted
to extract the IRQ polarity from the IRQ specifier in order to make this
choice.

That approach works fine if the IRQ signal is routed directly from the
PMIC to the IRQ controller with no intervening logic. However, if the
signal is inverted between the two, this approach gets the wrong answer.

Add an additional optional DT property which indicates that such an
inversion occurs. This allows DT to give complete information about the
desired IRQ output polarity to software.

An alternative would have been to add a new non-optional DT parameter to
indicate the exact desired output polarity. However, this would have been
an incompatible change to the DT binding.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
v2: Split V1's patch 1/2 into separate patches 1/3 and 2/3.
---
 Documentation/devicetree/bindings/mfd/palmas.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/palmas.txt b/Documentation/devicetree/bindings/mfd/palmas.txt
index e5f0f8303461..76ec509d5f87 100644
--- a/Documentation/devicetree/bindings/mfd/palmas.txt
+++ b/Documentation/devicetree/bindings/mfd/palmas.txt
@@ -18,6 +18,12 @@ Required properties:
   ti,tps659038
 and also the generic series names
   ti,palmas
+- interrupts : Should contain a single entry for the IRQ output.
+- ti,irq-externally-inverted : If missing, the polarity of the Palmas IRQ
+  output should be set to the opposite of the polarity indicated by the IRQ
+  specifier in the interrupts property. If absent, the polarity should be
+  configured to match. This allows the representation of an inverter between
+  the Palmas IRQ output and the interrupt parent's IRQ input.
 - interrupt-controller : palmas has its own internal IRQs
 - #interrupt-cells : should be set to 2 for IRQ number and flags
   The first cell is the IRQ number.
-- 
1.8.1.5

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

* [PATCH V2 2/3] mfd: palmas: support IRQ inversion at the board level
  2014-02-27 20:51 ` Stephen Warren
@ 2014-02-27 20:51     ` Stephen Warren
  -1 siblings, 0 replies; 22+ messages in thread
From: Stephen Warren @ 2014-02-27 20:51 UTC (permalink / raw)
  To: Samuel Ortiz, Lee Jones
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA, J Keerthy, Ian Lartey,
	Stephen Warren

From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Implement the new DT property ti,irq-externally-inverted, and add an
equivalent platform data field to match. This allows the driver to
correctly automatically configure the IRQ output polarity when the board
or SoC contains an inverter between the Palmas IRQ output and IRQ
controller input.

Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
v2: Split V1's patch 1/2 into separate patches 1/3 and 2/3.

If this patch (and likely 1/3 too) could be applied to its own branch
(w/ signed tag) in the MFD tree, that would great; then I can pull patch
it into the Tegra tree so that I can apply patch 3/3 on top. Thanks.
---
 drivers/mfd/palmas.c       | 4 ++++
 include/linux/mfd/palmas.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
index d280d789e55a..f4ea932adf8d 100644
--- a/drivers/mfd/palmas.c
+++ b/drivers/mfd/palmas.c
@@ -293,6 +293,8 @@ static int palmas_set_pdata_irq_flag(struct i2c_client *i2c,
 	}
 
 	pdata->irq_flags = irqd_get_trigger_type(irq_data);
+	pdata->irq_external_inversion = of_property_read_bool(i2c->dev.of_node,
+						"ti,irq-externally-inverted");
 	dev_info(&i2c->dev, "Irq flag is 0x%08x\n", pdata->irq_flags);
 	return 0;
 }
@@ -447,6 +449,8 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
 		reg = PALMAS_POLARITY_CTRL_INT_POLARITY;
 	else
 		reg = 0;
+	if (pdata->irq_external_inversion)
+		reg ^= PALMAS_POLARITY_CTRL_INT_POLARITY;
 	ret = palmas_update_bits(palmas, PALMAS_PU_PD_OD_BASE,
 			PALMAS_POLARITY_CTRL, PALMAS_POLARITY_CTRL_INT_POLARITY,
 			reg);
diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
index 9974e387e483..2fdf08c50a48 100644
--- a/include/linux/mfd/palmas.h
+++ b/include/linux/mfd/palmas.h
@@ -292,6 +292,7 @@ struct palmas_clk_platform_data {
 
 struct palmas_platform_data {
 	int irq_flags;
+	bool irq_external_inversion;
 	int gpio_base;
 
 	/* bit value to be loaded to the POWER_CTRL register */
-- 
1.8.1.5

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

* [PATCH V2 2/3] mfd: palmas: support IRQ inversion at the board level
@ 2014-02-27 20:51     ` Stephen Warren
  0 siblings, 0 replies; 22+ messages in thread
From: Stephen Warren @ 2014-02-27 20:51 UTC (permalink / raw)
  To: linux-arm-kernel

From: Stephen Warren <swarren@nvidia.com>

Implement the new DT property ti,irq-externally-inverted, and add an
equivalent platform data field to match. This allows the driver to
correctly automatically configure the IRQ output polarity when the board
or SoC contains an inverter between the Palmas IRQ output and IRQ
controller input.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
v2: Split V1's patch 1/2 into separate patches 1/3 and 2/3.

If this patch (and likely 1/3 too) could be applied to its own branch
(w/ signed tag) in the MFD tree, that would great; then I can pull patch
it into the Tegra tree so that I can apply patch 3/3 on top. Thanks.
---
 drivers/mfd/palmas.c       | 4 ++++
 include/linux/mfd/palmas.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
index d280d789e55a..f4ea932adf8d 100644
--- a/drivers/mfd/palmas.c
+++ b/drivers/mfd/palmas.c
@@ -293,6 +293,8 @@ static int palmas_set_pdata_irq_flag(struct i2c_client *i2c,
 	}
 
 	pdata->irq_flags = irqd_get_trigger_type(irq_data);
+	pdata->irq_external_inversion = of_property_read_bool(i2c->dev.of_node,
+						"ti,irq-externally-inverted");
 	dev_info(&i2c->dev, "Irq flag is 0x%08x\n", pdata->irq_flags);
 	return 0;
 }
@@ -447,6 +449,8 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
 		reg = PALMAS_POLARITY_CTRL_INT_POLARITY;
 	else
 		reg = 0;
+	if (pdata->irq_external_inversion)
+		reg ^= PALMAS_POLARITY_CTRL_INT_POLARITY;
 	ret = palmas_update_bits(palmas, PALMAS_PU_PD_OD_BASE,
 			PALMAS_POLARITY_CTRL, PALMAS_POLARITY_CTRL_INT_POLARITY,
 			reg);
diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
index 9974e387e483..2fdf08c50a48 100644
--- a/include/linux/mfd/palmas.h
+++ b/include/linux/mfd/palmas.h
@@ -292,6 +292,7 @@ struct palmas_clk_platform_data {
 
 struct palmas_platform_data {
 	int irq_flags;
+	bool irq_external_inversion;
 	int gpio_base;
 
 	/* bit value to be loaded to the POWER_CTRL register */
-- 
1.8.1.5

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

* [PATCH V2 3/3] ARM: tegra: fix Dalmore PMIC IRQ polarity
  2014-02-27 20:51 ` Stephen Warren
@ 2014-02-27 20:51     ` Stephen Warren
  -1 siblings, 0 replies; 22+ messages in thread
From: Stephen Warren @ 2014-02-27 20:51 UTC (permalink / raw)
  To: Samuel Ortiz, Lee Jones
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA, J Keerthy, Ian Lartey,
	Stephen Warren

From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

The Tegra PMC's resume-from-sleep logic wants an active-low IRQ input
from the PMIC. However, the PMIC IRQ is also routed to the GIC, which
only supports active high IRQs (or rising edge). Hence, the signal must
be inverted in the PMC before being routed to the GIC. This implies that
the PMC DT property nvidia,invert-interrupt must be set, and it is.

The PMIC's DT interrupts property must represent the IRQ level at the
GIC, since that is the PMIC's parent IRQ controller. Fix the PMIC's
interrupts property to correctly describe the GIC input polarity.

However, the PMIC IRQ output's polarity is programmable in HW, and by
default follows the parent IRQ controller's input polarity. We need to
have an active-low output due to the inversion inside the Tegra PMC.
Hence, add the ti,irq-externally-inverted property to the PMIC.

Reported-by: Stefan Agner <stefan-XLVq0VzYD2Y@public.gmane.org>
Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v2: No change.
---
 arch/arm/boot/dts/tegra114-dalmore.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/tegra114-dalmore.dts b/arch/arm/boot/dts/tegra114-dalmore.dts
index 8de543777882..2977206cafc9 100644
--- a/arch/arm/boot/dts/tegra114-dalmore.dts
+++ b/arch/arm/boot/dts/tegra114-dalmore.dts
@@ -893,7 +893,8 @@
 		palmas: tps65913@58 {
 			compatible = "ti,palmas";
 			reg = <0x58>;
-			interrupts = <0 86 IRQ_TYPE_LEVEL_LOW>;
+			interrupts = <0 86 IRQ_TYPE_LEVEL_HIGH>;
+			ti,irq-externally-inverted;
 
 			#interrupt-cells = <2>;
 			interrupt-controller;
-- 
1.8.1.5

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

* [PATCH V2 3/3] ARM: tegra: fix Dalmore PMIC IRQ polarity
@ 2014-02-27 20:51     ` Stephen Warren
  0 siblings, 0 replies; 22+ messages in thread
From: Stephen Warren @ 2014-02-27 20:51 UTC (permalink / raw)
  To: linux-arm-kernel

From: Stephen Warren <swarren@nvidia.com>

The Tegra PMC's resume-from-sleep logic wants an active-low IRQ input
from the PMIC. However, the PMIC IRQ is also routed to the GIC, which
only supports active high IRQs (or rising edge). Hence, the signal must
be inverted in the PMC before being routed to the GIC. This implies that
the PMC DT property nvidia,invert-interrupt must be set, and it is.

The PMIC's DT interrupts property must represent the IRQ level at the
GIC, since that is the PMIC's parent IRQ controller. Fix the PMIC's
interrupts property to correctly describe the GIC input polarity.

However, the PMIC IRQ output's polarity is programmable in HW, and by
default follows the parent IRQ controller's input polarity. We need to
have an active-low output due to the inversion inside the Tegra PMC.
Hence, add the ti,irq-externally-inverted property to the PMIC.

Reported-by: Stefan Agner <stefan@agner.ch>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
---
v2: No change.
---
 arch/arm/boot/dts/tegra114-dalmore.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/tegra114-dalmore.dts b/arch/arm/boot/dts/tegra114-dalmore.dts
index 8de543777882..2977206cafc9 100644
--- a/arch/arm/boot/dts/tegra114-dalmore.dts
+++ b/arch/arm/boot/dts/tegra114-dalmore.dts
@@ -893,7 +893,8 @@
 		palmas: tps65913 at 58 {
 			compatible = "ti,palmas";
 			reg = <0x58>;
-			interrupts = <0 86 IRQ_TYPE_LEVEL_LOW>;
+			interrupts = <0 86 IRQ_TYPE_LEVEL_HIGH>;
+			ti,irq-externally-inverted;
 
 			#interrupt-cells = <2>;
 			interrupt-controller;
-- 
1.8.1.5

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

* Re: [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level
  2014-02-27 20:51 ` Stephen Warren
@ 2014-02-27 21:02     ` Graeme Gregory
  -1 siblings, 0 replies; 22+ messages in thread
From: Graeme Gregory @ 2014-02-27 21:02 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Samuel Ortiz, Lee Jones, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Stephen Warren, Pawel Moll,
	Ian Campbell, J Keerthy, Ian Lartey, Rob Herring, Kumar Gala,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Thu, Feb 27, 2014 at 01:51:19PM -0700, Stephen Warren wrote:
> From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> Some boards or SoCs have an inverter between the PMIC IRQ output pin and
> the IRQ controller input signal.
> 
> The IRQ specifier in DT is meant to represent the IRQ flags at the input
> to the IRQ controller.
> 
> The Palmas HW's IRQ output has configurable polarity. Software needs to
> know which polarity to choose for the IRQ output. Software may be tempted
> to extract the IRQ polarity from the IRQ specifier in order to make this
> choice.
> 
> That approach works fine if the IRQ signal is routed directly from the
> PMIC to the IRQ controller with no intervening logic. However, if the
> signal is inverted between the two, this approach gets the wrong answer.
> 
> Add an additional optional DT property which indicates that such an
> inversion occurs. This allows DT to give complete information about the
> desired IRQ output polarity to software.
> 
> An alternative would have been to add a new non-optional DT parameter to
> indicate the exact desired output polarity. However, this would have been
> an incompatible change to the DT binding.
> 
> Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> v2: Split V1's patch 1/2 into separate patches 1/3 and 2/3.
> ---
>  Documentation/devicetree/bindings/mfd/palmas.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/palmas.txt b/Documentation/devicetree/bindings/mfd/palmas.txt
> index e5f0f8303461..76ec509d5f87 100644
> --- a/Documentation/devicetree/bindings/mfd/palmas.txt
> +++ b/Documentation/devicetree/bindings/mfd/palmas.txt
> @@ -18,6 +18,12 @@ Required properties:
>    ti,tps659038
>  and also the generic series names
>    ti,palmas
> +- interrupts : Should contain a single entry for the IRQ output.
> +- ti,irq-externally-inverted : If missing, the polarity of the Palmas IRQ
> +  output should be set to the opposite of the polarity indicated by the IRQ
> +  specifier in the interrupts property. If absent, the polarity should be
> +  configured to match. This allows the representation of an inverter between
> +  the Palmas IRQ output and the interrupt parent's IRQ input.

This has got to be the wrong way to do things, all this leads to is every
device doing this property in its own way and having totally inconsistent
properties all meaning the same thing.

If there is some other hardware inverting lines then there should be
a generic binding for this in DT. This is not describing the palmas hardware
but some external object to the palmas.

Graeme

>  - interrupt-controller : palmas has its own internal IRQs
>  - #interrupt-cells : should be set to 2 for IRQ number and flags
>    The first cell is the IRQ number.
> -- 
> 1.8.1.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level
@ 2014-02-27 21:02     ` Graeme Gregory
  0 siblings, 0 replies; 22+ messages in thread
From: Graeme Gregory @ 2014-02-27 21:02 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Feb 27, 2014 at 01:51:19PM -0700, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Some boards or SoCs have an inverter between the PMIC IRQ output pin and
> the IRQ controller input signal.
> 
> The IRQ specifier in DT is meant to represent the IRQ flags at the input
> to the IRQ controller.
> 
> The Palmas HW's IRQ output has configurable polarity. Software needs to
> know which polarity to choose for the IRQ output. Software may be tempted
> to extract the IRQ polarity from the IRQ specifier in order to make this
> choice.
> 
> That approach works fine if the IRQ signal is routed directly from the
> PMIC to the IRQ controller with no intervening logic. However, if the
> signal is inverted between the two, this approach gets the wrong answer.
> 
> Add an additional optional DT property which indicates that such an
> inversion occurs. This allows DT to give complete information about the
> desired IRQ output polarity to software.
> 
> An alternative would have been to add a new non-optional DT parameter to
> indicate the exact desired output polarity. However, this would have been
> an incompatible change to the DT binding.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
> v2: Split V1's patch 1/2 into separate patches 1/3 and 2/3.
> ---
>  Documentation/devicetree/bindings/mfd/palmas.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/palmas.txt b/Documentation/devicetree/bindings/mfd/palmas.txt
> index e5f0f8303461..76ec509d5f87 100644
> --- a/Documentation/devicetree/bindings/mfd/palmas.txt
> +++ b/Documentation/devicetree/bindings/mfd/palmas.txt
> @@ -18,6 +18,12 @@ Required properties:
>    ti,tps659038
>  and also the generic series names
>    ti,palmas
> +- interrupts : Should contain a single entry for the IRQ output.
> +- ti,irq-externally-inverted : If missing, the polarity of the Palmas IRQ
> +  output should be set to the opposite of the polarity indicated by the IRQ
> +  specifier in the interrupts property. If absent, the polarity should be
> +  configured to match. This allows the representation of an inverter between
> +  the Palmas IRQ output and the interrupt parent's IRQ input.

This has got to be the wrong way to do things, all this leads to is every
device doing this property in its own way and having totally inconsistent
properties all meaning the same thing.

If there is some other hardware inverting lines then there should be
a generic binding for this in DT. This is not describing the palmas hardware
but some external object to the palmas.

Graeme

>  - interrupt-controller : palmas has its own internal IRQs
>  - #interrupt-cells : should be set to 2 for IRQ number and flags
>    The first cell is the IRQ number.
> -- 
> 1.8.1.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level
  2014-02-27 21:02     ` Graeme Gregory
@ 2014-02-27 21:35       ` Stephen Warren
  -1 siblings, 0 replies; 22+ messages in thread
From: Stephen Warren @ 2014-02-27 21:35 UTC (permalink / raw)
  To: Graeme Gregory
  Cc: Samuel Ortiz, Lee Jones, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Stephen Warren, Pawel Moll,
	Ian Campbell, J Keerthy, Ian Lartey, Rob Herring, Kumar Gala,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On 02/27/2014 02:02 PM, Graeme Gregory wrote:
> On Thu, Feb 27, 2014 at 01:51:19PM -0700, Stephen Warren wrote:
>> Some boards or SoCs have an inverter between the PMIC IRQ output pin and
>> the IRQ controller input signal.
>>
>> The IRQ specifier in DT is meant to represent the IRQ flags at the input
>> to the IRQ controller.
>>
>> The Palmas HW's IRQ output has configurable polarity. Software needs to
>> know which polarity to choose for the IRQ output. Software may be tempted
>> to extract the IRQ polarity from the IRQ specifier in order to make this
>> choice.
>>
>> That approach works fine if the IRQ signal is routed directly from the
>> PMIC to the IRQ controller with no intervening logic. However, if the
>> signal is inverted between the two, this approach gets the wrong answer.
>>
>> Add an additional optional DT property which indicates that such an
>> inversion occurs. This allows DT to give complete information about the
>> desired IRQ output polarity to software.
>>
>> An alternative would have been to add a new non-optional DT parameter to
>> indicate the exact desired output polarity. However, this would have been
>> an incompatible change to the DT binding.

>> diff --git a/Documentation/devicetree/bindings/mfd/palmas.txt b/Documentation/devicetree/bindings/mfd/palmas.txt

>> +- ti,irq-externally-inverted : If missing, the polarity of the Palmas IRQ
>> +  output should be set to the opposite of the polarity indicated by the IRQ
>> +  specifier in the interrupts property. If absent, the polarity should be
>> +  configured to match. This allows the representation of an inverter between
>> +  the Palmas IRQ output and the interrupt parent's IRQ input.
> 
> This has got to be the wrong way to do things, all this leads to is every
> device doing this property in its own way and having totally inconsistent
> properties all meaning the same thing.
> 
> If there is some other hardware inverting lines then there should be
> a generic binding for this in DT. This is not describing the palmas hardware
> but some external object to the palmas.

I'd be fine with removing the "ti," vendor prefix from the property
name, and promoting it to be a cross-device standard.

I'm not sure that many devices will need this though; most don't have
configurable output polarity. Still, I guess that shouldn't stop us from
creating standards for the cases where it is needed.

If the DT reviewers can ack the concept, I'm happy to respin the patch
with the more generic property name.

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

* [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level
@ 2014-02-27 21:35       ` Stephen Warren
  0 siblings, 0 replies; 22+ messages in thread
From: Stephen Warren @ 2014-02-27 21:35 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/27/2014 02:02 PM, Graeme Gregory wrote:
> On Thu, Feb 27, 2014 at 01:51:19PM -0700, Stephen Warren wrote:
>> Some boards or SoCs have an inverter between the PMIC IRQ output pin and
>> the IRQ controller input signal.
>>
>> The IRQ specifier in DT is meant to represent the IRQ flags at the input
>> to the IRQ controller.
>>
>> The Palmas HW's IRQ output has configurable polarity. Software needs to
>> know which polarity to choose for the IRQ output. Software may be tempted
>> to extract the IRQ polarity from the IRQ specifier in order to make this
>> choice.
>>
>> That approach works fine if the IRQ signal is routed directly from the
>> PMIC to the IRQ controller with no intervening logic. However, if the
>> signal is inverted between the two, this approach gets the wrong answer.
>>
>> Add an additional optional DT property which indicates that such an
>> inversion occurs. This allows DT to give complete information about the
>> desired IRQ output polarity to software.
>>
>> An alternative would have been to add a new non-optional DT parameter to
>> indicate the exact desired output polarity. However, this would have been
>> an incompatible change to the DT binding.

>> diff --git a/Documentation/devicetree/bindings/mfd/palmas.txt b/Documentation/devicetree/bindings/mfd/palmas.txt

>> +- ti,irq-externally-inverted : If missing, the polarity of the Palmas IRQ
>> +  output should be set to the opposite of the polarity indicated by the IRQ
>> +  specifier in the interrupts property. If absent, the polarity should be
>> +  configured to match. This allows the representation of an inverter between
>> +  the Palmas IRQ output and the interrupt parent's IRQ input.
> 
> This has got to be the wrong way to do things, all this leads to is every
> device doing this property in its own way and having totally inconsistent
> properties all meaning the same thing.
> 
> If there is some other hardware inverting lines then there should be
> a generic binding for this in DT. This is not describing the palmas hardware
> but some external object to the palmas.

I'd be fine with removing the "ti," vendor prefix from the property
name, and promoting it to be a cross-device standard.

I'm not sure that many devices will need this though; most don't have
configurable output polarity. Still, I guess that shouldn't stop us from
creating standards for the cases where it is needed.

If the DT reviewers can ack the concept, I'm happy to respin the patch
with the more generic property name.

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

* Re: [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level
  2014-02-27 21:35       ` Stephen Warren
@ 2014-02-28  5:58           ` Mark Brown
  -1 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2014-02-28  5:58 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Graeme Gregory, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Stephen Warren, Samuel Ortiz, Pawel Moll, Ian Campbell,
	J Keerthy, Ian Lartey, Rob Herring, Kumar Gala,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA, Lee Jones,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

[-- Attachment #1: Type: text/plain, Size: 1734 bytes --]

On Thu, Feb 27, 2014 at 02:35:42PM -0700, Stephen Warren wrote:
> On 02/27/2014 02:02 PM, Graeme Gregory wrote:

> >> +- ti,irq-externally-inverted : If missing, the polarity of the Palmas IRQ
> >> +  output should be set to the opposite of the polarity indicated by the IRQ
> >> +  specifier in the interrupts property. If absent, the polarity should be
> >> +  configured to match. This allows the representation of an inverter between
> >> +  the Palmas IRQ output and the interrupt parent's IRQ input.

> > This has got to be the wrong way to do things, all this leads to is every
> > device doing this property in its own way and having totally inconsistent
> > properties all meaning the same thing.

> > If there is some other hardware inverting lines then there should be
> > a generic binding for this in DT. This is not describing the palmas hardware
> > but some external object to the palmas.

> I'd be fine with removing the "ti," vendor prefix from the property
> name, and promoting it to be a cross-device standard.

> I'm not sure that many devices will need this though; most don't have
> configurable output polarity. Still, I guess that shouldn't stop us from
> creating standards for the cases where it is needed.

It's really common for PMICs and CODECs to be able to set any interrupt
polarity at least.

> If the DT reviewers can ack the concept, I'm happy to respin the patch
> with the more generic property name.

I'm not sure that renaming the property really deals with the concerns
though since drivers still all need to manually add support for this,
shouldn't there be an interrupt controller described in the DT which
just chains on to the parent with the polarity inverted to do the
impedence match?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level
@ 2014-02-28  5:58           ` Mark Brown
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2014-02-28  5:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Feb 27, 2014 at 02:35:42PM -0700, Stephen Warren wrote:
> On 02/27/2014 02:02 PM, Graeme Gregory wrote:

> >> +- ti,irq-externally-inverted : If missing, the polarity of the Palmas IRQ
> >> +  output should be set to the opposite of the polarity indicated by the IRQ
> >> +  specifier in the interrupts property. If absent, the polarity should be
> >> +  configured to match. This allows the representation of an inverter between
> >> +  the Palmas IRQ output and the interrupt parent's IRQ input.

> > This has got to be the wrong way to do things, all this leads to is every
> > device doing this property in its own way and having totally inconsistent
> > properties all meaning the same thing.

> > If there is some other hardware inverting lines then there should be
> > a generic binding for this in DT. This is not describing the palmas hardware
> > but some external object to the palmas.

> I'd be fine with removing the "ti," vendor prefix from the property
> name, and promoting it to be a cross-device standard.

> I'm not sure that many devices will need this though; most don't have
> configurable output polarity. Still, I guess that shouldn't stop us from
> creating standards for the cases where it is needed.

It's really common for PMICs and CODECs to be able to set any interrupt
polarity at least.

> If the DT reviewers can ack the concept, I'm happy to respin the patch
> with the more generic property name.

I'm not sure that renaming the property really deals with the concerns
though since drivers still all need to manually add support for this,
shouldn't there be an interrupt controller described in the DT which
just chains on to the parent with the polarity inverted to do the
impedence match?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140228/9b669b7a/attachment.sig>

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

* Re: [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level
  2014-02-28  5:58           ` Mark Brown
@ 2014-02-28 16:34               ` Stephen Warren
  -1 siblings, 0 replies; 22+ messages in thread
From: Stephen Warren @ 2014-02-28 16:34 UTC (permalink / raw)
  To: Mark Brown
  Cc: Graeme Gregory, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Stephen Warren, Samuel Ortiz, Pawel Moll, Ian Campbell,
	J Keerthy, Ian Lartey, Rob Herring, Kumar Gala,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA, Lee Jones,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On 02/27/2014 10:58 PM, Mark Brown wrote:
> On Thu, Feb 27, 2014 at 02:35:42PM -0700, Stephen Warren wrote:
>> On 02/27/2014 02:02 PM, Graeme Gregory wrote:
> 
>>>> +- ti,irq-externally-inverted : If missing, the polarity of the Palmas IRQ
>>>> +  output should be set to the opposite of the polarity indicated by the IRQ
>>>> +  specifier in the interrupts property. If absent, the polarity should be
>>>> +  configured to match. This allows the representation of an inverter between
>>>> +  the Palmas IRQ output and the interrupt parent's IRQ input.
> 
>>> This has got to be the wrong way to do things, all this leads to is every
>>> device doing this property in its own way and having totally inconsistent
>>> properties all meaning the same thing.
> 
>>> If there is some other hardware inverting lines then there should be
>>> a generic binding for this in DT. This is not describing the palmas hardware
>>> but some external object to the palmas.
> 
>> I'd be fine with removing the "ti," vendor prefix from the property
>> name, and promoting it to be a cross-device standard.
> 
>> I'm not sure that many devices will need this though; most don't have
>> configurable output polarity. Still, I guess that shouldn't stop us from
>> creating standards for the cases where it is needed.
> 
> It's really common for PMICs and CODECs to be able to set any interrupt
> polarity at least.
> 
>> If the DT reviewers can ack the concept, I'm happy to respin the patch
>> with the more generic property name.
> 
> I'm not sure that renaming the property really deals with the concerns
> though since drivers still all need to manually add support for this,
> shouldn't there be an interrupt controller described in the DT which
> just chains on to the parent with the polarity inverted to do the
> impedence match?

I had thought of that when first dealing with this a couple years ago,
but Olof suggested that was too complicated.

Certainly, adding an "inverting" interrupt controller into the path
would solve the problem without inventing any new concept in DT.
However, the inverter really isn't an interrupt controller in any
meaningful fashion. It has no status/mask/enable/... registers at all;
it's nothing more than an inverter gate, at least in my case. Hence, I'm
actually not convinced that adding an extra interrupt controller into
the path is correct here.

Another alternative might be to add an extra IRQ bit in the IRQ
specifier (and something similar would be needed for GPIO specifiers)
that indicates "inversion between source and destination". This could be
queried by drivers in exactly the same way as the existing polarity/type
IRQ flags. We'd need to update each individual IRQ controller binding to
enable that flag, since each binding defines its own definition of such
flags. (although in practice since most use the same centrally suggested
flags, this wouldn't be any more than just saying yes, this binding
allows that new flag to be used).

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

* [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level
@ 2014-02-28 16:34               ` Stephen Warren
  0 siblings, 0 replies; 22+ messages in thread
From: Stephen Warren @ 2014-02-28 16:34 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/27/2014 10:58 PM, Mark Brown wrote:
> On Thu, Feb 27, 2014 at 02:35:42PM -0700, Stephen Warren wrote:
>> On 02/27/2014 02:02 PM, Graeme Gregory wrote:
> 
>>>> +- ti,irq-externally-inverted : If missing, the polarity of the Palmas IRQ
>>>> +  output should be set to the opposite of the polarity indicated by the IRQ
>>>> +  specifier in the interrupts property. If absent, the polarity should be
>>>> +  configured to match. This allows the representation of an inverter between
>>>> +  the Palmas IRQ output and the interrupt parent's IRQ input.
> 
>>> This has got to be the wrong way to do things, all this leads to is every
>>> device doing this property in its own way and having totally inconsistent
>>> properties all meaning the same thing.
> 
>>> If there is some other hardware inverting lines then there should be
>>> a generic binding for this in DT. This is not describing the palmas hardware
>>> but some external object to the palmas.
> 
>> I'd be fine with removing the "ti," vendor prefix from the property
>> name, and promoting it to be a cross-device standard.
> 
>> I'm not sure that many devices will need this though; most don't have
>> configurable output polarity. Still, I guess that shouldn't stop us from
>> creating standards for the cases where it is needed.
> 
> It's really common for PMICs and CODECs to be able to set any interrupt
> polarity at least.
> 
>> If the DT reviewers can ack the concept, I'm happy to respin the patch
>> with the more generic property name.
> 
> I'm not sure that renaming the property really deals with the concerns
> though since drivers still all need to manually add support for this,
> shouldn't there be an interrupt controller described in the DT which
> just chains on to the parent with the polarity inverted to do the
> impedence match?

I had thought of that when first dealing with this a couple years ago,
but Olof suggested that was too complicated.

Certainly, adding an "inverting" interrupt controller into the path
would solve the problem without inventing any new concept in DT.
However, the inverter really isn't an interrupt controller in any
meaningful fashion. It has no status/mask/enable/... registers at all;
it's nothing more than an inverter gate, at least in my case. Hence, I'm
actually not convinced that adding an extra interrupt controller into
the path is correct here.

Another alternative might be to add an extra IRQ bit in the IRQ
specifier (and something similar would be needed for GPIO specifiers)
that indicates "inversion between source and destination". This could be
queried by drivers in exactly the same way as the existing polarity/type
IRQ flags. We'd need to update each individual IRQ controller binding to
enable that flag, since each binding defines its own definition of such
flags. (although in practice since most use the same centrally suggested
flags, this wouldn't be any more than just saying yes, this binding
allows that new flag to be used).

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

* Re: [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level
  2014-02-28 16:34               ` Stephen Warren
@ 2014-03-01  3:13                   ` Mark Brown
  -1 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2014-03-01  3:13 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Graeme Gregory, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Stephen Warren, Samuel Ortiz, Pawel Moll, Ian Campbell,
	J Keerthy, Ian Lartey, Rob Herring, Kumar Gala,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA, Lee Jones,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

[-- Attachment #1: Type: text/plain, Size: 1332 bytes --]

On Fri, Feb 28, 2014 at 09:34:33AM -0700, Stephen Warren wrote:
> On 02/27/2014 10:58 PM, Mark Brown wrote:

> > I'm not sure that renaming the property really deals with the concerns
> > though since drivers still all need to manually add support for this,
> > shouldn't there be an interrupt controller described in the DT which
> > just chains on to the parent with the polarity inverted to do the
> > impedence match?

> I had thought of that when first dealing with this a couple years ago,
> but Olof suggested that was too complicated.

It's not obvious to me that it should be especially hard but I've not
thought about it too deeply.

> Another alternative might be to add an extra IRQ bit in the IRQ
> specifier (and something similar would be needed for GPIO specifiers)
> that indicates "inversion between source and destination". This could be
> queried by drivers in exactly the same way as the existing polarity/type
> IRQ flags. We'd need to update each individual IRQ controller binding to
> enable that flag, since each binding defines its own definition of such
> flags. (although in practice since most use the same centrally suggested
> flags, this wouldn't be any more than just saying yes, this binding
> allows that new flag to be used).

Yes, doing something on the controller side seems more obvious here.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level
@ 2014-03-01  3:13                   ` Mark Brown
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2014-03-01  3:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 28, 2014 at 09:34:33AM -0700, Stephen Warren wrote:
> On 02/27/2014 10:58 PM, Mark Brown wrote:

> > I'm not sure that renaming the property really deals with the concerns
> > though since drivers still all need to manually add support for this,
> > shouldn't there be an interrupt controller described in the DT which
> > just chains on to the parent with the polarity inverted to do the
> > impedence match?

> I had thought of that when first dealing with this a couple years ago,
> but Olof suggested that was too complicated.

It's not obvious to me that it should be especially hard but I've not
thought about it too deeply.

> Another alternative might be to add an extra IRQ bit in the IRQ
> specifier (and something similar would be needed for GPIO specifiers)
> that indicates "inversion between source and destination". This could be
> queried by drivers in exactly the same way as the existing polarity/type
> IRQ flags. We'd need to update each individual IRQ controller binding to
> enable that flag, since each binding defines its own definition of such
> flags. (although in practice since most use the same centrally suggested
> flags, this wouldn't be any more than just saying yes, this binding
> allows that new flag to be used).

Yes, doing something on the controller side seems more obvious here.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140301/be9631ae/attachment.sig>

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

* Re: [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level
  2014-02-28 16:34               ` Stephen Warren
@ 2014-03-03  0:52                   ` Graeme Gregory
  -1 siblings, 0 replies; 22+ messages in thread
From: Graeme Gregory @ 2014-03-03  0:52 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Mark Brown, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Stephen Warren, Samuel Ortiz, Pawel Moll, Ian Campbell,
	J Keerthy, Ian Lartey, Rob Herring, Kumar Gala,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA, Lee Jones,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Fri, Feb 28, 2014 at 09:34:33AM -0700, Stephen Warren wrote:
> On 02/27/2014 10:58 PM, Mark Brown wrote:
> > On Thu, Feb 27, 2014 at 02:35:42PM -0700, Stephen Warren wrote:
> >> On 02/27/2014 02:02 PM, Graeme Gregory wrote:
> > 
> >>>> +- ti,irq-externally-inverted : If missing, the polarity of the Palmas IRQ
> >>>> +  output should be set to the opposite of the polarity indicated by the IRQ
> >>>> +  specifier in the interrupts property. If absent, the polarity should be
> >>>> +  configured to match. This allows the representation of an inverter between
> >>>> +  the Palmas IRQ output and the interrupt parent's IRQ input.
> > 
> >>> This has got to be the wrong way to do things, all this leads to is every
> >>> device doing this property in its own way and having totally inconsistent
> >>> properties all meaning the same thing.
> > 
> >>> If there is some other hardware inverting lines then there should be
> >>> a generic binding for this in DT. This is not describing the palmas hardware
> >>> but some external object to the palmas.
> > 
> >> I'd be fine with removing the "ti," vendor prefix from the property
> >> name, and promoting it to be a cross-device standard.
> > 
> >> I'm not sure that many devices will need this though; most don't have
> >> configurable output polarity. Still, I guess that shouldn't stop us from
> >> creating standards for the cases where it is needed.
> > 
> > It's really common for PMICs and CODECs to be able to set any interrupt
> > polarity at least.
> > 
> >> If the DT reviewers can ack the concept, I'm happy to respin the patch
> >> with the more generic property name.
> > 
> > I'm not sure that renaming the property really deals with the concerns
> > though since drivers still all need to manually add support for this,
> > shouldn't there be an interrupt controller described in the DT which
> > just chains on to the parent with the polarity inverted to do the
> > impedence match?
> 
> I had thought of that when first dealing with this a couple years ago,
> but Olof suggested that was too complicated.
> 
> Certainly, adding an "inverting" interrupt controller into the path
> would solve the problem without inventing any new concept in DT.
> However, the inverter really isn't an interrupt controller in any
> meaningful fashion. It has no status/mask/enable/... registers at all;
> it's nothing more than an inverter gate, at least in my case. Hence, I'm
> actually not convinced that adding an extra interrupt controller into
> the path is correct here.
> 
A interupt controller inline that does the inversion was my first thought
but I think that is probably overkill unless there is a whole range
of different interupt filtering operations.

> Another alternative might be to add an extra IRQ bit in the IRQ
> specifier (and something similar would be needed for GPIO specifiers)
> that indicates "inversion between source and destination". This could be
> queried by drivers in exactly the same way as the existing polarity/type
> IRQ flags. We'd need to update each individual IRQ controller binding to
> enable that flag, since each binding defines its own definition of such
> flags. (although in practice since most use the same centrally suggested
> flags, this wouldn't be any more than just saying yes, this binding
> allows that new flag to be used).
> 
A new flag would meet my concerns of every chip doing basic inversion
in different ways.

Graeme

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

* [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level
@ 2014-03-03  0:52                   ` Graeme Gregory
  0 siblings, 0 replies; 22+ messages in thread
From: Graeme Gregory @ 2014-03-03  0:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 28, 2014 at 09:34:33AM -0700, Stephen Warren wrote:
> On 02/27/2014 10:58 PM, Mark Brown wrote:
> > On Thu, Feb 27, 2014 at 02:35:42PM -0700, Stephen Warren wrote:
> >> On 02/27/2014 02:02 PM, Graeme Gregory wrote:
> > 
> >>>> +- ti,irq-externally-inverted : If missing, the polarity of the Palmas IRQ
> >>>> +  output should be set to the opposite of the polarity indicated by the IRQ
> >>>> +  specifier in the interrupts property. If absent, the polarity should be
> >>>> +  configured to match. This allows the representation of an inverter between
> >>>> +  the Palmas IRQ output and the interrupt parent's IRQ input.
> > 
> >>> This has got to be the wrong way to do things, all this leads to is every
> >>> device doing this property in its own way and having totally inconsistent
> >>> properties all meaning the same thing.
> > 
> >>> If there is some other hardware inverting lines then there should be
> >>> a generic binding for this in DT. This is not describing the palmas hardware
> >>> but some external object to the palmas.
> > 
> >> I'd be fine with removing the "ti," vendor prefix from the property
> >> name, and promoting it to be a cross-device standard.
> > 
> >> I'm not sure that many devices will need this though; most don't have
> >> configurable output polarity. Still, I guess that shouldn't stop us from
> >> creating standards for the cases where it is needed.
> > 
> > It's really common for PMICs and CODECs to be able to set any interrupt
> > polarity at least.
> > 
> >> If the DT reviewers can ack the concept, I'm happy to respin the patch
> >> with the more generic property name.
> > 
> > I'm not sure that renaming the property really deals with the concerns
> > though since drivers still all need to manually add support for this,
> > shouldn't there be an interrupt controller described in the DT which
> > just chains on to the parent with the polarity inverted to do the
> > impedence match?
> 
> I had thought of that when first dealing with this a couple years ago,
> but Olof suggested that was too complicated.
> 
> Certainly, adding an "inverting" interrupt controller into the path
> would solve the problem without inventing any new concept in DT.
> However, the inverter really isn't an interrupt controller in any
> meaningful fashion. It has no status/mask/enable/... registers at all;
> it's nothing more than an inverter gate, at least in my case. Hence, I'm
> actually not convinced that adding an extra interrupt controller into
> the path is correct here.
> 
A interupt controller inline that does the inversion was my first thought
but I think that is probably overkill unless there is a whole range
of different interupt filtering operations.

> Another alternative might be to add an extra IRQ bit in the IRQ
> specifier (and something similar would be needed for GPIO specifiers)
> that indicates "inversion between source and destination". This could be
> queried by drivers in exactly the same way as the existing polarity/type
> IRQ flags. We'd need to update each individual IRQ controller binding to
> enable that flag, since each binding defines its own definition of such
> flags. (although in practice since most use the same centrally suggested
> flags, this wouldn't be any more than just saying yes, this binding
> allows that new flag to be used).
> 
A new flag would meet my concerns of every chip doing basic inversion
in different ways.

Graeme

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

* Re: [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level
  2014-03-03  0:52                   ` Graeme Gregory
@ 2014-03-03 16:41                     ` Stephen Warren
  -1 siblings, 0 replies; 22+ messages in thread
From: Stephen Warren @ 2014-03-03 16:41 UTC (permalink / raw)
  To: Graeme Gregory
  Cc: Mark Brown, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Stephen Warren, Samuel Ortiz, Pawel Moll, Ian Campbell,
	J Keerthy, Ian Lartey, Rob Herring, Kumar Gala,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA, Lee Jones,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On 03/02/2014 05:52 PM, Graeme Gregory wrote:
> On Fri, Feb 28, 2014 at 09:34:33AM -0700, Stephen Warren wrote:
>> On 02/27/2014 10:58 PM, Mark Brown wrote:
>>> On Thu, Feb 27, 2014 at 02:35:42PM -0700, Stephen Warren wrote:
>>>> On 02/27/2014 02:02 PM, Graeme Gregory wrote:
>>>
>>>>>> +- ti,irq-externally-inverted : If missing, the polarity of the Palmas IRQ
>>>>>> +  output should be set to the opposite of the polarity indicated by the IRQ
>>>>>> +  specifier in the interrupts property. If absent, the polarity should be
>>>>>> +  configured to match. This allows the representation of an inverter between
>>>>>> +  the Palmas IRQ output and the interrupt parent's IRQ input.
>>>
>>>>> This has got to be the wrong way to do things, all this leads to is every
>>>>> device doing this property in its own way and having totally inconsistent
>>>>> properties all meaning the same thing.
...
>> Another alternative might be to add an extra IRQ bit in the IRQ
>> specifier (and something similar would be needed for GPIO specifiers)
>> that indicates "inversion between source and destination". This could be
>> queried by drivers in exactly the same way as the existing polarity/type
>> IRQ flags. We'd need to update each individual IRQ controller binding to
>> enable that flag, since each binding defines its own definition of such
>> flags. (although in practice since most use the same centrally suggested
>> flags, this wouldn't be any more than just saying yes, this binding
>> allows that new flag to be used).
>>
> A new flag would meet my concerns of every chip doing basic inversion
> in different ways.

OK, I'll look into a new flag.

The one thing I worry about is that we can either:

1) Have every IC driver with a configurable output polarity read a DT
flag (and we can fully standardize it's name), which is 1 line of code
per IC driver.

or:

2) We can go through every single interrupt controller's DT binding and
driver, and implement (document and parse) the new IRQ specifier flag
there, and pass it throgh the Linux IRQ stack, yet we still need code in
every IC driver to actually read the flag, so we haven't removed any
code. It seems /much/ simpler, and no more of a maintenance or
consistency burden, to just have the IC driver read the flag directly
from their own DT.

Still, as I said, I'll go try and code it up and see how bad it is in
practice.

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

* [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level
@ 2014-03-03 16:41                     ` Stephen Warren
  0 siblings, 0 replies; 22+ messages in thread
From: Stephen Warren @ 2014-03-03 16:41 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/02/2014 05:52 PM, Graeme Gregory wrote:
> On Fri, Feb 28, 2014 at 09:34:33AM -0700, Stephen Warren wrote:
>> On 02/27/2014 10:58 PM, Mark Brown wrote:
>>> On Thu, Feb 27, 2014 at 02:35:42PM -0700, Stephen Warren wrote:
>>>> On 02/27/2014 02:02 PM, Graeme Gregory wrote:
>>>
>>>>>> +- ti,irq-externally-inverted : If missing, the polarity of the Palmas IRQ
>>>>>> +  output should be set to the opposite of the polarity indicated by the IRQ
>>>>>> +  specifier in the interrupts property. If absent, the polarity should be
>>>>>> +  configured to match. This allows the representation of an inverter between
>>>>>> +  the Palmas IRQ output and the interrupt parent's IRQ input.
>>>
>>>>> This has got to be the wrong way to do things, all this leads to is every
>>>>> device doing this property in its own way and having totally inconsistent
>>>>> properties all meaning the same thing.
...
>> Another alternative might be to add an extra IRQ bit in the IRQ
>> specifier (and something similar would be needed for GPIO specifiers)
>> that indicates "inversion between source and destination". This could be
>> queried by drivers in exactly the same way as the existing polarity/type
>> IRQ flags. We'd need to update each individual IRQ controller binding to
>> enable that flag, since each binding defines its own definition of such
>> flags. (although in practice since most use the same centrally suggested
>> flags, this wouldn't be any more than just saying yes, this binding
>> allows that new flag to be used).
>>
> A new flag would meet my concerns of every chip doing basic inversion
> in different ways.

OK, I'll look into a new flag.

The one thing I worry about is that we can either:

1) Have every IC driver with a configurable output polarity read a DT
flag (and we can fully standardize it's name), which is 1 line of code
per IC driver.

or:

2) We can go through every single interrupt controller's DT binding and
driver, and implement (document and parse) the new IRQ specifier flag
there, and pass it throgh the Linux IRQ stack, yet we still need code in
every IC driver to actually read the flag, so we haven't removed any
code. It seems /much/ simpler, and no more of a maintenance or
consistency burden, to just have the IC driver read the flag directly
from their own DT.

Still, as I said, I'll go try and code it up and see how bad it is in
practice.

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

* Re: [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level
  2014-03-03 16:41                     ` Stephen Warren
@ 2014-03-04  3:50                         ` Mark Brown
  -1 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2014-03-04  3:50 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Graeme Gregory, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Stephen Warren, Samuel Ortiz, Pawel Moll, Ian Campbell,
	J Keerthy, Ian Lartey, Rob Herring, Kumar Gala,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA, Lee Jones,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

[-- Attachment #1: Type: text/plain, Size: 756 bytes --]

On Mon, Mar 03, 2014 at 09:41:36AM -0700, Stephen Warren wrote:

> 2) We can go through every single interrupt controller's DT binding and
> driver, and implement (document and parse) the new IRQ specifier flag
> there, and pass it throgh the Linux IRQ stack, yet we still need code in
> every IC driver to actually read the flag, so we haven't removed any
> code. It seems /much/ simpler, and no more of a maintenance or
> consistency burden, to just have the IC driver read the flag directly
> from their own DT.

Why does this have to be handled by the chip drivers?  Can't the
interrupt controller configured in a way that the chip drivers never
need to see it at all, for example by adding a "override the mode for
this input" flag inside the device?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level
@ 2014-03-04  3:50                         ` Mark Brown
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2014-03-04  3:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Mar 03, 2014 at 09:41:36AM -0700, Stephen Warren wrote:

> 2) We can go through every single interrupt controller's DT binding and
> driver, and implement (document and parse) the new IRQ specifier flag
> there, and pass it throgh the Linux IRQ stack, yet we still need code in
> every IC driver to actually read the flag, so we haven't removed any
> code. It seems /much/ simpler, and no more of a maintenance or
> consistency burden, to just have the IC driver read the flag directly
> from their own DT.

Why does this have to be handled by the chip drivers?  Can't the
interrupt controller configured in a way that the chip drivers never
need to see it at all, for example by adding a "override the mode for
this input" flag inside the device?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140304/f6e579d3/attachment.sig>

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

end of thread, other threads:[~2014-03-04  3:50 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-27 20:51 [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level Stephen Warren
2014-02-27 20:51 ` Stephen Warren
     [not found] ` <1393534281-30759-1-git-send-email-swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2014-02-27 20:51   ` [PATCH V2 2/3] mfd: " Stephen Warren
2014-02-27 20:51     ` Stephen Warren
2014-02-27 20:51   ` [PATCH V2 3/3] ARM: tegra: fix Dalmore PMIC IRQ polarity Stephen Warren
2014-02-27 20:51     ` Stephen Warren
2014-02-27 21:02   ` [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level Graeme Gregory
2014-02-27 21:02     ` Graeme Gregory
2014-02-27 21:35     ` Stephen Warren
2014-02-27 21:35       ` Stephen Warren
     [not found]       ` <530FAFAE.5050800-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2014-02-28  5:58         ` Mark Brown
2014-02-28  5:58           ` Mark Brown
     [not found]           ` <20140228055818.GA29849-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2014-02-28 16:34             ` Stephen Warren
2014-02-28 16:34               ` Stephen Warren
     [not found]               ` <5310BA99.4050203-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2014-03-01  3:13                 ` Mark Brown
2014-03-01  3:13                   ` Mark Brown
2014-03-03  0:52                 ` Graeme Gregory
2014-03-03  0:52                   ` Graeme Gregory
2014-03-03 16:41                   ` Stephen Warren
2014-03-03 16:41                     ` Stephen Warren
     [not found]                     ` <5314B0C0.4040008-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2014-03-04  3:50                       ` Mark Brown
2014-03-04  3:50                         ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.