The match function used in gpio_device_find() should not modify the
contents of passed opaque pointer, because such modification would not
be necessary for actual matching and it could lead to quite unreadable,
spaghetti code.
Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
drivers/gpio/gpiolib-acpi.c | 2 +-
drivers/gpio/gpiolib-of.c | 7 ++++---
drivers/gpio/gpiolib.c | 6 +++---
include/linux/gpio/driver.h | 2 +-
4 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index cd3e9657cc36..899cd505073e 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -126,7 +126,7 @@ static DEFINE_MUTEX(acpi_gpio_deferred_req_irqs_lock);
static LIST_HEAD(acpi_gpio_deferred_req_irqs_list);
static bool acpi_gpio_deferred_req_irqs_done;
-static int acpi_gpiochip_find(struct gpio_chip *gc, void *data)
+static int acpi_gpiochip_find(struct gpio_chip *gc, const void *data)
{
return device_match_acpi_handle(&gc->gpiodev->dev, data);
}
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 77509aa19900..35d717fd393f 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -118,9 +118,10 @@ int of_gpio_get_count(struct device *dev, const char *con_id)
return ret ? ret : -ENOENT;
}
-static int of_gpiochip_match_node_and_xlate(struct gpio_chip *chip, void *data)
+static int of_gpiochip_match_node_and_xlate(struct gpio_chip *chip,
+ const void *data)
{
- struct of_phandle_args *gpiospec = data;
+ const struct of_phandle_args *gpiospec = data;
return device_match_of_node(&chip->gpiodev->dev, gpiospec->np) &&
chip->of_xlate &&
@@ -852,7 +853,7 @@ static void of_gpiochip_remove_hog(struct gpio_chip *chip,
gpiochip_free_own_desc(desc);
}
-static int of_gpiochip_match_node(struct gpio_chip *chip, void *data)
+static int of_gpiochip_match_node(struct gpio_chip *chip, const void *data)
{
return device_match_of_node(&chip->gpiodev->dev, data);
}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 24d046268a01..0e332b24c7b8 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1114,7 +1114,7 @@ EXPORT_SYMBOL_GPL(gpiochip_remove);
*/
struct gpio_device *gpio_device_find(void *data,
int (*match)(struct gpio_chip *gc,
- void *data))
+ const void *data))
{
struct gpio_device *gdev;
@@ -1136,7 +1136,7 @@ struct gpio_device *gpio_device_find(void *data,
}
EXPORT_SYMBOL_GPL(gpio_device_find);
-static int gpio_chip_match_by_label(struct gpio_chip *gc, void *label)
+static int gpio_chip_match_by_label(struct gpio_chip *gc, const void *label)
{
return gc->label && !strcmp(gc->label, label);
}
@@ -1156,7 +1156,7 @@ struct gpio_device *gpio_device_find_by_label(const char *label)
}
EXPORT_SYMBOL_GPL(gpio_device_find_by_label);
-static int gpio_chip_match_by_fwnode(struct gpio_chip *gc, void *fwnode)
+static int gpio_chip_match_by_fwnode(struct gpio_chip *gc, const void *fwnode)
{
return device_match_fwnode(&gc->gpiodev->dev, fwnode);
}
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 3a37d058cfcf..51b23211794d 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -629,7 +629,7 @@ int devm_gpiochip_add_data_with_key(struct device *dev, struct gpio_chip *gc,
struct lock_class_key *request_key);
struct gpio_device *gpio_device_find(void *data,
- int (*match)(struct gpio_chip *gc, void *data));
+ int (*match)(struct gpio_chip *gc, const void *data));
struct gpio_device *gpio_device_find_by_label(const char *label);
struct gpio_device *gpio_device_find_by_fwnode(const struct fwnode_handle *fwnode);
--
2.34.1
On Thu, Feb 08, 2024 at 09:27:04PM +0100, Krzysztof Kozlowski wrote:
> The match function used in gpio_device_find() should not modify the
> contents of passed opaque pointer, because such modification would not
> be necessary for actual matching and it could lead to quite unreadable,
> spaghetti code.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
Reviewed-by: Mika Westerberg <[email protected]>
On Thu, Feb 8, 2024 at 9:27 PM Krzysztof Kozlowski
<[email protected]> wrote:
> The match function used in gpio_device_find() should not modify the
> contents of passed opaque pointer, because such modification would not
> be necessary for actual matching and it could lead to quite unreadable,
> spaghetti code.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
It's what we call a pure function, good const-correctness fix.
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Thu, Feb 08, 2024 at 09:27:04PM +0100, Krzysztof Kozlowski wrote:
> The match function used in gpio_device_find() should not modify the
> contents of passed opaque pointer, because such modification would not
> be necessary for actual matching and it could lead to quite unreadable,
> spaghetti code.
IIRC I also asked Bart about this in his series where something similar
was used.
Good it's not only me who sees the benefits from this.
FWIW,
Reviewed-by: Andy Shevchenko <[email protected]>
--
With Best Regards,
Andy Shevchenko
On Thu, Feb 8, 2024 at 9:27 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> The match function used in gpio_device_find() should not modify the
> contents of passed opaque pointer, because such modification would not
> be necessary for actual matching and it could lead to quite unreadable,
> spaghetti code.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
Applied, thanks!
Bart