[v2] gpio: sim: fix the chip_name configfs item

[PATCH v2] gpio: sim: fix the chip_name configfs item

Posted by Bartosz Golaszewski 3 years, 9 months ago

The chip_name configs attribute always displays the device name of the
first GPIO bank because the logic of the relevant function is simply
wrong.

Fix it by correctly comparing the bank's swnode against the GPIO
device's children.

Fixes: cb8c474e79be ("gpio: sim: new testing module")
Cc: stable@vger.kernel.org
Reported-by: Kent Gibson <warthog618@gmail.com>
Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
v1 -> v2:
- use device_match_fwnode for shorter code

 drivers/gpio/gpio-sim.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c
index 98109839102f..1020c2feb249 100644
--- a/drivers/gpio/gpio-sim.c
+++ b/drivers/gpio/gpio-sim.c
@@ -991,28 +991,22 @@ static struct configfs_attribute *gpio_sim_device_config_attrs[] = {
 };
 
 struct gpio_sim_chip_name_ctx {
-	struct gpio_sim_device *dev;
+	struct fwnode_handle *swnode;
 	char *page;
 };
 
 static int gpio_sim_emit_chip_name(struct device *dev, void *data)
 {
 	struct gpio_sim_chip_name_ctx *ctx = data;
-	struct fwnode_handle *swnode;
-	struct gpio_sim_bank *bank;
 
 	/* This would be the sysfs device exported in /sys/class/gpio. */
 	if (dev->class)
 		return 0;
 
-	swnode = dev_fwnode(dev);
+	if (device_match_fwnode(dev, ctx->swnode))
+		return sprintf(ctx->page, "%s\n", dev_name(dev));
 
-	list_for_each_entry(bank, &ctx->dev->bank_list, siblings) {
-		if (bank->swnode == swnode)
-			return sprintf(ctx->page, "%s\n", dev_name(dev));
-	}
-
-	return -ENODATA;
+	return 0;
 }
 
 static ssize_t gpio_sim_bank_config_chip_name_show(struct config_item *item,
@@ -1020,7 +1014,7 @@ static ssize_t gpio_sim_bank_config_chip_name_show(struct config_item *item,
 {
 	struct gpio_sim_bank *bank = to_gpio_sim_bank(item);
 	struct gpio_sim_device *dev = gpio_sim_bank_get_device(bank);
-	struct gpio_sim_chip_name_ctx ctx = { dev, page };
+	struct gpio_sim_chip_name_ctx ctx = { bank->swnode, page };
 	int ret;
 
 	mutex_lock(&dev->lock);
-- 
2.34.1

Re: [PATCH v2] gpio: sim: fix the chip_name configfs item

Posted by Andy Shevchenko 3 years, 9 months ago

On Tue, Jul 12, 2022 at 9:46 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> The chip_name configs attribute always displays the device name of the
> first GPIO bank because the logic of the relevant function is simply
> wrong.
>
> Fix it by correctly comparing the bank's swnode against the GPIO
> device's children.

Taking into account that name swnode is used in other places in the
code, I'm fine with this version,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Fixes: cb8c474e79be ("gpio: sim: new testing module")
> Cc: stable@vger.kernel.org
> Reported-by: Kent Gibson <warthog618@gmail.com>
> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
> ---
> v1 -> v2:
> - use device_match_fwnode for shorter code
>
>  drivers/gpio/gpio-sim.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c
> index 98109839102f..1020c2feb249 100644
> --- a/drivers/gpio/gpio-sim.c
> +++ b/drivers/gpio/gpio-sim.c
> @@ -991,28 +991,22 @@ static struct configfs_attribute *gpio_sim_device_config_attrs[] = {
>  };
>
>  struct gpio_sim_chip_name_ctx {
> -       struct gpio_sim_device *dev;
> +       struct fwnode_handle *swnode;
>         char *page;
>  };
>
>  static int gpio_sim_emit_chip_name(struct device *dev, void *data)
>  {
>         struct gpio_sim_chip_name_ctx *ctx = data;
> -       struct fwnode_handle *swnode;
> -       struct gpio_sim_bank *bank;
>
>         /* This would be the sysfs device exported in /sys/class/gpio. */
>         if (dev->class)
>                 return 0;
>
> -       swnode = dev_fwnode(dev);
> +       if (device_match_fwnode(dev, ctx->swnode))
> +               return sprintf(ctx->page, "%s\n", dev_name(dev));
>
> -       list_for_each_entry(bank, &ctx->dev->bank_list, siblings) {
> -               if (bank->swnode == swnode)
> -                       return sprintf(ctx->page, "%s\n", dev_name(dev));
> -       }
> -
> -       return -ENODATA;
> +       return 0;
>  }
>
>  static ssize_t gpio_sim_bank_config_chip_name_show(struct config_item *item,
> @@ -1020,7 +1014,7 @@ static ssize_t gpio_sim_bank_config_chip_name_show(struct config_item *item,
>  {
>         struct gpio_sim_bank *bank = to_gpio_sim_bank(item);
>         struct gpio_sim_device *dev = gpio_sim_bank_get_device(bank);
> -       struct gpio_sim_chip_name_ctx ctx = { dev, page };
> +       struct gpio_sim_chip_name_ctx ctx = { bank->swnode, page };
>         int ret;
>
>         mutex_lock(&dev->lock);
> --
> 2.34.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] gpio: sim: fix the chip_name configfs item

Posted by Kent Gibson 3 years, 9 months ago

On Tue, Jul 12, 2022 at 09:40:55AM +0200, Bartosz Golaszewski wrote:
> The chip_name configs attribute always displays the device name of the
> first GPIO bank because the logic of the relevant function is simply
> wrong.
> 
> Fix it by correctly comparing the bank's swnode against the GPIO
> device's children.
> 
> Fixes: cb8c474e79be ("gpio: sim: new testing module")
> Cc: stable@vger.kernel.org
> Reported-by: Kent Gibson <warthog618@gmail.com>
> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
> ---
> v1 -> v2:
> - use device_match_fwnode for shorter code
> 

Works for me.

Reviewed-and-tested-by: Kent Gibson <warthog618@gmail.com>

Cheers,
Kent.