1- From e6d479eeaa03b9073304c0475dd72d781911b196 Mon Sep 17 00:00:00 2001
1+ From 770d943aac95c337f91280425f154846a9a41ef6 Mon Sep 17 00:00:00 2001
22From: Viacheslav Bocharov <v@baodeep.com>
33Date: Tue, 9 Jun 2026 16:37:20 +0300
44Subject: [PATCH] gpio: shared-proxy: always serialize with a sleeping mutex
@@ -39,6 +39,14 @@ underlying GPIO through the cansleep value accessors: those are valid
3939for both sleeping and non-sleeping chips, so value access keeps working
4040on fast controllers, at the cost of no longer being atomic.
4141
42+ With every vote edge now driven through the cansleep value setter,
43+ gpio_shared_proxy_set_unlocked() no longer needs a per-call setter: drop
44+ its set_func callback and call gpiod_set_value_cansleep() directly. The
45+ shared direction_output path reaches it only once the line is already an
46+ output, so driving the value there is equivalent to re-issuing
47+ gpiod_direction_output(), without the redundant per-edge re-assertion of
48+ drive config and bias.
49+
4250This is observable: consumers gating on gpiod_cansleep() take their
4351sleeping branch on a proxied GPIO (mmc-pwrseq-emmc skips its
4452emergency-restart reset handler; its normal reset is unaffected), and
@@ -59,39 +67,129 @@ Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
5967Closes: https://lore.kernel.org/all/00107523-7737-4b92-a785-14ce4e93b8cb@samsung.com/
6068Signed-off-by: Viacheslav Bocharov <v@baodeep.com>
6169---
62- drivers/gpio/gpio-shared-proxy.c | 43 +++++++----- --------------------
63- drivers/gpio/gpiolib-shared.c | 9 ++-- ---
64- drivers/gpio/gpiolib-shared.h | 31 +++++++++--- -----------
65- 3 files changed, 23 insertions(+), 60 deletions(-)
70+ drivers/gpio/gpio-shared-proxy.c | 76 ++++++++++++ --------------------
71+ drivers/gpio/gpiolib-shared.c | 9 +---
72+ drivers/gpio/gpiolib-shared.h | 28 + -----------
73+ 3 files changed, 32 insertions(+), 81 deletions(-)
6674
6775diff --git a/drivers/gpio/gpio-shared-proxy.c b/drivers/gpio/gpio-shared-proxy.c
68- index 6941e4be6cf1..856e5b9d6163 100644
76+ index 6941e4be6cf1..10ca2ef77ef3 100644
6977--- a/drivers/gpio/gpio-shared-proxy.c
7078+++ b/drivers/gpio/gpio-shared-proxy.c
71- @@ -109,7 +109,7 @@ static void gpio_shared_proxy_free(struct gpio_chip *gc, unsigned int offset)
79+ @@ -9,8 +9,10 @@
80+ #include <linux/err.h>
81+ #include <linux/gpio/consumer.h>
82+ #include <linux/gpio/driver.h>
83+ + #include <linux/lockdep.h>
84+ #include <linux/mod_devicetable.h>
85+ #include <linux/module.h>
86+ + #include <linux/mutex.h>
87+ #include <linux/string_choices.h>
88+ #include <linux/types.h>
89+
90+ @@ -24,15 +26,13 @@ struct gpio_shared_proxy_data {
91+ };
92+
93+ static int
94+ - gpio_shared_proxy_set_unlocked(struct gpio_shared_proxy_data *proxy,
95+ - int (*set_func)(struct gpio_desc *desc, int value),
96+ - int value)
97+ + gpio_shared_proxy_set_unlocked(struct gpio_shared_proxy_data *proxy, int value)
98+ {
99+ struct gpio_shared_desc *shared_desc = proxy->shared_desc;
100+ struct gpio_desc *desc = shared_desc->desc;
101+ int ret = 0;
102+
103+ - gpio_shared_lockdep_assert(shared_desc);
104+ + lockdep_assert_held(&shared_desc->mutex);
105+
106+ if (value) {
107+ /* User wants to set value to high. */
108+ @@ -46,7 +46,7 @@ gpio_shared_proxy_set_unlocked(struct gpio_shared_proxy_data *proxy,
109+ * Current value is low, need to actually set value
110+ * to high.
111+ */
112+ - ret = set_func(desc, 1);
113+ + ret = gpiod_set_value_cansleep(desc, 1);
114+ if (ret)
115+ goto out;
116+ }
117+ @@ -65,7 +65,7 @@ gpio_shared_proxy_set_unlocked(struct gpio_shared_proxy_data *proxy,
118+ /* We previously voted for high. */
119+ if (shared_desc->highcnt == 1) {
120+ /* This is the last remaining vote for high, set value to low. */
121+ - ret = set_func(desc, 0);
122+ + ret = gpiod_set_value_cansleep(desc, 0);
123+ if (ret)
124+ goto out;
125+ }
126+ @@ -89,7 +89,7 @@ static int gpio_shared_proxy_request(struct gpio_chip *gc, unsigned int offset)
127+ struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
128+ struct gpio_shared_desc *shared_desc = proxy->shared_desc;
129+
130+ - guard(gpio_shared_desc_lock)(shared_desc);
131+ + guard(mutex)(&shared_desc->mutex);
132+
133+ proxy->shared_desc->usecnt++;
134+
135+ @@ -105,11 +105,10 @@ static void gpio_shared_proxy_free(struct gpio_chip *gc, unsigned int offset)
136+ struct gpio_shared_desc *shared_desc = proxy->shared_desc;
137+ int ret;
138+
139+ - guard(gpio_shared_desc_lock)(shared_desc);
140+ + guard(mutex)(&shared_desc->mutex);
72141
73142 if (proxy->voted_high) {
74- ret = gpio_shared_proxy_set_unlocked(proxy,
143+ - ret = gpio_shared_proxy_set_unlocked(proxy,
75144- shared_desc->can_sleep ? gpiod_set_value_cansleep : gpiod_set_value, 0);
76- + gpiod_set_value_cansleep , 0);
145+ + ret = gpio_shared_proxy_set_unlocked(proxy , 0);
77146 if (ret)
78147 dev_err(proxy->dev,
79148 "Failed to unset the shared GPIO value on release: %d\n", ret);
80- @@ -222,13 +222,6 @@ static int gpio_shared_proxy_direction_output(struct gpio_chip *gc,
81- return gpio_shared_proxy_set_unlocked(proxy, gpiod_direction_output, value);
82- }
149+ @@ -129,7 +128,7 @@ static int gpio_shared_proxy_set_config(struct gpio_chip *gc,
150+ struct gpio_desc *desc = shared_desc->desc;
151+ int ret;
152+
153+ - guard(gpio_shared_desc_lock)(shared_desc);
154+ + guard(mutex)(&shared_desc->mutex);
155+
156+ if (shared_desc->usecnt > 1) {
157+ if (shared_desc->cfg != cfg) {
158+ @@ -157,7 +156,7 @@ static int gpio_shared_proxy_direction_input(struct gpio_chip *gc,
159+ struct gpio_desc *desc = shared_desc->desc;
160+ int dir;
161+
162+ - guard(gpio_shared_desc_lock)(shared_desc);
163+ + guard(mutex)(&shared_desc->mutex);
164+
165+ if (shared_desc->usecnt == 1) {
166+ dev_dbg(proxy->dev,
167+ @@ -187,7 +186,7 @@ static int gpio_shared_proxy_direction_output(struct gpio_chip *gc,
168+ struct gpio_desc *desc = shared_desc->desc;
169+ int ret, dir;
83170
171+ - guard(gpio_shared_desc_lock)(shared_desc);
172+ + guard(mutex)(&shared_desc->mutex);
173+
174+ if (shared_desc->usecnt == 1) {
175+ dev_dbg(proxy->dev,
176+ @@ -219,14 +218,7 @@ static int gpio_shared_proxy_direction_output(struct gpio_chip *gc,
177+ return -EPERM;
178+ }
179+
180+ - return gpio_shared_proxy_set_unlocked(proxy, gpiod_direction_output, value);
181+ - }
182+ -
84183- static int gpio_shared_proxy_get(struct gpio_chip *gc, unsigned int offset)
85184- {
86185- struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
87186-
88187- return gpiod_get_value(proxy->shared_desc->desc);
89- - }
90- -
188+ + return gpio_shared_proxy_set_unlocked(proxy, value);
189+ }
190+
91191 static int gpio_shared_proxy_get_cansleep(struct gpio_chip *gc,
92- unsigned int offset)
93- {
94- @@ -237,29 +230,15 @@ static int gpio_shared_proxy_get_cansleep(struct gpio_chip *gc,
192+ @@ -237,29 +229,14 @@ static int gpio_shared_proxy_get_cansleep(struct gpio_chip *gc,
95193 return gpiod_get_value_cansleep(proxy->shared_desc->desc);
96194 }
97195
@@ -118,19 +216,27 @@ index 6941e4be6cf1..856e5b9d6163 100644
118216 struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
119217
120218- return gpio_shared_proxy_do_set(proxy, gpiod_set_value_cansleep, value);
121- + guard(gpio_shared_desc_lock)( proxy->shared_desc);
219+ + guard(mutex)(& proxy->shared_desc->mutex );
122220+
123- + return gpio_shared_proxy_set_unlocked(proxy, gpiod_set_value_cansleep,
124- + value);
221+ + return gpio_shared_proxy_set_unlocked(proxy, value);
125222 }
126223
127224 static int gpio_shared_proxy_get_direction(struct gpio_chip *gc,
128- @@ -302,20 +281,16 @@ static int gpio_shared_proxy_probe(struct auxiliary_device *adev,
225+ @@ -302,20 +279,25 @@ static int gpio_shared_proxy_probe(struct auxiliary_device *adev,
129226 gc->label = dev_name(dev);
130227 gc->parent = dev;
131228 gc->owner = THIS_MODULE;
132229- gc->can_sleep = shared_desc->can_sleep;
133- + /* Always a sleeping gpiochip: see the lock comment in gpiolib-shared.h. */
230+ + /*
231+ + * Under the descriptor mutex the proxy may call
232+ + * gpiod_set_config()/gpiod_direction_*(), which can reach pinctrl
233+ + * paths that take a mutex (e.g. gpiod_set_config() ->
234+ + * gpiochip_generic_config() -> pinctrl_gpio_set_config()), independent
235+ + * of the underlying chip's can_sleep. So the descriptor lock must be a
236+ + * mutex and the proxy gpiochip is therefore always sleeping; drive the
237+ + * underlying GPIO through the cansleep value accessors, which are valid
238+ + * for both sleeping and non-sleeping chips.
239+ + */
134240+ gc->can_sleep = true;
135241
136242 gc->request = gpio_shared_proxy_request;
@@ -178,18 +284,21 @@ index de72776fb154..495bd3d0ddf0 100644
178284 return shared_desc;
179285 }
180286diff --git a/drivers/gpio/gpiolib-shared.h b/drivers/gpio/gpiolib-shared.h
181- index 15e72a8dcdb1..5c725118b1af 100644
287+ index 15e72a8dcdb1..bbdc0ab7b647 100644
182288--- a/drivers/gpio/gpiolib-shared.h
183289+++ b/drivers/gpio/gpiolib-shared.h
184- @@ -6,7 +6,6 @@
185- #include <linux/cleanup.h>
186- #include <linux/lockdep.h>
290+ @@ -3,10 +3,7 @@
291+ #ifndef __LINUX_GPIO_SHARED_H
292+ #define __LINUX_GPIO_SHARED_H
293+
294+ - #include <linux/cleanup.h>
295+ - #include <linux/lockdep.h>
187296 #include <linux/mutex.h>
188297- #include <linux/spinlock.h>
189298
190299 struct gpio_device;
191300 struct gpio_desc;
192- @@ -42,35 +41,29 @@ static inline int gpio_shared_add_proxy_lookup(struct device *consumer,
301+ @@ -42,35 +39,12 @@ static inline int gpio_shared_add_proxy_lookup(struct device *consumer,
193302
194303 struct gpio_shared_desc {
195304 struct gpio_desc *desc;
@@ -206,15 +315,7 @@ index 15e72a8dcdb1..5c725118b1af 100644
206315
207316 struct gpio_shared_desc *devm_gpiod_shared_get(struct device *dev);
208317
209- + /*
210- + * Under this lock the proxy may call gpiod_set_config()/gpiod_direction_*(),
211- + * which can reach pinctrl paths that take a mutex (e.g. gpiod_set_config() ->
212- + * gpiochip_generic_config() -> pinctrl_gpio_set_config()), independent of the
213- + * underlying chip's can_sleep. A spinlock would run that sleeping call from
214- + * atomic context, so the descriptor lock must be a mutex and the proxy
215- + * gpiochip is therefore sleeping (can_sleep=true).
216- + */
217- DEFINE_LOCK_GUARD_1(gpio_shared_desc_lock, struct gpio_shared_desc,
318+ - DEFINE_LOCK_GUARD_1(gpio_shared_desc_lock, struct gpio_shared_desc,
218319- if (_T->lock->can_sleep)
219320- mutex_lock(&_T->lock->mutex);
220321- else
@@ -224,18 +325,15 @@ index 15e72a8dcdb1..5c725118b1af 100644
224325- else
225326- spin_unlock_irqrestore(&_T->lock->spinlock, _T->flags),
226327- unsigned long flags)
227- + mutex_lock(&_T->lock->mutex),
228- + mutex_unlock(&_T->lock->mutex))
229-
230- static inline void gpio_shared_lockdep_assert(struct gpio_shared_desc *shared_desc)
231- {
328+ -
329+ - static inline void gpio_shared_lockdep_assert(struct gpio_shared_desc *shared_desc)
330+ - {
232331- if (shared_desc->can_sleep)
233332- lockdep_assert_held(&shared_desc->mutex);
234333- else
235334- lockdep_assert_held(&shared_desc->spinlock);
236- + lockdep_assert_held(&shared_desc->mutex);
237- }
238-
335+ - }
336+ -
239337 #endif /* __LINUX_GPIO_SHARED_H */
240338- -
2413392.54.0
0 commit comments