Fix scary '(null)' in the messages and move to dev_err() in one case.
v4:
- combined [email protected]
and [email protected] (Bart)
Andy Shevchenko (1):
gpiolib: Fix debug messaging in gpiod_find_and_request()
Peng Fan (1):
gpiolib: use dev_err() when gpiod_configure_flags failed
drivers/gpio/gpiolib.c | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)
--
2.43.0.rc1.1.gbec44491f096
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 <[email protected]>
Fixes: 8eb1f71e7acc ("gpiolib: consolidate GPIO lookups")
Suggested-by: Dmitry Torokhov <[email protected]>
Tested-by: Ferry Toth <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
---
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 06f6c84fc7b1..b98cbf4335e3 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2396,6 +2396,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
@@ -2424,10 +2429,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;
}
@@ -2437,8 +2443,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);
}
@@ -4152,19 +4158,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);
}
@@ -4180,6 +4184,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.
@@ -4202,8 +4207,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;
}
@@ -4225,15 +4229,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);
}
@@ -4349,6 +4352,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)
@@ -4392,7 +4396,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;
}
--
2.43.0.rc1.1.gbec44491f096
From: Peng Fan <[email protected]>
When gpio-ranges property was missed to be added in the gpio node,
using dev_err() to show an error message will helping to locate issues
easier.
Signed-off-by: Peng Fan <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/gpio/gpiolib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index b98cbf4335e3..5589e085ba25 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4236,7 +4236,7 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
ret = gpiod_configure_flags(desc, con_id, lookupflags, flags);
if (ret < 0) {
gpiod_put(desc);
- dev_dbg(consumer, "setup of GPIO %s failed\n", name);
+ dev_err(consumer, "setup of GPIO %s failed: %d\n", name, ret);
return ERR_PTR(ret);
}
--
2.43.0.rc1.1.gbec44491f096
On Tue, Mar 26, 2024 at 3:26 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Tue, Mar 26, 2024 at 12:58:40PM +0100, Bartosz Golaszewski wrote:
> > On Mon, Mar 25, 2024 at 6:18 PM Andy Shevchenko
> > <[email protected]> wrote:
>
> ...
>
> > I queued this one for fixes. I will take patch 2/2 through the
> > for-next tree as it's not really a fix.
>
> But you asked me to sent them both at once... Whatever, thank you!
>
And then I realized after a second look that it makes more sense to
send them separately.
Bart
On Tue, Mar 26, 2024 at 12:58:40PM +0100, Bartosz Golaszewski wrote:
> On Mon, Mar 25, 2024 at 6:18 PM Andy Shevchenko
> <[email protected]> wrote:
..
> I queued this one for fixes. I will take patch 2/2 through the
> for-next tree as it's not really a fix.
But you asked me to sent them both at once... Whatever, thank you!
--
With Best Regards,
Andy Shevchenko
On Mon, Mar 25, 2024 at 6:18 PM Andy Shevchenko
<[email protected]> wrote:
>
> 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 <[email protected]>
> Fixes: 8eb1f71e7acc ("gpiolib: consolidate GPIO lookups")
> Suggested-by: Dmitry Torokhov <[email protected]>
> Tested-by: Ferry Toth <[email protected]>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
I queued this one for fixes. I will take patch 2/2 through the
for-next tree as it's not really a fix.
Bart
On Mon, Mar 25, 2024 at 6:18 PM Andy Shevchenko
<[email protected]> wrote:
>
> From: Peng Fan <[email protected]>
>
> When gpio-ranges property was missed to be added in the gpio node,
> using dev_err() to show an error message will helping to locate issues
> easier.
>
> Signed-off-by: Peng Fan <[email protected]>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/gpio/gpiolib.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index b98cbf4335e3..5589e085ba25 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -4236,7 +4236,7 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
> ret = gpiod_configure_flags(desc, con_id, lookupflags, flags);
> if (ret < 0) {
> gpiod_put(desc);
> - dev_dbg(consumer, "setup of GPIO %s failed\n", name);
> + dev_err(consumer, "setup of GPIO %s failed: %d\n", name, ret);
> return ERR_PTR(ret);
> }
>
> --
> 2.43.0.rc1.1.gbec44491f096
>
Applied, thanks!
Bart