Thread (64 messages) 64 messages, 12 authors, 2010-04-10

Re: [RFC] What are the goals for the architecture of an in-kernel IR system?

From: Mauro Carvalho Chehab <hidden>
Date: 2009-12-13 12:15:40
Also in: linux-media, lkml

Possibly related (same subject, not in this thread)

Dmitry Torokhov wrote:
On Sun, Dec 06, 2009 at 09:34:26PM +0100, Krzysztof Halasa wrote:
quoted
Jon Smirl [off-list ref] writes:
quoted
quoted
Once again: how about agreement about the LIRC interface
(kernel-userspace) and merging the actual LIRC code first? In-kernel
decoding can wait a bit, it doesn't change any kernel-user interface.
I'd like to see a semi-complete design for an in-kernel IR system
before anything is merged from any source.
This is a way to nowhere, there is no logical dependency between LIRC
and input layer IR.

There is only one thing which needs attention before/when merging LIRC:
the LIRC user-kernel interface. In-kernel "IR system" is irrelevant and,
actually, making a correct IR core design without the LIRC merged can be
only harder.
This sounds like "merge first, think later"...

The question is why we need to merge lirc interface right now, before we
agreed on the sybsystem architecture? Noone _in kernel_ user lirc-dev
yet and, looking at the way things are shaping, no drivers will be
_directly_ using it after it is complete. So, even if we merge it right
away, the code will have to be restructured and reworked. Unfortunately,
just merging what Jarod posted, will introduce sysfs hierarchy which
is userspace interface as well (although we not as good maintaining it
at times) and will add more constraints on us.

That is why I think we should go the other way around - introduce the
core which receivers could plug into and decoder framework and once it
is ready register lirc-dev as one of the available decoders.
I've committed already some IR restruct code on my linux-next -git tree:

http://git.kernel.org/?p=linux/kernel/git/mchehab/linux-next.git

The code there basically moves the input/evdev registering code and 
scancode/keycode management code into a separate ir-core module.

To make my life easy, I've moved the code temporarily into drivers/media/IR.
This way, it helps me to move V4L specific code outside ir-core and to later
use it for DVB. After having it done, probably the better is to move it to
be under /drivers or /drivers/input.

The enclosed patch just adds a skeleton for the new sysfs class for remote
controllers and registers an yet unused ir_protocol attribute, creating this
tree:

/sys/class/irrcv/
|-- irrcv0
|   |-- ir_protocol
|   |-- power
|   |   `-- wakeup
|   |-- subsystem -> ../../irrcv
|   `-- uevent
`-- irrcv1
    |-- ir_protocol
    |-- power
    |   `-- wakeup
    |-- subsystem -> ../../irrcv
    `-- uevent

While writing the code, it occurred to me that calling it as "IR" is not the better way,
since there's nothing on the code that is related to infra-red, but, instead, is
is related to remote controller.

So, if it is ok for everybudy, IMO, we should use, instead "rc" meaning remote controller,
naming the core module as "rc-core", putting it into drivers/rc.

Also, since the same rc chip can have a receiver and a transmitter, maybe we can create the
class as:
	/sys/class/rc
		rcrcv0/
		rcrcv1/
		...
		rctx0/
		rctx1/
		...

Comments?


---
 linux/drivers/media/IR/Makefile      |    2 
 linux/drivers/media/IR/ir-keytable.c |   17 +++++-
 linux/drivers/media/IR/ir-sysfs.c    |   94 +++++++++++++++++++++++++++++++++++
 linux/include/media/ir-core.h        |   12 +++-
 4 files changed, 119 insertions(+), 6 deletions(-)
--- master.orig/linux/drivers/media/IR/Makefile
+++ master/linux/drivers/media/IR/Makefile
@@ -1,5 +1,5 @@
 ir-common-objs  := ir-functions.o ir-keymaps.o
-ir-core-objs	:= ir-keytable.o
+ir-core-objs	:= ir-keytable.o ir-sysfs.o
 
 obj-$(CONFIG_IR_CORE) += ir-core.o
 obj-$(CONFIG_VIDEO_IR) += ir-common.o
--- master.orig/linux/drivers/media/IR/ir-keytable.c
+++ master/linux/drivers/media/IR/ir-keytable.c
@@ -448,12 +448,21 @@ int ir_input_register(struct input_dev *
 	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) {
-		kfree(rc_tab->scan);
-		kfree(ir_dev);
-		input_set_drvdata(input_dev, NULL);
+		input_unregister_device(input_dev);
+		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);
@@ -473,6 +482,8 @@ void ir_input_unregister(struct input_de
 	kfree(rc_tab->scan);
 	rc_tab->scan = NULL;
 
+	ir_unregister_class(dev);
+
 	kfree(ir_dev);
 	input_unregister_device(dev);
 }
--- /dev/null
+++ master/linux/drivers/media/IR/ir-sysfs.c
@@ -0,0 +1,94 @@
+/* ir-register.c - handle IR scancode->keycode tables
+ *
+ * Copyright (C) 2009 by Mauro Carvalho Chehab <mchehab@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ */
+
+#include <linux/input.h>
+#include <linux/device.h>
+#include <media/ir-core.h>
+
+#define IRRCV_NUM_DEVICES	256
+
+unsigned long ir_core_dev_number;
+
+static struct class *ir_input_class;
+
+static DEVICE_ATTR(ir_protocol, S_IRUGO | S_IWUSR, NULL, NULL);
+
+static struct attribute *ir_dev_attrs[] = {
+	&dev_attr_ir_protocol.attr,
+};
+
+int ir_register_class(struct input_dev *input_dev)
+{
+	int rc;
+	struct kobject *kobj;
+
+	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
+	int devno = find_first_zero_bit(&ir_core_dev_number,
+				        IRRCV_NUM_DEVICES);
+
+	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->devno = devno;
+	set_bit(devno, &ir_core_dev_number);
+
+	return 0;
+};
+
+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);
+
+	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);
+}
+
+static int __init ir_core_init(void)
+{
+	ir_input_class = class_create(THIS_MODULE, "irrcv");
+	if (IS_ERR(ir_input_class)) {
+		printk(KERN_ERR "ir_core: unable to register irrcv class\n");
+		return PTR_ERR(ir_input_class);
+	}
+
+	return 0;
+}
+
+static void __exit ir_core_exit(void)
+{
+	class_destroy(ir_input_class);
+}
+
+module_init(ir_core_init);
+module_exit(ir_core_exit);
--- master.orig/linux/include/media/ir-core.h
+++ master/linux/include/media/ir-core.h
@@ -42,8 +42,11 @@ struct ir_scancode_table {
 };
 
 struct ir_input_dev {
-	struct input_dev		*dev;
-	struct ir_scancode_table	rc_tab;
+	struct input_dev		*dev;		/* Input 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 */
 };
 
 /* Routines from ir-keytable.c */
@@ -59,4 +62,9 @@ int ir_input_register(struct input_dev *
 		      struct ir_scancode_table *ir_codes);
 void ir_input_unregister(struct input_dev *input_dev);
 
+/* Routines from ir-sysfs.c */
+
+int ir_register_class(struct input_dev *input_dev);
+void ir_unregister_class(struct input_dev *input_dev);
+
 #endif
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help