All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] gpio: cgbc: Add init_valid_mask() operation
@ 2026-10-16 17:13 Thomas Richard (congatec GmbH)
  2026-10-16 17:23 ` sashiko-bot
  2026-10-16 20:37 ` Linus Walleij
  0 siblings, 2 replies; 3+ messages in thread
From: Thomas Richard (congatec GmbH) @ 2026-10-16 17:13 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski
  Cc: Thomas Petazzoni, Werner Gartner, linux-gpio, linux-kernel,
	Thomas Richard (congatec GmbH)

Some pins can be reconfigured from the BIOS as alternate functions,
which makes the corresponding GPIO lines unusable. Add the
init_valid_mask() operation to mark as valid only the GPIOs which are
usable.

Signed-off-by: Thomas Richard (congatec GmbH) <thomas.richard@bootlin.com>
---
 drivers/gpio/gpio-cgbc.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpio/gpio-cgbc.c b/drivers/gpio/gpio-cgbc.c
index 84b5ed3c6e22..1b603d8eccda 100644
--- a/drivers/gpio/gpio-cgbc.c
+++ b/drivers/gpio/gpio-cgbc.c
@@ -18,6 +18,7 @@
 #define CGBC_GPIO_CMD_SET	0x65
 #define CGBC_GPIO_CMD_DIR_GET	0x66
 #define CGBC_GPIO_CMD_DIR_SET	0x67
+#define CGBC_GPIO_CMD_ENA_RD	0x84
 
 struct cgbc_gpio_data {
 	struct gpio_chip	chip;
@@ -146,6 +147,30 @@ static int cgbc_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
 		return GPIO_LINE_DIRECTION_IN;
 }
 
+static int cgbc_gpio_init_valid_mask(struct gpio_chip *chip,
+				     unsigned long *valid_mask,
+				     unsigned int ngpios)
+{
+	struct cgbc_gpio_data *gpio = gpiochip_get_data(chip);
+	struct cgbc_device_data *cgbc = gpio->cgbc;
+	int ret;
+	u8 val;
+
+	ret = cgbc_gpio_cmd(cgbc, CGBC_GPIO_CMD_ENA_RD, 0, 0, &val);
+	if (ret)
+		return ret;
+
+	*valid_mask = val;
+
+	ret = cgbc_gpio_cmd(cgbc, CGBC_GPIO_CMD_ENA_RD, 1, 0, &val);
+	if (ret)
+		return ret;
+
+	*valid_mask |= (val << 8);
+
+	return 0;
+}
+
 static int cgbc_gpio_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -172,6 +197,7 @@ static int cgbc_gpio_probe(struct platform_device *pdev)
 	chip->get_direction = cgbc_gpio_get_direction;
 	chip->get = cgbc_gpio_get;
 	chip->set = cgbc_gpio_set;
+	chip->init_valid_mask = cgbc_gpio_init_valid_mask;
 	chip->ngpio = CGBC_GPIO_NGPIO;
 
 	ret = devm_mutex_init(dev, &gpio->lock);

---
base-commit: 7b47e2aec8d31ded7b996b7676d4d01d5e71736d
change-id: 20261016-cgbc-gpio-valid-mask-a08c365d0a4f

Best regards,
--  
Thomas Richard (congatec GmbH) <thomas.richard@bootlin.com>


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

* Re: [PATCH] gpio: cgbc: Add init_valid_mask() operation
  2026-10-16 17:13 [PATCH] gpio: cgbc: Add init_valid_mask() operation Thomas Richard (congatec GmbH)
@ 2026-10-16 17:23 ` sashiko-bot
  2026-10-16 20:37 ` Linus Walleij
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-10-16 17:23 UTC (permalink / raw)
  To: Thomas Richard (congatec GmbH); +Cc: linux-gpio

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] gpio: cgbc: init_valid_mask clobbers firmware reserved ranges
- [Medium] gpio: cgbc: uninitialized stack variable due to missing short response check
--

commit b1d359743f45922b03a802e12705b29714b272cd
Author: Thomas Richard (congatec GmbH) <thomas.richard@bootlin.com>

gpio: cgbc: Add init_valid_mask() operation

This commit adds the init_valid_mask() operation to the cgbc gpio driver
to mark as valid only the GPIOs which are usable, excluding those
reconfigured by the BIOS as alternate functions.

> diff --git a/drivers/gpio/gpio-cgbc.c b/drivers/gpio/gpio-cgbc.c
> index 84b5ed3c6e22b..1b603d8eccdaa 100644
> --- a/drivers/gpio/gpio-cgbc.c
> +++ b/drivers/gpio/gpio-cgbc.c

[ ... ]

> @@ -146,6 +147,30 @@ static int cgbc_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
>  		return GPIO_LINE_DIRECTION_IN;
>  }
>  
> +static int cgbc_gpio_init_valid_mask(struct gpio_chip *chip,
> +				     unsigned long *valid_mask,
> +				     unsigned int ngpios)
> +{
> +	struct cgbc_gpio_data *gpio = gpiochip_get_data(chip);
> +	struct cgbc_device_data *cgbc = gpio->cgbc;
> +	int ret;
> +	u8 val;
> +
> +	ret = cgbc_gpio_cmd(cgbc, CGBC_GPIO_CMD_ENA_RD, 0, 0, &val);

[Severity: Medium]
If the hardware returns a short response, could val remain uninitialized here?

Looking at cgbc_command() in drivers/mfd/cgbc-core.c, if the hardware returns
an empty payload (istatus == 0) but a valid checksum, it returns 0 (success)
without writing to the output buffer:

	case CGBC_STATUS_DATA_READY:
		if (istatus > data_size)
			istatus = data_size;
		for (i = 0; i < istatus; i++) {
			_data[i] = ioread8(cgbc->io_cmd + CGBC_CMD_DATA + ((i + 1) % 4));
			...

If istatus is 0, this loop is skipped, val is untouched, and the uninitialized
stack variable is used to populate the valid mask.

> +	if (ret)
> +		return ret;
> +
> +	*valid_mask = val;

[Severity: Medium]
Does this direct assignment clobber firmware-reserved ranges?

The gpiolib core clears reserved bits via gpiochip_apply_reserved_ranges()
before calling the driver's init_valid_mask() callback. By using a direct
assignment instead of a bitwise operation (e.g., *valid_mask &= val), any
hardware lines explicitly reserved by the board firmware will be incorrectly
marked as valid again.

> +
> +	ret = cgbc_gpio_cmd(cgbc, CGBC_GPIO_CMD_ENA_RD, 1, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	*valid_mask |= (val << 8);
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-cgbc-gpio-valid-mask-v1-1-938202276e26@bootlin.com?part=1

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

* Re: [PATCH] gpio: cgbc: Add init_valid_mask() operation
  2026-10-16 17:13 [PATCH] gpio: cgbc: Add init_valid_mask() operation Thomas Richard (congatec GmbH)
  2026-10-16 17:23 ` sashiko-bot
@ 2026-10-16 20:37 ` Linus Walleij
  1 sibling, 0 replies; 3+ messages in thread
From: Linus Walleij @ 2026-10-16 20:37 UTC (permalink / raw)
  To: Thomas Richard (congatec GmbH)
  Cc: Bartosz Golaszewski, Thomas Petazzoni, Werner Gartner, linux-gpio,
	linux-kernel

On Fri, Oct 16, 2026 at 7:13 PM Thomas Richard (congatec GmbH)
<thomas.richard@bootlin.com> wrote:

> Some pins can be reconfigured from the BIOS as alternate functions,
> which makes the corresponding GPIO lines unusable. Add the
> init_valid_mask() operation to mark as valid only the GPIOs which are
> usable.
>
> Signed-off-by: Thomas Richard (congatec GmbH) <thomas.richard@bootlin.com>

Sashiko's review looks like nonsense, this is good to go.
Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

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

end of thread, other threads:[~2026-10-16 20:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-16 17:13 [PATCH] gpio: cgbc: Add init_valid_mask() operation Thomas Richard (congatec GmbH)
2026-10-16 17:23 ` sashiko-bot
2026-10-16 20:37 ` Linus Walleij

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.