2022-12-23 03:10:12

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH 0/2] HID: i2c-hid: use standard debugging APIs

When trying to debug i2c-hid I was not able to use the builtin debug
mechanism because the "debug" module parameter is not writable.

We can change i2c-hid to use the standard kernel debugging APIs which are
more discoverable and have more features.

There is a bit of a stilistic conflict between consistently using
i2c_hid_dbg() and consistently using dev_<level>().

The second patch aligns debugging on i2c_hid_dbg().
If this is unneeded, feel free to drop it.

It would also be easy to just use dev_dbg() everywhere for consistency.

To: Jiri Kosina <[email protected]>
To: Benjamin Tissoires <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Thomas Weißschuh <[email protected]>

---
Thomas Weißschuh (2):
HID: i2c-hid: switch to standard debugging APIs
HID: i2c-hid: use uniform debugging APIs

drivers/hid/i2c-hid/i2c-hid-core.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)
---
base-commit: 8395ae05cb5a2e31d36106e8c85efa11cda849be
change-id: 20221223-hid-dbg-cdc1d56d8cee

Best regards,
--
Thomas Weißschuh <[email protected]>


2022-12-23 03:37:46

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH 1/2] HID: i2c-hid: switch to standard debugging APIs

Instead of implementing a custom form of dynamic debugging we can use
the standard debugging APIs. If the kernel is built with
CONFIG_DYNAMIC_DEBUG this will be more discoverable and featureful.

Also the previous module parameter "debug" is read-only so it can't
actually be enabled.

Signed-off-by: Thomas Weißschuh <[email protected]>
---
drivers/hid/i2c-hid/i2c-hid-core.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index b86b62f97108..b4ed21f25600 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -67,16 +67,7 @@
#define I2C_HID_PWR_ON 0x00
#define I2C_HID_PWR_SLEEP 0x01

-/* debug option */
-static bool debug;
-module_param(debug, bool, 0444);
-MODULE_PARM_DESC(debug, "print a lot of debug information");
-
-#define i2c_hid_dbg(ihid, fmt, arg...) \
-do { \
- if (debug) \
- dev_printk(KERN_DEBUG, &(ihid)->client->dev, fmt, ##arg); \
-} while (0)
+#define i2c_hid_dbg(ihid, ...) dev_dbg(&(ihid)->client->dev, __VA_ARGS__)

struct i2c_hid_desc {
__le16 wHIDDescLength;

--
2.39.0

2022-12-23 03:46:14

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH 2/2] HID: i2c-hid: use uniform debugging APIs

Only two locations in i2c-hid are using the standard dev_dbg() APIs.
The rest are all using the custom i2c_hid_dbg(), which in turn uses
dev_dbg().

Signed-off-by: Thomas Weißschuh <[email protected]>
---
drivers/hid/i2c-hid/i2c-hid-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index b4ed21f25600..c83c93716782 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -850,7 +850,7 @@ static int i2c_hid_init_irq(struct i2c_client *client)
unsigned long irqflags = 0;
int ret;

- dev_dbg(&client->dev, "Requesting IRQ: %d\n", client->irq);
+ i2c_hid_dbg(ihid, "Requesting IRQ: %d\n", client->irq);

if (!irq_get_trigger_type(client->irq))
irqflags = IRQF_TRIGGER_LOW;
@@ -994,7 +994,7 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
/* Make sure there is something at this address */
ret = i2c_smbus_read_byte(client);
if (ret < 0) {
- dev_dbg(&client->dev, "nothing at this address: %d\n", ret);
+ i2c_hid_dbg(ihid, "nothing at this address: %d\n", ret);
ret = -ENXIO;
goto err_powered;
}

--
2.39.0

2023-01-18 10:06:56

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH 0/2] HID: i2c-hid: use standard debugging APIs

On Fri, 23 Dec 2022, Thomas Weißschuh wrote:

> When trying to debug i2c-hid I was not able to use the builtin debug
> mechanism because the "debug" module parameter is not writable.
>
> We can change i2c-hid to use the standard kernel debugging APIs which are
> more discoverable and have more features.
>
> There is a bit of a stilistic conflict between consistently using
> i2c_hid_dbg() and consistently using dev_<level>().
>
> The second patch aligns debugging on i2c_hid_dbg().
> If this is unneeded, feel free to drop it.
>
> It would also be easy to just use dev_dbg() everywhere for consistency.
>
> To: Jiri Kosina <[email protected]>
> To: Benjamin Tissoires <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Thomas Weißschuh <[email protected]>
>
> ---
> Thomas Weißschuh (2):
> HID: i2c-hid: switch to standard debugging APIs
> HID: i2c-hid: use uniform debugging APIs
>
> drivers/hid/i2c-hid/i2c-hid-core.c | 15 +++------------
> 1 file changed, 3 insertions(+), 12 deletions(-)

Applied to for-6.3/i2c-hid branch, thanks Thomas.

--
Jiri Kosina
SUSE Labs