* [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt @ 2015-11-30 22:47 Christophe Ricard 2015-11-30 22:47 ` [PATCH v3 1/2] acpi: Rename acpi_gsi_get_irq_type to acpi_get_irq_type and export symbol Christophe Ricard ` (3 more replies) 0 siblings, 4 replies; 11+ messages in thread From: Christophe Ricard @ 2015-11-30 22:47 UTC (permalink / raw) To: rjw, lenb, linus.walleij, gnurou, andriy.shevchenko, mika.westerberg Cc: linux-i2c, linux-gpio, linux-acpi, Christophe Ricard Hi, This serie only move from RFC to final. I am also adding gpio folks for potential feedback. ACPI probing method does not retrieve irq_type from a gpio interrupt declared with GpioInt as it is done with devicetree probing. In other terms, irq_get_trigger_type will always send back 0. Those 2 patches propose a way to retrieve the correct interrupt polarity/type from a GpioInt acpi declaration when using irq_get_trigger_type. Best Regards Christophe Christophe Ricard (2): acpi: Rename acpi_gsi_get_irq_type to acpi_get_irq_type and export symbol ACPI / gpio: Add irq_type when a gpio is used as an interrupt drivers/acpi/gsi.c | 21 +-------------------- drivers/acpi/utils.c | 21 +++++++++++++++++++++ drivers/gpio/gpiolib-acpi.c | 5 ++++- include/linux/acpi.h | 2 ++ 4 files changed, 28 insertions(+), 21 deletions(-) -- 2.1.4 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/2] acpi: Rename acpi_gsi_get_irq_type to acpi_get_irq_type and export symbol 2015-11-30 22:47 [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt Christophe Ricard @ 2015-11-30 22:47 ` Christophe Ricard 2015-11-30 22:47 ` [PATCH v3 2/2] ACPI / gpio: Add irq_type when a gpio is used as an interrupt Christophe Ricard ` (2 subsequent siblings) 3 siblings, 0 replies; 11+ messages in thread From: Christophe Ricard @ 2015-11-30 22:47 UTC (permalink / raw) To: rjw, lenb, linus.walleij, gnurou, andriy.shevchenko, mika.westerberg Cc: linux-i2c, linux-gpio, linux-acpi, Christophe Ricard acpi_gsi_get_irq_type could be use out of gsi purpose. Rename and make it available as a utility function. Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com> --- drivers/acpi/gsi.c | 21 +-------------------- drivers/acpi/utils.c | 21 +++++++++++++++++++++ include/linux/acpi.h | 2 ++ 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c index fa4585a..2828351 100644 --- a/drivers/acpi/gsi.c +++ b/drivers/acpi/gsi.c @@ -17,25 +17,6 @@ enum acpi_irq_model_id acpi_irq_model; static struct fwnode_handle *acpi_gsi_domain_id; -static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity) -{ - switch (polarity) { - case ACPI_ACTIVE_LOW: - return trigger == ACPI_EDGE_SENSITIVE ? - IRQ_TYPE_EDGE_FALLING : - IRQ_TYPE_LEVEL_LOW; - case ACPI_ACTIVE_HIGH: - return trigger == ACPI_EDGE_SENSITIVE ? - IRQ_TYPE_EDGE_RISING : - IRQ_TYPE_LEVEL_HIGH; - case ACPI_ACTIVE_BOTH: - if (trigger == ACPI_EDGE_SENSITIVE) - return IRQ_TYPE_EDGE_BOTH; - default: - return IRQ_TYPE_NONE; - } -} - /** * acpi_gsi_to_irq() - Retrieve the linux irq number for a given GSI * @gsi: GSI IRQ number to map @@ -82,7 +63,7 @@ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, fwspec.fwnode = acpi_gsi_domain_id; fwspec.param[0] = gsi; - fwspec.param[1] = acpi_gsi_get_irq_type(trigger, polarity); + fwspec.param[1] = acpi_get_irq_type(trigger, polarity); fwspec.param_count = 2; return irq_create_fwspec_mapping(&fwspec); diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c index 475c907..715b24b 100644 --- a/drivers/acpi/utils.c +++ b/drivers/acpi/utils.c @@ -33,6 +33,27 @@ #define _COMPONENT ACPI_BUS_COMPONENT ACPI_MODULE_NAME("utils"); +unsigned int acpi_get_irq_type(int trigger, int polarity) +{ + switch (polarity) { + case ACPI_ACTIVE_LOW: + return trigger == ACPI_EDGE_SENSITIVE ? + IRQ_TYPE_EDGE_FALLING : + IRQ_TYPE_LEVEL_LOW; + case ACPI_ACTIVE_HIGH: + return trigger == ACPI_EDGE_SENSITIVE ? + IRQ_TYPE_EDGE_RISING : + IRQ_TYPE_LEVEL_HIGH; + case ACPI_ACTIVE_BOTH: + if (trigger == ACPI_EDGE_SENSITIVE) + return IRQ_TYPE_EDGE_BOTH; + default: + return IRQ_TYPE_NONE; + } +} +EXPORT_SYMBOL_GPL(acpi_get_irq_type); + + /* -------------------------------------------------------------------------- Object Evaluation Helpers -------------------------------------------------------------------------- */ diff --git a/include/linux/acpi.h b/include/linux/acpi.h index d863e12..b0c5a11 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -204,6 +204,8 @@ int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 *gsi); void acpi_set_irq_model(enum acpi_irq_model_id model, struct fwnode_handle *fwnode); +unsigned int acpi_get_irq_type(int trigger, int polarity); + #ifdef CONFIG_X86_IO_APIC extern int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity); #else -- 2.1.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 2/2] ACPI / gpio: Add irq_type when a gpio is used as an interrupt 2015-11-30 22:47 [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt Christophe Ricard 2015-11-30 22:47 ` [PATCH v3 1/2] acpi: Rename acpi_gsi_get_irq_type to acpi_get_irq_type and export symbol Christophe Ricard @ 2015-11-30 22:47 ` Christophe Ricard 2015-12-10 17:00 ` Linus Walleij 2015-12-01 1:03 ` [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt Wolfram Sang 2015-12-01 11:21 ` Mika Westerberg 3 siblings, 1 reply; 11+ messages in thread From: Christophe Ricard @ 2015-11-30 22:47 UTC (permalink / raw) To: rjw, lenb, linus.walleij, gnurou, andriy.shevchenko, mika.westerberg Cc: linux-i2c, linux-gpio, linux-acpi, Christophe Ricard When a gpio is used as an interrupt, the irq_type was not available for device driver. It is not align with devicetree probing. Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com> --- drivers/gpio/gpiolib-acpi.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c index bbcac3a..cff8736 100644 --- a/drivers/gpio/gpiolib-acpi.c +++ b/drivers/gpio/gpiolib-acpi.c @@ -416,9 +416,12 @@ static int acpi_find_gpio(struct acpi_resource *ares, void *data) * GpioIo is used then the only way to set the flag is * to use _DSD "gpios" property. */ - if (lookup->info.gpioint) + if (lookup->info.gpioint) { lookup->info.active_low = agpio->polarity == ACPI_ACTIVE_LOW; + irq_set_irq_type(gpiod_to_irq(lookup->desc), + acpi_get_irq_type(agpio->triggering, agpio->polarity)); + } } return 1; -- 2.1.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/2] ACPI / gpio: Add irq_type when a gpio is used as an interrupt 2015-11-30 22:47 ` [PATCH v3 2/2] ACPI / gpio: Add irq_type when a gpio is used as an interrupt Christophe Ricard @ 2015-12-10 17:00 ` Linus Walleij 0 siblings, 0 replies; 11+ messages in thread From: Linus Walleij @ 2015-12-10 17:00 UTC (permalink / raw) To: Christophe Ricard Cc: Rafael J. Wysocki, Len Brown, Alexandre Courbot, Andy Shevchenko, Mika Westerberg, linux-i2c@vger.kernel.org, linux-gpio@vger.kernel.org, ACPI Devel Maling List, Christophe Ricard On Mon, Nov 30, 2015 at 11:47 PM, Christophe Ricard <christophe.ricard@gmail.com> wrote: > When a gpio is used as an interrupt, the irq_type was not available for > device driver. It is not align with devicetree probing. > > Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com> Acked-by: Linus Walleij <linus.walleij@linaro.org> Rafael you can merge this into the ACPI tree. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt 2015-11-30 22:47 [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt Christophe Ricard 2015-11-30 22:47 ` [PATCH v3 1/2] acpi: Rename acpi_gsi_get_irq_type to acpi_get_irq_type and export symbol Christophe Ricard 2015-11-30 22:47 ` [PATCH v3 2/2] ACPI / gpio: Add irq_type when a gpio is used as an interrupt Christophe Ricard @ 2015-12-01 1:03 ` Wolfram Sang 2015-12-01 7:03 ` Christophe Ricard 2015-12-01 11:21 ` Mika Westerberg 3 siblings, 1 reply; 11+ messages in thread From: Wolfram Sang @ 2015-12-01 1:03 UTC (permalink / raw) To: Christophe Ricard Cc: rjw, lenb, linus.walleij, gnurou, andriy.shevchenko, mika.westerberg, linux-i2c, linux-gpio, linux-acpi, Christophe Ricard [-- Attachment #1: Type: text/plain, Size: 129 bytes --] > This serie only move from RFC to final. I am also adding gpio folks for potential > feedback. Why is this relevant for I2C? [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt 2015-12-01 1:03 ` [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt Wolfram Sang @ 2015-12-01 7:03 ` Christophe Ricard 0 siblings, 0 replies; 11+ messages in thread From: Christophe Ricard @ 2015-12-01 7:03 UTC (permalink / raw) To: Wolfram Sang Cc: rjw, lenb, linus.walleij, gnurou, andriy.shevchenko, mika.westerberg, linux-i2c, linux-gpio, linux-acpi, Christophe Ricard Hi Wolfram, This is correct that this serie does not impact i2c core at all. Best Regards Christophe On 01/12/2015 02:03, Wolfram Sang wrote: >> This serie only move from RFC to final. I am also adding gpio folks for potential >> feedback. > Why is this relevant for I2C? > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt 2015-11-30 22:47 [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt Christophe Ricard ` (2 preceding siblings ...) 2015-12-01 1:03 ` [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt Wolfram Sang @ 2015-12-01 11:21 ` Mika Westerberg [not found] ` <CALD+uuxJw5tQ2XY99nHigXdNkcXGnng+r9NZ8H9TqUF9GbgW=A@mail.gmail.com> 3 siblings, 1 reply; 11+ messages in thread From: Mika Westerberg @ 2015-12-01 11:21 UTC (permalink / raw) To: Christophe Ricard Cc: rjw, lenb, linus.walleij, gnurou, andriy.shevchenko, linux-i2c, linux-gpio, linux-acpi, Christophe Ricard On Mon, Nov 30, 2015 at 11:47:51PM +0100, Christophe Ricard wrote: > ACPI probing method does not retrieve irq_type from a gpio interrupt declared > with GpioInt as it is done with devicetree probing. In other terms, irq_get_trigger_type > will always send back 0. Is this real problem you are solving here? Also where does the device tree version set triggering flags for a GPIO irq? ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <CALD+uuxJw5tQ2XY99nHigXdNkcXGnng+r9NZ8H9TqUF9GbgW=A@mail.gmail.com>]
* Re: [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt [not found] ` <CALD+uuxJw5tQ2XY99nHigXdNkcXGnng+r9NZ8H9TqUF9GbgW=A@mail.gmail.com> @ 2015-12-01 13:03 ` Mika Westerberg 2015-12-04 23:36 ` Christophe Ricard 0 siblings, 1 reply; 11+ messages in thread From: Mika Westerberg @ 2015-12-01 13:03 UTC (permalink / raw) To: Christophe Ricard Cc: rjw, Len Brown, Linus Walleij, Alexandre Courbot, andriy.shevchenko, linux-i2c, linux-gpio, linux-acpi, Christophe Ricard On Tue, Dec 01, 2015 at 01:25:50PM +0100, Christophe Ricard wrote: > For example during an i2c_device_probe where an i2c slave device > describe in devicetree has an interrupts property. > i2c_device_probe (drivers/i2c/i2c-core.c), retrieves irq property from > of_irq_get which will looks for an "interrupts" property in > of_irq_parse_of (drivers/of/irq.c). > of_irq_get will then call irq_create_mapping (kernel/irq/irqdomain.c) > which will set the irq_type retrieved during the interrupts node > parsing. Found it now thanks. > This will allow from an i2c slave drivers to configure an interrupt > handler matching the exact devicetree data for the interrupts property > of the i2c slave node. Makes sense. > Now for the same kind of i2c driver using acpi description, the GpioInt > polarity/type is at the moment never kept in the irq property. > It is possible to check that following about the same path... > i2c_device_probe (drivers/i2c/i2c-core.c), retrieves irq property from > acpi_dev_gpio_irq_get but does not save the irq_type. > This would allow not to have to use an additional gpio field and all > the configuration step to configure the gpio interrupt correctly in a > device driver and taking a real benefit of the GpioInt acpi keyword > compare to GpioIo keyword. > Most the of the drivers based on acpi description retrieve gpio number > to assign an interrupt and a fix polarity. I believe my patchset > proposal would improve this and allow to > be much closer with devicetree. > Do you see any issue with this ? No, but I wonder if it would be better to do this in acpi_dev_gpio_irq_get() instead of acpi_find_gpio() which gets called everytime a GPIO is looked up? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt 2015-12-01 13:03 ` Mika Westerberg @ 2015-12-04 23:36 ` Christophe Ricard 2015-12-07 10:52 ` Mika Westerberg 0 siblings, 1 reply; 11+ messages in thread From: Christophe Ricard @ 2015-12-04 23:36 UTC (permalink / raw) To: Mika Westerberg Cc: rjw, Len Brown, Linus Walleij, Alexandre Courbot, andriy.shevchenko, linux-i2c, linux-gpio, linux-acpi, Christophe Ricard Hi Mika, On 01/12/2015 14:03, Mika Westerberg wrote: [...] >> Now for the same kind of i2c driver using acpi description, the GpioInt >> polarity/type is at the moment never kept in the irq property. >> It is possible to check that following about the same path... >> i2c_device_probe (drivers/i2c/i2c-core.c), retrieves irq property from >> acpi_dev_gpio_irq_get but does not save the irq_type. >> This would allow not to have to use an additional gpio field and all >> the configuration step to configure the gpio interrupt correctly in a >> device driver and taking a real benefit of the GpioInt acpi keyword >> compare to GpioIo keyword. >> Most the of the drivers based on acpi description retrieve gpio number >> to assign an interrupt and a fix polarity. I believe my patchset >> proposal would improve this and allow to >> be much closer with devicetree. >> Do you see any issue with this ? > No, but I wonder if it would be better to do this in acpi_dev_gpio_irq_get() > instead of acpi_find_gpio() which gets called everytime a GPIO is looked up? I believe, setting the irq type requires triggering and polarity data stored into a struct acpi_resource_gpio. acpi_dev_gpio_irq_get call acpi_get_gpiod_by_index which run acpi_find_gpio. Actually acpi_dev_gpio_irq_get is called everytime an i2c device slave is probed, acpi_find_gpio will get called to "register" gpios. If done correctly, i think this will be done only once... The only improvement i may see would be to add an extra field in the acpi_gpio_info structure in drivers/gpio/gpiolib.h (for example int irq_type). And call irq_set_irq_type in acpi_dev_gpio_irq_get if the gpio is an interrupt. I guess the current proposal and this one are equivalent... What do you think ? Best Regards Christophe ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt 2015-12-04 23:36 ` Christophe Ricard @ 2015-12-07 10:52 ` Mika Westerberg 2015-12-07 10:53 ` Christophe Henri RICARD 0 siblings, 1 reply; 11+ messages in thread From: Mika Westerberg @ 2015-12-07 10:52 UTC (permalink / raw) To: Christophe Ricard Cc: rjw, Len Brown, Linus Walleij, Alexandre Courbot, andriy.shevchenko, linux-i2c, linux-gpio, linux-acpi, Christophe Ricard On Sat, Dec 05, 2015 at 12:36:19AM +0100, Christophe Ricard wrote: > >No, but I wonder if it would be better to do this in acpi_dev_gpio_irq_get() > >instead of acpi_find_gpio() which gets called everytime a GPIO is looked up? > I believe, setting the irq type requires triggering and polarity data stored > into a struct acpi_resource_gpio. > > acpi_dev_gpio_irq_get call acpi_get_gpiod_by_index which run acpi_find_gpio. > > Actually acpi_dev_gpio_irq_get is called everytime an i2c device slave is > probed, acpi_find_gpio will get called to "register" gpios. > If done correctly, i think this will be done only once... > > The only improvement i may see would be to add an extra field in the > acpi_gpio_info structure in drivers/gpio/gpiolib.h (for example int > irq_type). > And call irq_set_irq_type in acpi_dev_gpio_irq_get if the gpio is an > interrupt. > > I guess the current proposal and this one are equivalent... > > What do you think ? Actually I think it may be useful to add triggering flags to acpi_gpio_info. ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt 2015-12-07 10:52 ` Mika Westerberg @ 2015-12-07 10:53 ` Christophe Henri RICARD 0 siblings, 0 replies; 11+ messages in thread From: Christophe Henri RICARD @ 2015-12-07 10:53 UTC (permalink / raw) To: Mika Westerberg, Christophe Ricard Cc: rjw@rjwysocki.net, Len Brown, Linus Walleij, Alexandre Courbot, andriy.shevchenko@linux.intel.com, linux-i2c@vger.kernel.org, linux-gpio@vger.kernel.org, linux-acpi@vger.kernel.org Hi Mika, Ok, i will send a RFC serie tonight. I have also noticed the spi core set the irq field to -1 in acpi. This will be fixed in this serie. Best Regards Christophe -----Original Message----- From: Mika Westerberg [mailto:mika.westerberg@linux.intel.com] Sent: lundi 7 décembre 2015 11:52 To: Christophe Ricard Cc: rjw@rjwysocki.net; Len Brown; Linus Walleij; Alexandre Courbot; andriy.shevchenko@linux.intel.com; linux-i2c@vger.kernel.org; linux-gpio@vger.kernel.org; linux-acpi@vger.kernel.org; Christophe Henri RICARD Subject: Re: [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt On Sat, Dec 05, 2015 at 12:36:19AM +0100, Christophe Ricard wrote: > >No, but I wonder if it would be better to do this in > >acpi_dev_gpio_irq_get() instead of acpi_find_gpio() which gets called everytime a GPIO is looked up? > I believe, setting the irq type requires triggering and polarity data > stored into a struct acpi_resource_gpio. > > acpi_dev_gpio_irq_get call acpi_get_gpiod_by_index which run acpi_find_gpio. > > Actually acpi_dev_gpio_irq_get is called everytime an i2c device slave > is probed, acpi_find_gpio will get called to "register" gpios. > If done correctly, i think this will be done only once... > > The only improvement i may see would be to add an extra field in the > acpi_gpio_info structure in drivers/gpio/gpiolib.h (for example int > irq_type). > And call irq_set_irq_type in acpi_dev_gpio_irq_get if the gpio is an > interrupt. > > I guess the current proposal and this one are equivalent... > > What do you think ? Actually I think it may be useful to add triggering flags to acpi_gpio_info. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-12-10 17:00 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-11-30 22:47 [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt Christophe Ricard 2015-11-30 22:47 ` [PATCH v3 1/2] acpi: Rename acpi_gsi_get_irq_type to acpi_get_irq_type and export symbol Christophe Ricard 2015-11-30 22:47 ` [PATCH v3 2/2] ACPI / gpio: Add irq_type when a gpio is used as an interrupt Christophe Ricard 2015-12-10 17:00 ` Linus Walleij 2015-12-01 1:03 ` [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt Wolfram Sang 2015-12-01 7:03 ` Christophe Ricard 2015-12-01 11:21 ` Mika Westerberg [not found] ` <CALD+uuxJw5tQ2XY99nHigXdNkcXGnng+r9NZ8H9TqUF9GbgW=A@mail.gmail.com> 2015-12-01 13:03 ` Mika Westerberg 2015-12-04 23:36 ` Christophe Ricard 2015-12-07 10:52 ` Mika Westerberg 2015-12-07 10:53 ` Christophe Henri RICARD
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).