[PATCH] V4L/DVB: ir: Add a link to associate /sys/class/ir/irrcv with the input device

[PATCH] V4L/DVB: ir: Add a link to associate /sys/class/ir/irrcv with the input device

[Mauro Carvalho Chehab]

In order to allow userspace programs to autoload an IR table, a link is
needed to point to the corresponding input device.

$ tree /sys/class/irrcv/irrcv0
/sys/class/irrcv/irrcv0
|-- current_protocol
|-- input -> ../../../pci0000:00/0000:00:0b.1/usb1/1-3/input/input22
|-- power
|   `-- wakeup
|-- subsystem -> ../../../../class/irrcv
`-- uevent

It is now easy to associate an irrcv device with the corresponding
device node, at the input interface.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/IR/ir-sysfs.c b/drivers/media/IR/ir-sysfs.c
index bf5fbcd..7de32e7 100644
--- a/drivers/media/IR/ir-sysfs.c
+++ b/drivers/media/IR/ir-sysfs.c
@@ -138,6 +138,7 @@ int ir_register_class(struct input_dev *input_dev)
 {
 	int rc;
 	struct kobject *kobj;
+	const char *path;
 
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
 	int devno = find_first_zero_bit(&ir_core_dev_number,
@@ -152,13 +153,26 @@ int ir_register_class(struct input_dev *input_dev)
 					  "irrcv%d", devno);
 	kobj = &ir_dev->class_dev->kobj;
 
-	printk(KERN_WARNING "Creating IR device %s\n", kobject_name(kobj));
 	rc = sysfs_create_group(kobj, &ir_dev->attr);
 	if (unlikely(rc < 0)) {
 		device_destroy(ir_input_class, input_dev->dev.devt);
 		return -ENOMEM;
 	}
 
+	rc = sysfs_create_link(kobj, &input_dev->dev.kobj, "input");
+	if (unlikely(rc < 0)) {
+		sysfs_remove_group(kobj, &ir_dev->attr);
+		device_destroy(ir_input_class, input_dev->dev.devt);
+		return -ENOMEM;
+	}
+
+	path = kobject_get_path(&input_dev->dev.kobj, GFP_KERNEL);
+	printk(KERN_INFO "%s: %s associated with sysfs %s\n",
+		kobject_name(kobj),
+		input_dev->name ? input_dev->name : "Unspecified device",
+		path ? path : "N/A");
+	kfree(path);
+
 	ir_dev->devno = devno;
 	set_bit(devno, &ir_core_dev_number);
 
@@ -181,6 +195,8 @@ void ir_unregister_class(struct input_dev *input_dev)
 
 	kobj = &ir_dev->class_dev->kobj;
 
+	sysfs_remove_link(kobj, "input");
+
 	sysfs_remove_group(kobj, &ir_dev->attr);
 	device_destroy(ir_input_class, input_dev->dev.devt);
 
-- 
1.6.6.1

Re: [PATCH] V4L/DVB: ir: Add a link to associate /sys/class/ir/irrcv with the input device

[Dmitry Torokhov]

Hi Mauro,

On Thu, Mar 11, 2010 at 12:46:19PM -0300, Mauro Carvalho Chehab wrote:
> In order to allow userspace programs to autoload an IR table, a link is
> needed to point to the corresponding input device.
> 
> $ tree /sys/class/irrcv/irrcv0
> /sys/class/irrcv/irrcv0
> |-- current_protocol
> |-- input -> ../../../pci0000:00/0000:00:0b.1/usb1/1-3/input/input22
> |-- power
> |   `-- wakeup
> |-- subsystem -> ../../../../class/irrcv
> `-- uevent
> 
> It is now easy to associate an irrcv device with the corresponding
> device node, at the input interface.
> 

I guess the question is why don't you make input device a child of your
irrcvX device? Then I believe driver core will link them properly. It
will also ensure proper power management hierarchy.

That probably will require you changing from class_dev into device but
that's the direction kernel is going to anyway.

Thanks.

-- 
Dmitry

Re: [PATCH] V4L/DVB: ir: Add a link to associate /sys/class/ir/irrcv with the input device

[Mauro Carvalho Chehab]

Dmitry,

Dmitry Torokhov wrote:
> Hi Mauro,
> 
> On Thu, Mar 11, 2010 at 12:46:19PM -0300, Mauro Carvalho Chehab wrote:
>> In order to allow userspace programs to autoload an IR table, a link is
>> needed to point to the corresponding input device.
>>
>> $ tree /sys/class/irrcv/irrcv0
>> /sys/class/irrcv/irrcv0
>> |-- current_protocol
>> |-- input -> ../../../pci0000:00/0000:00:0b.1/usb1/1-3/input/input22
>> |-- power
>> |   `-- wakeup
>> |-- subsystem -> ../../../../class/irrcv
>> `-- uevent
>>
>> It is now easy to associate an irrcv device with the corresponding
>> device node, at the input interface.
>>
> 
> I guess the question is why don't you make input device a child of your
> irrcvX device? Then I believe driver core will link them properly. It
> will also ensure proper power management hierarchy.
> 
> That probably will require you changing from class_dev into device but
> that's the direction kernel is going to anyway.

I remember you asked me to create a separate class for IR. The current code
does it, using class_create(). Once the class is created, I'm using 
device_create() to create the nodes. 

The current code is:

int ir_register_class(struct input_dev *input_dev)
{
	...
	ir_dev->class_dev = device_create(ir_input_class, NULL,
					  input_dev->dev.devt, ir_dev,
					  "irrcv%d", devno);
	...
	rc = sysfs_create_group(kobj, &ir_dev->attr);
	...
	rc = sysfs_create_link(kobj, &input_dev->dev.kobj, "input");
	...
	return 0;
};

int ir_input_register(struct input_dev *input_dev,
		      const struct ir_scancode_table *rc_tab,
		      const struct ir_dev_props *props)
{
	...
	rc = input_register_device(input_dev);
	...
	rc = ir_register_class(input_dev);
	...
}

static int __init ir_core_init(void)
{
	ir_input_class = class_create(THIS_MODULE, "irrcv");
	...
}

I couldn't find any other way to create the link without explicitly
calling sysfs_create_link().

Do you have a better idea?

-- 

Cheers,
Mauro

Re: [PATCH] V4L/DVB: ir: Add a link to associate /sys/class/ir/irrcv with the input device

[Mauro Carvalho Chehab]

Dmitry Torokhov wrote:
> Hi Mauro,
> 
> On Thu, Mar 11, 2010 at 12:46:19PM -0300, Mauro Carvalho Chehab wrote:
>> In order to allow userspace programs to autoload an IR table, a link is
>> needed to point to the corresponding input device.
>>
>> $ tree /sys/class/irrcv/irrcv0
>> /sys/class/irrcv/irrcv0
>> |-- current_protocol
>> |-- input -> ../../../pci0000:00/0000:00:0b.1/usb1/1-3/input/input22
>> |-- power
>> |   `-- wakeup
>> |-- subsystem -> ../../../../class/irrcv
>> `-- uevent
>>
>> It is now easy to associate an irrcv device with the corresponding
>> device node, at the input interface.
>>
> 
> I guess the question is why don't you make input device a child of your
> irrcvX device? Then I believe driver core will link them properly. It
> will also ensure proper power management hierarchy.
> 
> That probably will require you changing from class_dev into device but
> that's the direction kernel is going to anyway.

Done, see enclosed. It is now using class_register/device_register. The
newly created device for irrcv is used as the parent for input_dev->dev.

The resulting code looked cleaner after the change ;)

Cheers,
Mauro

---

V4L/DVB: ir: use a real device instead of a virtual class
    
Change the ir-sysfs approach to create irrcv0 as a device, instead of
using class_dev. Also, change the way input is registered, in order
to make its parent to be the irrcv device.

Due to this change, now the event device is created under
/sys/class/ir/irrcv class:

/sys/class/irrcv/irrcv0/
|-- current_protocol
|-- device -> ../../../1-3
|-- input9
|   |-- capabilities
|   |   |-- abs
...

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/IR/ir-keytable.c b/drivers/media/IR/ir-keytable.c
index 0903f53..c9c0a54 100644
--- a/drivers/media/IR/ir-keytable.c
+++ b/drivers/media/IR/ir-keytable.c
@@ -448,22 +448,15 @@ int ir_input_register(struct input_dev *input_dev,
 	input_dev->setkeycode = ir_setkeycode;
 	input_set_drvdata(input_dev, ir_dev);
 
-	rc = input_register_device(input_dev);
-	if (rc < 0)
-		goto err;
-
 	rc = ir_register_class(input_dev);
-	if (rc < 0) {
-		input_unregister_device(input_dev);
+	if (rc < 0)
 		goto err;
-	}
 
 	return 0;
 
 err:
 	kfree(rc_tab->scan);
 	kfree(ir_dev);
-	input_set_drvdata(input_dev, NULL);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(ir_input_register);
@@ -492,7 +485,6 @@ void ir_input_unregister(struct input_dev *dev)
 	ir_unregister_class(dev);
 
 	kfree(ir_dev);
-	input_unregister_device(dev);
 }
 EXPORT_SYMBOL_GPL(ir_input_unregister);
 
diff --git a/drivers/media/IR/ir-sysfs.c b/drivers/media/IR/ir-sysfs.c
index bf5fbcd..1bb011a 100644
--- a/drivers/media/IR/ir-sysfs.c
+++ b/drivers/media/IR/ir-sysfs.c
@@ -22,7 +22,15 @@
 static unsigned long ir_core_dev_number;
 
 /* class for /sys/class/irrcv */
-static struct class *ir_input_class;
+static char *ir_devnode(struct device *dev, mode_t *mode)
+{
+	return kasprintf(GFP_KERNEL, "irrcv/%s", dev_name(dev));
+}
+
+struct class ir_input_class = {
+	.name		= "irrcv",
+	.devnode	= ir_devnode,
+};
 
 /**
  * show_protocol() - shows the current IR protocol
@@ -128,6 +136,20 @@ static struct attribute *ir_dev_attrs[] = {
 	NULL,
 };
 
+static struct attribute_group ir_dev_attr_grp = {
+	.attrs	=ir_dev_attrs,
+};
+
+static const struct attribute_group *ir_dev_attr_groups[] = {
+	&ir_dev_attr_grp,
+	NULL
+};
+
+static struct device_type ir_dev_type = {
+	.groups		= ir_dev_attr_groups,
+};
+
+
 /**
  * ir_register_class() - creates the sysfs for /sys/class/irrcv/irrcv?
  * @input_dev:	the struct input_dev descriptor of the device
@@ -137,7 +159,7 @@ static struct attribute *ir_dev_attrs[] = {
 int ir_register_class(struct input_dev *input_dev)
 {
 	int rc;
-	struct kobject *kobj;
+	const char *path;
 
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
 	int devno = find_first_zero_bit(&ir_core_dev_number,
@@ -146,19 +168,31 @@ int ir_register_class(struct input_dev *input_dev)
 	if (unlikely(devno < 0))
 		return devno;
 
-	ir_dev->attr.attrs = ir_dev_attrs;
-	ir_dev->class_dev = device_create(ir_input_class, NULL,
-					  input_dev->dev.devt, ir_dev,
-					  "irrcv%d", devno);
-	kobj = &ir_dev->class_dev->kobj;
-
-	printk(KERN_WARNING "Creating IR device %s\n", kobject_name(kobj));
-	rc = sysfs_create_group(kobj, &ir_dev->attr);
-	if (unlikely(rc < 0)) {
-		device_destroy(ir_input_class, input_dev->dev.devt);
-		return -ENOMEM;
+	ir_dev->dev.type = &ir_dev_type;
+	ir_dev->dev.class = &ir_input_class;
+	ir_dev->dev.parent = input_dev->dev.parent;
+	dev_set_name(&ir_dev->dev, "irrcv%d", devno);
+	rc = device_register(&ir_dev->dev);
+	if (rc)
+		return rc;
+
+
+	input_dev->dev.parent = &ir_dev->dev;
+	rc = input_register_device(input_dev);
+	if (rc < 0) {
+		device_del(&ir_dev->dev);
+		return rc;
 	}
 
+	__module_get(THIS_MODULE);
+
+	path = kobject_get_path(&input_dev->dev.kobj, GFP_KERNEL);
+	printk(KERN_INFO "%s: %s associated with sysfs %s\n",
+		dev_name(&ir_dev->dev),
+		input_dev->name ? input_dev->name : "Unspecified device",
+		path ? path : "N/A");
+	kfree(path);
+
 	ir_dev->devno = devno;
 	set_bit(devno, &ir_core_dev_number);
 
@@ -175,16 +209,12 @@ int ir_register_class(struct input_dev *input_dev)
 void ir_unregister_class(struct input_dev *input_dev)
 {
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	struct kobject *kobj;
 
 	clear_bit(ir_dev->devno, &ir_core_dev_number);
+	input_unregister_device(input_dev);
+	device_del(&ir_dev->dev);
 
-	kobj = &ir_dev->class_dev->kobj;
-
-	sysfs_remove_group(kobj, &ir_dev->attr);
-	device_destroy(ir_input_class, input_dev->dev.devt);
-
-	kfree(ir_dev->attr.name);
+	module_put(THIS_MODULE);
 }
 
 /*
@@ -193,10 +223,10 @@ void ir_unregister_class(struct input_dev *input_dev)
 
 static int __init ir_core_init(void)
 {
-	ir_input_class = class_create(THIS_MODULE, "irrcv");
-	if (IS_ERR(ir_input_class)) {
+	int rc = class_register(&ir_input_class);
+	if (rc) {
 		printk(KERN_ERR "ir_core: unable to register irrcv class\n");
-		return PTR_ERR(ir_input_class);
+		return rc;
 	}
 
 	return 0;
@@ -204,7 +234,7 @@ static int __init ir_core_init(void)
 
 static void __exit ir_core_exit(void)
 {
-	class_destroy(ir_input_class);
+	class_unregister(&ir_input_class);
 }
 
 module_init(ir_core_init);
diff --git a/include/media/ir-core.h b/include/media/ir-core.h
index 61c223b..ce9f347 100644
--- a/include/media/ir-core.h
+++ b/include/media/ir-core.h
@@ -47,11 +47,9 @@ struct ir_dev_props {
 
 
 struct ir_input_dev {
-	struct input_dev		*dev;		/* Input device*/
+	struct device			dev;		/* device */
 	struct ir_scancode_table	rc_tab;		/* scan/key table */
 	unsigned long			devno;		/* device number */
-	struct attribute_group		attr;		/* IR attributes */
-	struct device			*class_dev;	/* virtual class dev */
 	const struct ir_dev_props	*props;		/* Device properties */
 };
 #define to_ir_input_dev(_attr) container_of(_attr, struct ir_input_dev, attr)

Re: [PATCH] V4L/DVB: ir: Add a link to associate /sys/class/ir/irrcv with the input device

[Dmitry Torokhov]

On Fri, Mar 12, 2010 at 01:32:23AM -0300, Mauro Carvalho Chehab wrote:
> Dmitry Torokhov wrote:
> > Hi Mauro,
> > 
> > On Thu, Mar 11, 2010 at 12:46:19PM -0300, Mauro Carvalho Chehab wrote:
> >> In order to allow userspace programs to autoload an IR table, a link is
> >> needed to point to the corresponding input device.
> >>
> >> $ tree /sys/class/irrcv/irrcv0
> >> /sys/class/irrcv/irrcv0
> >> |-- current_protocol
> >> |-- input -> ../../../pci0000:00/0000:00:0b.1/usb1/1-3/input/input22
> >> |-- power
> >> |   `-- wakeup
> >> |-- subsystem -> ../../../../class/irrcv
> >> `-- uevent
> >>
> >> It is now easy to associate an irrcv device with the corresponding
> >> device node, at the input interface.
> >>
> > 
> > I guess the question is why don't you make input device a child of your
> > irrcvX device? Then I believe driver core will link them properly. It
> > will also ensure proper power management hierarchy.
> > 
> > That probably will require you changing from class_dev into device but
> > that's the direction kernel is going to anyway.
> 
> Done, see enclosed. It is now using class_register/device_register. The
> newly created device for irrcv is used as the parent for input_dev->dev.
> 
> The resulting code looked cleaner after the change ;)
>

It is indeed better, however I wonder if current hierarchy expresses the
hardware in best way. You currently have irrcv devices grow in parallel
with input devices whereas I would expect input devices be children of
irrcv devices:


	parent (PCI board, USB) -> irrcvX -> input1
                                          -> input2
					 ...

This way your PM sequence as follows - input core does its thing and
releases all pressed keys, etc, then you can shut off the receiver and
then board driver can shut doen the main piece. Otherwise irrcv0 suspend
may be racing with input suspend and so forth.

Thanks.

-- 
Dmitry

Re: [PATCH] V4L/DVB: ir: Add a link to associate /sys/class/ir/irrcv with the input device

[Mauro Carvalho Chehab]

Dmitry Torokhov wrote:
> On Fri, Mar 12, 2010 at 01:32:23AM -0300, Mauro Carvalho Chehab wrote:
>> Dmitry Torokhov wrote:
>>> Hi Mauro,
>>>
>>> On Thu, Mar 11, 2010 at 12:46:19PM -0300, Mauro Carvalho Chehab wrote:
>>>> In order to allow userspace programs to autoload an IR table, a link is
>>>> needed to point to the corresponding input device.
>>>>
>>>> $ tree /sys/class/irrcv/irrcv0
>>>> /sys/class/irrcv/irrcv0
>>>> |-- current_protocol
>>>> |-- input -> ../../../pci0000:00/0000:00:0b.1/usb1/1-3/input/input22
>>>> |-- power
>>>> |   `-- wakeup
>>>> |-- subsystem -> ../../../../class/irrcv
>>>> `-- uevent
>>>>
>>>> It is now easy to associate an irrcv device with the corresponding
>>>> device node, at the input interface.
>>>>
>>> I guess the question is why don't you make input device a child of your
>>> irrcvX device? Then I believe driver core will link them properly. It
>>> will also ensure proper power management hierarchy.
>>>
>>> That probably will require you changing from class_dev into device but
>>> that's the direction kernel is going to anyway.
>> Done, see enclosed. It is now using class_register/device_register. The
>> newly created device for irrcv is used as the parent for input_dev->dev.
>>
>> The resulting code looked cleaner after the change ;)
>>
> 
> It is indeed better, however I wonder if current hierarchy expresses the
> hardware in best way. You currently have irrcv devices grow in parallel
> with input devices whereas I would expect input devices be children of
> irrcv devices:
> 
> 
> 	parent (PCI board, USB) -> irrcvX -> input1
>                                           -> input2
> 					 ...
> 

It is representing it right:

usb1/1-3 -> irrcv -> irrcv0 -> input7 -> event7

The only extra attribute there is the class name "irrcv", but this seems
coherent with the other classes on this device (dvb, sound, power, video4linux).

The created tree is:

$ tree /sys/class/irrcv/
/sys/class/irrcv/
`-- irrcv0 -> ../../devices/pci0000:00/0000:00:0b.1/usb1/1-3/irrcv/irrcv0

$ tree /sys/devices/pci0000:00/0000:00:0b.1/usb1/1-3/
/sys/devices/pci0000:00/0000:00:0b.1/usb1/1-3/
|-- 1-3:1.0
|   |-- bAlternateSetting
|   |-- bInterfaceClass
|   |-- bInterfaceNumber
|   |-- bInterfaceProtocol
|   |-- bInterfaceSubClass
|   |-- bNumEndpoints
|   |-- driver -> ../../../../../../bus/usb/drivers/em28xx
|   |-- ep_81
|   |   |-- bEndpointAddress
|   |   |-- bInterval
|   |   |-- bLength
|   |   |-- bmAttributes
|   |   |-- direction
|   |   |-- interval
|   |   |-- power
|   |   |   `-- wakeup
|   |   |-- type
|   |   |-- uevent
|   |   `-- wMaxPacketSize
|   |-- ep_82
|   |   |-- bEndpointAddress
|   |   |-- bInterval
|   |   |-- bLength
|   |   |-- bmAttributes
|   |   |-- direction
|   |   |-- interval
|   |   |-- power
|   |   |   `-- wakeup
|   |   |-- type
|   |   |-- uevent
|   |   `-- wMaxPacketSize
|   |-- ep_83
|   |   |-- bEndpointAddress
|   |   |-- bInterval
|   |   |-- bLength
|   |   |-- bmAttributes
|   |   |-- direction
|   |   |-- interval
|   |   |-- power
|   |   |   `-- wakeup
|   |   |-- type
|   |   |-- uevent
|   |   `-- wMaxPacketSize
|   |-- ep_84
|   |   |-- bEndpointAddress
|   |   |-- bInterval
|   |   |-- bLength
|   |   |-- bmAttributes
|   |   |-- direction
|   |   |-- interval
|   |   |-- power
|   |   |   `-- wakeup
|   |   |-- type
|   |   |-- uevent
|   |   `-- wMaxPacketSize
|   |-- modalias
|   |-- power
|   |   `-- wakeup
|   |-- subsystem -> ../../../../../../bus/usb
|   |-- supports_autosuspend
|   |-- uevent
|   `-- video4linux
|       |-- vbi2
|       |   |-- dev
|       |   |-- device -> ../../../1-3:1.0
|       |   |-- index
|       |   |-- name
|       |   |-- power
|       |   |   `-- wakeup
|       |   |-- subsystem -> ../../../../../../../../class/video4linux
|       |   `-- uevent
|       `-- video2
|           |-- dev
|           |-- device -> ../../../1-3:1.0
|           |-- index
|           |-- name
|           |-- power
|           |   `-- wakeup
|           |-- subsystem -> ../../../../../../../../class/video4linux
|           `-- uevent
|-- authorized
|-- bcdDevice
|-- bConfigurationValue
|-- bDeviceClass
|-- bDeviceProtocol
|-- bDeviceSubClass
|-- bmAttributes
|-- bMaxPacketSize0
|-- bMaxPower
|-- bNumConfigurations
|-- bNumInterfaces
|-- busnum
|-- configuration
|-- descriptors
|-- dev
|-- devnum
|-- devpath
|-- driver -> ../../../../../bus/usb/drivers/usb
|-- dvb
|   |-- dvb0.demux0
|   |   |-- dev
|   |   |-- device -> ../../../1-3
|   |   |-- power
|   |   |   `-- wakeup
|   |   |-- subsystem -> ../../../../../../../class/dvb
|   |   `-- uevent
|   |-- dvb0.dvr0
|   |   |-- dev
|   |   |-- device -> ../../../1-3
|   |   |-- power
|   |   |   `-- wakeup
|   |   |-- subsystem -> ../../../../../../../class/dvb
|   |   `-- uevent
|   |-- dvb0.frontend0
|   |   |-- dev
|   |   |-- device -> ../../../1-3
|   |   |-- power
|   |   |   `-- wakeup
|   |   |-- subsystem -> ../../../../../../../class/dvb
|   |   `-- uevent
|   `-- dvb0.net0
|       |-- dev
|       |-- device -> ../../../1-3
|       |-- power
|       |   `-- wakeup
|       |-- subsystem -> ../../../../../../../class/dvb
|       `-- uevent
|-- ep_00
|   |-- bEndpointAddress
|   |-- bInterval
|   |-- bLength
|   |-- bmAttributes
|   |-- direction
|   |-- interval
|   |-- power
|   |   `-- wakeup
|   |-- type
|   |-- uevent
|   `-- wMaxPacketSize
|-- i2c-3
|   |-- 3-005c
|   |   |-- driver -> ../../../../../../../bus/i2c/drivers/tvp5150
|   |   |-- modalias
|   |   |-- name
|   |   |-- power
|   |   |   `-- wakeup
|   |   |-- subsystem -> ../../../../../../../bus/i2c
|   |   `-- uevent
|   |-- 3-0061
|   |   |-- driver -> ../../../../../../../bus/i2c/drivers/tuner
|   |   |-- modalias
|   |   |-- name
|   |   |-- power
|   |   |   `-- wakeup
|   |   |-- subsystem -> ../../../../../../../bus/i2c
|   |   `-- uevent
|   |-- delete_device
|   |-- device -> ../../1-3
|   |-- name
|   |-- new_device
|   |-- power
|   |   `-- wakeup
|   |-- subsystem -> ../../../../../../bus/i2c
|   `-- uevent
|-- idProduct
|-- idVendor
|-- irrcv
|   `-- irrcv0
|       |-- current_protocol
|       |-- device -> ../../../1-3
|       |-- input7
|       |   |-- capabilities
|       |   |   |-- abs
|       |   |   |-- ev
|       |   |   |-- ff
|       |   |   |-- key
|       |   |   |-- led
|       |   |   |-- msc
|       |   |   |-- rel
|       |   |   |-- snd
|       |   |   `-- sw
|       |   |-- device -> ../../irrcv0
|       |   |-- event7
|       |   |   |-- dev
|       |   |   |-- device -> ../../input7
|       |   |   |-- power
|       |   |   |   `-- wakeup
|       |   |   |-- subsystem -> ../../../../../../../../../class/input
|       |   |   `-- uevent
|       |   |-- id
|       |   |   |-- bustype
|       |   |   |-- product
|       |   |   |-- vendor
|       |   |   `-- version
|       |   |-- modalias
|       |   |-- name
|       |   |-- phys
|       |   |-- power
|       |   |   `-- wakeup
|       |   |-- subsystem -> ../../../../../../../../class/input
|       |   |-- uevent
|       |   `-- uniq
|       |-- power
|       |   `-- wakeup
|       |-- subsystem -> ../../../../../../../class/irrcv
|       `-- uevent
|-- maxchild
|-- power
|   |-- active_duration
|   |-- autosuspend
|   |-- connected_duration
|   |-- level
|   |-- persist
|   `-- wakeup
|-- product
|-- quirks
|-- remove
|-- serial
|-- sound
|   `-- card1
|       |-- controlC1
|       |   |-- dev
|       |   |-- device -> ../../card1
|       |   |-- power
|       |   |   `-- wakeup
|       |   |-- subsystem -> ../../../../../../../../class/sound
|       |   `-- uevent
|       |-- device -> ../../../1-3
|       |-- id
|       |-- number
|       |-- pcmC1D0c
|       |   |-- dev
|       |   |-- device -> ../../card1
|       |   |-- pcm_class
|       |   |-- power
|       |   |   `-- wakeup
|       |   |-- subsystem -> ../../../../../../../../class/sound
|       |   `-- uevent
|       |-- power
|       |   `-- wakeup
|       |-- subsystem -> ../../../../../../../class/sound
|       `-- uevent
|-- speed
|-- subsystem -> ../../../../../bus/usb
|-- uevent
|-- urbnum
`-- version


$ tree /sys/class/irrcv/irrcv0/
/sys/class/irrcv/irrcv0/
|-- current_protocol
|-- device -> ../../../1-3
|-- input7
|   |-- capabilities
|   |   |-- abs
|   |   |-- ev
|   |   |-- ff
|   |   |-- key
|   |   |-- led
|   |   |-- msc
|   |   |-- rel
|   |   |-- snd
|   |   `-- sw
|   |-- device -> ../../irrcv0
|   |-- event7
|   |   |-- dev
|   |   |-- device -> ../../input7
|   |   |-- power
|   |   |   `-- wakeup
|   |   |-- subsystem -> ../../../../../../../../../class/input
|   |   `-- uevent
|   |-- id
|   |   |-- bustype
|   |   |-- product
|   |   |-- vendor
|   |   `-- version
|   |-- modalias
|   |-- name
|   |-- phys
|   |-- power
|   |   `-- wakeup
|   |-- subsystem -> ../../../../../../../../class/input
|   |-- uevent
|   `-- uniq
|-- power
|   `-- wakeup
|-- subsystem -> ../../../../../../../class/irrcv
`-- uevent

13 directories, 25 files

> This way your PM sequence as follows - input core does its thing and
> releases all pressed keys, etc, then you can shut off the receiver and
> then board driver can shut doen the main piece. Otherwise irrcv0 suspend
> may be racing with input suspend and so forth.
> 
> Thanks.
> 


-- 

Cheers,
Mauro

Re: [PATCH] V4L/DVB: ir: Add a link to associate /sys/class/ir/irrcv with the input device

[Dmitry Torokhov]

On Sat, Mar 13, 2010 at 05:59:34PM -0300, Mauro Carvalho Chehab wrote:
> Dmitry Torokhov wrote:
> > On Fri, Mar 12, 2010 at 01:32:23AM -0300, Mauro Carvalho Chehab wrote:
> >> Dmitry Torokhov wrote:
> >>> Hi Mauro,
> >>>
> >>> On Thu, Mar 11, 2010 at 12:46:19PM -0300, Mauro Carvalho Chehab wrote:
> >>>> In order to allow userspace programs to autoload an IR table, a link is
> >>>> needed to point to the corresponding input device.
> >>>>
> >>>> $ tree /sys/class/irrcv/irrcv0
> >>>> /sys/class/irrcv/irrcv0
> >>>> |-- current_protocol
> >>>> |-- input -> ../../../pci0000:00/0000:00:0b.1/usb1/1-3/input/input22
> >>>> |-- power
> >>>> |   `-- wakeup
> >>>> |-- subsystem -> ../../../../class/irrcv
> >>>> `-- uevent
> >>>>
> >>>> It is now easy to associate an irrcv device with the corresponding
> >>>> device node, at the input interface.
> >>>>
> >>> I guess the question is why don't you make input device a child of your
> >>> irrcvX device? Then I believe driver core will link them properly. It
> >>> will also ensure proper power management hierarchy.
> >>>
> >>> That probably will require you changing from class_dev into device but
> >>> that's the direction kernel is going to anyway.
> >> Done, see enclosed. It is now using class_register/device_register. The
> >> newly created device for irrcv is used as the parent for input_dev->dev.
> >>
> >> The resulting code looked cleaner after the change ;)
> >>
> > 
> > It is indeed better, however I wonder if current hierarchy expresses the
> > hardware in best way. You currently have irrcv devices grow in parallel
> > with input devices whereas I would expect input devices be children of
> > irrcv devices:
> > 
> > 
> > 	parent (PCI board, USB) -> irrcvX -> input1
> >                                           -> input2
> > 					 ...
> > 
> 
> It is representing it right:
> 
> usb1/1-3 -> irrcv -> irrcv0 -> input7 -> event7
> 
> The only extra attribute there is the class name "irrcv", but this seems
> coherent with the other classes on this device (dvb, sound, power, video4linux).
> 

Ah, yes, I saw where you take input device's parent and use it as
irrcv's parent but missed the piece where you substitute original
input device's parent with irrcv. I bit sneaky to my taste but I guess
can be cleaned up later.

-- 
Dmitry

Re: [PATCH] V4L/DVB: ir: Add a link to associate /sys/class/ir/irrcv with the input device

[Mauro Carvalho Chehab]

Dmitry Torokhov wrote:
> On Sat, Mar 13, 2010 at 05:59:34PM -0300, Mauro Carvalho Chehab wrote:

>> It is representing it right:
>>
>> usb1/1-3 -> irrcv -> irrcv0 -> input7 -> event7
>>
>> The only extra attribute there is the class name "irrcv", but this seems
>> coherent with the other classes on this device (dvb, sound, power, video4linux).
>>
> 
> Ah, yes, I saw where you take input device's parent and use it as
> irrcv's parent but missed the piece where you substitute original
> input device's parent with irrcv. I bit sneaky to my taste but I guess
> can be cleaned up later.

Yes. I opted to do this little trick to avoid changing the interfaces
on the other modules, and to minimize the needs when porting from input.
We can later clean it if/when needed.

-- 

Cheers,
Mauro