gpio: pisosr: Use devm_gpiochip_add_data() to simplify remove path
PATCH v3 1/1 gpio: pisosr: Use devm_gpiochip_add_data() to simplify remove path
Use devm version of gpiochip add function to handle removal for us.
While here update copyright and module author.
---
Changes from v2:
- Rebase on v6.4-rc2
Changes from v1:
- Use devm to cleanup mutex
drivers/gpio/gpio-pisosr.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-)
drivers/gpio/gpio-pisosr.c +14 -16
@@ -1,7 +1,7 @@// SPDX-License-Identifier: GPL-2.0-only /* -* Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/ -* Andrew F. Davis <[email protected]> +* Copyright (C) 2015-2023 Texas Instruments Incorporated - https://www.ti.com/ +* Andrew Davis <[email protected]> */ #include <linux/bitmap.h>
@@ -116,6 +116,13 @@ static const struct gpio_chip template_chip = { .can_sleep = true, }; +static void pisosr_mutex_destroy (void *data) +{ +struct mutex *lock = data; + +mutex_destroy (lock); +} +static int pisosr_gpio_probe (struct spi_device *spi) {struct device *dev = &spi->dev;
@@ -126,8 +133,6 @@ static int pisosr_gpio_probe(struct spi_device *spi)if (!gpio)return -ENOMEM ; -spi_set_drvdata (spi, gpio); - gpio->chip = template_chip; gpio->chip.parent = dev;of_property_read_u16 (dev->of_node,"ngpios" , &gpio->chip.ngpio);
@@ -145,8 +150,11 @@ static int pisosr_gpio_probe(struct spi_device *spi) "Unable to allocate load GPIO\n");mutex_init (&gpio->lock); + ret =devm_add_action_or_reset (dev, pisosr_mutex_destroy, &gpio->lock); +if (ret) +return ret; - ret =gpiochip_add_data (&gpio->chip, gpio); + ret =devm_gpiochip_add_data (dev, &gpio->chip, gpio);if (ret <0 ) {dev_err (dev,"Unable to register gpiochip\n" );return ret;
@@ -155,15 +163,6 @@ static int pisosr_gpio_probe(struct spi_device *spi)return 0 ; } -static void pisosr_gpio_remove (struct spi_device *spi) -{ -struct pisosr_gpio *gpio =spi_get_drvdata (spi); - -gpiochip_remove (&gpio->chip); - -mutex_destroy (&gpio->lock); -} -static const struct spi_device_id pisosr_gpio_id_table[] = { {"pisosr-gpio" , }, {/* sentinel */ }
@@ -182,11 +181,10 @@ static struct spi_driver pisosr_gpio_driver = { .of_match_table = pisosr_gpio_of_match_table, }, .probe = pisosr_gpio_probe, - .remove = pisosr_gpio_remove, .id_table = pisosr_gpio_id_table, };module_spi_driver (pisosr_gpio_driver); -MODULE_AUTHOR ("Andrew F. Davis <[email protected]>" ); +MODULE_AUTHOR ("Andrew Davis <[email protected]>" );MODULE_DESCRIPTION ("SPI Compatible PISO Shift Register GPIO Driver" );MODULE_LICENSE ("GPL v2" );
Signature · 1 line
2.39.2
Mon, May 15, 2023 at 12:43:21PM -0500, Andrew Davis kirjoitti:
Quoted 3 lines
Use devm version of gpiochip add function to handle removal for us.
While here update copyright and module author.
...
drivers/gpio/gpio-pisosr.c static const struct gpio_chip template_chip = { · line 124
Quoted 6 lines
drivers/gpio/gpio-pisosr.c +6
-0+static void pisosr_mutex_destroy (void *data) +{ +struct mutex *lock = data; + +mutex_destroy (lock); +}
No need to cast void * explicitly.
static void pisosr_mutex_destroy(void *lock)
{
mutex_destroy(lock);
}Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 7 lines · On 5/15/23 1:34 PM, [email protected] wrote:
Mon, May 15, 2023 at 12:43:21PM -0500, Andrew Davis kirjoitti:
Quoted 3 lines
Use devm version of gpiochip add function to handle removal for us.
While here update copyright and module author.
...
Quoted 9 lines
drivers/gpio/gpio-pisosr.c static const struct gpio_chip template_chip = { · line 124
Quoted 6 lines
drivers/gpio/gpio-pisosr.c +6
-0+static void pisosr_mutex_destroy (void *data) +{ +struct mutex *lock = data; + +mutex_destroy (lock); +}No need to cast void * explicitly.
No need sure, but I really think it looks cleaner to be explicit with the types here. If you feel strongly I can change it though.
Andrew
Quoted 5 lines
static void
pisosr_mutex_destroy(void *lock){ mutex_destroy(lock); }
Quoted 1 line · Andrew Davis
On 5/15/23 1:34 PM, [email protected] wrote:
...
Quoted 10 lines
Quoted 9 lines
drivers/gpio/gpio-pisosr.c static const struct gpio_chip template_chip = { · line 124
Quoted 6 lines
drivers/gpio/gpio-pisosr.c +6
-0+static void pisosr_mutex_destroy (void *data) +{ +struct mutex *lock = data; + +mutex_destroy (lock); +}No need to cast void * explicitly.
Quoted 2 lines
No need sure, but I really think it looks cleaner to be explicit with the types here.
The parameter is void * and since it's a simplest wrapper on another function there is no value to have a clear type. It brings nothing in my opinion.
Quoted 2 lines
If you feel strongly I
can change it though.
I would go with less LoCs.
Quoted 4 lines
Quoted 4 lines
static void
pisosr_mutex_destroy(void *lock){ mutex_destroy(lock); }
Signature · 2 lines
With Best Regards, Andy Shevchenko