[19/19] lirc_dev: consistent device registration printk
diff mbox

Message ID 149839397127.28811.15601147061333876867.stgit@zeus.hardeman.nu
State New
Headers show

Commit Message

David Härdeman June 25, 2017, 12:32 p.m. UTC
This patch changes the message that is printed on lirc device registration to
make it more consistent with the input and rc subsystems.

Before:
  rc rc0: rc-core loopback device as /devices/virtual/rc/rc0
  input: rc-core loopback device as /devices/virtual/rc/rc0/input43
  lirc lirc0: lirc_dev: driver ir-lirc-codec (rc-loopback) registered at minor = 0

After:
  rc rc0: rc-core loopback device as /devices/virtual/rc/rc0
  input: rc-core loopback device as /devices/virtual/rc/rc0/input23
  lirc lirc0: rc-core loopback device as /devices/virtual/rc/rc0/lirc0

Signed-off-by: David Härdeman <david@hardeman.nu>
---
 drivers/media/rc/ir-lirc-codec.c |    3 +--
 drivers/media/rc/lirc_dev.c      |    6 ++++--
 2 files changed, 5 insertions(+), 4 deletions(-)

Comments

Sean Young Aug. 1, 2017, 9:20 p.m. UTC | #1
On Sun, Jun 25, 2017 at 02:32:51PM +0200, David Härdeman wrote:
> This patch changes the message that is printed on lirc device registration to
> make it more consistent with the input and rc subsystems.
> 
> Before:
>   rc rc0: rc-core loopback device as /devices/virtual/rc/rc0
>   input: rc-core loopback device as /devices/virtual/rc/rc0/input43
>   lirc lirc0: lirc_dev: driver ir-lirc-codec (rc-loopback) registered at minor = 0
> 
> After:
>   rc rc0: rc-core loopback device as /devices/virtual/rc/rc0
>   input: rc-core loopback device as /devices/virtual/rc/rc0/input23
>   lirc lirc0: rc-core loopback device as /devices/virtual/rc/rc0/lirc0

There is a couple of problems with this.

1. lirc_dev name is 40 bytes, but rc_dev input_name has no limit, so it ends
up truncating which you don't want. For example:

[  106.303589] rc rc2: IguanaWorks USB IR Transceiver version 0x0308 as /devices/pci0000:00/0000:00:1a.1/usb4/4-2/4-2.3/4-2.3:1.0/rc/rc2
[  106.303664] input: IguanaWorks USB IR Transceiver version 0x0308 as /devices/pci0000:00/0000:00:1a.1/usb4/4-2/4-2.3/4-2.3:1.0/rc/rc2/input24
[  106.307272] lirc lirc2: IguanaWorks USB IR Transceiver version  as /devices/pci0000:00/0000:00:1a.1/usb4/4-2/4-2.3/4-2.3:1.0/rc/rc2/lirc2
[  233.005834] rc rc2: Media Center Ed. eHome Infrared Remote Transceiver (0609:031d) as /devices/pci0000:00/0000:00:1a.1/usb4/4-2/4-2.3/4-2.3:1.0/rc/rc2
[  233.005907] input: Media Center Ed. eHome Infrared Remote Transceiver (0609:031d) as /devices/pci0000:00/0000:00:1a.1/usb4/4-2/4-2.3/4-2.3:1.0/rc/rc2/input25
[  233.006307] lirc lirc2: Media Center Ed. eHome Infrared Remote  as /devices/pci0000:00/0000:00:1a.1/usb4/4-2/4-2.3/4-2.3:1.0/rc/rc2/lirc2

2. Documentation/media/uapi/rc/lirc-dev-intro.rst explicitly mentions this 
message, so that will need updating too. On a related note, should we be
changing messages, esp. documented ones? I agree the original message is
a bit ugly.

3. I like have the driver name in the message. As you can see above, you
have no idea you're plugging in an mceusb device (iguanair is a bit
more obvious). Then again none of the other messages mention this, maybe
it be added there.

As it is the patch can't be applied.

Maybe lirc_dev name can be changed to const char* so we avoid the strcpy
and truncation.


Sean

> 
> Signed-off-by: David Härdeman <david@hardeman.nu>
> ---
>  drivers/media/rc/ir-lirc-codec.c |    3 +--
>  drivers/media/rc/lirc_dev.c      |    6 ++++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
> index 05f88401f694..4f33516a95a3 100644
> --- a/drivers/media/rc/ir-lirc-codec.c
> +++ b/drivers/media/rc/ir-lirc-codec.c
> @@ -595,8 +595,7 @@ static int ir_lirc_register(struct rc_dev *dev)
>  	if (dev->max_timeout)
>  		features |= LIRC_CAN_SET_REC_TIMEOUT;
>  
> -	snprintf(ldev->name, sizeof(ldev->name), "ir-lirc-codec (%s)",
> -		 dev->driver_name);
> +	snprintf(ldev->name, sizeof(ldev->name), "%s", dev->input_name);
>  	ldev->features = features;
>  	ldev->data = &dev->raw->lirc;
>  	ldev->buf = NULL;
> diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
> index c1c917932f7e..03430a1fb192 100644
> --- a/drivers/media/rc/lirc_dev.c
> +++ b/drivers/media/rc/lirc_dev.c
> @@ -105,6 +105,7 @@ int lirc_register_device(struct lirc_dev *d)
>  {
>  	int minor;
>  	int err;
> +	const char *path;
>  
>  	if (!d) {
>  		pr_err("driver pointer must be not NULL!\n");
> @@ -171,8 +172,9 @@ int lirc_register_device(struct lirc_dev *d)
>  		return err;
>  	}
>  
> -	dev_info(&d->dev, "lirc_dev: driver %s registered at minor = %d\n",
> -		 d->name, d->minor);
> +	path = kobject_get_path(&d->dev.kobj, GFP_KERNEL);
> +	dev_info(&d->dev, "%s as %s\n", d->name, path ?: "N/A");
> +	kfree(path);
>  
>  	return 0;
>  }

Patch
diff mbox

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index 05f88401f694..4f33516a95a3 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -595,8 +595,7 @@  static int ir_lirc_register(struct rc_dev *dev)
 	if (dev->max_timeout)
 		features |= LIRC_CAN_SET_REC_TIMEOUT;
 
-	snprintf(ldev->name, sizeof(ldev->name), "ir-lirc-codec (%s)",
-		 dev->driver_name);
+	snprintf(ldev->name, sizeof(ldev->name), "%s", dev->input_name);
 	ldev->features = features;
 	ldev->data = &dev->raw->lirc;
 	ldev->buf = NULL;
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index c1c917932f7e..03430a1fb192 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -105,6 +105,7 @@  int lirc_register_device(struct lirc_dev *d)
 {
 	int minor;
 	int err;
+	const char *path;
 
 	if (!d) {
 		pr_err("driver pointer must be not NULL!\n");
@@ -171,8 +172,9 @@  int lirc_register_device(struct lirc_dev *d)
 		return err;
 	}
 
-	dev_info(&d->dev, "lirc_dev: driver %s registered at minor = %d\n",
-		 d->name, d->minor);
+	path = kobject_get_path(&d->dev.kobj, GFP_KERNEL);
+	dev_info(&d->dev, "%s as %s\n", d->name, path ?: "N/A");
+	kfree(path);
 
 	return 0;
 }