| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f83d7735-0351-481a-af3a-16137f078058@gmail.com>
Date: Wed, 20 Mar 2024 23:19:55 +0100
From: Ferry Toth <fntoth@...il.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
Linus Walleij <linus.walleij@...aro.org>, linux-gpio@...r.kernel.org,
linux-kernel@...r.kernel.org
Cc: Bartosz Golaszewski <brgl@...ev.pl>, Stephen Boyd <swboyd@...omium.org>,
Ferry Toth <ftoth@...londelft.nl>,
Dmitry Torokhov <dmitry.torokhov@...il.com>
Subject: Re: [PATCH v2 1/1] gpiolib: Fix debug messaging in
gpiod_find_and_request()
Hi
Op 20-03-2024 om 17:58 schreef Andy Shevchenko:
> When consolidating GPIO lookups in ACPI code, the debug messaging
> had been reworked that the user may see
>
> [ 13.401147] (NULL device *): using ACPI '\_SB.LEDS.led-0' for '(null)' GPIO lookup
> [ 13.401378] gpio gpiochip0: Persistence not supported for GPIO 40
> [ 13.401402] gpio-40 (?): no flags found for (null)
>
> instead of
>
> [ 14.182962] gpio gpiochip0: Persistence not supported for GPIO 40
> [ 14.182994] gpio-40 (?): no flags found for gpios
>
> The '(null)' parts are less informative and likely scare the users.
> Replace them by '(default)' which can point out to the default connection
> IDs, such as 'gpios'.
>
> While at it, amend other places where con_id is used in the messages.
>
> Reported-by: Ferry Toth <ftoth@...londelft.nl>
> Fixes: 8eb1f71e7acc ("gpiolib: consolidate GPIO lookups")
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@...il.c > Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> ---
> v2: completele reworked solution of
> 20231019173457.2445119-1-andriy.shevchenko@...ux.intel.com
> drivers/gpio/gpiolib.c | 32 ++++++++++++++++++--------------
> 1 file changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index e2e583b40207..7d26e5de0b44 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -2401,6 +2401,11 @@ char *gpiochip_dup_line_label(struct gpio_chip *gc, unsigned int offset)
> }
> EXPORT_SYMBOL_GPL(gpiochip_dup_line_label);
>
> +static inline const char *function_name_or_default(const char *con_id)
> +{
> + return con_id ?: "(default)";
> +}
> +
> /**
> * gpiochip_request_own_desc - Allow GPIO chip to request its own descriptor
> * @gc: GPIO chip
> @@ -2429,10 +2434,11 @@ struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc,
> enum gpiod_flags dflags)
> {
> struct gpio_desc *desc = gpiochip_get_desc(gc, hwnum);
> + const char *name = function_name_or_default(label);
> int ret;
>
> if (IS_ERR(desc)) {
> - chip_err(gc, "failed to get GPIO descriptor\n");
> + chip_err(gc, "failed to get GPIO %s descriptor\n", name);
> return desc;
> }
>
> @@ -2442,8 +2448,8 @@ struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc,
>
> ret = gpiod_configure_flags(desc, label, lflags, dflags);
> if (ret) {
> - chip_err(gc, "setup of own GPIO %s failed\n", label);
> gpiod_free_commit(desc);
> + chip_err(gc, "setup of own GPIO %s failed\n", name);
> return ERR_PTR(ret);
> }
>
> @@ -4157,19 +4163,17 @@ static struct gpio_desc *gpiod_find_by_fwnode(struct fwnode_handle *fwnode,
> enum gpiod_flags *flags,
> unsigned long *lookupflags)
> {
> + const char *name = function_name_or_default(con_id);
> struct gpio_desc *desc = ERR_PTR(-ENOENT);
>
> if (is_of_node(fwnode)) {
> - dev_dbg(consumer, "using DT '%pfw' for '%s' GPIO lookup\n",
> - fwnode, con_id);
> + dev_dbg(consumer, "using DT '%pfw' for '%s' GPIO lookup\n", fwnode, name);
> desc = of_find_gpio(to_of_node(fwnode), con_id, idx, lookupflags);
> } else if (is_acpi_node(fwnode)) {
> - dev_dbg(consumer, "using ACPI '%pfw' for '%s' GPIO lookup\n",
> - fwnode, con_id);
> + dev_dbg(consumer, "using ACPI '%pfw' for '%s' GPIO lookup\n", fwnode, name);
> desc = acpi_find_gpio(fwnode, con_id, idx, flags, lookupflags);
> } else if (is_software_node(fwnode)) {
> - dev_dbg(consumer, "using swnode '%pfw' for '%s' GPIO lookup\n",
> - fwnode, con_id);
> + dev_dbg(consumer, "using swnode '%pfw' for '%s' GPIO lookup\n", fwnode, name);
> desc = swnode_find_gpio(fwnode, con_id, idx, lookupflags);
> }
>
> @@ -4185,6 +4189,7 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
> bool platform_lookup_allowed)
> {
> unsigned long lookupflags = GPIO_LOOKUP_FLAGS_DEFAULT;
> + const char *name = function_name_or_default(con_id);
> /*
> * scoped_guard() is implemented as a for loop, meaning static
> * analyzers will complain about these two not being initialized.
> @@ -4207,8 +4212,7 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
> }
>
> if (IS_ERR(desc)) {
> - dev_dbg(consumer, "No GPIO consumer %s found\n",
> - con_id);
> + dev_dbg(consumer, "No GPIO consumer %s found\n", name);
> return desc;
> }
>
> @@ -4230,15 +4234,14 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
> *
> * FIXME: Make this more sane and safe.
> */
> - dev_info(consumer,
> - "nonexclusive access to GPIO for %s\n", con_id);
> + dev_info(consumer, "nonexclusive access to GPIO for %s\n", name);
> return desc;
> }
>
> ret = gpiod_configure_flags(desc, con_id, lookupflags, flags);
> if (ret < 0) {
> - dev_dbg(consumer, "setup of GPIO %s failed\n", con_id);
> gpiod_put(desc);
> + dev_dbg(consumer, "setup of GPIO %s failed\n", name);
> return ERR_PTR(ret);
> }
>
> @@ -4354,6 +4357,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_optional);
> int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
> unsigned long lflags, enum gpiod_flags dflags)
> {
> + const char *name = function_name_or_default(con_id);
> int ret;
>
> if (lflags & GPIO_ACTIVE_LOW)
> @@ -4397,7 +4401,7 @@ int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
>
> /* No particular flag request, return here... */
> if (!(dflags & GPIOD_FLAGS_BIT_DIR_SET)) {
> - gpiod_dbg(desc, "no flags found for %s\n", con_id);
> + gpiod_dbg(desc, "no flags found for GPIO %s\n", name);
> return 0;
> }
>
Tested-by: Ferry Toth <ftoth@...londelft.nl> on Intel Edison (mrfld)
Powered by blists - more mailing lists