Dmitry Torokhov wrote: > On Sun, Dec 06, 2009 at 09:34:26PM +0100, Krzysztof Halasa wrote: >> Jon Smirl <jonsmirl@xxxxxxxxx> writes: >> >>>> 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@xxxxxxxxxx> + * + * 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 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
