gpio: Hide valid_mask from direct assignments

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@linaro.org>
Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/4547ca90d910d60cab3d56d864d59ddde47a5e93.1741180097.git.mazziesaccount@gmail.com
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 4c15a70d4d80030f7633d562e59ed67b439c6ba2..e5eb3f0ee0710cf35f38a993a6e1841b8d2172b5 100644 (file)
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -672,7 +672,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);
@@ -686,8 +686,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);
@@ -696,7 +696,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;
@@ -704,7 +704,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)
@@ -735,7 +735,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);
 
@@ -743,9 +743,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 58af0491e60eb190a200c3fbce31804d6456dd56..a738e6c647d8b6b9b0d7d6054683520621bd6550 100644 (file)
--- 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 e3b59fda62e03223541b4a02f8caae5485c02815..e6e5304c99ca4241b0e729f41d1b605fff11de22 100644 (file)
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -514,14 +514,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