* [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level @ 2014-02-27 20:51 Stephen Warren [not found] ` <1393534281-30759-1-git-send-email-swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 0 siblings, 1 reply; 11+ 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] 11+ messages in thread
[parent not found: <1393534281-30759-1-git-send-email-swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]
* [PATCH V2 2/3] mfd: palmas: support IRQ inversion at the board level [not found] ` <1393534281-30759-1-git-send-email-swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> @ 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 21:02 ` [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level Graeme Gregory 2 siblings, 0 replies; 11+ 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] 11+ messages in thread
* [PATCH V2 3/3] ARM: tegra: fix Dalmore PMIC IRQ polarity [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 21:02 ` [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level Graeme Gregory 2 siblings, 0 replies; 11+ 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] 11+ messages in thread
* Re: [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level [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 ` [PATCH V2 3/3] ARM: tegra: fix Dalmore PMIC IRQ polarity Stephen Warren @ 2014-02-27 21:02 ` Graeme Gregory 2014-02-27 21:35 ` Stephen Warren 2 siblings, 1 reply; 11+ 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] 11+ messages in thread
* Re: [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level 2014-02-27 21:02 ` [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level Graeme Gregory @ 2014-02-27 21:35 ` Stephen Warren [not found] ` <530FAFAE.5050800-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 0 siblings, 1 reply; 11+ 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] 11+ messages in thread
[parent not found: <530FAFAE.5050800-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]
* Re: [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level [not found] ` <530FAFAE.5050800-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> @ 2014-02-28 5:58 ` Mark Brown [not found] ` <20140228055818.GA29849-GFdadSzt00ze9xe1eoZjHA@public.gmane.org> 0 siblings, 1 reply; 11+ 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] 11+ messages in thread
[parent not found: <20140228055818.GA29849-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>]
* Re: [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level [not found] ` <20140228055818.GA29849-GFdadSzt00ze9xe1eoZjHA@public.gmane.org> @ 2014-02-28 16:34 ` Stephen Warren [not found] ` <5310BA99.4050203-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 0 siblings, 1 reply; 11+ 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] 11+ messages in thread
[parent not found: <5310BA99.4050203-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]
* Re: [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level [not found] ` <5310BA99.4050203-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> @ 2014-03-01 3:13 ` Mark Brown 2014-03-03 0:52 ` Graeme Gregory 1 sibling, 0 replies; 11+ 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] 11+ messages in thread
* Re: [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level [not found] ` <5310BA99.4050203-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 2014-03-01 3:13 ` Mark Brown @ 2014-03-03 0:52 ` Graeme Gregory 2014-03-03 16:41 ` Stephen Warren 1 sibling, 1 reply; 11+ 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] 11+ 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 [not found] ` <5314B0C0.4040008-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 0 siblings, 1 reply; 11+ 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] 11+ messages in thread
[parent not found: <5314B0C0.4040008-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]
* Re: [PATCH V2 1/3] dt: palmas: support IRQ inversion at the board level [not found] ` <5314B0C0.4040008-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> @ 2014-03-04 3:50 ` Mark Brown 0 siblings, 0 replies; 11+ 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] 11+ messages in thread
end of thread, other threads:[~2014-03-04 3:50 UTC | newest] Thread overview: 11+ 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 [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 ` [PATCH V2 3/3] ARM: tegra: fix Dalmore PMIC IRQ polarity 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:35 ` Stephen Warren [not found] ` <530FAFAE.5050800-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 2014-02-28 5:58 ` Mark Brown [not found] ` <20140228055818.GA29849-GFdadSzt00ze9xe1eoZjHA@public.gmane.org> 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-03 0:52 ` Graeme Gregory 2014-03-03 16:41 ` Stephen Warren [not found] ` <5314B0C0.4040008-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 2014-03-04 3:50 ` Mark Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).