gpiolib: protect gpio_chip with SRCU in array_info paths in multi get/set
Subsystems (2)
PATCH v2 1/1 gpiolib: protect gpio_chip with SRCU in array_info paths in multi get/set
From: Bartosz Golaszewski <[email protected]>
During the locking rework in GPIOLIB, we omitted one important use-case,
namely: setting and getting values for GPIO descriptor arrays with array_info present.
This patch does two things: first it makes struct gpio_array store the address of the underlying GPIO device and not chip. Next: it protects the chip with SRCU from removal in gpiod_get_array_value_complex() and gpiod_set_array_value_complex().
---
Changes in v2:
- fix sparse warning about differing address spaces by checking for open-drain/source flags directly in the descriptor
drivers/gpio/gpiolib.c | 48 +++++++++++++++++++++++++++++------------- drivers/gpio/gpiolib.h | 4 ++-- 2 files changed, 35 insertions(+), 17 deletions(-)
drivers/gpio/gpiolib.c +33 -15
@@ -3128,6 +3128,8 @@ static int gpiod_get_raw_value_commit(const struct gpio_desc *desc)static int gpio_chip_get_multiple (struct gpio_chip *gc,unsigned long *mask,unsigned long *bits) { +lockdep_assert_held (&gc->gpiodev->srcu); +if (gc->get_multiple)return gc->get_multiple (gc, mask, bits);if (gc->get) {
@@ -3158,6 +3160,7 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,struct gpio_array *array_info, unsigned long *value_bitmap) { + struct gpio_chip *gc;int ret, i =0 ;/*
@@ -3169,10 +3172,15 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep, array_size <= array_info->size && (void *)array_info == desc_array + array_info->size) {if (!can_sleep) -WARN_ON (array_info->chip->can_sleep); +WARN_ON (array_info->gdev->can_sleep); - ret =gpio_chip_get_multiple (array_info->chip, - array_info->get_mask, +guard (srcu)(&array_info->gdev->srcu); + gc =srcu_dereference (array_info->gdev->chip, + &array_info->gdev->srcu); +if (!gc) +return -ENODEV ; + + ret =gpio_chip_get_multiple (gc, array_info->get_mask, value_bitmap);if (ret)return ret;
@@ -3453,6 +3461,8 @@ static void gpiod_set_raw_value_commit(struct gpio_desc *desc, bool value)static void gpio_chip_set_multiple (struct gpio_chip *gc,unsigned long *mask,unsigned long *bits) { +lockdep_assert_held (&gc->gpiodev->srcu); +if (gc->set_multiple) { gc->set_multiple (gc, mask, bits); }else {
@@ -3470,6 +3480,7 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,struct gpio_array *array_info, unsigned long *value_bitmap) { + struct gpio_chip *gc;int i =0 ;/*
@@ -3481,14 +3492,19 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep, array_size <= array_info->size && (void *)array_info == desc_array + array_info->size) {if (!can_sleep) -WARN_ON (array_info->chip->can_sleep); +WARN_ON (array_info->gdev->can_sleep); + +guard (srcu)(&array_info->gdev->srcu); + gc =srcu_dereference (array_info->gdev->chip, + &array_info->gdev->srcu); +if (!gc) +return -ENODEV ;if (!raw && !bitmap_empty (array_info->invert_mask, array_size))bitmap_xor (value_bitmap, value_bitmap, array_info->invert_mask, array_size); -gpio_chip_set_multiple (array_info->chip, array_info->set_mask, - value_bitmap); +gpio_chip_set_multiple (gc, array_info->set_mask, value_bitmap); i =find_first_zero_bit (array_info->set_mask, array_size);if (i == array_size)
@@ -4750,9 +4766,10 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev, {struct gpio_desc *desc;struct gpio_descs *descs; +struct gpio_device *gdev;struct gpio_array *array_info =NULL ; -struct gpio_chip *gc;int count, bitmap_size; +unsigned long dflags;size_t descs_size; count =gpiod_count (dev, con_id);
@@ -4773,7 +4790,7 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev, descs->desc[descs->ndescs] = desc; - gc =gpiod_to_chip (desc); + gdev =gpiod_to_gpio_device (desc);/* * If pin hardware number of array member 0 is also 0, select * its chip as a candidate for fast bitmap processing path.
@@ -4781,8 +4798,8 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,if (descs->ndescs ==0 &&gpio_chip_hwgpio (desc) ==0 ) {struct gpio_descs *array; - bitmap_size =BITS_TO_LONGS (gc->ngpio > count ? - gc->ngpio : count); + bitmap_size =BITS_TO_LONGS (gdev->ngpio > count ? + gdev->ngpio : count); array = krealloc(descs, descs_size + struct_size(array_info, invert_mask, 3 * bitmap_size),
@@ -4802,7 +4819,7 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev, array_info->desc = descs->desc; array_info->size = count; - array_info->chip = gc; + array_info->gdev = gdev;bitmap_set (array_info->get_mask, descs->ndescs, count - descs->ndescs); bitmap_set(array_info->set_mask, descs->ndescs,
@@ -4815,7 +4832,7 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,continue ;/* Unmark array members which don't belong to the 'fast' chip */ -if (array_info->chip != gc) { +if (array_info->gdev != gdev) {__clear_bit (descs->ndescs, array_info->get_mask);__clear_bit (descs->ndescs, array_info->set_mask); }
@@ -4838,9 +4855,10 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev, array_info->set_mask); } }else { + dflags =READ_ONCE (desc->flags);/* Exclude open drain or open source from fast output */ -if (gpiochip_line_is_open_drain (gc, descs->ndescs) || -gpiochip_line_is_open_source (gc, descs->ndescs)) +if (test_bit (FLAG_OPEN_DRAIN , &dflags) || +test_bit (FLAG_OPEN_SOURCE , &dflags))__clear_bit (descs->ndescs, array_info->set_mask);/* Identify 'fast' pins which require invertion */
@@ -4852,7 +4870,7 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,if (array_info)dev_dbg (dev,"GPIO array info: chip=%s, size=%d, get_mask=%lx, set_mask=%lx, invert_mask=%lx\n" , - array_info->chip->label, array_info->size, + array_info->gdev->label, array_info->size, *array_info->get_mask, *array_info->set_mask, *array_info->invert_mask);return descs;
drivers/gpio/gpiolib.h +2 -2
@@ -114,7 +114,7 @@ extern const char *const gpio_suffixes[];* * @desc: Array of pointers to the GPIO descriptors * @size: Number of elements in desc -* @chip: Parent GPIO chip +* @gdev: Parent GPIO device * @get_mask: Get mask used in fastpath * @set_mask: Set mask used in fastpath * @invert_mask: Invert mask used in fastpath
@@ -126,7 +126,7 @@ extern const char *const gpio_suffixes[];struct gpio_array {struct gpio_desc **desc;unsigned int size; -struct gpio_chip *chip; +struct gpio_device *gdev;unsigned long *get_mask;unsigned long *set_mask;unsigned long invert_mask[];
Signature · 1 line
2.45.2
Bartosz Golaszewski
AppliedFrom: Bartosz Golaszewski <[email protected]>
Quoted 10 lines · Bartosz Golaszewski
During the locking rework in GPIOLIB, we omitted one important use-case,
namely: setting and getting values for GPIO descriptor arrays witharray_infopresent.This patch does two things: first it makes struct
gpio_arraystore the address of the underlying GPIO device and not chip. Next: it protects the chip with SRCU from removal ingpiod_get_array_value_complex()andgpiod_set_array_value_complex().[...]
Applied, thanks!
[1/1] gpiolib: protect gpio_chip with SRCU in array_info paths in multi get/set
commit: 81570d6a7ad37033c7895811551a5a9023706eda
Best regards,
Signature · 1 line
Bartosz Golaszewski <[email protected]>