gpiolib: Avoid modifying GPIO chip fwnode
v2 cover gpiolib: Avoid modifying GPIO chip fwnode
Ideally the GPIO chip data structure has to be constant. In real life it's not true, but we can make it closer to that. Hence the series.
Bart, the idea is that this series has to land immediately after v6.5-rc1 is out so we will have longer time to fix downsides and regressions found, if any.
Benjamin, thank you for testing!
Changelog v2:
- replaced open coded device_match_acpi_handle()
- corrected commit message due to above change
- added tag (Benjamin)
Andy Shevchenko (3):
gpiolib: of: Don't use GPIO chip fwnode in of_gpiochip_*()
gpiolib: acpi: Don't use GPIO chip fwnode in acpi_gpiochip_find()
gpiolib: Do not alter GPIO chip fwnode member
drivers/gpio/gpiolib-acpi.c | 2 +- drivers/gpio/gpiolib-of.c | 6 +++--- drivers/gpio/gpiolib.c | 16 ++++++++-------- 3 files changed, 12 insertions(+), 12 deletions(-)
Signature · 1 line
2.40.0.1.gaa8946217a0b
PATCH v2 1/3 gpiolib: of: Don't use GPIO chip fwnode in of_gpiochip_*()
GPIO library should rely only on the GPIO device's fwnode. Hence, replace GPIO chip fwnode usage by respective OF node of the GPIO device.
JFYI, this is partial revert of 8afe82550240 ("gpiolib: of:
Prepare of_gpiochip_add() / of_gpiochip_remove() for fwnode").
--- drivers/gpio/gpiolib-of.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/gpio/gpiolib-of.c +3 -3
@@ -1078,16 +1078,16 @@ int of_gpiochip_add(struct gpio_chip *chip)if (ret)return ret; -fwnode_handle_get (chip->fwnode); +of_node_get (np); ret =of_gpiochip_scan_gpios (chip);if (ret) -fwnode_handle_put (chip->fwnode); +of_node_put (np);return ret; }void of_gpiochip_remove (struct gpio_chip *chip) { -fwnode_handle_put (chip->fwnode); +of_node_put (dev_of_node (&chip->gpiodev->dev)); }
Signature · 1 line
2.40.0.1.gaa8946217a0b
PATCH v2 2/3 gpiolib: acpi: Don't use GPIO chip fwnode in acpi_gpiochip_find()
In reply to Andy ShevchenkoGPIO library should rely only on the GPIO device's fwnode. Hence, replace GPIO chip fwnode usage by respective handle of the GPIO device.
--- drivers/gpio/gpiolib-acpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpio/gpiolib-acpi.c +1 -1
@@ -128,7 +128,7 @@ static bool acpi_gpio_deferred_req_irqs_done;static int acpi_gpiochip_find (struct gpio_chip *gc,void *data) { -return ACPI_HANDLE_FWNODE (gc->fwnode) == data; +return device_match_acpi_handle (&gc->gpiodev->dev, data); }/**
Signature · 1 line
2.40.0.1.gaa8946217a0b
Re: [PATCH v2 2/3] gpiolib: acpi: Don't use GPIO chip fwnode in acpi_gpiochip_find()
Quoted 16 lines · Andy Shevchenko
GPIO library should rely only on the GPIO device's fwnode. Hence, replace GPIO chip fwnode usage by respective handle of the GPIO device.
Signed-off-by Andy Shevchenko <[email protected]>Tested-by Benjamin Tissoires <[email protected]>--- drivers/gpio/gpiolib-acpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/gpio/gpiolib-acpi.c +0
-0
drivers/gpio/gpiolib-acpi.c static bool acpi_gpio_deferred_req_irqs_done; · line 131
Quoted 5 lines
drivers/gpio/gpiolib-acpi.c +1
-1@@ -128,7 +128,7 @@ static bool acpi_gpio_deferred_req_irqs_done;static int acpi_gpiochip_find (struct gpio_chip *gc,void *data) { -return ACPI_HANDLE_FWNODE (gc->fwnode) == data; +return device_match_acpi_handle (&gc->gpiodev->dev, data);
Quick confirmation that using "device_match_acpi_handle" also works in v2 (was easy enough to test given that everything was already set up locally).
Cheers,
Benjamin
Quoted 6 lines
drivers/gpio/gpiolib-acpi.c +0
-1}/** -- 2.40.0.1.gaa8946217a0b
Re: [PATCH v2 2/3] gpiolib: acpi: Don't use GPIO chip fwnode in acpi_gpiochip_find()
Quoted 9 lines · Benjamin Tissoires
drivers/gpio/gpiolib-acpi.c static bool acpi_gpio_deferred_req_irqs_done; · line 131
Quoted 2 lines · Andy Shevchenko
drivers/gpio/gpiolib-acpi.c +1
-1-return ACPI_HANDLE_FWNODE (gc->fwnode) == data; +return device_match_acpi_handle (&gc->gpiodev->dev, data);Quick confirmation that using "
device_match_acpi_handle" also works in v2 (was easy enough to test given that everything was already set up locally).
Thank you! I would be very surprised if not :-)
Signature · 2 lines
With Best Regards, Andy Shevchenko
PATCH v2 3/3 gpiolib: Do not alter GPIO chip fwnode member
In reply to Andy ShevchenkoIdeally we should not touch data in the given GPIO chip structure. Let's become closer to it by avoiding altering fwnode member.
The GPIO library must use fwnode from GPIO device and the drivers might use one from GPIO chip in case they initialized it.
--- drivers/gpio/gpiolib.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/gpio/gpiolib.c +8 -8
@@ -708,13 +708,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,int base =0 ;int ret =0 ; -/* -* If the calling driver did not initialize firmware node, do it here -* using the parent device, if any. -*/ -if (!gc->fwnode && gc->parent) - gc->fwnode =dev_fwnode (gc->parent); -/* * First: allocate and populate the internal stat container, and * set up the struct device.
@@ -729,7 +722,14 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, gc->gpiodev = gdev;gpiochip_set_data (gc, data); -device_set_node (&gdev->dev, gc->fwnode); +/* +* If the calling driver did not initialize firmware node, +* do it here using the parent device, if any. +*/ +if (gc->fwnode) +device_set_node (&gdev->dev, gc->fwnode); +else if (gc->parent) +device_set_node (&gdev->dev,dev_fwnode (gc->parent)); gdev->id =ida_alloc (&gpio_ida,GFP_KERNEL );if (gdev->id <0 ) {
Signature · 1 line
2.40.0.1.gaa8946217a0b
Quoted 3 lines · Andy Shevchenko
Ideally the GPIO chip data structure has to be constant. In real life it's not true, but we can make it closer to that. Hence the series.
The series looks completely reasonable to me:
Yours,
Linus Walleij
Bartosz Golaszewski
AppliedQuoted 21 lines · Andy Shevchenko
Ideally the GPIO chip data structure has to be constant. In real life it's not true, but we can make it closer to that. Hence the series.
Bart, the idea is that this series has to land immediately after v6.5-rc1 is out so we will have longer time to fix downsides and regressions found, if any.
Benjamin, thank you for testing!
Changelog v2:
- replaced open codeddevice_match_acpi_handle()
- corrected commit message due to above change
- added tag (Benjamin)Andy Shevchenko (3):
gpiolib: of: Don't use GPIO chip fwnode in of_gpiochip_*()
gpiolib: acpi: Don't use GPIO chip fwnode inacpi_gpiochip_find()
gpiolib: Do not alter GPIO chip fwnode member
Quoted 8 lines
drivers/gpio/gpiolib-acpi.c | 2 +- drivers/gpio/gpiolib-of.c | 6 +++--- drivers/gpio/gpiolib.c | 16 ++++++++-------- 3 files changed, 12 insertions(+), 12 deletions(-)--
2.40.0.1.gaa8946217a0b
Applied, thanks!
Bart