From: Maxim L. <max...@gm...> - 2010-07-28 15:14:30
|
Hi, This is first version of ported ENE driver to ir-core In meanwhile I fixed few bugs, added few things. Comments are welcome! Best regards, Maxim Levitsky |
From: Maxim L. <max...@gm...> - 2010-07-30 02:17:30
|
Hi, This is third revision of my patchset. Notable changes: * Added whitespace fixes from Jarod Wilson * 4 new bugs fixed (patches 04-07). Now in-kernel decoding works perfectly with all protocols it supports. * lirc interface additions cleaned up. no more wrong support for timeout reports new ioctl for learning mode still need to add carrier detect, timeout reports, and rx filter * replaced int with bool in my driver, plus few cleanups. * added myself to maintainers of the ene driver * added another PNP ID to ene driver Best regards, Maxim Levitsky |
From: Maxim L. <max...@gm...> - 2010-07-30 02:17:34
|
Move IR drives below separate menu. This allows to disable them. Also correct a typo. Signed-off-by: Maxim Levitsky <max...@gm...> --- drivers/media/IR/Kconfig | 10 +++++++--- 1 files changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/media/IR/Kconfig b/drivers/media/IR/Kconfig index e557ae0..fc48a3f 100644 --- a/drivers/media/IR/Kconfig +++ b/drivers/media/IR/Kconfig @@ -1,8 +1,10 @@ -config IR_CORE - tristate +menuconfig IR_CORE + tristate "Infrared remote controller adapters" depends on INPUT default INPUT +if IR_CORE + config VIDEO_IR tristate depends on IR_CORE @@ -16,7 +18,7 @@ config LIRC Enable this option to build the Linux Infrared Remote Control (LIRC) core device interface driver. The LIRC interface passes raw IR to and from userspace, where the - LIRC daemon handles protocol decoding for IR reception ann + LIRC daemon handles protocol decoding for IR reception and encoding for IR transmitting (aka "blasting"). source "drivers/media/IR/keymaps/Kconfig" @@ -102,3 +104,5 @@ config IR_MCEUSB To compile this driver as a module, choose M here: the module will be called mceusb. + +endif #IR_CORE -- 1.7.0.4 |
From: Maxim L. <max...@gm...> - 2010-07-30 02:17:37
|
* lirc: Don't propagate reset event to userspace * lirc: Remove strange logic from lirc that would make first sample always be pulse * Make TO_US macro actualy print what it should. Signed-off-by: Maxim Levitsky <max...@gm...> --- drivers/media/IR/ir-core-priv.h | 4 +--- drivers/media/IR/ir-lirc-codec.c | 14 ++++++++------ drivers/media/IR/ir-raw-event.c | 3 +++ 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h index babd520..dc26e2b 100644 --- a/drivers/media/IR/ir-core-priv.h +++ b/drivers/media/IR/ir-core-priv.h @@ -76,7 +76,6 @@ struct ir_raw_event_ctrl { struct lirc_codec { struct ir_input_dev *ir_dev; struct lirc_driver *drv; - int lircdata; } lirc; }; @@ -104,10 +103,9 @@ static inline void decrease_duration(struct ir_raw_event *ev, unsigned duration) ev->duration -= duration; } -#define TO_US(duration) (((duration) + 500) / 1000) +#define TO_US(duration) DIV_ROUND_CLOSEST((duration), 1000) #define TO_STR(is_pulse) ((is_pulse) ? "pulse" : "space") #define IS_RESET(ev) (ev.duration == 0) - /* * Routines from ir-sysfs.c - Meant to be called only internally inside * ir-core diff --git a/drivers/media/IR/ir-lirc-codec.c b/drivers/media/IR/ir-lirc-codec.c index 3ba482d..8ca01fd 100644 --- a/drivers/media/IR/ir-lirc-codec.c +++ b/drivers/media/IR/ir-lirc-codec.c @@ -32,6 +32,7 @@ static int ir_lirc_decode(struct input_dev *input_dev, struct ir_raw_event ev) { struct ir_input_dev *ir_dev = input_get_drvdata(input_dev); + int sample; if (!(ir_dev->raw->enabled_protocols & IR_TYPE_LIRC)) return 0; @@ -39,18 +40,21 @@ static int ir_lirc_decode(struct input_dev *input_dev, struct ir_raw_event ev) if (!ir_dev->raw->lirc.drv || !ir_dev->raw->lirc.drv->rbuf) return -EINVAL; + if (IS_RESET(ev)) + return 0; + IR_dprintk(2, "LIRC data transfer started (%uus %s)\n", TO_US(ev.duration), TO_STR(ev.pulse)); - ir_dev->raw->lirc.lircdata += ev.duration / 1000; + + sample = ev.duration / 1000; if (ev.pulse) - ir_dev->raw->lirc.lircdata |= PULSE_BIT; + sample |= PULSE_BIT; lirc_buffer_write(ir_dev->raw->lirc.drv->rbuf, - (unsigned char *) &ir_dev->raw->lirc.lircdata); + (unsigned char *) &sample); wake_up(&ir_dev->raw->lirc.drv->rbuf->wait_poll); - ir_dev->raw->lirc.lircdata = 0; return 0; } @@ -224,8 +228,6 @@ static int ir_lirc_register(struct input_dev *input_dev) ir_dev->raw->lirc.drv = drv; ir_dev->raw->lirc.ir_dev = ir_dev; - ir_dev->raw->lirc.lircdata = PULSE_MASK; - return 0; lirc_register_failed: diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c index 6f192ef..51f65da 100644 --- a/drivers/media/IR/ir-raw-event.c +++ b/drivers/media/IR/ir-raw-event.c @@ -66,6 +66,9 @@ int ir_raw_event_store(struct input_dev *input_dev, struct ir_raw_event *ev) if (!ir->raw) return -EINVAL; + IR_dprintk(2, "sample: (05%dus %s)\n", + TO_US(ev->duration), TO_STR(ev->pulse)); + if (kfifo_in(&ir->raw->kfifo, ev, sizeof(*ev)) != sizeof(*ev)) return -ENOMEM; -- 1.7.0.4 |
From: Maxim L. <max...@gm...> - 2010-07-30 02:17:40
|
Some handlers (lirc for example) allocates memory on initialization, doing so in atomic context is cumbersome. Fixes warning about sleeping function in atomic context. Signed-off-by: Maxim Levitsky <max...@gm...> --- drivers/media/IR/ir-raw-event.c | 28 ++++++++++++++-------------- 1 files changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c index 51f65da..9d5c029 100644 --- a/drivers/media/IR/ir-raw-event.c +++ b/drivers/media/IR/ir-raw-event.c @@ -13,7 +13,7 @@ */ #include <linux/workqueue.h> -#include <linux/spinlock.h> +#include <linux/mutex.h> #include <linux/sched.h> #include "ir-core-priv.h" @@ -24,7 +24,7 @@ static LIST_HEAD(ir_raw_client_list); /* Used to handle IR raw handler extensions */ -static DEFINE_SPINLOCK(ir_raw_handler_lock); +static DEFINE_MUTEX(ir_raw_handler_lock); static LIST_HEAD(ir_raw_handler_list); static u64 available_protocols; @@ -41,10 +41,10 @@ static void ir_raw_event_work(struct work_struct *work) container_of(work, struct ir_raw_event_ctrl, rx_work); while (kfifo_out(&raw->kfifo, &ev, sizeof(ev)) == sizeof(ev)) { - spin_lock(&ir_raw_handler_lock); + mutex_lock(&ir_raw_handler_lock); list_for_each_entry(handler, &ir_raw_handler_list, list) handler->decode(raw->input_dev, ev); - spin_unlock(&ir_raw_handler_lock); + mutex_unlock(&ir_raw_handler_lock); raw->prev_ev = ev; } } @@ -150,9 +150,9 @@ u64 ir_raw_get_allowed_protocols() { u64 protocols; - spin_lock(&ir_raw_handler_lock); + mutex_lock(&ir_raw_handler_lock); protocols = available_protocols; - spin_unlock(&ir_raw_handler_lock); + mutex_unlock(&ir_raw_handler_lock); return protocols; } @@ -180,12 +180,12 @@ int ir_raw_event_register(struct input_dev *input_dev) return rc; } - spin_lock(&ir_raw_handler_lock); + mutex_lock(&ir_raw_handler_lock); list_add_tail(&ir->raw->list, &ir_raw_client_list); list_for_each_entry(handler, &ir_raw_handler_list, list) if (handler->raw_register) handler->raw_register(ir->raw->input_dev); - spin_unlock(&ir_raw_handler_lock); + mutex_unlock(&ir_raw_handler_lock); return 0; } @@ -200,12 +200,12 @@ void ir_raw_event_unregister(struct input_dev *input_dev) cancel_work_sync(&ir->raw->rx_work); - spin_lock(&ir_raw_handler_lock); + mutex_lock(&ir_raw_handler_lock); list_del(&ir->raw->list); list_for_each_entry(handler, &ir_raw_handler_list, list) if (handler->raw_unregister) handler->raw_unregister(ir->raw->input_dev); - spin_unlock(&ir_raw_handler_lock); + mutex_unlock(&ir_raw_handler_lock); kfifo_free(&ir->raw->kfifo); kfree(ir->raw); @@ -220,13 +220,13 @@ int ir_raw_handler_register(struct ir_raw_handler *ir_raw_handler) { struct ir_raw_event_ctrl *raw; - spin_lock(&ir_raw_handler_lock); + mutex_lock(&ir_raw_handler_lock); list_add_tail(&ir_raw_handler->list, &ir_raw_handler_list); if (ir_raw_handler->raw_register) list_for_each_entry(raw, &ir_raw_client_list, list) ir_raw_handler->raw_register(raw->input_dev); available_protocols |= ir_raw_handler->protocols; - spin_unlock(&ir_raw_handler_lock); + mutex_unlock(&ir_raw_handler_lock); return 0; } @@ -236,13 +236,13 @@ void ir_raw_handler_unregister(struct ir_raw_handler *ir_raw_handler) { struct ir_raw_event_ctrl *raw; - spin_lock(&ir_raw_handler_lock); + mutex_lock(&ir_raw_handler_lock); list_del(&ir_raw_handler->list); if (ir_raw_handler->raw_unregister) list_for_each_entry(raw, &ir_raw_client_list, list) ir_raw_handler->raw_unregister(raw->input_dev); available_protocols &= ~ir_raw_handler->protocols; - spin_unlock(&ir_raw_handler_lock); + mutex_unlock(&ir_raw_handler_lock); } EXPORT_SYMBOL(ir_raw_handler_unregister); -- 1.7.0.4 |
From: Maxim L. <max...@gm...> - 2010-07-30 02:17:44
|
It is prefectly possible to have ir_raw_event_work running concurently on two cpus, thus we must protect it from that situation. Maybe better solution is to ditch the workqueue at all and use good 'ol thread per receiver, and just wake it up... Signed-off-by: Maxim Levitsky <max...@gm...> --- drivers/media/IR/ir-raw-event.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c index 9d5c029..4098748 100644 --- a/drivers/media/IR/ir-raw-event.c +++ b/drivers/media/IR/ir-raw-event.c @@ -40,13 +40,16 @@ static void ir_raw_event_work(struct work_struct *work) struct ir_raw_event_ctrl *raw = container_of(work, struct ir_raw_event_ctrl, rx_work); + mutex_lock(&ir_raw_handler_lock); + while (kfifo_out(&raw->kfifo, &ev, sizeof(ev)) == sizeof(ev)) { - mutex_lock(&ir_raw_handler_lock); list_for_each_entry(handler, &ir_raw_handler_list, list) handler->decode(raw->input_dev, ev); - mutex_unlock(&ir_raw_handler_lock); raw->prev_ev = ev; } + + mutex_unlock(&ir_raw_handler_lock); + } /** -- 1.7.0.4 |
From: Andy W. <aw...@md...> - 2010-07-30 02:42:17
|
On Fri, 2010-07-30 at 05:17 +0300, Maxim Levitsky wrote: > It is prefectly possible to have ir_raw_event_work > running concurently on two cpus, thus we must protect > it from that situation. Yup, the work is marked as not pending (and hence reschedulable) just before the work handler is run. > Maybe better solution is to ditch the workqueue at all > and use good 'ol thread per receiver, and just wake it up... I suppose you could also use a single threaded workqueue instead of a mutex, and let a bit test provide exclusivity. With the mutex, when the second thread finally obtains the lock, there will likely not be anything for it to do. Regards, Andy > Signed-off-by: Maxim Levitsky <max...@gm...> > --- > drivers/media/IR/ir-raw-event.c | 7 +++++-- > 1 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c > index 9d5c029..4098748 100644 > --- a/drivers/media/IR/ir-raw-event.c > +++ b/drivers/media/IR/ir-raw-event.c > @@ -40,13 +40,16 @@ static void ir_raw_event_work(struct work_struct *work) > struct ir_raw_event_ctrl *raw = > container_of(work, struct ir_raw_event_ctrl, rx_work); > > + mutex_lock(&ir_raw_handler_lock); > + > while (kfifo_out(&raw->kfifo, &ev, sizeof(ev)) == sizeof(ev)) { > - mutex_lock(&ir_raw_handler_lock); > list_for_each_entry(handler, &ir_raw_handler_list, list) > handler->decode(raw->input_dev, ev); > - mutex_unlock(&ir_raw_handler_lock); > raw->prev_ev = ev; > } > + > + mutex_unlock(&ir_raw_handler_lock); > + > } > > /** |
From: Maxim L. <max...@gm...> - 2010-07-30 11:03:01
|
On Thu, 2010-07-29 at 22:42 -0400, Andy Walls wrote: > On Fri, 2010-07-30 at 05:17 +0300, Maxim Levitsky wrote: > > It is prefectly possible to have ir_raw_event_work > > running concurently on two cpus, thus we must protect > > it from that situation. > > Yup, the work is marked as not pending (and hence reschedulable) just > before the work handler is run. > > > > Maybe better solution is to ditch the workqueue at all > > and use good 'ol thread per receiver, and just wake it up... > > I suppose you could also use a single threaded workqueue instead of a > mutex, and let a bit test provide exclusivity. With the mutex, when the > second thread finally obtains the lock, there will likely not be > anything for it to do. Mutex there is for another reason, to protect against decoder insert/removal. However, I think its best just to use a bare kthread for the purpose of this. Best regards, Maxim Levitsky |
From: Maxim L. <max...@gm...> - 2010-07-30 02:17:46
|
Currently, jvc decoder will attempt misdetect next press as a repeat of last keypress, therefore second keypress isn't detected. Signed-off-by: Maxim Levitsky <max...@gm...> --- drivers/media/IR/ir-jvc-decoder.c | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-) diff --git a/drivers/media/IR/ir-jvc-decoder.c b/drivers/media/IR/ir-jvc-decoder.c index 8894d8b..77a89c4 100644 --- a/drivers/media/IR/ir-jvc-decoder.c +++ b/drivers/media/IR/ir-jvc-decoder.c @@ -32,6 +32,7 @@ enum jvc_state { STATE_BIT_SPACE, STATE_TRAILER_PULSE, STATE_TRAILER_SPACE, + STATE_CHECK_REPEAT, }; /** @@ -60,6 +61,7 @@ static int ir_jvc_decode(struct input_dev *input_dev, struct ir_raw_event ev) IR_dprintk(2, "JVC decode started at state %d (%uus %s)\n", data->state, TO_US(ev.duration), TO_STR(ev.pulse)); +again: switch (data->state) { case STATE_INACTIVE: @@ -149,8 +151,18 @@ static int ir_jvc_decode(struct input_dev *input_dev, struct ir_raw_event ev) } data->count = 0; - data->state = STATE_BIT_PULSE; + data->state = STATE_CHECK_REPEAT; return 0; + + case STATE_CHECK_REPEAT: + if (!ev.pulse) + break; + + if (eq_margin(ev.duration, JVC_HEADER_PULSE, JVC_UNIT / 2)) + data->state = STATE_INACTIVE; + else + data->state = STATE_BIT_PULSE; + goto again; } out: -- 1.7.0.4 |
From: Maxim L. <max...@gm...> - 2010-07-30 02:17:48
|
Repeat space is 4 units, not 8. Current code would never trigger a repeat. However that isn't true for NECX, so repeat there must be handled differently. Signed-off-by: Maxim Levitsky <max...@gm...> --- drivers/media/IR/ir-nec-decoder.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c index 52e0f37..1c0cf03 100644 --- a/drivers/media/IR/ir-nec-decoder.c +++ b/drivers/media/IR/ir-nec-decoder.c @@ -20,7 +20,7 @@ #define NEC_HEADER_PULSE (16 * NEC_UNIT) #define NECX_HEADER_PULSE (8 * NEC_UNIT) /* Less common NEC variant */ #define NEC_HEADER_SPACE (8 * NEC_UNIT) -#define NEC_REPEAT_SPACE (8 * NEC_UNIT) +#define NEC_REPEAT_SPACE (4 * NEC_UNIT) #define NEC_BIT_PULSE (1 * NEC_UNIT) #define NEC_BIT_0_SPACE (1 * NEC_UNIT) #define NEC_BIT_1_SPACE (3 * NEC_UNIT) -- 1.7.0.4 |
From: Andy W. <aw...@md...> - 2010-07-30 02:49:34
|
On Fri, 2010-07-30 at 05:17 +0300, Maxim Levitsky wrote: > Repeat space is 4 units, not 8. > Current code would never trigger a repeat. Yup. Page 11, line (4) http://www.datasheetcatalog.org/datasheet/nec/UPD6122G-002.pdf Reviewed-by: Andy Walls <aw...@md...> Regards, Andy > However that isn't true for NECX, so repeat there > must be handled differently. > > Signed-off-by: Maxim Levitsky <max...@gm...> > --- > drivers/media/IR/ir-nec-decoder.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c > index 52e0f37..1c0cf03 100644 > --- a/drivers/media/IR/ir-nec-decoder.c > +++ b/drivers/media/IR/ir-nec-decoder.c > @@ -20,7 +20,7 @@ > #define NEC_HEADER_PULSE (16 * NEC_UNIT) > #define NECX_HEADER_PULSE (8 * NEC_UNIT) /* Less common NEC variant */ > #define NEC_HEADER_SPACE (8 * NEC_UNIT) > -#define NEC_REPEAT_SPACE (8 * NEC_UNIT) > +#define NEC_REPEAT_SPACE (4 * NEC_UNIT) > #define NEC_BIT_PULSE (1 * NEC_UNIT) > #define NEC_BIT_0_SPACE (1 * NEC_UNIT) > #define NEC_BIT_1_SPACE (3 * NEC_UNIT) |
From: Maxim L. <max...@gm...> - 2010-07-30 02:17:51
|
This adds support for repeat detecting for NECX variant Tested with uneversal remote Signed-off-by: Maxim Levitsky <max...@gm...> --- drivers/media/IR/ir-core-priv.h | 1 + drivers/media/IR/ir-nec-decoder.c | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h index dc26e2b..494e1f8 100644 --- a/drivers/media/IR/ir-core-priv.h +++ b/drivers/media/IR/ir-core-priv.h @@ -45,6 +45,7 @@ struct ir_raw_event_ctrl { int state; unsigned count; u32 bits; + bool is_nec_x; } nec; struct rc5_dec { int state; diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c index 1c0cf03..59127b1 100644 --- a/drivers/media/IR/ir-nec-decoder.c +++ b/drivers/media/IR/ir-nec-decoder.c @@ -26,6 +26,7 @@ #define NEC_BIT_1_SPACE (3 * NEC_UNIT) #define NEC_TRAILER_PULSE (1 * NEC_UNIT) #define NEC_TRAILER_SPACE (10 * NEC_UNIT) /* even longer in reality */ +#define NECX_REPEAT_BITS 1 enum nec_state { STATE_INACTIVE, @@ -67,8 +68,11 @@ static int ir_nec_decode(struct input_dev *input_dev, struct ir_raw_event ev) if (!ev.pulse) break; - if (!eq_margin(ev.duration, NEC_HEADER_PULSE, NEC_UNIT / 2) && - !eq_margin(ev.duration, NECX_HEADER_PULSE, NEC_UNIT / 2)) + if (eq_margin(ev.duration, NEC_HEADER_PULSE, NEC_UNIT / 2)) + data->is_nec_x = false; + else if (eq_margin(ev.duration, NECX_HEADER_PULSE, NEC_UNIT / 2)) + data->is_nec_x = true; + else break; data->count = 0; @@ -105,6 +109,14 @@ static int ir_nec_decode(struct input_dev *input_dev, struct ir_raw_event ev) if (ev.pulse) break; + if (geq_margin(ev.duration, NEC_TRAILER_SPACE, NEC_UNIT / 2) && + data->is_nec_x && data->count == NECX_REPEAT_BITS) { + IR_dprintk(1, "Repeat last key\n"); + ir_repeat(input_dev); + data->state = STATE_INACTIVE; + return 0; + } + data->bits <<= 1; if (eq_margin(ev.duration, NEC_BIT_1_SPACE, NEC_UNIT / 2)) data->bits |= 1; -- 1.7.0.4 |
From: Maxim L. <max...@gm...> - 2010-07-30 02:17:53
|
Currently, ir device registration fails if keymap requested by driver is not found. Fix that by always compiling in the empty keymap, and using it as a failback. Signed-off-by: Maxim Levitsky <max...@gm...> --- drivers/media/IR/ir-core-priv.h | 3 +- drivers/media/IR/ir-sysfs.c | 2 + drivers/media/IR/keymaps/Makefile | 1 - drivers/media/IR/keymaps/rc-empty.c | 44 ----------------------------------- drivers/media/IR/rc-map.c | 23 ++++++++++++++++++ include/media/ir-core.h | 8 ++++- 6 files changed, 33 insertions(+), 48 deletions(-) delete mode 100644 drivers/media/IR/keymaps/rc-empty.c diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h index 494e1f8..fe84374 100644 --- a/drivers/media/IR/ir-core-priv.h +++ b/drivers/media/IR/ir-core-priv.h @@ -125,7 +125,8 @@ int ir_raw_handler_register(struct ir_raw_handler *ir_raw_handler); void ir_raw_handler_unregister(struct ir_raw_handler *ir_raw_handler); void ir_raw_init(void); - +int ir_rcmap_init(void); +void ir_rcmap_cleanup(void); /* * Decoder initialization code * diff --git a/drivers/media/IR/ir-sysfs.c b/drivers/media/IR/ir-sysfs.c index a841e51..936dff8 100644 --- a/drivers/media/IR/ir-sysfs.c +++ b/drivers/media/IR/ir-sysfs.c @@ -341,6 +341,7 @@ static int __init ir_core_init(void) /* Initialize/load the decoders/keymap code that will be used */ ir_raw_init(); + ir_rcmap_init(); return 0; } @@ -348,6 +349,7 @@ static int __init ir_core_init(void) static void __exit ir_core_exit(void) { class_unregister(&ir_input_class); + ir_rcmap_cleanup(); } module_init(ir_core_init); diff --git a/drivers/media/IR/keymaps/Makefile b/drivers/media/IR/keymaps/Makefile index 86d3d1f..24992cd 100644 --- a/drivers/media/IR/keymaps/Makefile +++ b/drivers/media/IR/keymaps/Makefile @@ -17,7 +17,6 @@ obj-$(CONFIG_RC_MAP) += rc-adstech-dvb-t-pci.o \ rc-dm1105-nec.o \ rc-dntv-live-dvb-t.o \ rc-dntv-live-dvbt-pro.o \ - rc-empty.o \ rc-em-terratec.o \ rc-encore-enltv2.o \ rc-encore-enltv.o \ diff --git a/drivers/media/IR/keymaps/rc-empty.c b/drivers/media/IR/keymaps/rc-empty.c deleted file mode 100644 index 3b338d8..0000000 --- a/drivers/media/IR/keymaps/rc-empty.c +++ /dev/null @@ -1,44 +0,0 @@ -/* empty.h - Keytable for empty Remote Controller - * - * keymap imported from ir-keymaps.c - * - * Copyright (c) 2010 by Mauro Carvalho Chehab <mc...@re...> - * - * 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; either version 2 of the License, or - * (at your option) any later version. - */ - -#include <media/rc-map.h> - -/* empty keytable, can be used as placeholder for not-yet created keytables */ - -static struct ir_scancode empty[] = { - { 0x2a, KEY_COFFEE }, -}; - -static struct rc_keymap empty_map = { - .map = { - .scan = empty, - .size = ARRAY_SIZE(empty), - .ir_type = IR_TYPE_UNKNOWN, /* Legacy IR type */ - .name = RC_MAP_EMPTY, - } -}; - -static int __init init_rc_map_empty(void) -{ - return ir_register_map(&empty_map); -} - -static void __exit exit_rc_map_empty(void) -{ - ir_unregister_map(&empty_map); -} - -module_init(init_rc_map_empty) -module_exit(exit_rc_map_empty) - -MODULE_LICENSE("GPL"); -MODULE_AUTHOR("Mauro Carvalho Chehab <mc...@re...>"); diff --git a/drivers/media/IR/rc-map.c b/drivers/media/IR/rc-map.c index 46a8f15..689143f 100644 --- a/drivers/media/IR/rc-map.c +++ b/drivers/media/IR/rc-map.c @@ -82,3 +82,26 @@ void ir_unregister_map(struct rc_keymap *map) } EXPORT_SYMBOL_GPL(ir_unregister_map); + +static struct ir_scancode empty[] = { + { 0x2a, KEY_COFFEE }, +}; + +static struct rc_keymap empty_map = { + .map = { + .scan = empty, + .size = ARRAY_SIZE(empty), + .ir_type = IR_TYPE_UNKNOWN, /* Legacy IR type */ + .name = RC_MAP_EMPTY, + } +}; + +int ir_rcmap_init(void) +{ + return ir_register_map(&empty_map); +} + +void ir_rcmap_cleanup(void) +{ + ir_unregister_map(&empty_map); +} diff --git a/include/media/ir-core.h b/include/media/ir-core.h index 513e60d..197d05a 100644 --- a/include/media/ir-core.h +++ b/include/media/ir-core.h @@ -110,8 +110,12 @@ static inline int ir_input_register(struct input_dev *dev, return -EINVAL; ir_codes = get_rc_map(map_name); - if (!ir_codes) - return -EINVAL; + if (!ir_codes) { + ir_codes = get_rc_map(RC_MAP_EMPTY); + + if (!ir_codes) + return -EINVAL; + } rc = __ir_input_register(dev, ir_codes, props, driver_name); if (rc < 0) -- 1.7.0.4 |
From: Maxim L. <max...@gm...> - 2010-07-30 02:17:56
|
Some ir input devices have small buffer, and interrupt the host each time it is full (or half full) Add a helper that automaticly handles timeouts, and also automaticly merges samples of same time (space-space) Such samples might be placed by hardware because size of sample in the buffer is small (a byte for example). Also remove constness from ir_dev_props, because it now contains timeout settings that driver might want to change Signed-off-by: Maxim Levitsky <max...@gm...> --- drivers/media/IR/ir-core-priv.h | 1 + drivers/media/IR/ir-keytable.c | 2 +- drivers/media/IR/ir-raw-event.c | 84 +++++++++++++++++++++++++++++++++++++++ include/media/ir-core.h | 23 +++++++++- 4 files changed, 106 insertions(+), 4 deletions(-) diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h index fe84374..841b76c 100644 --- a/drivers/media/IR/ir-core-priv.h +++ b/drivers/media/IR/ir-core-priv.h @@ -41,6 +41,7 @@ struct ir_raw_event_ctrl { /* raw decoder state follows */ struct ir_raw_event prev_ev; + struct ir_raw_event this_ev; struct nec_dec { int state; unsigned count; diff --git a/drivers/media/IR/ir-keytable.c b/drivers/media/IR/ir-keytable.c index 94a8577..34b9c07 100644 --- a/drivers/media/IR/ir-keytable.c +++ b/drivers/media/IR/ir-keytable.c @@ -428,7 +428,7 @@ static void ir_close(struct input_dev *input_dev) */ int __ir_input_register(struct input_dev *input_dev, const struct ir_scancode_table *rc_tab, - const struct ir_dev_props *props, + struct ir_dev_props *props, const char *driver_name) { struct ir_input_dev *ir_dev; diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c index 4098748..5a6f8ce 100644 --- a/drivers/media/IR/ir-raw-event.c +++ b/drivers/media/IR/ir-raw-event.c @@ -132,6 +132,90 @@ int ir_raw_event_store_edge(struct input_dev *input_dev, enum raw_event_type typ EXPORT_SYMBOL_GPL(ir_raw_event_store_edge); /** + * ir_raw_event_store_with_filter() - pass next pulse/space to decoders with some processing + * @input_dev: the struct input_dev device descriptor + * @type: the type of the event that has occurred + * + * This routine (which may be called from an interrupt context) works + * in similiar manner to ir_raw_event_store_edge. + * This routine is intended for devices with limited internal buffer + * It automerges samples of same type, and handles timeouts + */ +int ir_raw_event_store_with_filter(struct input_dev *input_dev, + struct ir_raw_event *ev) +{ + struct ir_input_dev *ir = input_get_drvdata(input_dev); + struct ir_raw_event_ctrl *raw = ir->raw; + + if (!raw || !ir->props) + return -EINVAL; + + /* Ignore spaces in idle mode */ + if (ir->idle && !ev->pulse) + return 0; + else if (ir->idle) + ir_raw_event_set_idle(input_dev, 0); + + if (!raw->this_ev.duration) { + raw->this_ev = *ev; + } else if (ev->pulse == raw->this_ev.pulse) { + raw->this_ev.duration += ev->duration; + } else { + ir_raw_event_store(input_dev, &raw->this_ev); + raw->this_ev = *ev; + } + + /* Enter idle mode if nessesary */ + if (!ev->pulse && ir->props->timeout && + raw->this_ev.duration >= ir->props->timeout) + ir_raw_event_set_idle(input_dev, 1); + return 0; +} +EXPORT_SYMBOL_GPL(ir_raw_event_store_with_filter); + +void ir_raw_event_set_idle(struct input_dev *input_dev, int idle) +{ + struct ir_input_dev *ir = input_get_drvdata(input_dev); + struct ir_raw_event_ctrl *raw = ir->raw; + ktime_t now; + u64 delta; + + if (!ir->props) + return; + + if (!ir->raw) + goto out; + + if (idle) { + IR_dprintk(2, "enter idle mode\n"); + raw->last_event = ktime_get(); + } else { + IR_dprintk(2, "exit idle mode\n"); + + now = ktime_get(); + delta = ktime_to_ns(ktime_sub(now, ir->raw->last_event)); + + WARN_ON(raw->this_ev.pulse); + + raw->this_ev.duration = + min(raw->this_ev.duration + delta, + (u64)IR_MAX_DURATION); + + ir_raw_event_store(input_dev, &raw->this_ev); + + if (raw->this_ev.duration == IR_MAX_DURATION) + ir_raw_event_reset(input_dev); + + raw->this_ev.duration = 0; + } +out: + if (ir->props->s_idle) + ir->props->s_idle(ir->props->priv, idle); + ir->idle = idle; +} +EXPORT_SYMBOL_GPL(ir_raw_event_set_idle); + +/** * ir_raw_event_handle() - schedules the decoding of stored ir data * @input_dev: the struct input_dev device descriptor * diff --git a/include/media/ir-core.h b/include/media/ir-core.h index 197d05a..7ad39fe 100644 --- a/include/media/ir-core.h +++ b/include/media/ir-core.h @@ -41,6 +41,9 @@ enum rc_driver_type { * anything with it. Yet, as the same keycode table can be used with other * devices, a mask is provided to allow its usage. Drivers should generally * leave this field in blank + * @timeout: optional time after which device stops sending data + * @min_timeout: minimum timeout supported by device + * @max_timeout: maximum timeout supported by device * @priv: driver-specific data, to be used on the callbacks * @change_protocol: allow changing the protocol used on hardware decoders * @open: callback to allow drivers to enable polling/irq when IR input device @@ -50,11 +53,19 @@ enum rc_driver_type { * @s_tx_mask: set transmitter mask (for devices with multiple tx outputs) * @s_tx_carrier: set transmit carrier frequency * @tx_ir: transmit IR + * @s_idle: optional: enable/disable hardware idle mode, upon which, + * device doesn't interrupt host untill it sees IR data */ struct ir_dev_props { enum rc_driver_type driver_type; unsigned long allowed_protos; u32 scanmask; + + u64 timeout; + u64 min_timeout; + u64 max_timeout; + + void *priv; int (*change_protocol)(void *priv, u64 ir_type); int (*open)(void *priv); @@ -62,6 +73,7 @@ struct ir_dev_props { int (*s_tx_mask)(void *priv, u32 mask); int (*s_tx_carrier)(void *priv, u32 carrier); int (*tx_ir)(void *priv, int *txbuf, u32 n); + void (*s_idle)(void *priv, int enable); }; struct ir_input_dev { @@ -69,9 +81,10 @@ struct ir_input_dev { char *driver_name; /* Name of the driver module */ struct ir_scancode_table rc_tab; /* scan/key table */ unsigned long devno; /* device number */ - const struct ir_dev_props *props; /* Device properties */ + struct ir_dev_props *props; /* Device properties */ struct ir_raw_event_ctrl *raw; /* for raw pulse/space events */ struct input_dev *input_dev; /* the input device associated with this device */ + bool idle; /* key info - needed by IR keycode handlers */ spinlock_t keylock; /* protects the below members */ @@ -95,12 +108,12 @@ enum raw_event_type { /* From ir-keytable.c */ int __ir_input_register(struct input_dev *dev, const struct ir_scancode_table *ir_codes, - const struct ir_dev_props *props, + struct ir_dev_props *props, const char *driver_name); static inline int ir_input_register(struct input_dev *dev, const char *map_name, - const struct ir_dev_props *props, + struct ir_dev_props *props, const char *driver_name) { struct ir_scancode_table *ir_codes; struct ir_input_dev *ir_dev; @@ -148,6 +161,10 @@ struct ir_raw_event { void ir_raw_event_handle(struct input_dev *input_dev); int ir_raw_event_store(struct input_dev *input_dev, struct ir_raw_event *ev); int ir_raw_event_store_edge(struct input_dev *input_dev, enum raw_event_type type); +int ir_raw_event_store_with_filter(struct input_dev *input_dev, + struct ir_raw_event *ev); +void ir_raw_event_set_idle(struct input_dev *input_dev, int idle); + static inline void ir_raw_event_reset(struct input_dev *input_dev) { struct ir_raw_event ev = { .pulse = false, .duration = 0 }; -- 1.7.0.4 |
From: Maxim L. <max...@gm...> - 2010-07-30 02:17:57
|
Still missing features: carrier report & timeout reports. Will need to pack these into ir_raw_event Signed-off-by: Maxim Levitsky <max...@gm...> --- drivers/media/IR/ir-core-priv.h | 1 + drivers/media/IR/ir-lirc-codec.c | 112 +++++++++++++++++++++++++++++++------- include/media/ir-core.h | 14 +++++ include/media/lirc.h | 5 ++- 4 files changed, 112 insertions(+), 20 deletions(-) diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h index 841b76c..068807d 100644 --- a/drivers/media/IR/ir-core-priv.h +++ b/drivers/media/IR/ir-core-priv.h @@ -78,6 +78,7 @@ struct ir_raw_event_ctrl { struct lirc_codec { struct ir_input_dev *ir_dev; struct lirc_driver *drv; + int carrier_low; } lirc; }; diff --git a/drivers/media/IR/ir-lirc-codec.c b/drivers/media/IR/ir-lirc-codec.c index 8ca01fd..5d5150f 100644 --- a/drivers/media/IR/ir-lirc-codec.c +++ b/drivers/media/IR/ir-lirc-codec.c @@ -46,7 +46,6 @@ static int ir_lirc_decode(struct input_dev *input_dev, struct ir_raw_event ev) IR_dprintk(2, "LIRC data transfer started (%uus %s)\n", TO_US(ev.duration), TO_STR(ev.pulse)); - sample = ev.duration / 1000; if (ev.pulse) sample |= PULSE_BIT; @@ -96,13 +95,14 @@ out: return ret; } -static long ir_lirc_ioctl(struct file *filep, unsigned int cmd, unsigned long arg) +static long ir_lirc_ioctl(struct file *filep, unsigned int cmd, + unsigned long __user arg) { struct lirc_codec *lirc; struct ir_input_dev *ir_dev; int ret = 0; void *drv_data; - unsigned long val; + unsigned long val = 0; lirc = lirc_get_pdata(filep); if (!lirc) @@ -114,47 +114,106 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int cmd, unsigned long ar drv_data = ir_dev->props->priv; - switch (cmd) { - case LIRC_SET_TRANSMITTER_MASK: + if (_IOC_DIR(cmd) & _IOC_WRITE) { ret = get_user(val, (unsigned long *)arg); if (ret) return ret; + } + + switch (cmd) { + + /* legacy support */ + case LIRC_GET_SEND_MODE: + val = LIRC_CAN_SEND_PULSE & LIRC_CAN_SEND_MASK; + break; + + case LIRC_SET_SEND_MODE: + if (val != (LIRC_MODE_PULSE & LIRC_CAN_SEND_MASK)) + return -EINVAL; + break; - if (ir_dev->props && ir_dev->props->s_tx_mask) + /* TX settings */ + case LIRC_SET_TRANSMITTER_MASK: + if (ir_dev->props->s_tx_mask) ret = ir_dev->props->s_tx_mask(drv_data, (u32)val); else return -EINVAL; break; case LIRC_SET_SEND_CARRIER: - ret = get_user(val, (unsigned long *)arg); - if (ret) - return ret; - - if (ir_dev->props && ir_dev->props->s_tx_carrier) + if (ir_dev->props->s_tx_carrier) ir_dev->props->s_tx_carrier(drv_data, (u32)val); else return -EINVAL; break; - case LIRC_GET_SEND_MODE: - val = LIRC_CAN_SEND_PULSE & LIRC_CAN_SEND_MASK; - ret = put_user(val, (unsigned long *)arg); + case LIRC_SET_SEND_DUTY_CYCLE: + if (!ir_dev->props->s_tx_duty_cycle) + return -ENOSYS; + + if (val <= 0 || val >= 100) + return -EINVAL; + + ir_dev->props->s_tx_duty_cycle(ir_dev->props->priv, val); break; - case LIRC_SET_SEND_MODE: - ret = get_user(val, (unsigned long *)arg); - if (ret) - return ret; + /* RX settings */ + case LIRC_SET_REC_CARRIER: + if (ir_dev->props->s_rx_carrier_range) + ret = ir_dev->props->s_rx_carrier_range( + ir_dev->props->priv, + ir_dev->raw->lirc.carrier_low, val); + else + return -ENOSYS; - if (val != (LIRC_MODE_PULSE & LIRC_CAN_SEND_MASK)) + if (!ret) + ir_dev->raw->lirc.carrier_low = 0; + break; + + case LIRC_SET_REC_CARRIER_RANGE: + if (val >= 0) + ir_dev->raw->lirc.carrier_low = val; + break; + + + case LIRC_GET_REC_RESOLUTION: + val = ir_dev->props->rx_resolution; + break; + + case LIRC_SET_WIDEBAND_RECEIVER: + if (ir_dev->props->s_learning_mode) + return ir_dev->props->s_learning_mode( + ir_dev->props->priv, !!val); + else + return -ENOSYS; + + /* Generic timeout support */ + case LIRC_GET_MIN_TIMEOUT: + if (!ir_dev->props->max_timeout) + return -ENOSYS; + val = ir_dev->props->min_timeout / 1000; + break; + + case LIRC_GET_MAX_TIMEOUT: + if (!ir_dev->props->max_timeout) + return -ENOSYS; + val = ir_dev->props->max_timeout / 1000; + break; + + case LIRC_SET_REC_TIMEOUT: + if (val < ir_dev->props->min_timeout || + val > ir_dev->props->max_timeout) return -EINVAL; + ir_dev->props->timeout = val * 1000; break; default: return lirc_dev_fop_ioctl(filep, cmd, arg); } + if (_IOC_DIR(cmd) & _IOC_READ) + ret = put_user(val, (unsigned long *)arg); + return ret; } @@ -200,13 +259,28 @@ static int ir_lirc_register(struct input_dev *input_dev) features = LIRC_CAN_REC_MODE2; if (ir_dev->props->tx_ir) { + features |= LIRC_CAN_SEND_PULSE; if (ir_dev->props->s_tx_mask) features |= LIRC_CAN_SET_TRANSMITTER_MASK; if (ir_dev->props->s_tx_carrier) features |= LIRC_CAN_SET_SEND_CARRIER; + + if (ir_dev->props->s_tx_duty_cycle) + features |= LIRC_CAN_SET_REC_DUTY_CYCLE; } + if (ir_dev->props->s_rx_carrier_range) + features |= LIRC_CAN_SET_REC_CARRIER | + LIRC_CAN_SET_REC_CARRIER_RANGE; + + if (ir_dev->props->s_learning_mode) + features |= LIRC_CAN_HAVE_WIDEBAND_RECEIVER; + + if (ir_dev->props->max_timeout) + features |= LIRC_CAN_SET_REC_TIMEOUT; + + snprintf(drv->name, sizeof(drv->name), "ir-lirc-codec (%s)", ir_dev->driver_name); drv->minor = -1; diff --git a/include/media/ir-core.h b/include/media/ir-core.h index 7ad39fe..ed9c1cb 100644 --- a/include/media/ir-core.h +++ b/include/media/ir-core.h @@ -44,6 +44,8 @@ enum rc_driver_type { * @timeout: optional time after which device stops sending data * @min_timeout: minimum timeout supported by device * @max_timeout: maximum timeout supported by device + * @rx_resolution : resolution (in ns) of input sampler + * @tx_resolution: resolution (in ns) of output sampler * @priv: driver-specific data, to be used on the callbacks * @change_protocol: allow changing the protocol used on hardware decoders * @open: callback to allow drivers to enable polling/irq when IR input device @@ -52,9 +54,16 @@ enum rc_driver_type { * is opened. * @s_tx_mask: set transmitter mask (for devices with multiple tx outputs) * @s_tx_carrier: set transmit carrier frequency + * @s_tx_duty_cycle: set transmit duty cycle (0% - 100%) + * @s_rx_carrier: inform driver about carrier it is expected to handle * @tx_ir: transmit IR * @s_idle: optional: enable/disable hardware idle mode, upon which, +<<<<<<< current * device doesn't interrupt host untill it sees IR data +======= + device doesn't interrupt host untill it sees IR data + * @s_learning_mode: enable wide band receiver used for learning +>>>>>>> patched */ struct ir_dev_props { enum rc_driver_type driver_type; @@ -65,6 +74,8 @@ struct ir_dev_props { u64 min_timeout; u64 max_timeout; + u32 rx_resolution; + u32 tx_resolution; void *priv; int (*change_protocol)(void *priv, u64 ir_type); @@ -72,8 +83,11 @@ struct ir_dev_props { void (*close)(void *priv); int (*s_tx_mask)(void *priv, u32 mask); int (*s_tx_carrier)(void *priv, u32 carrier); + int (*s_tx_duty_cycle)(void *priv, u32 duty_cycle); + int (*s_rx_carrier_range)(void *priv, u32 min, u32 max); int (*tx_ir)(void *priv, int *txbuf, u32 n); void (*s_idle)(void *priv, int enable); + int (*s_learning_mode)(void *priv, int enable); }; struct ir_input_dev { diff --git a/include/media/lirc.h b/include/media/lirc.h index 42c467c..bda3d03 100644 --- a/include/media/lirc.h +++ b/include/media/lirc.h @@ -77,6 +77,7 @@ #define LIRC_CAN_SET_REC_FILTER 0x08000000 #define LIRC_CAN_MEASURE_CARRIER 0x02000000 +#define LIRC_CAN_HAVE_WIDEBAND_RECEIVER 0x04000000 #define LIRC_CAN_SEND(x) ((x)&LIRC_CAN_SEND_MASK) #define LIRC_CAN_REC(x) ((x)&LIRC_CAN_REC_MASK) @@ -145,7 +146,7 @@ * if enabled from the next key press on the driver will send * LIRC_MODE2_FREQUENCY packets */ -#define LIRC_SET_MEASURE_CARRIER_MODE _IOW('i', 0x0000001d, __u32) +#define LIRC_SET_MEASURE_CARRIER_MODE _IOW('i', 0x0000001d, __u32) /* * to set a range use @@ -162,4 +163,6 @@ #define LIRC_SETUP_START _IO('i', 0x00000021) #define LIRC_SETUP_END _IO('i', 0x00000022) +#define LIRC_SET_WIDEBAND_RECEIVER _IOW('i', 0x00000023, __u32) + #endif -- 1.7.0.4 |
From: Maxim L. <max...@gm...> - 2010-07-30 02:18:00
|
This way it is possible to use evtest to create keymap for unknown remote. Signed-off-by: Maxim Levitsky <max...@gm...> --- drivers/media/IR/ir-keytable.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/drivers/media/IR/ir-keytable.c b/drivers/media/IR/ir-keytable.c index 34b9c07..ba7678a 100644 --- a/drivers/media/IR/ir-keytable.c +++ b/drivers/media/IR/ir-keytable.c @@ -339,6 +339,8 @@ void ir_repeat(struct input_dev *dev) spin_lock_irqsave(&ir->keylock, flags); + input_event(dev, EV_MSC, MSC_SCAN, ir->last_scancode); + if (!ir->keypressed) goto out; @@ -370,6 +372,8 @@ void ir_keydown(struct input_dev *dev, int scancode, u8 toggle) spin_lock_irqsave(&ir->keylock, flags); + input_event(dev, EV_MSC, MSC_SCAN, scancode); + /* Repeat event? */ if (ir->keypressed && ir->last_scancode == scancode && @@ -383,9 +387,11 @@ void ir_keydown(struct input_dev *dev, int scancode, u8 toggle) ir->last_toggle = toggle; ir->last_keycode = keycode; + if (keycode == KEY_RESERVED) goto out; + /* Register a keypress */ ir->keypressed = true; IR_dprintk(1, "%s: key down event, key 0x%04x, scancode 0x%04x\n", @@ -480,6 +486,8 @@ int __ir_input_register(struct input_dev *input_dev, set_bit(EV_KEY, input_dev->evbit); set_bit(EV_REP, input_dev->evbit); + set_bit(EV_MSC, input_dev->evbit); + set_bit(MSC_SCAN, input_dev->mscbit); if (ir_setkeytable(input_dev, &ir_dev->rc_tab, rc_tab)) { rc = -ENOMEM; -- 1.7.0.4 |
From: Jon S. <jon...@gm...> - 2010-07-30 02:39:17
|
On Thu, Jul 29, 2010 at 10:17 PM, Maxim Levitsky <max...@gm...> wrote: > note that error_adjustment module option is added. > This allows to reduce input samples by a percent. > This makes input on my system more correct. > > Default is 4% as it works best here. > > Note that only normal input is adjusted. I don't know > what adjustments to apply to fan tachometer input. > Maybe it is accurate already. Do you have the manual for the ENE chip in English? or do you read Chinese? Maybe you can figure out why the readings are off by 4%. I suspect that someone has set a clock divider wrong when programming the chip. For example setting the divider for a 25Mhz clock when the clock is actually 26Mhz would cause the error you are seeing. Or they just made a mistake in computing the divisor. It is probably a bug in the BIOS of your laptop. If that's the case you could add a quirk in the system boot code to fix the register setting. -- Jon Smirl jon...@gm... |
From: Andy W. <aw...@md...> - 2010-07-30 03:45:35
|
On Thu, 2010-07-29 at 22:39 -0400, Jon Smirl wrote: > On Thu, Jul 29, 2010 at 10:17 PM, Maxim Levitsky > <max...@gm...> wrote: > > note that error_adjustment module option is added. > > This allows to reduce input samples by a percent. > > This makes input on my system more correct. > > > > Default is 4% as it works best here. > > > > Note that only normal input is adjusted. I don't know > > what adjustments to apply to fan tachometer input. > > Maybe it is accurate already. > > Do you have the manual for the ENE chip in English? or do you read Chinese? The datasheet for a similar chip, the KB3700, is out there in English, but it doesn't have CIR. You might find these links mildly interesting: http://www.coreboot.org/Embedded_controller http://wiki.laptop.org/go/Embedded_controller http://lists.laptop.org/pipermail/openec/2008-July/000108.html Regards, Andy > Maybe you can figure out why the readings are off by 4%. I suspect > that someone has set a clock divider wrong when programming the chip. > For example setting the divider for a 25Mhz clock when the clock is > actually 26Mhz would cause the error you are seeing. Or they just made > a mistake in computing the divisor. It is probably a bug in the BIOS > of your laptop. If that's the case you could add a quirk in the > system boot code to fix the register setting. > |
From: Maxim L. <max...@gm...> - 2010-07-30 11:36:12
|
On Thu, 2010-07-29 at 23:46 -0400, Andy Walls wrote: > On Thu, 2010-07-29 at 22:39 -0400, Jon Smirl wrote: > > On Thu, Jul 29, 2010 at 10:17 PM, Maxim Levitsky > > <max...@gm...> wrote: > > > note that error_adjustment module option is added. > > > This allows to reduce input samples by a percent. > > > This makes input on my system more correct. > > > > > > Default is 4% as it works best here. > > > > > > Note that only normal input is adjusted. I don't know > > > what adjustments to apply to fan tachometer input. > > > Maybe it is accurate already. > > > > Do you have the manual for the ENE chip in English? or do you read Chinese? > > The datasheet for a similar chip, the KB3700, is out there in English, > but it doesn't have CIR. > > You might find these links mildly interesting: > > http://www.coreboot.org/Embedded_controller > http://wiki.laptop.org/go/Embedded_controller > http://lists.laptop.org/pipermail/openec/2008-July/000108.html Nope, I have read that. > > Regards, > Andy > > > Maybe you can figure out why the readings are off by 4%. I suspect > > that someone has set a clock divider wrong when programming the chip. > > For example setting the divider for a 25Mhz clock when the clock is > > actually 26Mhz would cause the error you are seeing. Or they just made > > a mistake in computing the divisor. It is probably a bug in the BIOS > > of your laptop. If that's the case you could add a quirk in the > > system boot code to fix the register setting. I figured out how windows driver compensates for the offset, and do the same in my driver. I think the problem is solved. Best regards, Maxim Levitsky |
From: Jon S. <jon...@gm...> - 2010-07-30 11:51:38
|
On Fri, Jul 30, 2010 at 7:36 AM, Maxim Levitsky <max...@gm...> wrote: > On Thu, 2010-07-29 at 23:46 -0400, Andy Walls wrote: >> On Thu, 2010-07-29 at 22:39 -0400, Jon Smirl wrote: >> > On Thu, Jul 29, 2010 at 10:17 PM, Maxim Levitsky >> > <max...@gm...> wrote: >> > > note that error_adjustment module option is added. >> > > This allows to reduce input samples by a percent. >> > > This makes input on my system more correct. >> > > >> > > Default is 4% as it works best here. >> > > >> > > Note that only normal input is adjusted. I don't know >> > > what adjustments to apply to fan tachometer input. >> > > Maybe it is accurate already. >> > >> > Do you have the manual for the ENE chip in English? or do you read Chinese? >> >> The datasheet for a similar chip, the KB3700, is out there in English, >> but it doesn't have CIR. >> >> You might find these links mildly interesting: >> >> http://www.coreboot.org/Embedded_controller >> http://wiki.laptop.org/go/Embedded_controller >> http://lists.laptop.org/pipermail/openec/2008-July/000108.html > > Nope, I have read that. >> >> Regards, >> Andy >> >> > Maybe you can figure out why the readings are off by 4%. I suspect >> > that someone has set a clock divider wrong when programming the chip. >> > For example setting the divider for a 25Mhz clock when the clock is >> > actually 26Mhz would cause the error you are seeing. Or they just made >> > a mistake in computing the divisor. It is probably a bug in the BIOS >> > of your laptop. If that's the case you could add a quirk in the >> > system boot code to fix the register setting. > > I figured out how windows driver compensates for the offset, and do the > same in my driver. I think the problem is solved. > Should that be a <= or >= instead of !=? + if (pll_freq != 1000) Programming the PLL wrong would cause the 4% error. hw_revision = ene_hw_read_reg(dev, ENE_HW_VERSION); old_ver = ene_hw_read_reg(dev, ENE_HW_VER_OLD); + pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) << 4) + + (ene_hw_read_reg(dev, ENE_PLLFRL) >> 2); + + if (pll_freq != 1000) + dev->rx_period_adjust = 4; + else + dev->rx_period_adjust = 2; + + + ene_printk(KERN_NOTICE, "PLL freq = %d\n", pll_freq); + if (hw_revision == 0xFF) { > > Best regards, > Maxim Levitsky > > -- Jon Smirl jon...@gm... |
From: Maxim L. <max...@gm...> - 2010-07-30 11:54:35
|
On Fri, 2010-07-30 at 07:51 -0400, Jon Smirl wrote: > On Fri, Jul 30, 2010 at 7:36 AM, Maxim Levitsky <max...@gm...> wrote: > > On Thu, 2010-07-29 at 23:46 -0400, Andy Walls wrote: > >> On Thu, 2010-07-29 at 22:39 -0400, Jon Smirl wrote: > >> > On Thu, Jul 29, 2010 at 10:17 PM, Maxim Levitsky > >> > <max...@gm...> wrote: > >> > > note that error_adjustment module option is added. > >> > > This allows to reduce input samples by a percent. > >> > > This makes input on my system more correct. > >> > > > >> > > Default is 4% as it works best here. > >> > > > >> > > Note that only normal input is adjusted. I don't know > >> > > what adjustments to apply to fan tachometer input. > >> > > Maybe it is accurate already. > >> > > >> > Do you have the manual for the ENE chip in English? or do you read Chinese? > >> > >> The datasheet for a similar chip, the KB3700, is out there in English, > >> but it doesn't have CIR. > >> > >> You might find these links mildly interesting: > >> > >> http://www.coreboot.org/Embedded_controller > >> http://wiki.laptop.org/go/Embedded_controller > >> http://lists.laptop.org/pipermail/openec/2008-July/000108.html > > > > Nope, I have read that. > >> > >> Regards, > >> Andy > >> > >> > Maybe you can figure out why the readings are off by 4%. I suspect > >> > that someone has set a clock divider wrong when programming the chip. > >> > For example setting the divider for a 25Mhz clock when the clock is > >> > actually 26Mhz would cause the error you are seeing. Or they just made > >> > a mistake in computing the divisor. It is probably a bug in the BIOS > >> > of your laptop. If that's the case you could add a quirk in the > >> > system boot code to fix the register setting. > > > > I figured out how windows driver compensates for the offset, and do the > > same in my driver. I think the problem is solved. > > > > Should that be a <= or >= instead of !=? > + if (pll_freq != 1000) This is how its done in windows driver. > > Programming the PLL wrong would cause the 4% error. > > hw_revision = ene_hw_read_reg(dev, ENE_HW_VERSION); > old_ver = ene_hw_read_reg(dev, ENE_HW_VER_OLD); > > + pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) << 4) + > + (ene_hw_read_reg(dev, ENE_PLLFRL) >> 2); > + > + if (pll_freq != 1000) > + dev->rx_period_adjust = 4; > + else > + dev->rx_period_adjust = 2; > + > + > + ene_printk(KERN_NOTICE, "PLL freq = %d\n", pll_freq); > + > if (hw_revision == 0xFF) { > > > > > > > Best regards, > > Maxim Levitsky > > > > > > > |
From: Jon S. <jon...@gm...> - 2010-07-30 12:02:25
|
On Fri, Jul 30, 2010 at 7:54 AM, Maxim Levitsky <max...@gm...> wrote: > On Fri, 2010-07-30 at 07:51 -0400, Jon Smirl wrote: >> On Fri, Jul 30, 2010 at 7:36 AM, Maxim Levitsky <max...@gm...> wrote: >> > On Thu, 2010-07-29 at 23:46 -0400, Andy Walls wrote: >> >> On Thu, 2010-07-29 at 22:39 -0400, Jon Smirl wrote: >> >> > On Thu, Jul 29, 2010 at 10:17 PM, Maxim Levitsky >> >> > <max...@gm...> wrote: >> >> > > note that error_adjustment module option is added. >> >> > > This allows to reduce input samples by a percent. >> >> > > This makes input on my system more correct. >> >> > > >> >> > > Default is 4% as it works best here. >> >> > > >> >> > > Note that only normal input is adjusted. I don't know >> >> > > what adjustments to apply to fan tachometer input. >> >> > > Maybe it is accurate already. >> >> > >> >> > Do you have the manual for the ENE chip in English? or do you read Chinese? >> >> >> >> The datasheet for a similar chip, the KB3700, is out there in English, >> >> but it doesn't have CIR. >> >> >> >> You might find these links mildly interesting: >> >> >> >> http://www.coreboot.org/Embedded_controller >> >> http://wiki.laptop.org/go/Embedded_controller >> >> http://lists.laptop.org/pipermail/openec/2008-July/000108.html >> > >> > Nope, I have read that. >> >> >> >> Regards, >> >> Andy >> >> >> >> > Maybe you can figure out why the readings are off by 4%. I suspect >> >> > that someone has set a clock divider wrong when programming the chip. >> >> > For example setting the divider for a 25Mhz clock when the clock is >> >> > actually 26Mhz would cause the error you are seeing. Or they just made >> >> > a mistake in computing the divisor. It is probably a bug in the BIOS >> >> > of your laptop. If that's the case you could add a quirk in the >> >> > system boot code to fix the register setting. >> > >> > I figured out how windows driver compensates for the offset, and do the >> > same in my driver. I think the problem is solved. >> > >> >> Should that be a <= or >= instead of !=? >> + if (pll_freq != 1000) > > This is how its done in windows driver. That doesn't mean it is bug free. Experimenting with changing the PLL frequency register may correct the error. Try taking 96% of pll_freq and write it back into these register. This would be easy to fix with a manual. The root problem is almost certainly a bug in the way the PLLs were programmed. I don't like putting in fudge factors like the 4% correction. What happens if a later version of the hardware has fixed firmware? I normal user is never going to figure out that they need to change the fudge factor. + pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) << 4) + + (ene_hw_read_reg(dev, ENE_PLLFRL) >> 2); + >> >> Programming the PLL wrong would cause the 4% error. >> >> hw_revision = ene_hw_read_reg(dev, ENE_HW_VERSION); >> old_ver = ene_hw_read_reg(dev, ENE_HW_VER_OLD); >> >> + pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) << 4) + >> + (ene_hw_read_reg(dev, ENE_PLLFRL) >> 2); >> + >> + if (pll_freq != 1000) >> + dev->rx_period_adjust = 4; >> + else >> + dev->rx_period_adjust = 2; >> + >> + >> + ene_printk(KERN_NOTICE, "PLL freq = %d\n", pll_freq); >> + >> if (hw_revision == 0xFF) { >> >> >> >> > >> > Best regards, >> > Maxim Levitsky >> > >> > >> >> >> > > > -- Jon Smirl jon...@gm... |
From: Jon S. <jon...@gm...> - 2010-07-30 12:07:41
|
On Fri, Jul 30, 2010 at 8:02 AM, Jon Smirl <jon...@gm...> wrote: > On Fri, Jul 30, 2010 at 7:54 AM, Maxim Levitsky <max...@gm...> wrote: >> On Fri, 2010-07-30 at 07:51 -0400, Jon Smirl wrote: >>> On Fri, Jul 30, 2010 at 7:36 AM, Maxim Levitsky <max...@gm...> wrote: >>> > On Thu, 2010-07-29 at 23:46 -0400, Andy Walls wrote: >>> >> On Thu, 2010-07-29 at 22:39 -0400, Jon Smirl wrote: >>> >> > On Thu, Jul 29, 2010 at 10:17 PM, Maxim Levitsky >>> >> > <max...@gm...> wrote: >>> >> > > note that error_adjustment module option is added. >>> >> > > This allows to reduce input samples by a percent. >>> >> > > This makes input on my system more correct. >>> >> > > >>> >> > > Default is 4% as it works best here. >>> >> > > >>> >> > > Note that only normal input is adjusted. I don't know >>> >> > > what adjustments to apply to fan tachometer input. >>> >> > > Maybe it is accurate already. >>> >> > >>> >> > Do you have the manual for the ENE chip in English? or do you read Chinese? >>> >> >>> >> The datasheet for a similar chip, the KB3700, is out there in English, >>> >> but it doesn't have CIR. >>> >> >>> >> You might find these links mildly interesting: >>> >> >>> >> http://www.coreboot.org/Embedded_controller >>> >> http://wiki.laptop.org/go/Embedded_controller >>> >> http://lists.laptop.org/pipermail/openec/2008-July/000108.html >>> > >>> > Nope, I have read that. >>> >> >>> >> Regards, >>> >> Andy >>> >> >>> >> > Maybe you can figure out why the readings are off by 4%. I suspect >>> >> > that someone has set a clock divider wrong when programming the chip. >>> >> > For example setting the divider for a 25Mhz clock when the clock is >>> >> > actually 26Mhz would cause the error you are seeing. Or they just made >>> >> > a mistake in computing the divisor. It is probably a bug in the BIOS >>> >> > of your laptop. If that's the case you could add a quirk in the >>> >> > system boot code to fix the register setting. >>> > >>> > I figured out how windows driver compensates for the offset, and do the >>> > same in my driver. I think the problem is solved. >>> > >>> >>> Should that be a <= or >= instead of !=? >>> + if (pll_freq != 1000) >> >> This is how its done in windows driver. > > That doesn't mean it is bug free. > > Experimenting with changing the PLL frequency register may correct the > error. Try taking 96% of pll_freq and write it back into these > register. This would be easy to fix with a manual. The root problem is > almost certainly a bug in the way the PLLs were programmed. > > I don't like putting in fudge factors like the 4% correction. What > happens if a later version of the hardware has fixed firmware? I > normal user is never going to figure out that they need to change the > fudge factor. > > + pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) << 4) + > + (ene_hw_read_reg(dev, ENE_PLLFRL) >> 2); I can understand the shift of the high bits, but that shift of the low bits is unlikely. A manual would tell us if it is right. > + > > >>> >>> Programming the PLL wrong would cause the 4% error. >>> >>> hw_revision = ene_hw_read_reg(dev, ENE_HW_VERSION); >>> old_ver = ene_hw_read_reg(dev, ENE_HW_VER_OLD); >>> >>> + pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) << 4) + >>> + (ene_hw_read_reg(dev, ENE_PLLFRL) >> 2); >>> + >>> + if (pll_freq != 1000) >>> + dev->rx_period_adjust = 4; >>> + else >>> + dev->rx_period_adjust = 2; >>> + >>> + >>> + ene_printk(KERN_NOTICE, "PLL freq = %d\n", pll_freq); >>> + >>> if (hw_revision == 0xFF) { >>> >>> >>> >>> > >>> > Best regards, >>> > Maxim Levitsky >>> > >>> > >>> >>> >>> >> >> >> > > > > -- > Jon Smirl > jon...@gm... > -- Jon Smirl jon...@gm... |
From: Maxim L. <max...@gm...> - 2010-07-30 12:45:23
|
On Fri, 2010-07-30 at 08:07 -0400, Jon Smirl wrote: > On Fri, Jul 30, 2010 at 8:02 AM, Jon Smirl <jon...@gm...> wrote: > > On Fri, Jul 30, 2010 at 7:54 AM, Maxim Levitsky <max...@gm...> wrote: > >> On Fri, 2010-07-30 at 07:51 -0400, Jon Smirl wrote: > >>> On Fri, Jul 30, 2010 at 7:36 AM, Maxim Levitsky <max...@gm...> wrote: > >>> > On Thu, 2010-07-29 at 23:46 -0400, Andy Walls wrote: > >>> >> On Thu, 2010-07-29 at 22:39 -0400, Jon Smirl wrote: > >>> >> > On Thu, Jul 29, 2010 at 10:17 PM, Maxim Levitsky > >>> >> > <max...@gm...> wrote: > >>> >> > > note that error_adjustment module option is added. > >>> >> > > This allows to reduce input samples by a percent. > >>> >> > > This makes input on my system more correct. > >>> >> > > > >>> >> > > Default is 4% as it works best here. > >>> >> > > > >>> >> > > Note that only normal input is adjusted. I don't know > >>> >> > > what adjustments to apply to fan tachometer input. > >>> >> > > Maybe it is accurate already. > >>> >> > > >>> >> > Do you have the manual for the ENE chip in English? or do you read Chinese? > >>> >> > >>> >> The datasheet for a similar chip, the KB3700, is out there in English, > >>> >> but it doesn't have CIR. > >>> >> > >>> >> You might find these links mildly interesting: > >>> >> > >>> >> http://www.coreboot.org/Embedded_controller > >>> >> http://wiki.laptop.org/go/Embedded_controller > >>> >> http://lists.laptop.org/pipermail/openec/2008-July/000108.html > >>> > > >>> > Nope, I have read that. > >>> >> > >>> >> Regards, > >>> >> Andy > >>> >> > >>> >> > Maybe you can figure out why the readings are off by 4%. I suspect > >>> >> > that someone has set a clock divider wrong when programming the chip. > >>> >> > For example setting the divider for a 25Mhz clock when the clock is > >>> >> > actually 26Mhz would cause the error you are seeing. Or they just made > >>> >> > a mistake in computing the divisor. It is probably a bug in the BIOS > >>> >> > of your laptop. If that's the case you could add a quirk in the > >>> >> > system boot code to fix the register setting. > >>> > > >>> > I figured out how windows driver compensates for the offset, and do the > >>> > same in my driver. I think the problem is solved. > >>> > > >>> > >>> Should that be a <= or >= instead of !=? > >>> + if (pll_freq != 1000) > >> > >> This is how its done in windows driver. > > > > That doesn't mean it is bug free. This PLL frequency is likely to be chip internal frequency. And windows driver doesn't touch it. Its embedded controller, so I don't want to touch things I am not sure about. > > > > Experimenting with changing the PLL frequency register may correct the > > error. Try taking 96% of pll_freq and write it back into these > > register. This would be easy to fix with a manual. The root problem is > > almost certainly a bug in the way the PLLs were programmed. > > > > I don't like putting in fudge factors like the 4% correction. What > > happens if a later version of the hardware has fixed firmware? I > > normal user is never going to figure out that they need to change the > > fudge factor. I don't think that is a hardware bug, rather a limitation. Lets leave it as is. I will soon publish the driver on launchpad or something like that and try to contact users I debugged that driver with, and then see what ranges PLL register takes. > > > > + pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) << 4) + > > + (ene_hw_read_reg(dev, ENE_PLLFRL) >> 2); > > I can understand the shift of the high bits, but that shift of the low > bits is unlikely. A manual would tell us if it is right. > This shift is correct (according to datasheet, which contains mostly useless info, but it does dociment this reg briefly.) Best regards, Maxim Levitsky |
From: Andy W. <aw...@md...> - 2010-07-31 13:55:01
|
On Fri, 2010-07-30 at 15:45 +0300, Maxim Levitsky wrote: > On Fri, 2010-07-30 at 08:07 -0400, Jon Smirl wrote: > > On Fri, Jul 30, 2010 at 8:02 AM, Jon Smirl <jon...@gm...> wrote: > > > On Fri, Jul 30, 2010 at 7:54 AM, Maxim Levitsky <max...@gm...> wrote: > > > > > > > + pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) << 4) + > > > + (ene_hw_read_reg(dev, ENE_PLLFRL) >> 2); > > > > > > I can understand the shift of the high bits, but that shift of the low > > bits is unlikely. A manual would tell us if it is right. > > > This shift is correct (according to datasheet, which contains mostly > useless info, but it does dociment this reg briefly.) The KB3700 series datasheet indicates that the value from ENE_PLLFRL should be shifted by >> 4 bits, not by >> 2. Of course, the KB3700 isn't the exact same chip. Regards, Andy |