The valid_mask member of the struct gpio_chip is unconditionally written by the GPIO core at driver registration. Current documentation does not mention this but just says the valid_mask is used if it's not NULL. This lured me to try populating it directly in the GPIO driver probe instead of using the init_valid_mask() callback. It took some retries with different bitmaps and eventually a bit of code-reading to understand why the valid_mask was not obeyed. I could've avoided this trial and error if the valid_mask was hidden in the struct gpio_device instead of being a visible member of the struct gpio_chip. Help the next developer who decides to directly populate the valid_mask in struct gpio_chip by hiding the valid_mask in struct gpio_device and keep it internal to the GPIO core. Suggested-by: Linus Walleij <linus.walleij@xxxxxxxxxx> Signed-off-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx> --- Revision history: v1 => v2: - Hide the valid_mask instead of documenting it as internal to GPIO core as suggested by Linus W. https://lore.kernel.org/all/Z71qphikHPGB0Yuv@mva-rohm/ --- drivers/gpio/gpiolib.c | 16 ++++++++-------- drivers/gpio/gpiolib.h | 3 +++ include/linux/gpio/driver.h | 8 -------- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 7e36894bab11..37e1f277b0a8 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -652,7 +652,7 @@ static int gpiochip_apply_reserved_ranges(struct gpio_chip *gc) if (start >= gc->ngpio || start + count > gc->ngpio) continue; - bitmap_clear(gc->valid_mask, start, count); + bitmap_clear(gc->gpiodev->valid_mask, start, count); } kfree(ranges); @@ -666,8 +666,8 @@ static int gpiochip_init_valid_mask(struct gpio_chip *gc) if (!(gpiochip_count_reserved_ranges(gc) || gc->init_valid_mask)) return 0; - gc->valid_mask = gpiochip_allocate_mask(gc); - if (!gc->valid_mask) + gc->gpiodev->valid_mask = gpiochip_allocate_mask(gc); + if (!gc->gpiodev->valid_mask) return -ENOMEM; ret = gpiochip_apply_reserved_ranges(gc); @@ -676,7 +676,7 @@ static int gpiochip_init_valid_mask(struct gpio_chip *gc) if (gc->init_valid_mask) return gc->init_valid_mask(gc, - gc->valid_mask, + gc->gpiodev->valid_mask, gc->ngpio); return 0; @@ -684,7 +684,7 @@ static int gpiochip_init_valid_mask(struct gpio_chip *gc) static void gpiochip_free_valid_mask(struct gpio_chip *gc) { - gpiochip_free_mask(&gc->valid_mask); + gpiochip_free_mask(&gc->gpiodev->valid_mask); } static int gpiochip_add_pin_ranges(struct gpio_chip *gc) @@ -715,7 +715,7 @@ static int gpiochip_add_pin_ranges(struct gpio_chip *gc) */ const unsigned long *gpiochip_query_valid_mask(const struct gpio_chip *gc) { - return gc->valid_mask; + return gc->gpiodev->valid_mask; } EXPORT_SYMBOL_GPL(gpiochip_query_valid_mask); @@ -723,9 +723,9 @@ bool gpiochip_line_is_valid(const struct gpio_chip *gc, unsigned int offset) { /* No mask means all valid */ - if (likely(!gc->valid_mask)) + if (likely(!gc->gpiodev->valid_mask)) return true; - return test_bit(offset, gc->valid_mask); + return test_bit(offset, gc->gpiodev->valid_mask); } EXPORT_SYMBOL_GPL(gpiochip_line_is_valid); diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h index 147156ec502b..b9a4f161db53 100644 --- a/drivers/gpio/gpiolib.h +++ b/drivers/gpio/gpiolib.h @@ -32,6 +32,8 @@ * @chip: pointer to the corresponding gpiochip, holding static * data for this device * @descs: array of ngpio descriptors. + * @valid_mask: If not %NULL, holds bitmask of GPIOs which are valid to be + * used from the chip. * @desc_srcu: ensures consistent state of GPIO descriptors exposed to users * @ngpio: the number of GPIO lines on this GPIO device, equal to the size * of the @descs array. @@ -65,6 +67,7 @@ struct gpio_device { struct module *owner; struct gpio_chip __rcu *chip; struct gpio_desc *descs; + unsigned long *valid_mask; struct srcu_struct desc_srcu; unsigned int base; u16 ngpio; diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 7dfb8341b0e2..0e8621be7272 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -499,14 +499,6 @@ struct gpio_chip { struct gpio_irq_chip irq; #endif /* CONFIG_GPIOLIB_IRQCHIP */ - /** - * @valid_mask: - * - * If not %NULL, holds bitmask of GPIOs which are valid to be used - * from the chip. - */ - unsigned long *valid_mask; - #if defined(CONFIG_OF_GPIO) /* * If CONFIG_OF_GPIO is enabled, then all GPIO controllers described in -- 2.48.1
Attachment:
signature.asc
Description: PGP signature
