| Message ID | 20231130134630.18198-8-brgl@bgdev.pl |
|---|---|
| State | New |
| Headers | show |
| Series | gpio/pinctrl: replace gpiochip_is_requested() with a safer interface | expand |
On Thu, Nov 30, 2023 at 02:46:27PM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Use the new gpiochip_dup_line_label() helper to safely retrieve the > descriptor label. ... > seq_printf(s, > " gpio-%-3d (%-20.20s) %s %s %s pad-%-3d offset:0x%03x mux:%d %s%s%s", > pin, > - label, > + label ?: "Unrequested", This already fourth (?) duplication among drivers. Perhaps you want a helper: gpiochip_dup_line_label_fallback() // naming is up to you which will return the same for everybody and we don't need to hunt for the different meaning of "Unrequested". Also the word "Unrequested" is a bit doubtful as it can be a label, right? Something with special characters / spaces / etc would suit better? In any case it might require to add a warning (?) to the GPIO lib core when label gets assigned if it clashes with the "reserved" word. > val & BYT_INPUT_EN ? " " : "in", > val & BYT_OUTPUT_EN ? " " : "out", > str_hi_lo(val & BYT_LEVEL),
On Thu, Nov 30, 2023 at 5:36 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Nov 30, 2023 at 02:46:27PM +0100, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > Use the new gpiochip_dup_line_label() helper to safely retrieve the > > descriptor label. > > ... > > > seq_printf(s, > > " gpio-%-3d (%-20.20s) %s %s %s pad-%-3d offset:0x%03x mux:%d %s%s%s", > > pin, > > - label, > > + label ?: "Unrequested", > > This already fourth (?) duplication among drivers. > Perhaps you want a helper: > gpiochip_dup_line_label_fallback() // naming is up to you > which will return the same for everybody and we don't need to hunt for > the different meaning of "Unrequested". > IMO the overhead here is very small in return for better readability (IOW: `label ?: "Unrequested"` is more readable than some function named `gpiochip_dup_line_label_fallback()`). Given the string is in .rodata anyway, I wouldn't be surprised if adding a helper resulted in bigger code. > Also the word "Unrequested" is a bit doubtful as it can be a label, right? > Something with special characters / spaces / etc would suit better? > In any case it might require to add a warning (?) to the GPIO lib core > when label gets assigned if it clashes with the "reserved" word. > Agreed but this is a functional change in debugfs output. I know debugfs is not considered stable but I didn't write it, I don't know who's using it and I prefer to leave it be. Bart > > val & BYT_INPUT_EN ? " " : "in", > > val & BYT_OUTPUT_EN ? " " : "out", > > str_hi_lo(val & BYT_LEVEL), > > -- > With Best Regards, > Andy Shevchenko > >
diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c b/drivers/pinctrl/intel/pinctrl-baytrail.c index 3cd0798ee631..3c8c02043481 100644 --- a/drivers/pinctrl/intel/pinctrl-baytrail.c +++ b/drivers/pinctrl/intel/pinctrl-baytrail.c @@ -9,6 +9,7 @@ #include <linux/acpi.h> #include <linux/array_size.h> #include <linux/bitops.h> +#include <linux/cleanup.h> #include <linux/gpio/driver.h> #include <linux/init.h> #include <linux/interrupt.h> @@ -1173,7 +1174,6 @@ static void byt_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip) const char *pull_str = NULL; const char *pull = NULL; unsigned long flags; - const char *label; unsigned int pin; pin = vg->soc->pins[i].number; @@ -1200,9 +1200,10 @@ static void byt_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip) seq_printf(s, "Pin %i: can't retrieve community\n", pin); continue; } - label = gpiochip_is_requested(chip, i); - if (!label) - label = "Unrequested"; + + char *label __free(kfree) = gpiochip_dup_line_label(chip, i); + if (IS_ERR(label)) + continue; switch (conf0 & BYT_PULL_ASSIGN_MASK) { case BYT_PULL_ASSIGN_UP: @@ -1231,7 +1232,7 @@ static void byt_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip) seq_printf(s, " gpio-%-3d (%-20.20s) %s %s %s pad-%-3d offset:0x%03x mux:%d %s%s%s", pin, - label, + label ?: "Unrequested", val & BYT_INPUT_EN ? " " : "in", val & BYT_OUTPUT_EN ? " " : "out", str_hi_lo(val & BYT_LEVEL),