Commit 0102fbf5 authored Mar 11, 2025 by Bartosz Golaszewski

gpiolib: don't check the retval of get_direction() when registering a chip



During chip registration we should neither check the return value of
gc->get_direction() nor hold the SRCU lock when calling it. The former
is because pin controllers may have pins set to alternate functions and
return errors from their get_direction() callbacks. That's alright - we
should default to the safe INPUT state and not bail-out. The latter is
not needed because we haven't registered the chip yet so there's nothing
to protect against dynamic removal. In fact: we currently hit a lockdep
splat. Revert to calling the gc->get_direction() callback directly and
*not* checking its value.

Fixes: 9d846b1a ("gpiolib: check the return value of gpio_chip::get_direction()")
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Closes: https://lore.kernel.org/all/81f890fc-6688-42f0-9756-567efc8bb97a@samsung.com/


Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Link: https://lore.kernel.org/r/20250226-retval-fixes-v2-1-c8dc57182441@linaro.org


Tested-by: Gene C <arch@sapience.com>
Link: https://lore.kernel.org/r/20250311175631.83779-1-brgl@bgdev.pl


Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

parent 80e54e84

drivers/gpio/gpiolib.c

+11 −16

Original line number	Diff line number	Diff line
		@@ -1056,25 +1056,20 @@ int gpiochip_add_data_with_key(struct gpio_chip gc, void data,

		desc->gdev = gdev;

		if (gc->get_direction && gpiochip_line_is_valid(gc, desc_index)) {
		ret = gc->get_direction(gc, desc_index);
		if (ret < 0)
		/*
		* FIXME: Bail-out here once all GPIO drivers
		* are updated to not return errors in
		* situations that can be considered normal
		* operation.
		*/
		dev_warn(&gdev->dev,
		"%s: get_direction failed: %d\n",
		__func__, ret);

		assign_bit(FLAG_IS_OUT, &desc->flags, !ret);
		} else {
		* We would typically want to check the return value of
		* get_direction() here but we must not check the return value
		* and bail-out as pin controllers can have pins configured to
		* alternate functions and return -EINVAL. Also: there's no
		* need to take the SRCU lock here.
		*/
		if (gc->get_direction && gpiochip_line_is_valid(gc, desc_index))
		assign_bit(FLAG_IS_OUT, &desc->flags,
		!gc->get_direction(gc, desc_index));
		else
		assign_bit(FLAG_IS_OUT,
		&desc->flags, !gc->direction_input);
		}
		}

		ret = of_gpiochip_add(gc);
		if (ret)