Skip to content

Commit 534a2c6

Browse files
committed
Merge tag 'gpio-fixes-for-v6.14-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux
Pull gpio fixes from Bartosz Golaszewski: "There are two fixes for GPIO core: one adds missing retval checks to older code, while the second adds SRCU synchronization to legs in code that were missed during the big rework a few cycles back. There's also one small driver fix: - check the return value of the get_direction() callback in struct gpio_chip - protect the multi-line get/set legs in GPIO core with SRCU - fix a race condition in gpio-vf610" * tag 'gpio-fixes-for-v6.14-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux: gpiolib: don't bail out if get_direction() fails in gpiochip_add_data() gpiolib: protect gpio_chip with SRCU in array_info paths in multi get/set gpio: vf610: add locking to gpio direction functions gpiolib: check the return value of gpio_chip::get_direction()
2 parents 3344260 + 96fa9ec commit 534a2c6

File tree

3 files changed

+76
-32
lines changed

3 files changed

+76
-32
lines changed

drivers/gpio/gpio-vf610.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ struct vf610_gpio_port {
3636
struct clk *clk_port;
3737
struct clk *clk_gpio;
3838
int irq;
39+
spinlock_t lock; /* protect gpio direction registers */
3940
};
4041

4142
#define GPIO_PDOR 0x00
@@ -124,6 +125,7 @@ static int vf610_gpio_direction_input(struct gpio_chip *chip, unsigned int gpio)
124125
u32 val;
125126

126127
if (port->sdata->have_paddr) {
128+
guard(spinlock_irqsave)(&port->lock);
127129
val = vf610_gpio_readl(port->gpio_base + GPIO_PDDR);
128130
val &= ~mask;
129131
vf610_gpio_writel(val, port->gpio_base + GPIO_PDDR);
@@ -142,6 +144,7 @@ static int vf610_gpio_direction_output(struct gpio_chip *chip, unsigned int gpio
142144
vf610_gpio_set(chip, gpio, value);
143145

144146
if (port->sdata->have_paddr) {
147+
guard(spinlock_irqsave)(&port->lock);
145148
val = vf610_gpio_readl(port->gpio_base + GPIO_PDDR);
146149
val |= mask;
147150
vf610_gpio_writel(val, port->gpio_base + GPIO_PDDR);
@@ -297,6 +300,7 @@ static int vf610_gpio_probe(struct platform_device *pdev)
297300
return -ENOMEM;
298301

299302
port->sdata = device_get_match_data(dev);
303+
spin_lock_init(&port->lock);
300304

301305
dual_base = port->sdata->have_dual_base;
302306

drivers/gpio/gpiolib.c

Lines changed: 70 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,8 +1057,19 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
10571057
desc->gdev = gdev;
10581058

10591059
if (gc->get_direction && gpiochip_line_is_valid(gc, desc_index)) {
1060-
assign_bit(FLAG_IS_OUT,
1061-
&desc->flags, !gc->get_direction(gc, desc_index));
1060+
ret = gc->get_direction(gc, desc_index);
1061+
if (ret < 0)
1062+
/*
1063+
* FIXME: Bail-out here once all GPIO drivers
1064+
* are updated to not return errors in
1065+
* situations that can be considered normal
1066+
* operation.
1067+
*/
1068+
dev_warn(&gdev->dev,
1069+
"%s: get_direction failed: %d\n",
1070+
__func__, ret);
1071+
1072+
assign_bit(FLAG_IS_OUT, &desc->flags, !ret);
10621073
} else {
10631074
assign_bit(FLAG_IS_OUT,
10641075
&desc->flags, !gc->direction_input);
@@ -2728,13 +2739,18 @@ int gpiod_direction_input_nonotify(struct gpio_desc *desc)
27282739
if (guard.gc->direction_input) {
27292740
ret = guard.gc->direction_input(guard.gc,
27302741
gpio_chip_hwgpio(desc));
2731-
} else if (guard.gc->get_direction &&
2732-
(guard.gc->get_direction(guard.gc,
2733-
gpio_chip_hwgpio(desc)) != 1)) {
2734-
gpiod_warn(desc,
2735-
"%s: missing direction_input() operation and line is output\n",
2736-
__func__);
2737-
return -EIO;
2742+
} else if (guard.gc->get_direction) {
2743+
ret = guard.gc->get_direction(guard.gc,
2744+
gpio_chip_hwgpio(desc));
2745+
if (ret < 0)
2746+
return ret;
2747+
2748+
if (ret != GPIO_LINE_DIRECTION_IN) {
2749+
gpiod_warn(desc,
2750+
"%s: missing direction_input() operation and line is output\n",
2751+
__func__);
2752+
return -EIO;
2753+
}
27382754
}
27392755
if (ret == 0) {
27402756
clear_bit(FLAG_IS_OUT, &desc->flags);
@@ -2771,12 +2787,18 @@ static int gpiod_direction_output_raw_commit(struct gpio_desc *desc, int value)
27712787
gpio_chip_hwgpio(desc), val);
27722788
} else {
27732789
/* Check that we are in output mode if we can */
2774-
if (guard.gc->get_direction &&
2775-
guard.gc->get_direction(guard.gc, gpio_chip_hwgpio(desc))) {
2776-
gpiod_warn(desc,
2777-
"%s: missing direction_output() operation\n",
2778-
__func__);
2779-
return -EIO;
2790+
if (guard.gc->get_direction) {
2791+
ret = guard.gc->get_direction(guard.gc,
2792+
gpio_chip_hwgpio(desc));
2793+
if (ret < 0)
2794+
return ret;
2795+
2796+
if (ret != GPIO_LINE_DIRECTION_OUT) {
2797+
gpiod_warn(desc,
2798+
"%s: missing direction_output() operation\n",
2799+
__func__);
2800+
return -EIO;
2801+
}
27802802
}
27812803
/*
27822804
* If we can't actively set the direction, we are some
@@ -3129,6 +3151,8 @@ static int gpiod_get_raw_value_commit(const struct gpio_desc *desc)
31293151
static int gpio_chip_get_multiple(struct gpio_chip *gc,
31303152
unsigned long *mask, unsigned long *bits)
31313153
{
3154+
lockdep_assert_held(&gc->gpiodev->srcu);
3155+
31323156
if (gc->get_multiple)
31333157
return gc->get_multiple(gc, mask, bits);
31343158
if (gc->get) {
@@ -3159,6 +3183,7 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
31593183
struct gpio_array *array_info,
31603184
unsigned long *value_bitmap)
31613185
{
3186+
struct gpio_chip *gc;
31623187
int ret, i = 0;
31633188

31643189
/*
@@ -3170,10 +3195,15 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
31703195
array_size <= array_info->size &&
31713196
(void *)array_info == desc_array + array_info->size) {
31723197
if (!can_sleep)
3173-
WARN_ON(array_info->chip->can_sleep);
3198+
WARN_ON(array_info->gdev->can_sleep);
3199+
3200+
guard(srcu)(&array_info->gdev->srcu);
3201+
gc = srcu_dereference(array_info->gdev->chip,
3202+
&array_info->gdev->srcu);
3203+
if (!gc)
3204+
return -ENODEV;
31743205

3175-
ret = gpio_chip_get_multiple(array_info->chip,
3176-
array_info->get_mask,
3206+
ret = gpio_chip_get_multiple(gc, array_info->get_mask,
31773207
value_bitmap);
31783208
if (ret)
31793209
return ret;
@@ -3454,6 +3484,8 @@ static void gpiod_set_raw_value_commit(struct gpio_desc *desc, bool value)
34543484
static void gpio_chip_set_multiple(struct gpio_chip *gc,
34553485
unsigned long *mask, unsigned long *bits)
34563486
{
3487+
lockdep_assert_held(&gc->gpiodev->srcu);
3488+
34573489
if (gc->set_multiple) {
34583490
gc->set_multiple(gc, mask, bits);
34593491
} else {
@@ -3471,6 +3503,7 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
34713503
struct gpio_array *array_info,
34723504
unsigned long *value_bitmap)
34733505
{
3506+
struct gpio_chip *gc;
34743507
int i = 0;
34753508

34763509
/*
@@ -3482,14 +3515,19 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
34823515
array_size <= array_info->size &&
34833516
(void *)array_info == desc_array + array_info->size) {
34843517
if (!can_sleep)
3485-
WARN_ON(array_info->chip->can_sleep);
3518+
WARN_ON(array_info->gdev->can_sleep);
3519+
3520+
guard(srcu)(&array_info->gdev->srcu);
3521+
gc = srcu_dereference(array_info->gdev->chip,
3522+
&array_info->gdev->srcu);
3523+
if (!gc)
3524+
return -ENODEV;
34863525

34873526
if (!raw && !bitmap_empty(array_info->invert_mask, array_size))
34883527
bitmap_xor(value_bitmap, value_bitmap,
34893528
array_info->invert_mask, array_size);
34903529

3491-
gpio_chip_set_multiple(array_info->chip, array_info->set_mask,
3492-
value_bitmap);
3530+
gpio_chip_set_multiple(gc, array_info->set_mask, value_bitmap);
34933531

34943532
i = find_first_zero_bit(array_info->set_mask, array_size);
34953533
if (i == array_size)
@@ -4751,9 +4789,10 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
47514789
{
47524790
struct gpio_desc *desc;
47534791
struct gpio_descs *descs;
4792+
struct gpio_device *gdev;
47544793
struct gpio_array *array_info = NULL;
4755-
struct gpio_chip *gc;
47564794
int count, bitmap_size;
4795+
unsigned long dflags;
47574796
size_t descs_size;
47584797

47594798
count = gpiod_count(dev, con_id);
@@ -4774,16 +4813,16 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
47744813

47754814
descs->desc[descs->ndescs] = desc;
47764815

4777-
gc = gpiod_to_chip(desc);
4816+
gdev = gpiod_to_gpio_device(desc);
47784817
/*
47794818
* If pin hardware number of array member 0 is also 0, select
47804819
* its chip as a candidate for fast bitmap processing path.
47814820
*/
47824821
if (descs->ndescs == 0 && gpio_chip_hwgpio(desc) == 0) {
47834822
struct gpio_descs *array;
47844823

4785-
bitmap_size = BITS_TO_LONGS(gc->ngpio > count ?
4786-
gc->ngpio : count);
4824+
bitmap_size = BITS_TO_LONGS(gdev->ngpio > count ?
4825+
gdev->ngpio : count);
47874826

47884827
array = krealloc(descs, descs_size +
47894828
struct_size(array_info, invert_mask, 3 * bitmap_size),
@@ -4803,7 +4842,7 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
48034842

48044843
array_info->desc = descs->desc;
48054844
array_info->size = count;
4806-
array_info->chip = gc;
4845+
array_info->gdev = gdev;
48074846
bitmap_set(array_info->get_mask, descs->ndescs,
48084847
count - descs->ndescs);
48094848
bitmap_set(array_info->set_mask, descs->ndescs,
@@ -4816,7 +4855,7 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
48164855
continue;
48174856

48184857
/* Unmark array members which don't belong to the 'fast' chip */
4819-
if (array_info->chip != gc) {
4858+
if (array_info->gdev != gdev) {
48204859
__clear_bit(descs->ndescs, array_info->get_mask);
48214860
__clear_bit(descs->ndescs, array_info->set_mask);
48224861
}
@@ -4839,9 +4878,10 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
48394878
array_info->set_mask);
48404879
}
48414880
} else {
4881+
dflags = READ_ONCE(desc->flags);
48424882
/* Exclude open drain or open source from fast output */
4843-
if (gpiochip_line_is_open_drain(gc, descs->ndescs) ||
4844-
gpiochip_line_is_open_source(gc, descs->ndescs))
4883+
if (test_bit(FLAG_OPEN_DRAIN, &dflags) ||
4884+
test_bit(FLAG_OPEN_SOURCE, &dflags))
48454885
__clear_bit(descs->ndescs,
48464886
array_info->set_mask);
48474887
/* Identify 'fast' pins which require invertion */
@@ -4853,7 +4893,7 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
48534893
if (array_info)
48544894
dev_dbg(dev,
48554895
"GPIO array info: chip=%s, size=%d, get_mask=%lx, set_mask=%lx, invert_mask=%lx\n",
4856-
array_info->chip->label, array_info->size,
4896+
array_info->gdev->label, array_info->size,
48574897
*array_info->get_mask, *array_info->set_mask,
48584898
*array_info->invert_mask);
48594899
return descs;

drivers/gpio/gpiolib.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ extern const char *const gpio_suffixes[];
114114
*
115115
* @desc: Array of pointers to the GPIO descriptors
116116
* @size: Number of elements in desc
117-
* @chip: Parent GPIO chip
117+
* @gdev: Parent GPIO device
118118
* @get_mask: Get mask used in fastpath
119119
* @set_mask: Set mask used in fastpath
120120
* @invert_mask: Invert mask used in fastpath
@@ -126,7 +126,7 @@ extern const char *const gpio_suffixes[];
126126
struct gpio_array {
127127
struct gpio_desc **desc;
128128
unsigned int size;
129-
struct gpio_chip *chip;
129+
struct gpio_device *gdev;
130130
unsigned long *get_mask;
131131
unsigned long *set_mask;
132132
unsigned long invert_mask[];

0 commit comments

Comments
 (0)