GPIO library uses of_node and fwnode in the core in non-unified way.
The series cleans this up and improves IRQ domain creation for non-OF cases
where currently the names of the domain are 'unknown'.
This has been tested on Intel Galileo Gen 2.
In v3:
- fix subtle bug in gpiod_count
- make irq_domain_add_simple() static inline (Marc)
In v2:
- added a new patch due to functionality in irq_comain_add_simple() (Linus)
- tagged patches 2-4 (Linus)
- Cc'ed to Rafael
Andy Shevchenko (5):
irqdomain: Introduce irq_domain_create_simple() API
gpiolib: Unify the checks on fwnode type
gpiolib: Move of_node operations to gpiolib-of and correct fwnode use
gpiolib: Introduce acpi_gpio_dev_init() and call it from core
gpiolib: Reuse device's fwnode to create IRQ domain
Documentation/core-api/irq/irq-domain.rst | 22 ++++----
drivers/gpio/gpiolib-acpi.c | 7 +++
drivers/gpio/gpiolib-acpi.h | 4 ++
drivers/gpio/gpiolib-of.c | 6 ++-
drivers/gpio/gpiolib.c | 66 +++++++++--------------
include/linux/irqdomain.h | 19 +++++--
kernel/irq/irqdomain.c | 20 +++----
7 files changed, 77 insertions(+), 67 deletions(-)
--
2.30.1
When IRQ domain is created for an ACPI case, the name of it becomes unknown-%d
since for now it utilizes of_node member only and doesn't consider fwnode case.
Convert IRQ domain creation code to utilize fwnode instead.
Before/After the change on Intel Galileo Gen 2 with two GPIO (IRQ) controllers:
unknown-1 ==> \_SB.PCI0.GIP0.GPO
unknown-2 ==> \_SB.NIO3
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/gpio/gpiolib.c | 28 ++++++++--------------------
1 file changed, 8 insertions(+), 20 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 46ab1ce67ba0..d7897a77c3fd 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1457,9 +1457,9 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
struct lock_class_key *lock_key,
struct lock_class_key *request_key)
{
+ struct fwnode_handle *fwnode = dev_fwnode(&gc->gpiodev->dev);
struct irq_chip *irqchip = gc->irq.chip;
- const struct irq_domain_ops *ops = NULL;
- struct device_node *np;
+ const struct irq_domain_ops *ops;
unsigned int type;
unsigned int i;
@@ -1471,7 +1471,6 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
return -EINVAL;
}
- np = gc->gpiodev->dev.of_node;
type = gc->irq.default_type;
/*
@@ -1479,16 +1478,10 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
* used to configure the interrupts, as you may end up with
* conflicting triggers. Tell the user, and reset to NONE.
*/
- if (WARN(np && type != IRQ_TYPE_NONE,
- "%s: Ignoring %u default trigger\n", np->full_name, type))
+ if (WARN(fwnode && type != IRQ_TYPE_NONE,
+ "%pfw: Ignoring %u default trigger\n", fwnode, type))
type = IRQ_TYPE_NONE;
- if (has_acpi_companion(gc->parent) && type != IRQ_TYPE_NONE) {
- acpi_handle_warn(ACPI_HANDLE(gc->parent),
- "Ignoring %u default trigger\n", type);
- type = IRQ_TYPE_NONE;
- }
-
if (gc->to_irq)
chip_warn(gc, "to_irq is redefined in %s and you shouldn't rely on it\n", __func__);
@@ -1504,15 +1497,10 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
return ret;
} else {
/* Some drivers provide custom irqdomain ops */
- if (gc->irq.domain_ops)
- ops = gc->irq.domain_ops;
-
- if (!ops)
- ops = &gpiochip_domain_ops;
- gc->irq.domain = irq_domain_add_simple(np,
- gc->ngpio,
- gc->irq.first,
- ops, gc);
+ ops = gc->irq.domain_ops ?: &gpiochip_domain_ops;
+ gc->irq.domain = irq_domain_create_simple(fwnode, gc->ngpio,
+ gc->irq.first,
+ ops, gc);
if (!gc->irq.domain)
return -EINVAL;
}
--
2.30.1
In the ACPI case we may use the firmware node in the similar way
as it's done for OF case. We may use that fwnode for other purposes
in the future.
Signed-off-by: Andy Shevchenko <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
---
drivers/gpio/gpiolib-acpi.c | 7 +++++++
drivers/gpio/gpiolib-acpi.h | 4 ++++
drivers/gpio/gpiolib.c | 1 +
3 files changed, 12 insertions(+)
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 1aacd2a5a1fd..21750be9c489 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -1291,6 +1291,13 @@ void acpi_gpiochip_remove(struct gpio_chip *chip)
kfree(acpi_gpio);
}
+void acpi_gpio_dev_init(struct gpio_chip *gc, struct gpio_device *gdev)
+{
+ /* Set default fwnode to parent's one if present */
+ if (gc->parent)
+ ACPI_COMPANION_SET(&gdev->dev, ACPI_COMPANION(gc->parent));
+}
+
static int acpi_gpio_package_count(const union acpi_object *obj)
{
const union acpi_object *element = obj->package.elements;
diff --git a/drivers/gpio/gpiolib-acpi.h b/drivers/gpio/gpiolib-acpi.h
index e2edb632b2cc..e476558d9471 100644
--- a/drivers/gpio/gpiolib-acpi.h
+++ b/drivers/gpio/gpiolib-acpi.h
@@ -36,6 +36,8 @@ struct acpi_gpio_info {
void acpi_gpiochip_add(struct gpio_chip *chip);
void acpi_gpiochip_remove(struct gpio_chip *chip);
+void acpi_gpio_dev_init(struct gpio_chip *gc, struct gpio_device *gdev);
+
void acpi_gpiochip_request_interrupts(struct gpio_chip *chip);
void acpi_gpiochip_free_interrupts(struct gpio_chip *chip);
@@ -58,6 +60,8 @@ int acpi_gpio_count(struct device *dev, const char *con_id);
static inline void acpi_gpiochip_add(struct gpio_chip *chip) { }
static inline void acpi_gpiochip_remove(struct gpio_chip *chip) { }
+static inline void acpi_gpio_dev_init(struct gpio_chip *gc, struct gpio_device *gdev) { }
+
static inline void
acpi_gpiochip_request_interrupts(struct gpio_chip *chip) { }
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 2a3d562eb8c1..46ab1ce67ba0 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -590,6 +590,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
gc->gpiodev = gdev;
of_gpio_dev_init(gc, gdev);
+ acpi_gpio_dev_init(gc, gdev);
gdev->id = ida_alloc(&gpio_ida, GFP_KERNEL);
if (gdev->id < 0) {
--
2.30.1
The initial value of the OF node based on presence of parent, but
at the same time this operation somehow appeared separately from others
that handle the OF case. On the other hand there is no need to assign
dev->fwnode in the OF case if code properly retrieves fwnode, i.e.
via dev_fwnode() helper.
Amend gpiolib.c and gpiolib-of.c code in order to group OF operations.
Signed-off-by: Andy Shevchenko <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
---
drivers/gpio/gpiolib-of.c | 6 ++++--
drivers/gpio/gpiolib.c | 9 ++++-----
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index baf0153b7bca..bbcc7c073f63 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -1042,11 +1042,13 @@ void of_gpiochip_remove(struct gpio_chip *chip)
void of_gpio_dev_init(struct gpio_chip *gc, struct gpio_device *gdev)
{
+ /* Set default OF node to parent's one if present */
+ if (gc->parent)
+ gdev->dev.of_node = gc->parent->of_node;
+
/* If the gpiochip has an assigned OF node this takes precedence */
if (gc->of_node)
gdev->dev.of_node = gc->of_node;
else
gc->of_node = gdev->dev.of_node;
- if (gdev->dev.of_node)
- gdev->dev.fwnode = of_fwnode_handle(gdev->dev.of_node);
}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 083d7e1c7cde..2a3d562eb8c1 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -585,12 +585,9 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
if (!gdev)
return -ENOMEM;
gdev->dev.bus = &gpio_bus_type;
+ gdev->dev.parent = gc->parent;
gdev->chip = gc;
gc->gpiodev = gdev;
- if (gc->parent) {
- gdev->dev.parent = gc->parent;
- gdev->dev.of_node = gc->parent->of_node;
- }
of_gpio_dev_init(gc, gdev);
@@ -4212,11 +4209,13 @@ EXPORT_SYMBOL_GPL(gpiod_put_array);
static int gpio_bus_match(struct device *dev, struct device_driver *drv)
{
+ struct fwnode_handle *fwnode = dev_fwnode(dev);
+
/*
* Only match if the fwnode doesn't already have a proper struct device
* created for it.
*/
- if (dev->fwnode && dev->fwnode->dev != dev)
+ if (fwnode && fwnode->dev != dev)
return 0;
return 1;
}
--
2.30.1
On Thu, Mar 4, 2021 at 9:13 PM Andy Shevchenko
<[email protected]> wrote:
>
> GPIO library uses of_node and fwnode in the core in non-unified way.
> The series cleans this up and improves IRQ domain creation for non-OF cases
> where currently the names of the domain are 'unknown'.
>
> This has been tested on Intel Galileo Gen 2.
>
> In v3:
> - fix subtle bug in gpiod_count
> - make irq_domain_add_simple() static inline (Marc)
>
> In v2:
> - added a new patch due to functionality in irq_comain_add_simple() (Linus)
> - tagged patches 2-4 (Linus)
> - Cc'ed to Rafael
>
> Andy Shevchenko (5):
> irqdomain: Introduce irq_domain_create_simple() API
> gpiolib: Unify the checks on fwnode type
> gpiolib: Move of_node operations to gpiolib-of and correct fwnode use
> gpiolib: Introduce acpi_gpio_dev_init() and call it from core
> gpiolib: Reuse device's fwnode to create IRQ domain
[1-4/5] applied as 5.13 material and I have a minor comment regarding
the last patch (will send separately).
Thanks!
On Mon, Mar 8, 2021 at 7:22 PM Rafael J. Wysocki <[email protected]> wrote:
>
> On Thu, Mar 4, 2021 at 9:13 PM Andy Shevchenko
> <[email protected]> wrote:
> >
> > GPIO library uses of_node and fwnode in the core in non-unified way.
> > The series cleans this up and improves IRQ domain creation for non-OF cases
> > where currently the names of the domain are 'unknown'.
> >
> > This has been tested on Intel Galileo Gen 2.
> >
> > In v3:
> > - fix subtle bug in gpiod_count
> > - make irq_domain_add_simple() static inline (Marc)
> >
> > In v2:
> > - added a new patch due to functionality in irq_comain_add_simple() (Linus)
> > - tagged patches 2-4 (Linus)
> > - Cc'ed to Rafael
> >
> > Andy Shevchenko (5):
> > irqdomain: Introduce irq_domain_create_simple() API
> > gpiolib: Unify the checks on fwnode type
> > gpiolib: Move of_node operations to gpiolib-of and correct fwnode use
> > gpiolib: Introduce acpi_gpio_dev_init() and call it from core
> > gpiolib: Reuse device's fwnode to create IRQ domain
>
> [1-4/5] applied as 5.13 material and I have a minor comment regarding
> the last patch (will send separately).
>
> Thanks!
Hi Rafael!
AFAICT this should go through the GPIO tree as usual. Any reason for
you to pick these patches this time?
Bartosz
On Mon, Mar 8, 2021 at 8:23 PM Bartosz Golaszewski
<[email protected]> wrote:
>
> On Mon, Mar 8, 2021 at 7:22 PM Rafael J. Wysocki <[email protected]> wrote:
> >
> > On Thu, Mar 4, 2021 at 9:13 PM Andy Shevchenko
> > <[email protected]> wrote:
> > >
> > > GPIO library uses of_node and fwnode in the core in non-unified way.
> > > The series cleans this up and improves IRQ domain creation for non-OF cases
> > > where currently the names of the domain are 'unknown'.
> > >
> > > This has been tested on Intel Galileo Gen 2.
> > >
> > > In v3:
> > > - fix subtle bug in gpiod_count
> > > - make irq_domain_add_simple() static inline (Marc)
> > >
> > > In v2:
> > > - added a new patch due to functionality in irq_comain_add_simple() (Linus)
> > > - tagged patches 2-4 (Linus)
> > > - Cc'ed to Rafael
> > >
> > > Andy Shevchenko (5):
> > > irqdomain: Introduce irq_domain_create_simple() API
> > > gpiolib: Unify the checks on fwnode type
> > > gpiolib: Move of_node operations to gpiolib-of and correct fwnode use
> > > gpiolib: Introduce acpi_gpio_dev_init() and call it from core
> > > gpiolib: Reuse device's fwnode to create IRQ domain
> >
> > [1-4/5] applied as 5.13 material and I have a minor comment regarding
> > the last patch (will send separately).
> >
> > Thanks!
>
> Hi Rafael!
>
> AFAICT this should go through the GPIO tree as usual. Any reason for
> you to pick these patches this time?
My impression was that Andy wanted me to take them.
However, if you'd rather take care of them yourself, there you go!
I'll drop them now and assume that they will be routed through the GPIO tree.
Thanks!
On Mon, Mar 8, 2021 at 8:26 PM Rafael J. Wysocki <[email protected]> wrote:
>
> On Mon, Mar 8, 2021 at 8:23 PM Bartosz Golaszewski
> <[email protected]> wrote:
> >
> > On Mon, Mar 8, 2021 at 7:22 PM Rafael J. Wysocki <[email protected]> wrote:
> > >
> > > On Thu, Mar 4, 2021 at 9:13 PM Andy Shevchenko
> > > <[email protected]> wrote:
> > > >
> > > > GPIO library uses of_node and fwnode in the core in non-unified way.
> > > > The series cleans this up and improves IRQ domain creation for non-OF cases
> > > > where currently the names of the domain are 'unknown'.
> > > >
> > > > This has been tested on Intel Galileo Gen 2.
> > > >
> > > > In v3:
> > > > - fix subtle bug in gpiod_count
> > > > - make irq_domain_add_simple() static inline (Marc)
> > > >
> > > > In v2:
> > > > - added a new patch due to functionality in irq_comain_add_simple() (Linus)
> > > > - tagged patches 2-4 (Linus)
> > > > - Cc'ed to Rafael
> > > >
> > > > Andy Shevchenko (5):
> > > > irqdomain: Introduce irq_domain_create_simple() API
> > > > gpiolib: Unify the checks on fwnode type
> > > > gpiolib: Move of_node operations to gpiolib-of and correct fwnode use
> > > > gpiolib: Introduce acpi_gpio_dev_init() and call it from core
> > > > gpiolib: Reuse device's fwnode to create IRQ domain
> > >
> > > [1-4/5] applied as 5.13 material and I have a minor comment regarding
> > > the last patch (will send separately).
> > >
> > > Thanks!
> >
> > Hi Rafael!
> >
> > AFAICT this should go through the GPIO tree as usual. Any reason for
> > you to pick these patches this time?
>
> My impression was that Andy wanted me to take them.
>
> However, if you'd rather take care of them yourself, there you go!
>
> I'll drop them now and assume that they will be routed through the GPIO tree.
>
> Thanks!
They touch a lot of core GPIO code and are likely to conflict if any
other changes show up this release cycle. I'd rather take them through
the usual channel. Thanks!
Bartosz
On Mon, Mar 08, 2021 at 08:26:39PM +0100, Rafael J. Wysocki wrote:
> On Mon, Mar 8, 2021 at 8:23 PM Bartosz Golaszewski
> <[email protected]> wrote:
> > On Mon, Mar 8, 2021 at 7:22 PM Rafael J. Wysocki <[email protected]> wrote:
> > > On Thu, Mar 4, 2021 at 9:13 PM Andy Shevchenko
> > > <[email protected]> wrote:
> > AFAICT this should go through the GPIO tree as usual. Any reason for
> > you to pick these patches this time?
>
> My impression was that Andy wanted me to take them.
Hmm... I guess the MAINTAINERS pointed to your name due to changes in GPIO ACPI
library, but most of them indeed are GPIO core ones. Perhaps I have to clarify
that in the cover letter.
> However, if you'd rather take care of them yourself, there you go!
>
> I'll drop them now and assume that they will be routed through the GPIO tree.
Anyway, thank you for the review, it is useful!
--
With Best Regards,
Andy Shevchenko
On Mon, Mar 08, 2021 at 08:29:27PM +0100, Bartosz Golaszewski wrote:
> On Mon, Mar 8, 2021 at 8:26 PM Rafael J. Wysocki <[email protected]> wrote:
> > On Mon, Mar 8, 2021 at 8:23 PM Bartosz Golaszewski
> > <[email protected]> wrote:
...
> > My impression was that Andy wanted me to take them.
> >
> > However, if you'd rather take care of them yourself, there you go!
> >
> > I'll drop them now and assume that they will be routed through the GPIO tree.
> >
> > Thanks!
>
> They touch a lot of core GPIO code and are likely to conflict if any
> other changes show up this release cycle. I'd rather take them through
> the usual channel. Thanks!
Since now we have v4 based on Rafael's bleeding-edge, what do you want me to
do? Resend a v5 with all patches included?
--
With Best Regards,
Andy Shevchenko
On Mon, Mar 08, 2021 at 09:36:52PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 08, 2021 at 08:29:27PM +0100, Bartosz Golaszewski wrote:
> > On Mon, Mar 8, 2021 at 8:26 PM Rafael J. Wysocki <[email protected]> wrote:
> > > On Mon, Mar 8, 2021 at 8:23 PM Bartosz Golaszewski
> > > <[email protected]> wrote:
>
> ...
>
> > > My impression was that Andy wanted me to take them.
> > >
> > > However, if you'd rather take care of them yourself, there you go!
> > >
> > > I'll drop them now and assume that they will be routed through the GPIO tree.
> > >
> > > Thanks!
> >
> > They touch a lot of core GPIO code and are likely to conflict if any
> > other changes show up this release cycle. I'd rather take them through
> > the usual channel. Thanks!
>
> Since now we have v4 based on Rafael's bleeding-edge, what do you want me to
> do? Resend a v5 with all patches included?
I have decided to resend as usually it's better for maintainers.
But it appears I was too quick to miss Rafael's review tag / comments.
So, I will send v6 with those included.
--
With Best Regards,
Andy Shevchenko
On Mon, Mar 8, 2021 at 8:52 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Mon, Mar 08, 2021 at 09:36:52PM +0200, Andy Shevchenko wrote:
> > On Mon, Mar 08, 2021 at 08:29:27PM +0100, Bartosz Golaszewski wrote:
> > > On Mon, Mar 8, 2021 at 8:26 PM Rafael J. Wysocki <[email protected]> wrote:
> > > > On Mon, Mar 8, 2021 at 8:23 PM Bartosz Golaszewski
> > > > <[email protected]> wrote:
> >
> > ...
> >
> > > > My impression was that Andy wanted me to take them.
> > > >
> > > > However, if you'd rather take care of them yourself, there you go!
> > > >
> > > > I'll drop them now and assume that they will be routed through the GPIO tree.
> > > >
> > > > Thanks!
> > >
> > > They touch a lot of core GPIO code and are likely to conflict if any
> > > other changes show up this release cycle. I'd rather take them through
> > > the usual channel. Thanks!
> >
> > Since now we have v4 based on Rafael's bleeding-edge, what do you want me to
> > do? Resend a v5 with all patches included?
>
> I have decided to resend as usually it's better for maintainers.
>
> But it appears I was too quick to miss Rafael's review tag / comments.
>
> So, I will send v6 with those included.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
Does this series depend on patches already in Rafael's tree? If so,
maybe Rafael can provide me with an immutable tag to merge in?
Bartosz
On Tue, Mar 09, 2021 at 09:19:19AM +0100, Bartosz Golaszewski wrote:
> On Mon, Mar 8, 2021 at 8:52 PM Andy Shevchenko
> <[email protected]> wrote:
> > On Mon, Mar 08, 2021 at 09:36:52PM +0200, Andy Shevchenko wrote:
...
> > So, I will send v6 with those included.
>
> Does this series depend on patches already in Rafael's tree? If so,
> maybe Rafael can provide me with an immutable tag to merge in?
Not anymore since v5.12-rc2 has a necessary fix.
In any case I have sent a v6. It should be clean to apply on top of your for-next.
--
With Best Regards,
Andy Shevchenko