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

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

[Thomas Weißschuh]

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 <jikos@kernel.org>
To: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>

---
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 <linux@weissschuh.net>

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

[Thomas Weißschuh]

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 <linux@weissschuh.net>
---
 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

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

[Thomas Weißschuh]

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 <linux@weissschuh.net>
---
 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

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

[Jiri Kosina]

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 <jikos@kernel.org>
> To: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: linux-input@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> 
> ---
> 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