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)
- 2009-12-15 · Re: [RFC] What are the goals for the architecture of an in-kernel IR system? · Pavel Machek <hidden>
- 2009-12-13 · Re: [RFC] What are the goals for the architecture of an in-kernel IR system? · Pavel Machek <hidden>
- 2009-12-13 · Re: [RFC] What are the goals for the architecture of an in-kernel IR system? · <hidden>
- 2009-12-08 · Re: [RFC] What are the goals for the architecture of an in-kernel IR system? · Christoph Bartelmus <hidden>
- 2009-12-08 · Re: [RFC] What are the goals for the architecture of an in-kernel IR system? · Dmitry Torokhov <dmitry.torokhov@gmail.com>
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