From a619cba8c69c434258ff4101d463322cd63e1bdc Mon Sep 17 00:00:00 2001 From: Koichiro Den Date: Fri, 3 Jan 2025 23:18:26 +0900 Subject: [PATCH 1/5] gpio: virtuser: fix missing lookup table cleanups When a virtuser device is created via configfs and the probe fails due to an incorrect lookup table, the table is not removed. This prevents subsequent probe attempts from succeeding, even if the issue is corrected, unless the device is released. Additionally, cleanup is also needed in the less likely case of platform_device_register_full() failure. Besides, a consistent memory leak in lookup_table->dev_id was spotted using kmemleak by toggling the live state between 0 and 1 with a correct lookup table. Introduce gpio_virtuser_remove_lookup_table() as the counterpart to the existing gpio_virtuser_make_lookup_table() and call it from all necessary points to ensure proper cleanup. Fixes: 91581c4b3f29 ("gpio: virtuser: new virtual testing driver for the GPIO API") Signed-off-by: Koichiro Den Link: https://lore.kernel.org/r/20250103141829.430662-2-koichiro.den@canonical.com Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpio-virtuser.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/drivers/gpio/gpio-virtuser.c b/drivers/gpio/gpio-virtuser.c index 91b6352c957c..e89b1239b635 100644 --- a/drivers/gpio/gpio-virtuser.c +++ b/drivers/gpio/gpio-virtuser.c @@ -1439,6 +1439,15 @@ gpio_virtuser_make_lookup_table(struct gpio_virtuser_device *dev) return 0; } +static void +gpio_virtuser_remove_lookup_table(struct gpio_virtuser_device *dev) +{ + gpiod_remove_lookup_table(dev->lookup_table); + kfree(dev->lookup_table->dev_id); + kfree(dev->lookup_table); + dev->lookup_table = NULL; +} + static struct fwnode_handle * gpio_virtuser_make_device_swnode(struct gpio_virtuser_device *dev) { @@ -1487,10 +1496,8 @@ gpio_virtuser_device_activate(struct gpio_virtuser_device *dev) pdevinfo.fwnode = swnode; ret = gpio_virtuser_make_lookup_table(dev); - if (ret) { - fwnode_remove_software_node(swnode); - return ret; - } + if (ret) + goto err_remove_swnode; reinit_completion(&dev->probe_completion); dev->driver_bound = false; @@ -1498,23 +1505,31 @@ gpio_virtuser_device_activate(struct gpio_virtuser_device *dev) pdev = platform_device_register_full(&pdevinfo); if (IS_ERR(pdev)) { + ret = PTR_ERR(pdev); bus_unregister_notifier(&platform_bus_type, &dev->bus_notifier); - fwnode_remove_software_node(swnode); - return PTR_ERR(pdev); + goto err_remove_lookup_table; } wait_for_completion(&dev->probe_completion); bus_unregister_notifier(&platform_bus_type, &dev->bus_notifier); if (!dev->driver_bound) { - platform_device_unregister(pdev); - fwnode_remove_software_node(swnode); - return -ENXIO; + ret = -ENXIO; + goto err_unregister_pdev; } dev->pdev = pdev; return 0; + +err_unregister_pdev: + platform_device_unregister(pdev); +err_remove_lookup_table: + gpio_virtuser_remove_lookup_table(dev); +err_remove_swnode: + fwnode_remove_software_node(swnode); + + return ret; } static void @@ -1526,10 +1541,9 @@ gpio_virtuser_device_deactivate(struct gpio_virtuser_device *dev) swnode = dev_fwnode(&dev->pdev->dev); platform_device_unregister(dev->pdev); + gpio_virtuser_remove_lookup_table(dev); fwnode_remove_software_node(swnode); dev->pdev = NULL; - gpiod_remove_lookup_table(dev->lookup_table); - kfree(dev->lookup_table); } static ssize_t From 656cc2e892f128b03ea9ef19bd11d70f71d5472b Mon Sep 17 00:00:00 2001 From: Koichiro Den Date: Fri, 3 Jan 2025 23:18:27 +0900 Subject: [PATCH 2/5] gpio: virtuser: fix handling of multiple conn_ids in lookup table Creating a virtuser device via configfs with multiple conn_ids fails due to incorrect indexing of lookup entries. Correct the indexing logic to ensure proper functionality when multiple gpio_virtuser_lookup are created. Fixes: 91581c4b3f29 ("gpio: virtuser: new virtual testing driver for the GPIO API") Signed-off-by: Koichiro Den Link: https://lore.kernel.org/r/20250103141829.430662-3-koichiro.den@canonical.com Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpio-virtuser.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpio/gpio-virtuser.c b/drivers/gpio/gpio-virtuser.c index e89b1239b635..d6244f0d3bc7 100644 --- a/drivers/gpio/gpio-virtuser.c +++ b/drivers/gpio/gpio-virtuser.c @@ -1410,7 +1410,7 @@ gpio_virtuser_make_lookup_table(struct gpio_virtuser_device *dev) size_t num_entries = gpio_virtuser_get_lookup_count(dev); struct gpio_virtuser_lookup_entry *entry; struct gpio_virtuser_lookup *lookup; - unsigned int i = 0; + unsigned int i = 0, idx; lockdep_assert_held(&dev->lock); @@ -1424,12 +1424,12 @@ gpio_virtuser_make_lookup_table(struct gpio_virtuser_device *dev) return -ENOMEM; list_for_each_entry(lookup, &dev->lookup_list, siblings) { + idx = 0; list_for_each_entry(entry, &lookup->entry_list, siblings) { - table->table[i] = + table->table[i++] = GPIO_LOOKUP_IDX(entry->key, entry->offset < 0 ? U16_MAX : entry->offset, - lookup->con_id, i, entry->flags); - i++; + lookup->con_id, idx++, entry->flags); } } From c7c434c1dba955005f5161dae73f09c0a922cfa7 Mon Sep 17 00:00:00 2001 From: Koichiro Den Date: Fri, 3 Jan 2025 23:18:28 +0900 Subject: [PATCH 3/5] gpio: virtuser: lock up configfs that an instantiated device depends on Once a virtuser device is instantiated and actively used, allowing rmdir for its configfs serves no purpose and can be confusing. Userspace interacts with the virtual consumer at arbitrary times, meaning it depends on its existence. Make the subsystem itself depend on the configfs entry for a virtuser device while it is in active use. Signed-off-by: Koichiro Den Link: https://lore.kernel.org/r/20250103141829.430662-4-koichiro.den@canonical.com Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpio-virtuser.c | 49 ++++++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 8 deletions(-) diff --git a/drivers/gpio/gpio-virtuser.c b/drivers/gpio/gpio-virtuser.c index d6244f0d3bc7..e89f299f2140 100644 --- a/drivers/gpio/gpio-virtuser.c +++ b/drivers/gpio/gpio-virtuser.c @@ -1546,6 +1546,30 @@ gpio_virtuser_device_deactivate(struct gpio_virtuser_device *dev) dev->pdev = NULL; } +static void +gpio_virtuser_device_lockup_configfs(struct gpio_virtuser_device *dev, bool lock) +{ + struct configfs_subsystem *subsys = dev->group.cg_subsys; + struct gpio_virtuser_lookup_entry *entry; + struct gpio_virtuser_lookup *lookup; + + /* + * The device only needs to depend on leaf lookup entries. This is + * sufficient to lock up all the configfs entries that the + * instantiated, alive device depends on. + */ + list_for_each_entry(lookup, &dev->lookup_list, siblings) { + list_for_each_entry(entry, &lookup->entry_list, siblings) { + if (lock) + WARN_ON(configfs_depend_item_unlocked( + subsys, &entry->group.cg_item)); + else + configfs_undepend_item_unlocked( + &entry->group.cg_item); + } + } +} + static ssize_t gpio_virtuser_device_config_live_store(struct config_item *item, const char *page, size_t count) @@ -1558,15 +1582,24 @@ gpio_virtuser_device_config_live_store(struct config_item *item, if (ret) return ret; - guard(mutex)(&dev->lock); - - if (live == gpio_virtuser_device_is_live(dev)) - return -EPERM; - if (live) - ret = gpio_virtuser_device_activate(dev); - else - gpio_virtuser_device_deactivate(dev); + gpio_virtuser_device_lockup_configfs(dev, true); + + scoped_guard(mutex, &dev->lock) { + if (live == gpio_virtuser_device_is_live(dev)) + ret = -EPERM; + else if (live) + ret = gpio_virtuser_device_activate(dev); + else + gpio_virtuser_device_deactivate(dev); + } + + /* + * Undepend is required only if device disablement (live == 0) + * succeeds or if device enablement (live == 1) fails. + */ + if (live == !!ret) + gpio_virtuser_device_lockup_configfs(dev, false); return ret ?: count; } From 8bd76b3d3f3af7ac2898b6a27ad90c444fec418f Mon Sep 17 00:00:00 2001 From: Koichiro Den Date: Fri, 3 Jan 2025 23:18:29 +0900 Subject: [PATCH 4/5] gpio: sim: lock up configfs that an instantiated device depends on Once a sim device is instantiated and actively used, allowing rmdir for its configfs serves no purpose and can be confusing. Effectively, arbitrary users start depending on its existence. Make the subsystem itself depend on the configfs entry for a sim device while it is in active use. Signed-off-by: Koichiro Den Link: https://lore.kernel.org/r/20250103141829.430662-5-koichiro.den@canonical.com Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpio-sim.c | 48 +++++++++++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 7 deletions(-) diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c index f387dad81f29..686ae3d11ba3 100644 --- a/drivers/gpio/gpio-sim.c +++ b/drivers/gpio/gpio-sim.c @@ -1027,6 +1027,30 @@ static void gpio_sim_device_deactivate(struct gpio_sim_device *dev) dev->pdev = NULL; } +static void +gpio_sim_device_lockup_configfs(struct gpio_sim_device *dev, bool lock) +{ + struct configfs_subsystem *subsys = dev->group.cg_subsys; + struct gpio_sim_bank *bank; + struct gpio_sim_line *line; + + /* + * The device only needs to depend on leaf line entries. This is + * sufficient to lock up all the configfs entries that the + * instantiated, alive device depends on. + */ + list_for_each_entry(bank, &dev->bank_list, siblings) { + list_for_each_entry(line, &bank->line_list, siblings) { + if (lock) + WARN_ON(configfs_depend_item_unlocked( + subsys, &line->group.cg_item)); + else + configfs_undepend_item_unlocked( + &line->group.cg_item); + } + } +} + static ssize_t gpio_sim_device_config_live_store(struct config_item *item, const char *page, size_t count) @@ -1039,14 +1063,24 @@ gpio_sim_device_config_live_store(struct config_item *item, if (ret) return ret; - guard(mutex)(&dev->lock); + if (live) + gpio_sim_device_lockup_configfs(dev, true); - if (live == gpio_sim_device_is_live(dev)) - ret = -EPERM; - else if (live) - ret = gpio_sim_device_activate(dev); - else - gpio_sim_device_deactivate(dev); + scoped_guard(mutex, &dev->lock) { + if (live == gpio_sim_device_is_live(dev)) + ret = -EPERM; + else if (live) + ret = gpio_sim_device_activate(dev); + else + gpio_sim_device_deactivate(dev); + } + + /* + * Undepend is required only if device disablement (live == 0) + * succeeds or if device enablement (live == 1) fails. + */ + if (live == !!ret) + gpio_sim_device_lockup_configfs(dev, false); return ret ?: count; } From e59f4c97172de0c302894cfd5616161c1f0c4d85 Mon Sep 17 00:00:00 2001 From: Binbin Zhou Date: Tue, 7 Jan 2025 18:38:56 +0800 Subject: [PATCH 5/5] gpio: loongson: Fix Loongson-2K2000 ACPI GPIO register offset Since commit 3feb70a61740 ("gpio: loongson: add more gpio chip support"), the Loongson-2K2000 GPIO is supported. However, according to the firmware development specification, the Loongson-2K2000 ACPI GPIO register offsets in the driver do not match the register base addresses in the firmware, resulting in the registers not being accessed properly. Now, we fix it to ensure the GPIO function works properly. Cc: stable@vger.kernel.org Cc: Yinbo Zhu Fixes: 3feb70a61740 ("gpio: loongson: add more gpio chip support") Co-developed-by: Hongliang Wang Signed-off-by: Hongliang Wang Signed-off-by: Binbin Zhou Link: https://lore.kernel.org/r/20250107103856.1037222-1-zhoubinbin@loongson.cn Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpio-loongson-64bit.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpio/gpio-loongson-64bit.c b/drivers/gpio/gpio-loongson-64bit.c index 6749d4dd6d64..7f4d78fd800e 100644 --- a/drivers/gpio/gpio-loongson-64bit.c +++ b/drivers/gpio/gpio-loongson-64bit.c @@ -237,9 +237,9 @@ static const struct loongson_gpio_chip_data loongson_gpio_ls2k2000_data1 = { static const struct loongson_gpio_chip_data loongson_gpio_ls2k2000_data2 = { .label = "ls2k2000_gpio", .mode = BIT_CTRL_MODE, - .conf_offset = 0x84, - .in_offset = 0x88, - .out_offset = 0x80, + .conf_offset = 0x4, + .in_offset = 0x8, + .out_offset = 0x0, }; static const struct loongson_gpio_chip_data loongson_gpio_ls3a5000_data = {