Skip to content

Commit 4941648

Browse files
author
Bartosz Golaszewski
committed
gpio: shared: allow sharing a reset-gpios pin between reset-gpio and gpiolib
We currently support sharing GPIOs between multiple devices whose drivers use either the GPIOLIB API *OR* the reset control API but not both at the same time. There's an unlikely corner-case where a reset-gpios pin can be shared by one driver using the GPIOLIB API and a second using the reset API. This will currently not work as the reset-gpio consumers are not described in firmware at the time of scanning so the shared GPIO just chooses one of the proxies created for the consumers when the reset-gpio driver performs the lookup. This can of course conflict in the case described above. In order to fix it: if we deal with the "reset-gpios" pin that's shared acconding to the firmware node setup, create a proxy for each described consumer as well as another one for the potential reset-gpio device. To that end: rework the matching to take this into account. Fixes: 7b78b26 ("gpio: shared: handle the reset-gpios corner case") Link: https://lore.kernel.org/r/20251222-gpio-shared-reset-gpio-proxy-v1-3-8d4bba7d8c14@oss.qualcomm.com Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
1 parent cb0451e commit 4941648

1 file changed

Lines changed: 129 additions & 53 deletions

File tree

drivers/gpio/gpiolib-shared.c

Lines changed: 129 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,56 @@ gpio_shared_find_entry(struct fwnode_handle *controller_node,
7676
return NULL;
7777
}
7878

79+
static struct gpio_shared_ref *gpio_shared_make_ref(struct fwnode_handle *fwnode,
80+
const char *con_id,
81+
enum gpiod_flags flags)
82+
{
83+
char *con_id_cpy __free(kfree) = NULL;
84+
85+
struct gpio_shared_ref *ref __free(kfree) = kzalloc(sizeof(*ref), GFP_KERNEL);
86+
if (!ref)
87+
return NULL;
88+
89+
if (con_id) {
90+
con_id_cpy = kstrdup(con_id, GFP_KERNEL);
91+
if (!con_id_cpy)
92+
return NULL;
93+
}
94+
95+
ref->dev_id = ida_alloc(&gpio_shared_ida, GFP_KERNEL);
96+
if (ref->dev_id < 0)
97+
return NULL;
98+
99+
ref->flags = flags;
100+
ref->con_id = no_free_ptr(con_id_cpy);
101+
ref->fwnode = fwnode;
102+
mutex_init(&ref->lock);
103+
104+
return no_free_ptr(ref);
105+
}
106+
107+
static int gpio_shared_setup_reset_proxy(struct gpio_shared_entry *entry,
108+
enum gpiod_flags flags)
109+
{
110+
struct gpio_shared_ref *ref;
111+
112+
list_for_each_entry(ref, &entry->refs, list) {
113+
if (!ref->fwnode && ref->con_id && strcmp(ref->con_id, "reset") == 0)
114+
return 0;
115+
}
116+
117+
ref = gpio_shared_make_ref(NULL, "reset", flags);
118+
if (!ref)
119+
return -ENOMEM;
120+
121+
list_add_tail(&ref->list, &entry->refs);
122+
123+
pr_debug("Created a secondary shared GPIO reference for potential reset-gpio device for GPIO %u at %s\n",
124+
entry->offset, fwnode_get_name(entry->fwnode));
125+
126+
return 0;
127+
}
128+
79129
/* Handle all special nodes that we should ignore. */
80130
static bool gpio_shared_of_node_ignore(struct device_node *node)
81131
{
@@ -106,6 +156,7 @@ static int gpio_shared_of_traverse(struct device_node *curr)
106156
size_t con_id_len, suffix_len;
107157
struct fwnode_handle *fwnode;
108158
struct of_phandle_args args;
159+
struct gpio_shared_ref *ref;
109160
struct property *prop;
110161
unsigned int offset;
111162
const char *suffix;
@@ -138,6 +189,7 @@ static int gpio_shared_of_traverse(struct device_node *curr)
138189

139190
for (i = 0; i < count; i++) {
140191
struct device_node *np __free(device_node) = NULL;
192+
char *con_id __free(kfree) = NULL;
141193

142194
ret = of_parse_phandle_with_args(curr, prop->name,
143195
"#gpio-cells", i,
@@ -182,15 +234,6 @@ static int gpio_shared_of_traverse(struct device_node *curr)
182234
list_add_tail(&entry->list, &gpio_shared_list);
183235
}
184236

185-
struct gpio_shared_ref *ref __free(kfree) =
186-
kzalloc(sizeof(*ref), GFP_KERNEL);
187-
if (!ref)
188-
return -ENOMEM;
189-
190-
ref->fwnode = fwnode_handle_get(of_fwnode_handle(curr));
191-
ref->flags = args.args[1];
192-
mutex_init(&ref->lock);
193-
194237
if (strends(prop->name, "gpios"))
195238
suffix = "-gpios";
196239
else if (strends(prop->name, "gpio"))
@@ -202,27 +245,32 @@ static int gpio_shared_of_traverse(struct device_node *curr)
202245

203246
/* We only set con_id if there's actually one. */
204247
if (strcmp(prop->name, "gpios") && strcmp(prop->name, "gpio")) {
205-
ref->con_id = kstrdup(prop->name, GFP_KERNEL);
206-
if (!ref->con_id)
248+
con_id = kstrdup(prop->name, GFP_KERNEL);
249+
if (!con_id)
207250
return -ENOMEM;
208251

209-
con_id_len = strlen(ref->con_id);
252+
con_id_len = strlen(con_id);
210253
suffix_len = strlen(suffix);
211254

212-
ref->con_id[con_id_len - suffix_len] = '\0';
255+
con_id[con_id_len - suffix_len] = '\0';
213256
}
214257

215-
ref->dev_id = ida_alloc(&gpio_shared_ida, GFP_KERNEL);
216-
if (ref->dev_id < 0) {
217-
kfree(ref->con_id);
258+
ref = gpio_shared_make_ref(fwnode_handle_get(of_fwnode_handle(curr)),
259+
con_id, args.args[1]);
260+
if (!ref)
218261
return -ENOMEM;
219-
}
220262

221263
if (!list_empty(&entry->refs))
222264
pr_debug("GPIO %u at %s is shared by multiple firmware nodes\n",
223265
entry->offset, fwnode_get_name(entry->fwnode));
224266

225-
list_add_tail(&no_free_ptr(ref)->list, &entry->refs);
267+
list_add_tail(&ref->list, &entry->refs);
268+
269+
if (strcmp(prop->name, "reset-gpios") == 0) {
270+
ret = gpio_shared_setup_reset_proxy(entry, args.args[1]);
271+
if (ret)
272+
return ret;
273+
}
226274
}
227275
}
228276

@@ -306,20 +354,16 @@ static bool gpio_shared_dev_is_reset_gpio(struct device *consumer,
306354
struct fwnode_handle *reset_fwnode = dev_fwnode(consumer);
307355
struct fwnode_reference_args ref_args, aux_args;
308356
struct device *parent = consumer->parent;
357+
struct gpio_shared_ref *real_ref;
309358
bool match;
310359
int ret;
311360

361+
lockdep_assert_held(&ref->lock);
362+
312363
/* The reset-gpio device must have a parent AND a firmware node. */
313364
if (!parent || !reset_fwnode)
314365
return false;
315366

316-
/*
317-
* FIXME: use device_is_compatible() once the reset-gpio drivers gains
318-
* a compatible string which it currently does not have.
319-
*/
320-
if (!strstarts(dev_name(consumer), "reset.gpio."))
321-
return false;
322-
323367
/*
324368
* Parent of the reset-gpio auxiliary device is the GPIO chip whose
325369
* fwnode we stored in the entry structure.
@@ -328,33 +372,56 @@ static bool gpio_shared_dev_is_reset_gpio(struct device *consumer,
328372
return false;
329373

330374
/*
331-
* The device associated with the shared reference's firmware node is
332-
* the consumer of the reset control exposed by the reset-gpio device.
333-
* It must have a "reset-gpios" property that's referencing the entry's
334-
* firmware node.
335-
*
336-
* The reference args must agree between the real consumer and the
337-
* auxiliary reset-gpio device.
375+
* Now we need to find the actual pin we want to assign to this
376+
* reset-gpio device. To that end: iterate over the list of references
377+
* of this entry and see if there's one, whose reset-gpios property's
378+
* arguments match the ones from this consumer's node.
338379
*/
339-
ret = fwnode_property_get_reference_args(ref->fwnode, "reset-gpios",
340-
NULL, 2, 0, &ref_args);
341-
if (ret)
342-
return false;
380+
list_for_each_entry(real_ref, &entry->refs, list) {
381+
if (!real_ref->fwnode)
382+
continue;
383+
384+
/*
385+
* The device associated with the shared reference's firmware
386+
* node is the consumer of the reset control exposed by the
387+
* reset-gpio device. It must have a "reset-gpios" property
388+
* that's referencing the entry's firmware node.
389+
*
390+
* The reference args must agree between the real consumer and
391+
* the auxiliary reset-gpio device.
392+
*/
393+
ret = fwnode_property_get_reference_args(real_ref->fwnode,
394+
"reset-gpios",
395+
NULL, 2, 0, &ref_args);
396+
if (ret)
397+
continue;
398+
399+
ret = fwnode_property_get_reference_args(reset_fwnode, "reset-gpios",
400+
NULL, 2, 0, &aux_args);
401+
if (ret) {
402+
fwnode_handle_put(ref_args.fwnode);
403+
continue;
404+
}
405+
406+
match = ((ref_args.fwnode == entry->fwnode) &&
407+
(aux_args.fwnode == entry->fwnode) &&
408+
(ref_args.args[0] == aux_args.args[0]));
343409

344-
ret = fwnode_property_get_reference_args(reset_fwnode, "reset-gpios",
345-
NULL, 2, 0, &aux_args);
346-
if (ret) {
347410
fwnode_handle_put(ref_args.fwnode);
348-
return false;
349-
}
411+
fwnode_handle_put(aux_args.fwnode);
412+
413+
if (!match)
414+
continue;
350415

351-
match = ((ref_args.fwnode == entry->fwnode) &&
352-
(aux_args.fwnode == entry->fwnode) &&
353-
(ref_args.args[0] == aux_args.args[0]));
416+
/*
417+
* Reuse the fwnode of the real device, next time we'll use it
418+
* in the normal path.
419+
*/
420+
ref->fwnode = fwnode_handle_get(real_ref->fwnode);
421+
return true;
422+
}
354423

355-
fwnode_handle_put(ref_args.fwnode);
356-
fwnode_handle_put(aux_args.fwnode);
357-
return match;
424+
return false;
358425
}
359426
#else
360427
static bool gpio_shared_dev_is_reset_gpio(struct device *consumer,
@@ -379,12 +446,20 @@ int gpio_shared_add_proxy_lookup(struct device *consumer, const char *con_id,
379446

380447
list_for_each_entry(entry, &gpio_shared_list, list) {
381448
list_for_each_entry(ref, &entry->refs, list) {
382-
if (!device_match_fwnode(consumer, ref->fwnode) &&
383-
!gpio_shared_dev_is_reset_gpio(consumer, entry, ref))
384-
continue;
385-
386449
guard(mutex)(&ref->lock);
387450

451+
/*
452+
* FIXME: use device_is_compatible() once the reset-gpio
453+
* drivers gains a compatible string which it currently
454+
* does not have.
455+
*/
456+
if (!ref->fwnode && strstarts(dev_name(consumer), "reset.gpio.")) {
457+
if (!gpio_shared_dev_is_reset_gpio(consumer, entry, ref))
458+
continue;
459+
} else if (!device_match_fwnode(consumer, ref->fwnode)) {
460+
continue;
461+
}
462+
388463
if ((!con_id && ref->con_id) || (con_id && !ref->con_id) ||
389464
(con_id && ref->con_id && strcmp(con_id, ref->con_id) != 0))
390465
continue;
@@ -471,8 +546,9 @@ int gpio_device_setup_shared(struct gpio_device *gdev)
471546
entry->offset, gpio_device_get_label(gdev));
472547

473548
list_for_each_entry(ref, &entry->refs, list) {
474-
pr_debug("Setting up a shared GPIO entry for %s\n",
475-
fwnode_get_name(ref->fwnode));
549+
pr_debug("Setting up a shared GPIO entry for %s (con_id: '%s')\n",
550+
fwnode_get_name(ref->fwnode) ?: "(no fwnode)",
551+
ref->con_id ?: "(none)");
476552

477553
ret = gpio_shared_make_adev(gdev, entry, ref);
478554
if (ret)

0 commit comments

Comments
 (0)