Re: [PATCH v11 3/3] gpio: gpio-ltc4283: Add support for the LTC4283 Swap Controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Unconditional NULL pointer dereference in `ltc4283_init_valid_mask`.
- [Medium] Momentary hardware glitch when configuring GPIO as output.
--

commit 7f319a6f25c038e1ae21c4d383a2dec867c1cace
Author: Nuno Sá <nuno.sa@analog.com>

gpio: gpio-ltc4283: Add support for the LTC4283 Swap Controller

This commit adds GPIO support for the LTC4283 Negative Voltage Hot Swap
Controller. It provides a driver for up to 8 configurable pins that can
be used as GPIOs over an auxiliary bus.

> diff --git a/drivers/gpio/gpio-ltc4283.c b/drivers/gpio/gpio-ltc4283.c
> new file mode 100644
> index 000000000000..6609443c5d62
> --- /dev/null
> +++ b/drivers/gpio/gpio-ltc4283.c

[ ... ]

> @@ -0,0 +1,218 @@
> +static int ltc4283_gpio_direction_output(struct gpio_chip *gc, unsigned int off, int val)
> +{
> +	struct ltc4283_gpio *st = gpiochip_get_data(gc);
> +	int ret;
> +
> +	ret = ltc4283_gpio_direction_set(st, off, false);
> +	if (ret)
> +		return ret;
> +
> +	return __ltc4283_gpio_set_value(st, off, val);
> +}

Because ltc4283_gpio_direction_set() and __ltc4283_gpio_set_value() map to
separate regmap updates, could this sequence cause a momentary hardware
glitch?

Setting the direction to output before setting the intended value means the
pin might actively drive its previous state for a short time before the
second transaction completes.

Would it be safer to update the output register to the intended value before
enabling the output driver?

[ ... ]

> +static int ltc4283_init_valid_mask(struct gpio_chip *gc, unsigned long *valid_mask,
> +				   unsigned int ngpios)
> +{
> +	unsigned long *mask = dev_get_platdata(gc->parent);
> +
> +	bitmap_copy(valid_mask, mask, ngpios);
> +	return 0;
> +}

If the parent device is instantiated without platform data, could this
regression lead to a NULL pointer dereference?

If dev_get_platdata() returns NULL, bitmap_copy() will attempt to copy from
a NULL source pointer:

include/linux/bitmap.h:bitmap_copy() {
        ...
        if (small_const_nbits(nbits))
                *dst = *src;
        else
                memcpy(dst, src, len);
        ...
}

Should there be a check to ensure mask is not NULL before calling
bitmap_copy()?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260429-ltc4283-support-v11-0-27ccde619dad@analog.com?part=3