* [PATCH 1/3] gpiolib: Downgrade set_debounce() missing operation warning
@ 2013-09-06 12:38 Mark Brown
2013-09-06 12:38 ` [PATCH 2/3] gpiolib: Provide helper macros for logging of GPIO events Mark Brown
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Mark Brown @ 2013-09-06 12:38 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-gpio, linaro-kernel, Mark Brown
From: Mark Brown <broonie@linaro.org>
It is reasonable for a driver using a GPIO to call set_debounce() on GPIOs
that don't provide this support in hardware since the driver can fall back
on a software debounce implementation or otherwise not depend on success
so downgrade the log message for this to a debug one.
Signed-off-by: Mark Brown <broonie@linaro.org>
---
drivers/gpio/gpiolib.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index b762718..374cf9e 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1779,8 +1779,8 @@ static int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
chip = desc->chip;
if (!chip->set || !chip->set_debounce) {
- pr_warn("%s: missing set() or set_debounce() operations\n",
- __func__);
+ pr_debug("%s: missing set() or set_debounce() operations\n",
+ __func__);
return -EIO;
}
--
1.8.4.rc3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/3] gpiolib: Provide helper macros for logging of GPIO events 2013-09-06 12:38 [PATCH 1/3] gpiolib: Downgrade set_debounce() missing operation warning Mark Brown @ 2013-09-06 12:38 ` Mark Brown 2013-09-07 2:34 ` Alexandre Courbot 2013-09-06 12:38 ` [PATCH 3/3] gpiolib: Include GPIO label in log messages for GPIOs Mark Brown 2013-09-09 7:47 ` [PATCH 1/3] gpiolib: Downgrade set_debounce() missing operation warning Linus Walleij 2 siblings, 1 reply; 7+ messages in thread From: Mark Brown @ 2013-09-06 12:38 UTC (permalink / raw) To: Linus Walleij; +Cc: linux-gpio, linaro-kernel, Mark Brown From: Mark Brown <broonie@linaro.org> Currently many but not all GPIO log messages log the GPIO number and the formats vary. Ensure that this is done consistently by defining logging helpers which take the GPIO descriptor. The will help people pattern matching on logs and providing the number makes the log messages that omitted it more useful. Signed-off-by: Mark Brown <broonie@linaro.org> --- drivers/gpio/gpiolib.c | 52 +++++++++++++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 374cf9e..e389183 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -103,6 +103,18 @@ static int gpiod_export_link(struct device *dev, const char *name, static int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value); static void gpiod_unexport(struct gpio_desc *desc); +#define gpiod_emerg(desc, fmt, ...) \ + pr_emerg("gpio-%d: " fmt, desc_to_gpio(desc), ##__VA_ARGS__) +#define gpiod_crit(desc, fmt, ...) \ + pr_crit("gpio-%d: " fmt, desc_to_gpio(desc), ##__VA_ARGS__) +#define gpiod_err(desc, fmt, ...) \ + pr_err("gpio-%d: " fmt, desc_to_gpio(desc), ##__VA_ARGS__) +#define gpiod_warn(desc, fmt, ...) \ + pr_warn("gpio-%d: " fmt, desc_to_gpio(desc), ##__VA_ARGS__) +#define gpiod_info(desc, fmt, ...) \ + pr_info("gpio-%d: " fmt, desc_to_gpio(desc), ##__VA_ARGS__) +#define gpiod_dbg(desc, fmt, ...) \ + pr_debug("gpio-%d: " fmt, desc_to_gpio(desc), ##__VA_ARGS__) static inline void desc_set_label(struct gpio_desc *d, const char *label) { @@ -1636,8 +1648,9 @@ static int gpiod_direction_input(struct gpio_desc *desc) chip = desc->chip; if (!chip->get || !chip->direction_input) { - pr_warn("%s: missing get() or direction_input() operations\n", - __func__); + gpiod_warn(desc, + "%s: missing get() or direction_input() operations\n", + __func__); return -EIO; } @@ -1657,8 +1670,7 @@ static int gpiod_direction_input(struct gpio_desc *desc) if (status) { status = chip->request(chip, offset); if (status < 0) { - pr_debug("GPIO-%d: chip request fail, %d\n", - desc_to_gpio(desc), status); + gpiod_dbg(desc, "chip request fail, %d\n", status); /* and it's not available to anyone else ... * gpio_request() is the fully clean solution. */ @@ -1676,8 +1688,7 @@ lose: fail: spin_unlock_irqrestore(&gpio_lock, flags); if (status) - pr_debug("%s: gpio-%d status %d\n", __func__, - desc_to_gpio(desc), status); + gpiod_dbg(desc, "%s status %d\n", __func__, status); return status; } @@ -1709,8 +1720,9 @@ static int gpiod_direction_output(struct gpio_desc *desc, int value) chip = desc->chip; if (!chip->set || !chip->direction_output) { - pr_warn("%s: missing set() or direction_output() operations\n", - __func__); + gpiod_warn(desc, + "%s: missing set() or direction_output() operations\n", + __func__); return -EIO; } @@ -1730,8 +1742,7 @@ static int gpiod_direction_output(struct gpio_desc *desc, int value) if (status) { status = chip->request(chip, offset); if (status < 0) { - pr_debug("GPIO-%d: chip request fail, %d\n", - desc_to_gpio(desc), status); + gpiod_dbg(desc, "chip request fail, %d\n", status); /* and it's not available to anyone else ... * gpio_request() is the fully clean solution. */ @@ -1749,8 +1760,7 @@ lose: fail: spin_unlock_irqrestore(&gpio_lock, flags); if (status) - pr_debug("%s: gpio-%d status %d\n", __func__, - desc_to_gpio(desc), status); + gpiod_dbg(desc, "%s: gpio status %d\n", __func__, status); return status; } @@ -1779,8 +1789,9 @@ static int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce) chip = desc->chip; if (!chip->set || !chip->set_debounce) { - pr_debug("%s: missing set() or set_debounce() operations\n", - __func__); + gpiod_dbg(desc, + "%s: missing set() or set_debounce() operations\n", + __func__); return -EIO; } @@ -1802,8 +1813,7 @@ static int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce) fail: spin_unlock_irqrestore(&gpio_lock, flags); if (status) - pr_debug("%s: gpio-%d status %d\n", __func__, - desc_to_gpio(desc), status); + gpiod_dbg(desc, "%s: status %d\n", __func__, status); return status; } @@ -1891,8 +1901,9 @@ static void _gpio_set_open_drain_value(struct gpio_desc *desc, int value) } trace_gpio_direction(desc_to_gpio(desc), value, err); if (err < 0) - pr_err("%s: Error in set_value for open drain gpio%d err %d\n", - __func__, desc_to_gpio(desc), err); + gpiod_err(desc, + "%s: Error in set_value for open drain err %d\n", + __func__, err); } /* @@ -1918,8 +1929,9 @@ static void _gpio_set_open_source_value(struct gpio_desc *desc, int value) } trace_gpio_direction(desc_to_gpio(desc), !value, err); if (err < 0) - pr_err("%s: Error in set_value for open source gpio%d err %d\n", - __func__, desc_to_gpio(desc), err); + gpiod_err(desc, + "%s: Error in set_value for open source err %d\n", + __func__, err); } /** -- 1.8.4.rc3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] gpiolib: Provide helper macros for logging of GPIO events 2013-09-06 12:38 ` [PATCH 2/3] gpiolib: Provide helper macros for logging of GPIO events Mark Brown @ 2013-09-07 2:34 ` Alexandre Courbot 2013-09-07 13:10 ` Mark Brown 0 siblings, 1 reply; 7+ messages in thread From: Alexandre Courbot @ 2013-09-07 2:34 UTC (permalink / raw) To: Mark Brown Cc: Linus Walleij, linux-gpio@vger.kernel.org, linaro-kernel, Mark Brown On Fri, Sep 6, 2013 at 9:38 PM, Mark Brown <broonie@kernel.org> wrote: > From: Mark Brown <broonie@linaro.org> > > Currently many but not all GPIO log messages log the GPIO number and the > formats vary. Ensure that this is done consistently by defining logging > helpers which take the GPIO descriptor. > > The will help people pattern matching on logs and providing the number > makes the log messages that omitted it more useful. Acked-by: Alexandre Courbot <acourbot@nvidia.com> I like the idea of having log helpers, but we are trying to deprecate the representation of GPIO as integers. Well, for now this is indeed the only way to use them, so what I am saying might only apply when the gpiod RFC patches are refined and merged, but how about thinking of another way to represent them, e.g. by GPIO chip name/GPIO index wrt. their chip? The representation needs to be compact, which is another challenge. Note that this is not a comment against this patch (which definitely improves the situation), just some thinking about a problem we will likely face later. > > Signed-off-by: Mark Brown <broonie@linaro.org> > --- > drivers/gpio/gpiolib.c | 52 +++++++++++++++++++++++++++++++------------------- > 1 file changed, 32 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index 374cf9e..e389183 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -103,6 +103,18 @@ static int gpiod_export_link(struct device *dev, const char *name, > static int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value); > static void gpiod_unexport(struct gpio_desc *desc); > > +#define gpiod_emerg(desc, fmt, ...) \ > + pr_emerg("gpio-%d: " fmt, desc_to_gpio(desc), ##__VA_ARGS__) > +#define gpiod_crit(desc, fmt, ...) \ > + pr_crit("gpio-%d: " fmt, desc_to_gpio(desc), ##__VA_ARGS__) > +#define gpiod_err(desc, fmt, ...) \ > + pr_err("gpio-%d: " fmt, desc_to_gpio(desc), ##__VA_ARGS__) > +#define gpiod_warn(desc, fmt, ...) \ > + pr_warn("gpio-%d: " fmt, desc_to_gpio(desc), ##__VA_ARGS__) > +#define gpiod_info(desc, fmt, ...) \ > + pr_info("gpio-%d: " fmt, desc_to_gpio(desc), ##__VA_ARGS__) > +#define gpiod_dbg(desc, fmt, ...) \ > + pr_debug("gpio-%d: " fmt, desc_to_gpio(desc), ##__VA_ARGS__) > > static inline void desc_set_label(struct gpio_desc *d, const char *label) > { > @@ -1636,8 +1648,9 @@ static int gpiod_direction_input(struct gpio_desc *desc) > > chip = desc->chip; > if (!chip->get || !chip->direction_input) { > - pr_warn("%s: missing get() or direction_input() operations\n", > - __func__); > + gpiod_warn(desc, > + "%s: missing get() or direction_input() operations\n", > + __func__); > return -EIO; > } > > @@ -1657,8 +1670,7 @@ static int gpiod_direction_input(struct gpio_desc *desc) > if (status) { > status = chip->request(chip, offset); > if (status < 0) { > - pr_debug("GPIO-%d: chip request fail, %d\n", > - desc_to_gpio(desc), status); > + gpiod_dbg(desc, "chip request fail, %d\n", status); > /* and it's not available to anyone else ... > * gpio_request() is the fully clean solution. > */ > @@ -1676,8 +1688,7 @@ lose: > fail: > spin_unlock_irqrestore(&gpio_lock, flags); > if (status) > - pr_debug("%s: gpio-%d status %d\n", __func__, > - desc_to_gpio(desc), status); > + gpiod_dbg(desc, "%s status %d\n", __func__, status); > return status; > } > > @@ -1709,8 +1720,9 @@ static int gpiod_direction_output(struct gpio_desc *desc, int value) > > chip = desc->chip; > if (!chip->set || !chip->direction_output) { > - pr_warn("%s: missing set() or direction_output() operations\n", > - __func__); > + gpiod_warn(desc, > + "%s: missing set() or direction_output() operations\n", > + __func__); > return -EIO; > } > > @@ -1730,8 +1742,7 @@ static int gpiod_direction_output(struct gpio_desc *desc, int value) > if (status) { > status = chip->request(chip, offset); > if (status < 0) { > - pr_debug("GPIO-%d: chip request fail, %d\n", > - desc_to_gpio(desc), status); > + gpiod_dbg(desc, "chip request fail, %d\n", status); > /* and it's not available to anyone else ... > * gpio_request() is the fully clean solution. > */ > @@ -1749,8 +1760,7 @@ lose: > fail: > spin_unlock_irqrestore(&gpio_lock, flags); > if (status) > - pr_debug("%s: gpio-%d status %d\n", __func__, > - desc_to_gpio(desc), status); > + gpiod_dbg(desc, "%s: gpio status %d\n", __func__, status); > return status; > } > > @@ -1779,8 +1789,9 @@ static int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce) > > chip = desc->chip; > if (!chip->set || !chip->set_debounce) { > - pr_debug("%s: missing set() or set_debounce() operations\n", > - __func__); > + gpiod_dbg(desc, > + "%s: missing set() or set_debounce() operations\n", > + __func__); > return -EIO; > } > > @@ -1802,8 +1813,7 @@ static int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce) > fail: > spin_unlock_irqrestore(&gpio_lock, flags); > if (status) > - pr_debug("%s: gpio-%d status %d\n", __func__, > - desc_to_gpio(desc), status); > + gpiod_dbg(desc, "%s: status %d\n", __func__, status); > > return status; > } > @@ -1891,8 +1901,9 @@ static void _gpio_set_open_drain_value(struct gpio_desc *desc, int value) > } > trace_gpio_direction(desc_to_gpio(desc), value, err); > if (err < 0) > - pr_err("%s: Error in set_value for open drain gpio%d err %d\n", > - __func__, desc_to_gpio(desc), err); > + gpiod_err(desc, > + "%s: Error in set_value for open drain err %d\n", > + __func__, err); > } > > /* > @@ -1918,8 +1929,9 @@ static void _gpio_set_open_source_value(struct gpio_desc *desc, int value) > } > trace_gpio_direction(desc_to_gpio(desc), !value, err); > if (err < 0) > - pr_err("%s: Error in set_value for open source gpio%d err %d\n", > - __func__, desc_to_gpio(desc), err); > + gpiod_err(desc, > + "%s: Error in set_value for open source err %d\n", > + __func__, err); > } > > /** > -- > 1.8.4.rc3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-gpio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] gpiolib: Provide helper macros for logging of GPIO events 2013-09-07 2:34 ` Alexandre Courbot @ 2013-09-07 13:10 ` Mark Brown 0 siblings, 0 replies; 7+ messages in thread From: Mark Brown @ 2013-09-07 13:10 UTC (permalink / raw) To: Alexandre Courbot Cc: Linus Walleij, linux-gpio@vger.kernel.org, linaro-kernel [-- Attachment #1: Type: text/plain, Size: 651 bytes --] On Sat, Sep 07, 2013 at 11:34:36AM +0900, Alexandre Courbot wrote: > I like the idea of having log helpers, but we are trying to deprecate > the representation of GPIO as integers. Well, for now this is indeed > the only way to use them, so what I am saying might only apply when > the gpiod RFC patches are refined and merged, but how about thinking > of another way to represent them, e.g. by GPIO chip name/GPIO index > wrt. their chip? The representation needs to be compact, which is > another challenge. dev_name() plus label perhaps? Some of the labels do tend to get a bit long but if they're used this way it'd probably encourage brevity. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] gpiolib: Include GPIO label in log messages for GPIOs 2013-09-06 12:38 [PATCH 1/3] gpiolib: Downgrade set_debounce() missing operation warning Mark Brown 2013-09-06 12:38 ` [PATCH 2/3] gpiolib: Provide helper macros for logging of GPIO events Mark Brown @ 2013-09-06 12:38 ` Mark Brown 2013-09-09 7:47 ` [PATCH 1/3] gpiolib: Downgrade set_debounce() missing operation warning Linus Walleij 2 siblings, 0 replies; 7+ messages in thread From: Mark Brown @ 2013-09-06 12:38 UTC (permalink / raw) To: Linus Walleij; +Cc: linux-gpio, linaro-kernel, Mark Brown From: Mark Brown <broonie@linaro.org> Provide the human readable label for the GPIO as well as the number when we are recording it in order to improve the readability of log messages. Signed-off-by: Mark Brown <broonie@linaro.org> --- drivers/gpio/gpiolib.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index e389183..a3a41bc 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -103,6 +103,26 @@ static int gpiod_export_link(struct device *dev, const char *name, static int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value); static void gpiod_unexport(struct gpio_desc *desc); +#ifdef CONFIG_DEBUG_FS +#define gpiod_emerg(desc, fmt, ...) \ + pr_emerg("gpio-%d (%s): " fmt, desc_to_gpio(desc), desc->label, \ + ##__VA_ARGS__) +#define gpiod_crit(desc, fmt, ...) \ + pr_crit("gpio-%d (%s): " fmt, desc_to_gpio(desc), desc->label, \ + ##__VA_ARGS__) +#define gpiod_err(desc, fmt, ...) \ + pr_err("gpio-%d (%s): " fmt, desc_to_gpio(desc), desc->label, \ + ##__VA_ARGS__) +#define gpiod_warn(desc, fmt, ...) \ + pr_warn("gpio-%d (%s): " fmt, desc_to_gpio(desc), desc->label, \ + ##__VA_ARGS__) +#define gpiod_info(desc, fmt, ...) \ + pr_info("gpio-%d (%s): " fmt, desc_to_gpio(desc), desc->label, \ + ##__VA_ARGS__) +#define gpiod_dbg(desc, fmt, ...) \ + pr_debug("gpio-%d (%s): " fmt, desc_to_gpio(desc), desc->label, \ + ##__VA_ARGS__) +#else #define gpiod_emerg(desc, fmt, ...) \ pr_emerg("gpio-%d: " fmt, desc_to_gpio(desc), ##__VA_ARGS__) #define gpiod_crit(desc, fmt, ...) \ @@ -115,6 +135,7 @@ static void gpiod_unexport(struct gpio_desc *desc); pr_info("gpio-%d: " fmt, desc_to_gpio(desc), ##__VA_ARGS__) #define gpiod_dbg(desc, fmt, ...) \ pr_debug("gpio-%d: " fmt, desc_to_gpio(desc), ##__VA_ARGS__) +#endif static inline void desc_set_label(struct gpio_desc *d, const char *label) { -- 1.8.4.rc3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] gpiolib: Downgrade set_debounce() missing operation warning 2013-09-06 12:38 [PATCH 1/3] gpiolib: Downgrade set_debounce() missing operation warning Mark Brown 2013-09-06 12:38 ` [PATCH 2/3] gpiolib: Provide helper macros for logging of GPIO events Mark Brown 2013-09-06 12:38 ` [PATCH 3/3] gpiolib: Include GPIO label in log messages for GPIOs Mark Brown @ 2013-09-09 7:47 ` Linus Walleij 2013-09-09 9:26 ` Mark Brown 2 siblings, 1 reply; 7+ messages in thread From: Linus Walleij @ 2013-09-09 7:47 UTC (permalink / raw) To: Mark Brown; +Cc: linux-gpio@vger.kernel.org, linaro-kernel, Mark Brown On Fri, Sep 6, 2013 at 2:38 PM, Mark Brown <broonie@kernel.org> wrote: > +++ b/drivers/gpio/gpiolib.c > @@ -1779,8 +1779,8 @@ static int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce) > > chip = desc->chip; > if (!chip->set || !chip->set_debounce) { > - pr_warn("%s: missing set() or set_debounce() operations\n", > - __func__); > + pr_debug("%s: missing set() or set_debounce() operations\n", > + __func__); A patch like this is already upstream, but thanks anyway! Yours, Linus Walleij ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] gpiolib: Downgrade set_debounce() missing operation warning 2013-09-09 7:47 ` [PATCH 1/3] gpiolib: Downgrade set_debounce() missing operation warning Linus Walleij @ 2013-09-09 9:26 ` Mark Brown 0 siblings, 0 replies; 7+ messages in thread From: Mark Brown @ 2013-09-09 9:26 UTC (permalink / raw) To: Linus Walleij; +Cc: linux-gpio@vger.kernel.org, linaro-kernel [-- Attachment #1: Type: text/plain, Size: 201 bytes --] On Mon, Sep 09, 2013 at 09:47:17AM +0200, Linus Walleij wrote: > A patch like this is already upstream, but thanks anyway! Hrm, it hadn't found its way into -next on Friday but I see it's there now. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-09-09 9:26 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-09-06 12:38 [PATCH 1/3] gpiolib: Downgrade set_debounce() missing operation warning Mark Brown 2013-09-06 12:38 ` [PATCH 2/3] gpiolib: Provide helper macros for logging of GPIO events Mark Brown 2013-09-07 2:34 ` Alexandre Courbot 2013-09-07 13:10 ` Mark Brown 2013-09-06 12:38 ` [PATCH 3/3] gpiolib: Include GPIO label in log messages for GPIOs Mark Brown 2013-09-09 7:47 ` [PATCH 1/3] gpiolib: Downgrade set_debounce() missing operation warning Linus Walleij 2013-09-09 9:26 ` Mark Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).