linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/7] gpio: 74x164: Refactor and clean up the driver
@ 2025-02-03 12:17 Andy Shevchenko
  2025-02-03 12:17 ` [PATCH v1 1/7] gpio: 74x164: Remove unneeded dependency to OF_GPIO Andy Shevchenko
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Andy Shevchenko @ 2025-02-03 12:17 UTC (permalink / raw)
  To: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
	linux-hardening
  Cc: Bartosz Golaszewski, Kees Cook, Gustavo A. R. Silva,
	Andy Shevchenko

Seems like I have had a cleanup series for 74x164, but forgot to send it
last year, here it is.

Andy Shevchenko (7):
  gpio: 74x164: Remove unneeded dependency to OF_GPIO
  gpio: 74x164: Simplify code with cleanup helpers
  gpio: 74x164: Annotate buffer with __counted_by()
  gpio: 74x164: Make use of the macros from bits.h
  gpio: 74x164: Fully convert to use managed resources
  gpio: 74x164: Switch to use dev_err_probe()
  gpio: 74x164: Utilise temporary variable for struct device

 drivers/gpio/Kconfig       |  1 -
 drivers/gpio/gpio-74x164.c | 63 +++++++++++++++++++++-----------------
 2 files changed, 35 insertions(+), 29 deletions(-)

-- 
2.43.0.rc1.1336.g36b5255a03ac


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v1 1/7] gpio: 74x164: Remove unneeded dependency to OF_GPIO
  2025-02-03 12:17 [PATCH v1 0/7] gpio: 74x164: Refactor and clean up the driver Andy Shevchenko
@ 2025-02-03 12:17 ` Andy Shevchenko
  2025-02-07  8:01   ` Bartosz Golaszewski
  2025-02-03 12:17 ` [PATCH v1 2/7] gpio: 74x164: Simplify code with cleanup helpers Andy Shevchenko
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2025-02-03 12:17 UTC (permalink / raw)
  To: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
	linux-hardening
  Cc: Bartosz Golaszewski, Kees Cook, Gustavo A. R. Silva,
	Andy Shevchenko

Remove unneeded dependency to OF_GPIO which driver does not use.

Fixes: 3c7469514dbe ("gpio: 74x164: Make use of device properties")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index add5ad29a673..56c1f30ac195 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1790,7 +1790,6 @@ menu "SPI GPIO expanders"
 
 config GPIO_74X164
 	tristate "74x164 serial-in/parallel-out 8-bits shift register"
-	depends on OF_GPIO
 	help
 	  Driver for 74x164 compatible serial-in/parallel-out 8-outputs
 	  shift registers. This driver can be used to provide access
-- 
2.43.0.rc1.1336.g36b5255a03ac


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v1 2/7] gpio: 74x164: Simplify code with cleanup helpers
  2025-02-03 12:17 [PATCH v1 0/7] gpio: 74x164: Refactor and clean up the driver Andy Shevchenko
  2025-02-03 12:17 ` [PATCH v1 1/7] gpio: 74x164: Remove unneeded dependency to OF_GPIO Andy Shevchenko
@ 2025-02-03 12:17 ` Andy Shevchenko
  2025-02-10  9:12   ` Linus Walleij
  2025-02-03 12:17 ` [PATCH v1 3/7] gpio: 74x164: Annotate buffer with __counted_by() Andy Shevchenko
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2025-02-03 12:17 UTC (permalink / raw)
  To: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
	linux-hardening
  Cc: Bartosz Golaszewski, Kees Cook, Gustavo A. R. Silva,
	Andy Shevchenko

Use macros defined in linux/cleanup.h to automate resource lifetime
control in the driver.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-74x164.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
index fca6cd2eb1dd..70c662bbca7b 100644
--- a/drivers/gpio/gpio-74x164.c
+++ b/drivers/gpio/gpio-74x164.c
@@ -7,6 +7,7 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/cleanup.h>
 #include <linux/gpio/consumer.h>
 #include <linux/gpio/driver.h>
 #include <linux/module.h>
@@ -43,13 +44,10 @@ static int gen_74x164_get_value(struct gpio_chip *gc, unsigned offset)
 	struct gen_74x164_chip *chip = gpiochip_get_data(gc);
 	u8 bank = chip->registers - 1 - offset / 8;
 	u8 pin = offset % 8;
-	int ret;
 
-	mutex_lock(&chip->lock);
-	ret = (chip->buffer[bank] >> pin) & 0x1;
-	mutex_unlock(&chip->lock);
+	guard(mutex)(&chip->lock);
 
-	return ret;
+	return (chip->buffer[bank] >> pin) & 0x1;
 }
 
 static void gen_74x164_set_value(struct gpio_chip *gc,
@@ -59,14 +57,14 @@ static void gen_74x164_set_value(struct gpio_chip *gc,
 	u8 bank = chip->registers - 1 - offset / 8;
 	u8 pin = offset % 8;
 
-	mutex_lock(&chip->lock);
+	guard(mutex)(&chip->lock);
+
 	if (val)
 		chip->buffer[bank] |= (1 << pin);
 	else
 		chip->buffer[bank] &= ~(1 << pin);
 
 	__gen_74x164_write_config(chip);
-	mutex_unlock(&chip->lock);
 }
 
 static void gen_74x164_set_multiple(struct gpio_chip *gc, unsigned long *mask,
@@ -78,7 +76,8 @@ static void gen_74x164_set_multiple(struct gpio_chip *gc, unsigned long *mask,
 	size_t bank;
 	unsigned long bitmask;
 
-	mutex_lock(&chip->lock);
+	guard(mutex)(&chip->lock);
+
 	for_each_set_clump8(offset, bankmask, mask, chip->registers * 8) {
 		bank = chip->registers - 1 - offset / 8;
 		bitmask = bitmap_get_value8(bits, offset) & bankmask;
@@ -87,7 +86,6 @@ static void gen_74x164_set_multiple(struct gpio_chip *gc, unsigned long *mask,
 		chip->buffer[bank] |= bitmask;
 	}
 	__gen_74x164_write_config(chip);
-	mutex_unlock(&chip->lock);
 }
 
 static int gen_74x164_direction_output(struct gpio_chip *gc,
-- 
2.43.0.rc1.1336.g36b5255a03ac


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v1 3/7] gpio: 74x164: Annotate buffer with __counted_by()
  2025-02-03 12:17 [PATCH v1 0/7] gpio: 74x164: Refactor and clean up the driver Andy Shevchenko
  2025-02-03 12:17 ` [PATCH v1 1/7] gpio: 74x164: Remove unneeded dependency to OF_GPIO Andy Shevchenko
  2025-02-03 12:17 ` [PATCH v1 2/7] gpio: 74x164: Simplify code with cleanup helpers Andy Shevchenko
@ 2025-02-03 12:17 ` Andy Shevchenko
  2025-02-04  1:28   ` Gustavo A. R. Silva
  2025-02-03 12:17 ` [PATCH v1 4/7] gpio: 74x164: Make use of the macros from bits.h Andy Shevchenko
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2025-02-03 12:17 UTC (permalink / raw)
  To: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
	linux-hardening
  Cc: Bartosz Golaszewski, Kees Cook, Gustavo A. R. Silva,
	Andy Shevchenko

Add the __counted_by() compiler attribute to the flexible array member
volumes to improve access bounds-checking via CONFIG_UBSAN_BOUNDS and
CONFIG_FORTIFY_SOURCE.

Use struct_size() instead of manually calculating the number of bytes to
allocate the private structure with a buffer.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-74x164.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
index 70c662bbca7b..7844f8a58834 100644
--- a/drivers/gpio/gpio-74x164.c
+++ b/drivers/gpio/gpio-74x164.c
@@ -30,7 +30,7 @@ struct gen_74x164_chip {
 	 * register at the end of the transfer. So, to have a logical
 	 * numbering, store the bytes in reverse order.
 	 */
-	u8			buffer[];
+	u8			buffer[] __counted_by(registers);
 };
 
 static int __gen_74x164_write_config(struct gen_74x164_chip *chip)
@@ -97,6 +97,7 @@ static int gen_74x164_direction_output(struct gpio_chip *gc,
 
 static int gen_74x164_probe(struct spi_device *spi)
 {
+	struct device *dev = &spi->dev;
 	struct gen_74x164_chip *chip;
 	u32 nregs;
 	int ret;
@@ -116,10 +117,12 @@ static int gen_74x164_probe(struct spi_device *spi)
 		return -EINVAL;
 	}
 
-	chip = devm_kzalloc(&spi->dev, sizeof(*chip) + nregs, GFP_KERNEL);
+	chip = devm_kzalloc(dev, struct_size(chip, buffer, nregs), GFP_KERNEL);
 	if (!chip)
 		return -ENOMEM;
 
+	chip->registers = nregs;
+
 	chip->gpiod_oe = devm_gpiod_get_optional(&spi->dev, "enable",
 						 GPIOD_OUT_LOW);
 	if (IS_ERR(chip->gpiod_oe))
@@ -133,10 +136,7 @@ static int gen_74x164_probe(struct spi_device *spi)
 	chip->gpio_chip.set = gen_74x164_set_value;
 	chip->gpio_chip.set_multiple = gen_74x164_set_multiple;
 	chip->gpio_chip.base = -1;
-
-	chip->registers = nregs;
 	chip->gpio_chip.ngpio = GEN_74X164_NUMBER_GPIOS * chip->registers;
-
 	chip->gpio_chip.can_sleep = true;
 	chip->gpio_chip.parent = &spi->dev;
 	chip->gpio_chip.owner = THIS_MODULE;
-- 
2.43.0.rc1.1336.g36b5255a03ac


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v1 4/7] gpio: 74x164: Make use of the macros from bits.h
  2025-02-03 12:17 [PATCH v1 0/7] gpio: 74x164: Refactor and clean up the driver Andy Shevchenko
                   ` (2 preceding siblings ...)
  2025-02-03 12:17 ` [PATCH v1 3/7] gpio: 74x164: Annotate buffer with __counted_by() Andy Shevchenko
@ 2025-02-03 12:17 ` Andy Shevchenko
  2025-02-10  9:12   ` Linus Walleij
  2025-02-03 12:17 ` [PATCH v1 5/7] gpio: 74x164: Fully convert to use managed resources Andy Shevchenko
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2025-02-03 12:17 UTC (permalink / raw)
  To: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
	linux-hardening
  Cc: Bartosz Golaszewski, Kees Cook, Gustavo A. R. Silva,
	Andy Shevchenko

Make use of BIT() and GENMASK() where it makes sense.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-74x164.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
index 7844f8a58834..0f720d539fa7 100644
--- a/drivers/gpio/gpio-74x164.c
+++ b/drivers/gpio/gpio-74x164.c
@@ -47,7 +47,7 @@ static int gen_74x164_get_value(struct gpio_chip *gc, unsigned offset)
 
 	guard(mutex)(&chip->lock);
 
-	return (chip->buffer[bank] >> pin) & 0x1;
+	return !!(chip->buffer[bank] & BIT(pin));
 }
 
 static void gen_74x164_set_value(struct gpio_chip *gc,
@@ -60,9 +60,9 @@ static void gen_74x164_set_value(struct gpio_chip *gc,
 	guard(mutex)(&chip->lock);
 
 	if (val)
-		chip->buffer[bank] |= (1 << pin);
+		chip->buffer[bank] |= BIT(pin);
 	else
-		chip->buffer[bank] &= ~(1 << pin);
+		chip->buffer[bank] &= ~BIT(pin);
 
 	__gen_74x164_write_config(chip);
 }
-- 
2.43.0.rc1.1336.g36b5255a03ac


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v1 5/7] gpio: 74x164: Fully convert to use managed resources
  2025-02-03 12:17 [PATCH v1 0/7] gpio: 74x164: Refactor and clean up the driver Andy Shevchenko
                   ` (3 preceding siblings ...)
  2025-02-03 12:17 ` [PATCH v1 4/7] gpio: 74x164: Make use of the macros from bits.h Andy Shevchenko
@ 2025-02-03 12:17 ` Andy Shevchenko
  2025-02-07  8:22   ` Bartosz Golaszewski
  2025-02-03 12:17 ` [PATCH v1 6/7] gpio: 74x164: Switch to use dev_err_probe() Andy Shevchenko
  2025-02-03 12:17 ` [PATCH v1 7/7] gpio: 74x164: Utilise temporary variable for struct device Andy Shevchenko
  6 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2025-02-03 12:17 UTC (permalink / raw)
  To: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
	linux-hardening
  Cc: Bartosz Golaszewski, Kees Cook, Gustavo A. R. Silva,
	Andy Shevchenko

Convert the driver probe stage to use managed resources.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-74x164.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
index 0f720d539fa7..920d3b9c1087 100644
--- a/drivers/gpio/gpio-74x164.c
+++ b/drivers/gpio/gpio-74x164.c
@@ -95,6 +95,19 @@ static int gen_74x164_direction_output(struct gpio_chip *gc,
 	return 0;
 }
 
+static void gen_74x164_deactivate(void *data)
+{
+	struct gen_74x164_chip *chip = data;
+
+	gpiod_set_value_cansleep(chip->gpiod_oe, 0);
+}
+
+static int gen_74x164_activate(struct device *dev, struct gen_74x164_chip *chip)
+{
+	gpiod_set_value_cansleep(chip->gpiod_oe, 1);
+	return devm_add_action_or_reset(dev, gen_74x164_deactivate, chip);
+}
+
 static int gen_74x164_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
@@ -128,8 +141,6 @@ static int gen_74x164_probe(struct spi_device *spi)
 	if (IS_ERR(chip->gpiod_oe))
 		return PTR_ERR(chip->gpiod_oe);
 
-	spi_set_drvdata(spi, chip);
-
 	chip->gpio_chip.label = spi->modalias;
 	chip->gpio_chip.direction_output = gen_74x164_direction_output;
 	chip->gpio_chip.get = gen_74x164_get_value;
@@ -149,7 +160,9 @@ static int gen_74x164_probe(struct spi_device *spi)
 	if (ret)
 		return dev_err_probe(&spi->dev, ret, "Config write failed\n");
 
-	gpiod_set_value_cansleep(chip->gpiod_oe, 1);
+	ret = gen_74x164_activate(dev, chip);
+	if (ret)
+		return ret;
 
 	return devm_gpiochip_add_data(&spi->dev, &chip->gpio_chip, chip);
 }
@@ -181,7 +194,6 @@ static struct spi_driver gen_74x164_driver = {
 		.of_match_table	= gen_74x164_dt_ids,
 	},
 	.probe		= gen_74x164_probe,
-	.remove		= gen_74x164_remove,
 	.id_table	= gen_74x164_spi_ids,
 };
 module_spi_driver(gen_74x164_driver);
-- 
2.43.0.rc1.1336.g36b5255a03ac


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v1 6/7] gpio: 74x164: Switch to use dev_err_probe()
  2025-02-03 12:17 [PATCH v1 0/7] gpio: 74x164: Refactor and clean up the driver Andy Shevchenko
                   ` (4 preceding siblings ...)
  2025-02-03 12:17 ` [PATCH v1 5/7] gpio: 74x164: Fully convert to use managed resources Andy Shevchenko
@ 2025-02-03 12:17 ` Andy Shevchenko
  2025-02-11  9:15   ` Linus Walleij
  2025-02-03 12:17 ` [PATCH v1 7/7] gpio: 74x164: Utilise temporary variable for struct device Andy Shevchenko
  6 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2025-02-03 12:17 UTC (permalink / raw)
  To: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
	linux-hardening
  Cc: Bartosz Golaszewski, Kees Cook, Gustavo A. R. Silva,
	Andy Shevchenko

Switch to use dev_err_probe() to simplify the error path and
unify a message template.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-74x164.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
index 920d3b9c1087..2cf77c0a5615 100644
--- a/drivers/gpio/gpio-74x164.c
+++ b/drivers/gpio/gpio-74x164.c
@@ -125,10 +125,8 @@ static int gen_74x164_probe(struct spi_device *spi)
 		return ret;
 
 	ret = device_property_read_u32(&spi->dev, "registers-number", &nregs);
-	if (ret) {
-		dev_err(&spi->dev, "Missing 'registers-number' property.\n");
-		return -EINVAL;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "Missing 'registers-number' property.\n");
 
 	chip = devm_kzalloc(dev, struct_size(chip, buffer, nregs), GFP_KERNEL);
 	if (!chip)
-- 
2.43.0.rc1.1336.g36b5255a03ac


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v1 7/7] gpio: 74x164: Utilise temporary variable for struct device
  2025-02-03 12:17 [PATCH v1 0/7] gpio: 74x164: Refactor and clean up the driver Andy Shevchenko
                   ` (5 preceding siblings ...)
  2025-02-03 12:17 ` [PATCH v1 6/7] gpio: 74x164: Switch to use dev_err_probe() Andy Shevchenko
@ 2025-02-03 12:17 ` Andy Shevchenko
  6 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2025-02-03 12:17 UTC (permalink / raw)
  To: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
	linux-hardening
  Cc: Bartosz Golaszewski, Kees Cook, Gustavo A. R. Silva,
	Andy Shevchenko

We have a temporary variable to keep a pointer to struct device.
Utilise it where it makes sense.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-74x164.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
index 2cf77c0a5615..4f7837515e7a 100644
--- a/drivers/gpio/gpio-74x164.c
+++ b/drivers/gpio/gpio-74x164.c
@@ -124,7 +124,7 @@ static int gen_74x164_probe(struct spi_device *spi)
 	if (ret < 0)
 		return ret;
 
-	ret = device_property_read_u32(&spi->dev, "registers-number", &nregs);
+	ret = device_property_read_u32(dev, "registers-number", &nregs);
 	if (ret)
 		return dev_err_probe(dev, ret, "Missing 'registers-number' property.\n");
 
@@ -134,8 +134,7 @@ static int gen_74x164_probe(struct spi_device *spi)
 
 	chip->registers = nregs;
 
-	chip->gpiod_oe = devm_gpiod_get_optional(&spi->dev, "enable",
-						 GPIOD_OUT_LOW);
+	chip->gpiod_oe = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
 	if (IS_ERR(chip->gpiod_oe))
 		return PTR_ERR(chip->gpiod_oe);
 
@@ -147,7 +146,7 @@ static int gen_74x164_probe(struct spi_device *spi)
 	chip->gpio_chip.base = -1;
 	chip->gpio_chip.ngpio = GEN_74X164_NUMBER_GPIOS * chip->registers;
 	chip->gpio_chip.can_sleep = true;
-	chip->gpio_chip.parent = &spi->dev;
+	chip->gpio_chip.parent = dev;
 	chip->gpio_chip.owner = THIS_MODULE;
 
 	ret = devm_mutex_init(&spi->dev, &chip->lock);
-- 
2.43.0.rc1.1336.g36b5255a03ac


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 3/7] gpio: 74x164: Annotate buffer with __counted_by()
  2025-02-03 12:17 ` [PATCH v1 3/7] gpio: 74x164: Annotate buffer with __counted_by() Andy Shevchenko
@ 2025-02-04  1:28   ` Gustavo A. R. Silva
  0 siblings, 0 replies; 16+ messages in thread
From: Gustavo A. R. Silva @ 2025-02-04  1:28 UTC (permalink / raw)
  To: Andy Shevchenko, Bartosz Golaszewski, Linus Walleij, linux-gpio,
	linux-kernel, linux-hardening
  Cc: Bartosz Golaszewski, Kees Cook, Gustavo A. R. Silva



On 03/02/25 22:47, Andy Shevchenko wrote:
> Add the __counted_by() compiler attribute to the flexible array member
> volumes to improve access bounds-checking via CONFIG_UBSAN_BOUNDS and
> CONFIG_FORTIFY_SOURCE.
> 
> Use struct_size() instead of manually calculating the number of bytes to
> allocate the private structure with a buffer.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks!
--
Gustavo

> ---
>   drivers/gpio/gpio-74x164.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
> index 70c662bbca7b..7844f8a58834 100644
> --- a/drivers/gpio/gpio-74x164.c
> +++ b/drivers/gpio/gpio-74x164.c
> @@ -30,7 +30,7 @@ struct gen_74x164_chip {
>   	 * register at the end of the transfer. So, to have a logical
>   	 * numbering, store the bytes in reverse order.
>   	 */
> -	u8			buffer[];
> +	u8			buffer[] __counted_by(registers);
>   };
>   
>   static int __gen_74x164_write_config(struct gen_74x164_chip *chip)
> @@ -97,6 +97,7 @@ static int gen_74x164_direction_output(struct gpio_chip *gc,
>   
>   static int gen_74x164_probe(struct spi_device *spi)
>   {
> +	struct device *dev = &spi->dev;
>   	struct gen_74x164_chip *chip;
>   	u32 nregs;
>   	int ret;
> @@ -116,10 +117,12 @@ static int gen_74x164_probe(struct spi_device *spi)
>   		return -EINVAL;
>   	}
>   
> -	chip = devm_kzalloc(&spi->dev, sizeof(*chip) + nregs, GFP_KERNEL);
> +	chip = devm_kzalloc(dev, struct_size(chip, buffer, nregs), GFP_KERNEL);
>   	if (!chip)
>   		return -ENOMEM;
>   
> +	chip->registers = nregs;
> +
>   	chip->gpiod_oe = devm_gpiod_get_optional(&spi->dev, "enable",
>   						 GPIOD_OUT_LOW);
>   	if (IS_ERR(chip->gpiod_oe))
> @@ -133,10 +136,7 @@ static int gen_74x164_probe(struct spi_device *spi)
>   	chip->gpio_chip.set = gen_74x164_set_value;
>   	chip->gpio_chip.set_multiple = gen_74x164_set_multiple;
>   	chip->gpio_chip.base = -1;
> -
> -	chip->registers = nregs;
>   	chip->gpio_chip.ngpio = GEN_74X164_NUMBER_GPIOS * chip->registers;
> -
>   	chip->gpio_chip.can_sleep = true;
>   	chip->gpio_chip.parent = &spi->dev;
>   	chip->gpio_chip.owner = THIS_MODULE;


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 1/7] gpio: 74x164: Remove unneeded dependency to OF_GPIO
  2025-02-03 12:17 ` [PATCH v1 1/7] gpio: 74x164: Remove unneeded dependency to OF_GPIO Andy Shevchenko
@ 2025-02-07  8:01   ` Bartosz Golaszewski
  2025-02-07  8:56     ` Geert Uytterhoeven
  0 siblings, 1 reply; 16+ messages in thread
From: Bartosz Golaszewski @ 2025-02-07  8:01 UTC (permalink / raw)
  To: Andy Shevchenko, Geert Uytterhoeven
  Cc: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
	linux-hardening, Kees Cook, Gustavo A. R. Silva

On Mon, Feb 3, 2025 at 1:18 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Remove unneeded dependency to OF_GPIO which driver does not use.
>
> Fixes: 3c7469514dbe ("gpio: 74x164: Make use of device properties")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/gpio/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index add5ad29a673..56c1f30ac195 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1790,7 +1790,6 @@ menu "SPI GPIO expanders"
>
>  config GPIO_74X164
>         tristate "74x164 serial-in/parallel-out 8-bits shift register"
> -       depends on OF_GPIO
>         help
>           Driver for 74x164 compatible serial-in/parallel-out 8-outputs
>           shift registers. This driver can be used to provide access
> --
> 2.43.0.rc1.1336.g36b5255a03ac
>

Geert: Just Cc'ing you here because you yelled at me last time I
removed all dependencies. This driver is under the SPI section so it
does have the CONFIG_SPI_MASTER dependency. I think it's fine to drop
OF_GPIO here but I just wanted to run it by you.

Bart

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 5/7] gpio: 74x164: Fully convert to use managed resources
  2025-02-03 12:17 ` [PATCH v1 5/7] gpio: 74x164: Fully convert to use managed resources Andy Shevchenko
@ 2025-02-07  8:22   ` Bartosz Golaszewski
  0 siblings, 0 replies; 16+ messages in thread
From: Bartosz Golaszewski @ 2025-02-07  8:22 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
	linux-hardening, Kees Cook, Gustavo A. R. Silva

On Mon, Feb 3, 2025 at 1:18 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Convert the driver probe stage to use managed resources.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/gpio/gpio-74x164.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
> index 0f720d539fa7..920d3b9c1087 100644
> --- a/drivers/gpio/gpio-74x164.c
> +++ b/drivers/gpio/gpio-74x164.c
> @@ -95,6 +95,19 @@ static int gen_74x164_direction_output(struct gpio_chip *gc,
>         return 0;
>  }
>
> +static void gen_74x164_deactivate(void *data)
> +{
> +       struct gen_74x164_chip *chip = data;
> +
> +       gpiod_set_value_cansleep(chip->gpiod_oe, 0);
> +}
> +
> +static int gen_74x164_activate(struct device *dev, struct gen_74x164_chip *chip)
> +{
> +       gpiod_set_value_cansleep(chip->gpiod_oe, 1);
> +       return devm_add_action_or_reset(dev, gen_74x164_deactivate, chip);
> +}
> +
>  static int gen_74x164_probe(struct spi_device *spi)
>  {
>         struct device *dev = &spi->dev;
> @@ -128,8 +141,6 @@ static int gen_74x164_probe(struct spi_device *spi)
>         if (IS_ERR(chip->gpiod_oe))
>                 return PTR_ERR(chip->gpiod_oe);
>
> -       spi_set_drvdata(spi, chip);
> -
>         chip->gpio_chip.label = spi->modalias;
>         chip->gpio_chip.direction_output = gen_74x164_direction_output;
>         chip->gpio_chip.get = gen_74x164_get_value;
> @@ -149,7 +160,9 @@ static int gen_74x164_probe(struct spi_device *spi)
>         if (ret)
>                 return dev_err_probe(&spi->dev, ret, "Config write failed\n");
>
> -       gpiod_set_value_cansleep(chip->gpiod_oe, 1);
> +       ret = gen_74x164_activate(dev, chip);
> +       if (ret)
> +               return ret;
>
>         return devm_gpiochip_add_data(&spi->dev, &chip->gpio_chip, chip);
>  }
> @@ -181,7 +194,6 @@ static struct spi_driver gen_74x164_driver = {
>                 .of_match_table = gen_74x164_dt_ids,
>         },
>         .probe          = gen_74x164_probe,
> -       .remove         = gen_74x164_remove,

Did you forget to remove gen_74x164_remove()?

Bart

>         .id_table       = gen_74x164_spi_ids,
>  };
>  module_spi_driver(gen_74x164_driver);
> --
> 2.43.0.rc1.1336.g36b5255a03ac
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 1/7] gpio: 74x164: Remove unneeded dependency to OF_GPIO
  2025-02-07  8:01   ` Bartosz Golaszewski
@ 2025-02-07  8:56     ` Geert Uytterhoeven
  0 siblings, 0 replies; 16+ messages in thread
From: Geert Uytterhoeven @ 2025-02-07  8:56 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Andy Shevchenko, Bartosz Golaszewski, Linus Walleij, linux-gpio,
	linux-kernel, linux-hardening, Kees Cook, Gustavo A. R. Silva

Hi Bartosz,

On Fri, 7 Feb 2025 at 09:02, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> On Mon, Feb 3, 2025 at 1:18 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > Remove unneeded dependency to OF_GPIO which driver does not use.
> >
> > Fixes: 3c7469514dbe ("gpio: 74x164: Make use of device properties")
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/gpio/Kconfig | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> > index add5ad29a673..56c1f30ac195 100644
> > --- a/drivers/gpio/Kconfig
> > +++ b/drivers/gpio/Kconfig
> > @@ -1790,7 +1790,6 @@ menu "SPI GPIO expanders"
> >
> >  config GPIO_74X164
> >         tristate "74x164 serial-in/parallel-out 8-bits shift register"
> > -       depends on OF_GPIO
> >         help
> >           Driver for 74x164 compatible serial-in/parallel-out 8-outputs
> >           shift registers. This driver can be used to provide access

>
> Geert: Just Cc'ing you here because you yelled at me last time I
> removed all dependencies. This driver is under the SPI section so it
> does have the CONFIG_SPI_MASTER dependency. I think it's fine to drop
> OF_GPIO here but I just wanted to run it by you.

That's fine, as there is still the SPI_MASTER dependency, and the
driver uses device properties.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 2/7] gpio: 74x164: Simplify code with cleanup helpers
  2025-02-03 12:17 ` [PATCH v1 2/7] gpio: 74x164: Simplify code with cleanup helpers Andy Shevchenko
@ 2025-02-10  9:12   ` Linus Walleij
  0 siblings, 0 replies; 16+ messages in thread
From: Linus Walleij @ 2025-02-10  9:12 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Bartosz Golaszewski, linux-gpio, linux-kernel, linux-hardening,
	Bartosz Golaszewski, Kees Cook, Gustavo A. R. Silva

On Mon, Feb 3, 2025 at 1:18 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Use macros defined in linux/cleanup.h to automate resource lifetime
> control in the driver.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Clearly this is more readable and less prone to accidental bugs.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 4/7] gpio: 74x164: Make use of the macros from bits.h
  2025-02-03 12:17 ` [PATCH v1 4/7] gpio: 74x164: Make use of the macros from bits.h Andy Shevchenko
@ 2025-02-10  9:12   ` Linus Walleij
  2025-02-10  9:16     ` Andy Shevchenko
  0 siblings, 1 reply; 16+ messages in thread
From: Linus Walleij @ 2025-02-10  9:12 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Bartosz Golaszewski, linux-gpio, linux-kernel, linux-hardening,
	Bartosz Golaszewski, Kees Cook, Gustavo A. R. Silva

On Mon, Feb 3, 2025 at 1:18 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Make use of BIT() and GENMASK() where it makes sense.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 4/7] gpio: 74x164: Make use of the macros from bits.h
  2025-02-10  9:12   ` Linus Walleij
@ 2025-02-10  9:16     ` Andy Shevchenko
  0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2025-02-10  9:16 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Bartosz Golaszewski, linux-gpio, linux-kernel, linux-hardening,
	Bartosz Golaszewski, Kees Cook, Gustavo A. R. Silva

On Mon, Feb 10, 2025 at 10:12:52AM +0100, Linus Walleij wrote:
> On Mon, Feb 3, 2025 at 1:18 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> 
> > Make use of BIT() and GENMASK() where it makes sense.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Thanks, there is a v2 available.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 6/7] gpio: 74x164: Switch to use dev_err_probe()
  2025-02-03 12:17 ` [PATCH v1 6/7] gpio: 74x164: Switch to use dev_err_probe() Andy Shevchenko
@ 2025-02-11  9:15   ` Linus Walleij
  0 siblings, 0 replies; 16+ messages in thread
From: Linus Walleij @ 2025-02-11  9:15 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Bartosz Golaszewski, linux-gpio, linux-kernel, linux-hardening,
	Bartosz Golaszewski, Kees Cook, Gustavo A. R. Silva

On Mon, Feb 3, 2025 at 1:18 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Switch to use dev_err_probe() to simplify the error path and
> unify a message template.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2025-02-11  9:16 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-03 12:17 [PATCH v1 0/7] gpio: 74x164: Refactor and clean up the driver Andy Shevchenko
2025-02-03 12:17 ` [PATCH v1 1/7] gpio: 74x164: Remove unneeded dependency to OF_GPIO Andy Shevchenko
2025-02-07  8:01   ` Bartosz Golaszewski
2025-02-07  8:56     ` Geert Uytterhoeven
2025-02-03 12:17 ` [PATCH v1 2/7] gpio: 74x164: Simplify code with cleanup helpers Andy Shevchenko
2025-02-10  9:12   ` Linus Walleij
2025-02-03 12:17 ` [PATCH v1 3/7] gpio: 74x164: Annotate buffer with __counted_by() Andy Shevchenko
2025-02-04  1:28   ` Gustavo A. R. Silva
2025-02-03 12:17 ` [PATCH v1 4/7] gpio: 74x164: Make use of the macros from bits.h Andy Shevchenko
2025-02-10  9:12   ` Linus Walleij
2025-02-10  9:16     ` Andy Shevchenko
2025-02-03 12:17 ` [PATCH v1 5/7] gpio: 74x164: Fully convert to use managed resources Andy Shevchenko
2025-02-07  8:22   ` Bartosz Golaszewski
2025-02-03 12:17 ` [PATCH v1 6/7] gpio: 74x164: Switch to use dev_err_probe() Andy Shevchenko
2025-02-11  9:15   ` Linus Walleij
2025-02-03 12:17 ` [PATCH v1 7/7] gpio: 74x164: Utilise temporary variable for struct device Andy Shevchenko

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).