All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] ir-core: raw decoder framework changes
@ 2010-06-13 20:29 ` David Härdeman
  0 siblings, 0 replies; 21+ messages in thread
From: David Härdeman @ 2010-06-13 20:29 UTC (permalink / raw)
  To: jarod; +Cc: jarod, linux-media, mchehab, linux-input

The following two patches implement the same ir raw decoder centralization
changes I've proposed before, but now with some changes (client register
and unregister callbacks have been fixed wrt. module load order and
kept around) for lirc "decoding"...

---

David Härdeman (2):
      ir-core: centralize sysfs raw decoder enabling/disabling
      ir-core: move decoding state to ir_raw_event_ctrl


 drivers/media/IR/ir-core-priv.h    |   41 ++++++
 drivers/media/IR/ir-jvc-decoder.c  |  152 +---------------------
 drivers/media/IR/ir-nec-decoder.c  |  151 +---------------------
 drivers/media/IR/ir-raw-event.c    |  166 +++++++++++++-----------
 drivers/media/IR/ir-rc5-decoder.c  |  165 ++----------------------
 drivers/media/IR/ir-rc6-decoder.c  |  154 +---------------------
 drivers/media/IR/ir-sony-decoder.c |  155 ++--------------------
 drivers/media/IR/ir-sysfs.c        |  252 +++++++++++++++++++++---------------
 8 files changed, 334 insertions(+), 902 deletions(-)

-- 
David Härdeman

^ permalink raw reply	[flat|nested] 21+ messages in thread
* [PATCH 0/2] ir-core: raw decoder framework changes
@ 2010-06-13 20:29 ` David Härdeman
  0 siblings, 0 replies; 21+ messages in thread
From: David Härdeman @ 2010-06-13 20:29 UTC (permalink / raw)
  To: jarod; +Cc: jarod, linux-media, mchehab, linux-input

The following two patches implement the same ir raw decoder centralization
changes I've proposed before, but now with some changes (client register
and unregister callbacks have been fixed wrt. module load order and
kept around) for lirc "decoding"...

---

David Härdeman (2):
      ir-core: centralize sysfs raw decoder enabling/disabling
      ir-core: move decoding state to ir_raw_event_ctrl


 drivers/media/IR/ir-core-priv.h    |   41 ++++++
 drivers/media/IR/ir-jvc-decoder.c  |  152 +---------------------
 drivers/media/IR/ir-nec-decoder.c  |  151 +---------------------
 drivers/media/IR/ir-raw-event.c    |  166 +++++++++++++-----------
 drivers/media/IR/ir-rc5-decoder.c  |  165 ++----------------------
 drivers/media/IR/ir-rc6-decoder.c  |  154 +---------------------
 drivers/media/IR/ir-sony-decoder.c |  155 ++--------------------
 drivers/media/IR/ir-sysfs.c        |  252 +++++++++++++++++++++---------------
 8 files changed, 334 insertions(+), 902 deletions(-)

-- 
David Härdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread
* [PATCH 1/2] ir-core: centralize sysfs raw decoder enabling/disabling
  2010-06-13 20:29 ` David Härdeman
  (?)
@ 2010-06-13 20:29 ` David Härdeman
  2010-06-16 20:05     ` Jarod Wilson
                     ` (2 more replies)
  -1 siblings, 3 replies; 21+ messages in thread
From: David Härdeman @ 2010-06-13 20:29 UTC (permalink / raw)
  To: jarod; +Cc: jarod, linux-media, mchehab, linux-input

With the current logic, each raw decoder needs to add a copy of the exact
same sysfs code. This is both unnecessary and also means that (re)loading
an IR driver after raw decoder modules have been loaded won't work as
expected.

This patch moves that logic into ir-raw-event and adds a single sysfs
file per device.

Reading that file returns something like:

	"rc5 [rc6] nec jvc [sony]"

(with enabled protocols in [] brackets)

Writing either "+protocol" or "-protocol" to that file will
enable or disable the according protocol decoder.

An additional benefit is that the disabling of a decoder will be
remembered across module removal/insertion so a previously
disabled decoder won't suddenly be activated again. The default
setting is to enable all decoders.

This is also necessary for the next patch which moves even more decoder
state into the central raw decoding structs.

Signed-off-by: David Härdeman <david@hardeman.nu>
---
 drivers/media/IR/ir-core-priv.h    |    3 
 drivers/media/IR/ir-jvc-decoder.c  |   64 ---------
 drivers/media/IR/ir-nec-decoder.c  |   64 ---------
 drivers/media/IR/ir-raw-event.c    |  112 +++++++++-------
 drivers/media/IR/ir-rc5-decoder.c  |   64 ---------
 drivers/media/IR/ir-rc6-decoder.c  |   64 ---------
 drivers/media/IR/ir-sony-decoder.c |   64 ---------
 drivers/media/IR/ir-sysfs.c        |  252 +++++++++++++++++++++---------------
 8 files changed, 231 insertions(+), 456 deletions(-)

diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h
index b79446f..3072e55 100644
--- a/drivers/media/IR/ir-core-priv.h
+++ b/drivers/media/IR/ir-core-priv.h
@@ -21,6 +21,7 @@
 struct ir_raw_handler {
 	struct list_head list;
 
+	u64 protocols; /* which are handled by this handler */
 	int (*decode)(struct input_dev *input_dev, struct ir_raw_event event);
 	int (*raw_register)(struct input_dev *input_dev);
 	int (*raw_unregister)(struct input_dev *input_dev);
@@ -32,6 +33,7 @@ struct ir_raw_event_ctrl {
 	ktime_t				last_event;	/* when last event occurred */
 	enum raw_event_type		last_type;	/* last event type */
 	struct input_dev		*input_dev;	/* pointer to the parent input_dev */
+	u64				enabled_protocols; /* enabled raw protocol decoders */
 };
 
 /* macros for IR decoders */
@@ -73,6 +75,7 @@ void ir_unregister_class(struct input_dev *input_dev);
 /*
  * Routines from ir-raw-event.c to be used internally and by decoders
  */
+u64 ir_raw_get_allowed_protocols(void);
 int ir_raw_event_register(struct input_dev *input_dev);
 void ir_raw_event_unregister(struct input_dev *input_dev);
 int ir_raw_handler_register(struct ir_raw_handler *ir_raw_handler);
diff --git a/drivers/media/IR/ir-jvc-decoder.c b/drivers/media/IR/ir-jvc-decoder.c
index 0b80494..1055de4 100644
--- a/drivers/media/IR/ir-jvc-decoder.c
+++ b/drivers/media/IR/ir-jvc-decoder.c
@@ -41,7 +41,6 @@ enum jvc_state {
 struct decoder_data {
 	struct list_head	list;
 	struct ir_input_dev	*ir_dev;
-	int			enabled:1;
 
 	/* State machine control */
 	enum jvc_state		state;
@@ -72,53 +71,6 @@ static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
 	return data;
 }
 
-static ssize_t store_enabled(struct device *d,
-			     struct device_attribute *mattr,
-			     const char *buf,
-			     size_t len)
-{
-	unsigned long value;
-	struct ir_input_dev *ir_dev = dev_get_drvdata(d);
-	struct decoder_data *data = get_decoder_data(ir_dev);
-
-	if (!data)
-		return -EINVAL;
-
-	if (strict_strtoul(buf, 10, &value) || value > 1)
-		return -EINVAL;
-
-	data->enabled = value;
-
-	return len;
-}
-
-static ssize_t show_enabled(struct device *d,
-			     struct device_attribute *mattr, char *buf)
-{
-	struct ir_input_dev *ir_dev = dev_get_drvdata(d);
-	struct decoder_data *data = get_decoder_data(ir_dev);
-
-	if (!data)
-		return -EINVAL;
-
-	if (data->enabled)
-		return sprintf(buf, "1\n");
-	else
-	return sprintf(buf, "0\n");
-}
-
-static DEVICE_ATTR(enabled, S_IRUGO | S_IWUSR, show_enabled, store_enabled);
-
-static struct attribute *decoder_attributes[] = {
-	&dev_attr_enabled.attr,
-	NULL
-};
-
-static struct attribute_group decoder_attribute_group = {
-	.name	= "jvc_decoder",
-	.attrs	= decoder_attributes,
-};
-
 /**
  * ir_jvc_decode() - Decode one JVC pulse or space
  * @input_dev:	the struct input_dev descriptor of the device
@@ -135,7 +87,7 @@ static int ir_jvc_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 	if (!data)
 		return -EINVAL;
 
-	if (!data->enabled)
+	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_JVC))
 		return 0;
 
 	if (IS_RESET(ev)) {
@@ -253,21 +205,12 @@ static int ir_jvc_register(struct input_dev *input_dev)
 {
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
 	struct decoder_data *data;
-	int rc;
-
-	rc = sysfs_create_group(&ir_dev->dev.kobj, &decoder_attribute_group);
-	if (rc < 0)
-		return rc;
 
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (!data) {
-		sysfs_remove_group(&ir_dev->dev.kobj, &decoder_attribute_group);
+	if (!data)
 		return -ENOMEM;
-	}
 
 	data->ir_dev = ir_dev;
-	data->enabled = 1;
-
 	spin_lock(&decoder_lock);
 	list_add_tail(&data->list, &decoder_list);
 	spin_unlock(&decoder_lock);
@@ -284,8 +227,6 @@ static int ir_jvc_unregister(struct input_dev *input_dev)
 	if (!data)
 		return 0;
 
-	sysfs_remove_group(&ir_dev->dev.kobj, &decoder_attribute_group);
-
 	spin_lock(&decoder_lock);
 	list_del(&data->list);
 	spin_unlock(&decoder_lock);
@@ -294,6 +235,7 @@ static int ir_jvc_unregister(struct input_dev *input_dev)
 }
 
 static struct ir_raw_handler jvc_handler = {
+	.protocols	= IR_TYPE_JVC,
 	.decode		= ir_jvc_decode,
 	.raw_register	= ir_jvc_register,
 	.raw_unregister	= ir_jvc_unregister,
diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c
index ba79233..2cc2b92 100644
--- a/drivers/media/IR/ir-nec-decoder.c
+++ b/drivers/media/IR/ir-nec-decoder.c
@@ -43,7 +43,6 @@ enum nec_state {
 struct decoder_data {
 	struct list_head	list;
 	struct ir_input_dev	*ir_dev;
-	int			enabled:1;
 
 	/* State machine control */
 	enum nec_state		state;
@@ -71,53 +70,6 @@ static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
 	return data;
 }
 
-static ssize_t store_enabled(struct device *d,
-			     struct device_attribute *mattr,
-			     const char *buf,
-			     size_t len)
-{
-	unsigned long value;
-	struct ir_input_dev *ir_dev = dev_get_drvdata(d);
-	struct decoder_data *data = get_decoder_data(ir_dev);
-
-	if (!data)
-		return -EINVAL;
-
-	if (strict_strtoul(buf, 10, &value) || value > 1)
-		return -EINVAL;
-
-	data->enabled = value;
-
-	return len;
-}
-
-static ssize_t show_enabled(struct device *d,
-			     struct device_attribute *mattr, char *buf)
-{
-	struct ir_input_dev *ir_dev = dev_get_drvdata(d);
-	struct decoder_data *data = get_decoder_data(ir_dev);
-
-	if (!data)
-		return -EINVAL;
-
-	if (data->enabled)
-		return sprintf(buf, "1\n");
-	else
-	return sprintf(buf, "0\n");
-}
-
-static DEVICE_ATTR(enabled, S_IRUGO | S_IWUSR, show_enabled, store_enabled);
-
-static struct attribute *decoder_attributes[] = {
-	&dev_attr_enabled.attr,
-	NULL
-};
-
-static struct attribute_group decoder_attribute_group = {
-	.name	= "nec_decoder",
-	.attrs	= decoder_attributes,
-};
-
 /**
  * ir_nec_decode() - Decode one NEC pulse or space
  * @input_dev:	the struct input_dev descriptor of the device
@@ -136,7 +88,7 @@ static int ir_nec_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 	if (!data)
 		return -EINVAL;
 
-	if (!data->enabled)
+	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_NEC))
 		return 0;
 
 	if (IS_RESET(ev)) {
@@ -260,21 +212,12 @@ static int ir_nec_register(struct input_dev *input_dev)
 {
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
 	struct decoder_data *data;
-	int rc;
-
-	rc = sysfs_create_group(&ir_dev->dev.kobj, &decoder_attribute_group);
-	if (rc < 0)
-		return rc;
 
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (!data) {
-		sysfs_remove_group(&ir_dev->dev.kobj, &decoder_attribute_group);
+	if (!data)
 		return -ENOMEM;
-	}
 
 	data->ir_dev = ir_dev;
-	data->enabled = 1;
-
 	spin_lock(&decoder_lock);
 	list_add_tail(&data->list, &decoder_list);
 	spin_unlock(&decoder_lock);
@@ -291,8 +234,6 @@ static int ir_nec_unregister(struct input_dev *input_dev)
 	if (!data)
 		return 0;
 
-	sysfs_remove_group(&ir_dev->dev.kobj, &decoder_attribute_group);
-
 	spin_lock(&decoder_lock);
 	list_del(&data->list);
 	spin_unlock(&decoder_lock);
@@ -301,6 +242,7 @@ static int ir_nec_unregister(struct input_dev *input_dev)
 }
 
 static struct ir_raw_handler nec_handler = {
+	.protocols	= IR_TYPE_NEC,
 	.decode		= ir_nec_decode,
 	.raw_register	= ir_nec_register,
 	.raw_unregister	= ir_nec_unregister,
diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c
index ea68a3f..eeca8a8 100644
--- a/drivers/media/IR/ir-raw-event.c
+++ b/drivers/media/IR/ir-raw-event.c
@@ -21,8 +21,9 @@
 #define MAX_IR_EVENT_SIZE      512
 
 /* Used to handle IR raw handler extensions */
-static LIST_HEAD(ir_raw_handler_list);
 static DEFINE_SPINLOCK(ir_raw_handler_lock);
+static LIST_HEAD(ir_raw_handler_list);
+static u64 available_protocols;
 
 /**
  * RUN_DECODER()	- runs an operation on all IR decoders
@@ -65,52 +66,6 @@ static void ir_raw_event_work(struct work_struct *work)
 		RUN_DECODER(decode, raw->input_dev, ev);
 }
 
-int ir_raw_event_register(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir = input_get_drvdata(input_dev);
-	int rc;
-
-	ir->raw = kzalloc(sizeof(*ir->raw), GFP_KERNEL);
-	if (!ir->raw)
-		return -ENOMEM;
-
-	ir->raw->input_dev = input_dev;
-	INIT_WORK(&ir->raw->rx_work, ir_raw_event_work);
-
-	rc = kfifo_alloc(&ir->raw->kfifo, sizeof(s64) * MAX_IR_EVENT_SIZE,
-			 GFP_KERNEL);
-	if (rc < 0) {
-		kfree(ir->raw);
-		ir->raw = NULL;
-		return rc;
-	}
-
-	rc = RUN_DECODER(raw_register, input_dev);
-	if (rc < 0) {
-		kfifo_free(&ir->raw->kfifo);
-		kfree(ir->raw);
-		ir->raw = NULL;
-		return rc;
-	}
-
-	return rc;
-}
-
-void ir_raw_event_unregister(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir = input_get_drvdata(input_dev);
-
-	if (!ir->raw)
-		return;
-
-	cancel_work_sync(&ir->raw->rx_work);
-	RUN_DECODER(raw_unregister, input_dev);
-
-	kfifo_free(&ir->raw->kfifo);
-	kfree(ir->raw);
-	ir->raw = NULL;
-}
-
 /**
  * ir_raw_event_store() - pass a pulse/space duration to the raw ir decoders
  * @input_dev:	the struct input_dev device descriptor
@@ -204,6 +159,66 @@ void ir_raw_event_handle(struct input_dev *input_dev)
 }
 EXPORT_SYMBOL_GPL(ir_raw_event_handle);
 
+/* used internally by the sysfs interface */
+u64
+ir_raw_get_allowed_protocols()
+{
+	u64 protocols;
+	spin_lock(&ir_raw_handler_lock);
+	protocols = available_protocols;
+	spin_unlock(&ir_raw_handler_lock);
+	return protocols;
+}
+
+/*
+ * Used to (un)register raw event clients
+ */
+int ir_raw_event_register(struct input_dev *input_dev)
+{
+	struct ir_input_dev *ir = input_get_drvdata(input_dev);
+	int rc;
+
+	ir->raw = kzalloc(sizeof(*ir->raw), GFP_KERNEL);
+	if (!ir->raw)
+		return -ENOMEM;
+
+	ir->raw->input_dev = input_dev;
+	INIT_WORK(&ir->raw->rx_work, ir_raw_event_work);
+	ir->raw->enabled_protocols = ~0;
+	rc = kfifo_alloc(&ir->raw->kfifo, sizeof(s64) * MAX_IR_EVENT_SIZE,
+			 GFP_KERNEL);
+	if (rc < 0) {
+		kfree(ir->raw);
+		ir->raw = NULL;
+		return rc;
+	}
+
+	rc = RUN_DECODER(raw_register, input_dev);
+	if (rc < 0) {
+		kfifo_free(&ir->raw->kfifo);
+		kfree(ir->raw);
+		ir->raw = NULL;
+		return rc;
+	}
+
+	return rc;
+}
+
+void ir_raw_event_unregister(struct input_dev *input_dev)
+{
+	struct ir_input_dev *ir = input_get_drvdata(input_dev);
+
+	if (!ir->raw)
+		return;
+
+	cancel_work_sync(&ir->raw->rx_work);
+	RUN_DECODER(raw_unregister, input_dev);
+
+	kfifo_free(&ir->raw->kfifo);
+	kfree(ir->raw);
+	ir->raw = NULL;
+}
+
 /*
  * Extension interface - used to register the IR decoders
  */
@@ -212,7 +227,9 @@ int ir_raw_handler_register(struct ir_raw_handler *ir_raw_handler)
 {
 	spin_lock(&ir_raw_handler_lock);
 	list_add_tail(&ir_raw_handler->list, &ir_raw_handler_list);
+	available_protocols |= ir_raw_handler->protocols;
 	spin_unlock(&ir_raw_handler_lock);
+
 	return 0;
 }
 EXPORT_SYMBOL(ir_raw_handler_register);
@@ -221,6 +238,7 @@ void ir_raw_handler_unregister(struct ir_raw_handler *ir_raw_handler)
 {
 	spin_lock(&ir_raw_handler_lock);
 	list_del(&ir_raw_handler->list);
+	available_protocols &= ~ir_raw_handler->protocols;
 	spin_unlock(&ir_raw_handler_lock);
 }
 EXPORT_SYMBOL(ir_raw_handler_unregister);
diff --git a/drivers/media/IR/ir-rc5-decoder.c b/drivers/media/IR/ir-rc5-decoder.c
index 23cdb1b..1be8981 100644
--- a/drivers/media/IR/ir-rc5-decoder.c
+++ b/drivers/media/IR/ir-rc5-decoder.c
@@ -45,7 +45,6 @@ enum rc5_state {
 struct decoder_data {
 	struct list_head	list;
 	struct ir_input_dev	*ir_dev;
-	int			enabled:1;
 
 	/* State machine control */
 	enum rc5_state		state;
@@ -76,53 +75,6 @@ static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
 	return data;
 }
 
-static ssize_t store_enabled(struct device *d,
-			     struct device_attribute *mattr,
-			     const char *buf,
-			     size_t len)
-{
-	unsigned long value;
-	struct ir_input_dev *ir_dev = dev_get_drvdata(d);
-	struct decoder_data *data = get_decoder_data(ir_dev);
-
-	if (!data)
-		return -EINVAL;
-
-	if (strict_strtoul(buf, 10, &value) || value > 1)
-		return -EINVAL;
-
-	data->enabled = value;
-
-	return len;
-}
-
-static ssize_t show_enabled(struct device *d,
-			     struct device_attribute *mattr, char *buf)
-{
-	struct ir_input_dev *ir_dev = dev_get_drvdata(d);
-	struct decoder_data *data = get_decoder_data(ir_dev);
-
-	if (!data)
-		return -EINVAL;
-
-	if (data->enabled)
-		return sprintf(buf, "1\n");
-	else
-	return sprintf(buf, "0\n");
-}
-
-static DEVICE_ATTR(enabled, S_IRUGO | S_IWUSR, show_enabled, store_enabled);
-
-static struct attribute *decoder_attributes[] = {
-	&dev_attr_enabled.attr,
-	NULL
-};
-
-static struct attribute_group decoder_attribute_group = {
-	.name	= "rc5_decoder",
-	.attrs	= decoder_attributes,
-};
-
 /**
  * ir_rc5_decode() - Decode one RC-5 pulse or space
  * @input_dev:	the struct input_dev descriptor of the device
@@ -141,7 +93,7 @@ static int ir_rc5_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 	if (!data)
 		return -EINVAL;
 
-	if (!data->enabled)
+	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_RC5))
 		return 0;
 
 	if (IS_RESET(ev)) {
@@ -256,21 +208,12 @@ static int ir_rc5_register(struct input_dev *input_dev)
 {
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
 	struct decoder_data *data;
-	int rc;
-
-	rc = sysfs_create_group(&ir_dev->dev.kobj, &decoder_attribute_group);
-	if (rc < 0)
-		return rc;
 
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (!data) {
-		sysfs_remove_group(&ir_dev->dev.kobj, &decoder_attribute_group);
+	if (!data)
 		return -ENOMEM;
-	}
 
 	data->ir_dev = ir_dev;
-	data->enabled = 1;
-
 	spin_lock(&decoder_lock);
 	list_add_tail(&data->list, &decoder_list);
 	spin_unlock(&decoder_lock);
@@ -287,8 +230,6 @@ static int ir_rc5_unregister(struct input_dev *input_dev)
 	if (!data)
 		return 0;
 
-	sysfs_remove_group(&ir_dev->dev.kobj, &decoder_attribute_group);
-
 	spin_lock(&decoder_lock);
 	list_del(&data->list);
 	spin_unlock(&decoder_lock);
@@ -297,6 +238,7 @@ static int ir_rc5_unregister(struct input_dev *input_dev)
 }
 
 static struct ir_raw_handler rc5_handler = {
+	.protocols	= IR_TYPE_RC5,
 	.decode		= ir_rc5_decode,
 	.raw_register	= ir_rc5_register,
 	.raw_unregister	= ir_rc5_unregister,
diff --git a/drivers/media/IR/ir-rc6-decoder.c b/drivers/media/IR/ir-rc6-decoder.c
index 2bf479f..5e940a8 100644
--- a/drivers/media/IR/ir-rc6-decoder.c
+++ b/drivers/media/IR/ir-rc6-decoder.c
@@ -61,7 +61,6 @@ enum rc6_state {
 struct decoder_data {
 	struct list_head	list;
 	struct ir_input_dev	*ir_dev;
-	int			enabled:1;
 
 	/* State machine control */
 	enum rc6_state		state;
@@ -93,53 +92,6 @@ static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
 	return data;
 }
 
-static ssize_t store_enabled(struct device *d,
-			     struct device_attribute *mattr,
-			     const char *buf,
-			     size_t len)
-{
-	unsigned long value;
-	struct ir_input_dev *ir_dev = dev_get_drvdata(d);
-	struct decoder_data *data = get_decoder_data(ir_dev);
-
-	if (!data)
-		return -EINVAL;
-
-	if (strict_strtoul(buf, 10, &value) || value > 1)
-		return -EINVAL;
-
-	data->enabled = value;
-
-	return len;
-}
-
-static ssize_t show_enabled(struct device *d,
-			     struct device_attribute *mattr, char *buf)
-{
-	struct ir_input_dev *ir_dev = dev_get_drvdata(d);
-	struct decoder_data *data = get_decoder_data(ir_dev);
-
-	if (!data)
-		return -EINVAL;
-
-	if (data->enabled)
-		return sprintf(buf, "1\n");
-	else
-	return sprintf(buf, "0\n");
-}
-
-static DEVICE_ATTR(enabled, S_IRUGO | S_IWUSR, show_enabled, store_enabled);
-
-static struct attribute *decoder_attributes[] = {
-	&dev_attr_enabled.attr,
-	NULL
-};
-
-static struct attribute_group decoder_attribute_group = {
-	.name	= "rc6_decoder",
-	.attrs	= decoder_attributes,
-};
-
 static enum rc6_mode rc6_mode(struct decoder_data *data) {
 	switch (data->header & RC6_MODE_MASK) {
 	case 0:
@@ -171,7 +123,7 @@ static int ir_rc6_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 	if (!data)
 		return -EINVAL;
 
-	if (!data->enabled)
+	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_RC6))
 		return 0;
 
 	if (IS_RESET(ev)) {
@@ -352,21 +304,12 @@ static int ir_rc6_register(struct input_dev *input_dev)
 {
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
 	struct decoder_data *data;
-	int rc;
-
-	rc = sysfs_create_group(&ir_dev->dev.kobj, &decoder_attribute_group);
-	if (rc < 0)
-		return rc;
 
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (!data) {
-		sysfs_remove_group(&ir_dev->dev.kobj, &decoder_attribute_group);
+	if (!data)
 		return -ENOMEM;
-	}
 
 	data->ir_dev = ir_dev;
-	data->enabled = 1;
-
 	spin_lock(&decoder_lock);
 	list_add_tail(&data->list, &decoder_list);
 	spin_unlock(&decoder_lock);
@@ -383,8 +326,6 @@ static int ir_rc6_unregister(struct input_dev *input_dev)
 	if (!data)
 		return 0;
 
-	sysfs_remove_group(&ir_dev->dev.kobj, &decoder_attribute_group);
-
 	spin_lock(&decoder_lock);
 	list_del(&data->list);
 	spin_unlock(&decoder_lock);
@@ -393,6 +334,7 @@ static int ir_rc6_unregister(struct input_dev *input_dev)
 }
 
 static struct ir_raw_handler rc6_handler = {
+	.protocols	= IR_TYPE_RC6,
 	.decode		= ir_rc6_decode,
 	.raw_register	= ir_rc6_register,
 	.raw_unregister	= ir_rc6_unregister,
diff --git a/drivers/media/IR/ir-sony-decoder.c b/drivers/media/IR/ir-sony-decoder.c
index 9f440c5..8afd16a 100644
--- a/drivers/media/IR/ir-sony-decoder.c
+++ b/drivers/media/IR/ir-sony-decoder.c
@@ -38,7 +38,6 @@ enum sony_state {
 struct decoder_data {
 	struct list_head	list;
 	struct ir_input_dev	*ir_dev;
-	int			enabled:1;
 
 	/* State machine control */
 	enum sony_state		state;
@@ -66,53 +65,6 @@ static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
 	return data;
 }
 
-static ssize_t store_enabled(struct device *d,
-			     struct device_attribute *mattr,
-			     const char *buf,
-			     size_t len)
-{
-	unsigned long value;
-	struct ir_input_dev *ir_dev = dev_get_drvdata(d);
-	struct decoder_data *data = get_decoder_data(ir_dev);
-
-	if (!data)
-		return -EINVAL;
-
-	if (strict_strtoul(buf, 10, &value) || value > 1)
-		return -EINVAL;
-
-	data->enabled = value;
-
-	return len;
-}
-
-static ssize_t show_enabled(struct device *d,
-			     struct device_attribute *mattr, char *buf)
-{
-	struct ir_input_dev *ir_dev = dev_get_drvdata(d);
-	struct decoder_data *data = get_decoder_data(ir_dev);
-
-	if (!data)
-		return -EINVAL;
-
-	if (data->enabled)
-		return sprintf(buf, "1\n");
-	else
-	return sprintf(buf, "0\n");
-}
-
-static DEVICE_ATTR(enabled, S_IRUGO | S_IWUSR, show_enabled, store_enabled);
-
-static struct attribute *decoder_attributes[] = {
-	&dev_attr_enabled.attr,
-	NULL
-};
-
-static struct attribute_group decoder_attribute_group = {
-	.name	= "sony_decoder",
-	.attrs	= decoder_attributes,
-};
-
 /**
  * ir_sony_decode() - Decode one Sony pulse or space
  * @input_dev:	the struct input_dev descriptor of the device
@@ -131,7 +83,7 @@ static int ir_sony_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 	if (!data)
 		return -EINVAL;
 
-	if (!data->enabled)
+	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_SONY))
 		return 0;
 
 	if (IS_RESET(ev)) {
@@ -245,21 +197,12 @@ static int ir_sony_register(struct input_dev *input_dev)
 {
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
 	struct decoder_data *data;
-	int rc;
-
-	rc = sysfs_create_group(&ir_dev->dev.kobj, &decoder_attribute_group);
-	if (rc < 0)
-		return rc;
 
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (!data) {
-		sysfs_remove_group(&ir_dev->dev.kobj, &decoder_attribute_group);
+	if (!data)
 		return -ENOMEM;
-	}
 
 	data->ir_dev = ir_dev;
-	data->enabled = 1;
-
 	spin_lock(&decoder_lock);
 	list_add_tail(&data->list, &decoder_list);
 	spin_unlock(&decoder_lock);
@@ -276,8 +219,6 @@ static int ir_sony_unregister(struct input_dev *input_dev)
 	if (!data)
 		return 0;
 
-	sysfs_remove_group(&ir_dev->dev.kobj, &decoder_attribute_group);
-
 	spin_lock(&decoder_lock);
 	list_del(&data->list);
 	spin_unlock(&decoder_lock);
@@ -286,6 +227,7 @@ static int ir_sony_unregister(struct input_dev *input_dev)
 }
 
 static struct ir_raw_handler sony_handler = {
+	.protocols	= IR_TYPE_SONY,
 	.decode		= ir_sony_decode,
 	.raw_register	= ir_sony_register,
 	.raw_unregister	= ir_sony_unregister,
diff --git a/drivers/media/IR/ir-sysfs.c b/drivers/media/IR/ir-sysfs.c
index daf33c1..7ae5662 100644
--- a/drivers/media/IR/ir-sysfs.c
+++ b/drivers/media/IR/ir-sysfs.c
@@ -33,122 +33,178 @@ static struct class ir_input_class = {
 };
 
 /**
- * show_protocol() - shows the current IR protocol
+ * show_protocols() - shows the current IR protocol(s)
  * @d:		the device descriptor
  * @mattr:	the device attribute struct (unused)
  * @buf:	a pointer to the output buffer
  *
- * This routine is a callback routine for input read the IR protocol type.
- * it is trigged by reading /sys/class/rc/rc?/current_protocol.
- * It returns the protocol name, as understood by the driver.
+ * This routine is a callback routine for input read the IR protocol type(s).
+ * it is trigged by reading /sys/class/rc/rc?/protocols.
+ * It returns the protocol names of supported protocols.
+ * Enabled protocols are printed in brackets.
  */
-static ssize_t show_protocol(struct device *d,
-			     struct device_attribute *mattr, char *buf)
+static ssize_t show_protocols(struct device *d,
+			      struct device_attribute *mattr, char *buf)
 {
-	char *s;
 	struct ir_input_dev *ir_dev = dev_get_drvdata(d);
-	u64 ir_type = ir_dev->rc_tab.ir_type;
-
-	IR_dprintk(1, "Current protocol is %lld\n", (long long)ir_type);
-
-	/* FIXME: doesn't support multiple protocols at the same time */
-	if (ir_type == IR_TYPE_UNKNOWN)
-		s = "Unknown";
-	else if (ir_type == IR_TYPE_RC5)
-		s = "rc-5";
-	else if (ir_type == IR_TYPE_NEC)
-		s = "nec";
-	else if (ir_type == IR_TYPE_RC6)
-		s = "rc6";
-	else if (ir_type == IR_TYPE_JVC)
-		s = "jvc";
-	else if (ir_type == IR_TYPE_SONY)
-		s = "sony";
-	else
-		s = "other";
+	u64 allowed, enabled;
+	char *tmp = buf;
+
+	if (ir_dev->props->driver_type == RC_DRIVER_SCANCODE) {
+		enabled = ir_dev->rc_tab.ir_type;
+		allowed = ir_dev->props->allowed_protos;
+	} else {
+		enabled = ir_dev->raw->enabled_protocols;
+		allowed = ir_raw_get_allowed_protocols();
+	}
 
-	return sprintf(buf, "%s\n", s);
+	IR_dprintk(1, "allowed - 0x%llx, enabled - 0x%llx\n",
+		   (long long)allowed,
+		   (long long)enabled);
+
+	if (allowed & enabled & IR_TYPE_UNKNOWN)
+		tmp += sprintf(tmp, "[unknown] ");
+	else if (allowed & IR_TYPE_UNKNOWN)
+		tmp += sprintf(tmp, "unknown ");
+
+	if (allowed & enabled & IR_TYPE_RC5)
+		tmp += sprintf(tmp, "[rc5] ");
+	else if (allowed & IR_TYPE_RC5)
+		tmp += sprintf(tmp, "rc5 ");
+
+	if (allowed & enabled & IR_TYPE_NEC)
+		tmp += sprintf(tmp, "[nec] ");
+	else if (allowed & IR_TYPE_NEC)
+		tmp += sprintf(tmp, "nec ");
+
+	if (allowed & enabled & IR_TYPE_RC6)
+		tmp += sprintf(tmp, "[rc6] ");
+	else if (allowed & IR_TYPE_RC6)
+		tmp += sprintf(tmp, "rc6 ");
+
+	if (allowed & enabled & IR_TYPE_JVC)
+		tmp += sprintf(tmp, "[jvc] ");
+	else if (allowed & IR_TYPE_JVC)
+		tmp += sprintf(tmp, "jvc ");
+
+	if (allowed & enabled & IR_TYPE_SONY)
+		tmp += sprintf(tmp, "[sony] ");
+	else if (allowed & IR_TYPE_SONY)
+		tmp += sprintf(tmp, "sony ");
+
+	if (tmp != buf)
+		tmp--;
+	*tmp = '\n';
+	return tmp + 1 - buf;
 }
 
 /**
- * store_protocol() - shows the current IR protocol
+ * store_protocols() - changes the current IR protocol(s)
  * @d:		the device descriptor
  * @mattr:	the device attribute struct (unused)
  * @buf:	a pointer to the input buffer
  * @len:	length of the input buffer
  *
  * This routine is a callback routine for changing the IR protocol type.
- * it is trigged by reading /sys/class/rc/rc?/current_protocol.
- * It changes the IR the protocol name, if the IR type is recognized
- * by the driver.
- * If an unknown protocol name is used, returns -EINVAL.
+ * It is trigged by writing to /sys/class/rc/rc?/protocols.
+ * Writing "+proto" will add a protocol to the list of enabled protocols.
+ * Writing "-proto" will remove a protocol from the list of enabled protocols.
+ * Writing "proto" will enable only "proto".
+ * Returns -EINVAL if an invalid protocol combination or unknown protocol name
+ * is used, otherwise @len.
  */
-static ssize_t store_protocol(struct device *d,
-			      struct device_attribute *mattr,
-			      const char *data,
-			      size_t len)
+static ssize_t store_protocols(struct device *d,
+			       struct device_attribute *mattr,
+			       const char *data,
+			       size_t len)
 {
 	struct ir_input_dev *ir_dev = dev_get_drvdata(d);
-	u64 ir_type = 0;
-	int rc = -EINVAL;
+	bool enable, disable;
+	const char *tmp;
+	u64 type;
+	u64 mask;
+	int rc;
 	unsigned long flags;
-	char *buf;
-
-	while ((buf = strsep((char **) &data, " \n")) != NULL) {
-		if (!strcasecmp(buf, "rc-5") || !strcasecmp(buf, "rc5"))
-			ir_type |= IR_TYPE_RC5;
-		if (!strcasecmp(buf, "nec"))
-			ir_type |= IR_TYPE_NEC;
-		if (!strcasecmp(buf, "jvc"))
-			ir_type |= IR_TYPE_JVC;
-		if (!strcasecmp(buf, "sony"))
-			ir_type |= IR_TYPE_SONY;
+
+	tmp = skip_spaces(data);
+
+	if (*tmp == '+') {
+		enable = true;
+		disable = false;
+		tmp++;
+	} else if (*tmp == '-') {
+		enable = false;
+		disable = true;
+		tmp++;
+	} else {
+		enable = false;
+		disable = false;
 	}
 
-	if (!ir_type) {
+	if (!strncasecmp(tmp, "unknown", 7)) {
+		tmp += 7;
+		mask = IR_TYPE_UNKNOWN;
+	} else if (!strncasecmp(tmp, "rc5", 3)) {
+		tmp += 3;
+		mask = IR_TYPE_RC5;
+	} else if (!strncasecmp(tmp, "nec", 3)) {
+		tmp += 3;
+		mask = IR_TYPE_NEC;
+	} else if (!strncasecmp(tmp, "rc6", 3)) {
+		tmp += 3;
+		mask = IR_TYPE_RC6;
+	} else if (!strncasecmp(tmp, "jvc", 3)) {
+		tmp += 3;
+		mask = IR_TYPE_JVC;
+	} else if (!strncasecmp(tmp, "sony", 4)) {
+		tmp += 4;
+		mask = IR_TYPE_SONY;
+	} else {
 		IR_dprintk(1, "Unknown protocol\n");
 		return -EINVAL;
 	}
 
-	if (ir_dev->props && ir_dev->props->change_protocol)
-		rc = ir_dev->props->change_protocol(ir_dev->props->priv,
-						    ir_type);
-
-	if (rc < 0) {
-		IR_dprintk(1, "Error setting protocol to %lld\n",
-			   (long long)ir_type);
+	tmp = skip_spaces(tmp);
+	if (*tmp != '\0') {
+		IR_dprintk(1, "Invalid trailing characters\n");
 		return -EINVAL;
 	}
 
-	spin_lock_irqsave(&ir_dev->rc_tab.lock, flags);
-	ir_dev->rc_tab.ir_type = ir_type;
-	spin_unlock_irqrestore(&ir_dev->rc_tab.lock, flags);
+	if (ir_dev->props->driver_type == RC_DRIVER_SCANCODE)
+		type = ir_dev->rc_tab.ir_type;
+	else
+		type = ir_dev->raw->enabled_protocols;
 
-	IR_dprintk(1, "Current protocol(s) is(are) %lld\n",
-		   (long long)ir_type);
+	if (enable)
+		type |= mask;
+	else if (disable)
+		type &= ~mask;
+	else
+		type = mask;
 
-	return len;
-}
+	if (ir_dev->props && ir_dev->props->change_protocol) {
+		rc = ir_dev->props->change_protocol(ir_dev->props->priv,
+						    type);
+		if (rc < 0) {
+			IR_dprintk(1, "Error setting protocols to 0x%llx\n",
+				   (long long)type);
+			return -EINVAL;
+		}
+	}
 
-static ssize_t show_supported_protocols(struct device *d,
-			     struct device_attribute *mattr, char *buf)
-{
-	char *orgbuf = buf;
-	struct ir_input_dev *ir_dev = dev_get_drvdata(d);
+	if (ir_dev->props->driver_type == RC_DRIVER_SCANCODE) {
+		spin_lock_irqsave(&ir_dev->rc_tab.lock, flags);
+		ir_dev->rc_tab.ir_type = type;
+		spin_unlock_irqrestore(&ir_dev->rc_tab.lock, flags);
+	} else {
+		ir_dev->raw->enabled_protocols = type;
+	}
 
-	/* FIXME: doesn't support multiple protocols at the same time */
-	if (ir_dev->props->allowed_protos == IR_TYPE_UNKNOWN)
-		buf += sprintf(buf, "unknown ");
-	if (ir_dev->props->allowed_protos & IR_TYPE_RC5)
-		buf += sprintf(buf, "rc-5 ");
-	if (ir_dev->props->allowed_protos & IR_TYPE_NEC)
-		buf += sprintf(buf, "nec ");
-	if (buf == orgbuf)
-		buf += sprintf(buf, "other ");
 
-	buf += sprintf(buf - 1, "\n");
+	IR_dprintk(1, "Current protocol(s): 0x%llx\n",
+		   (long long)type);
 
-	return buf - orgbuf;
+	return len;
 }
 
 #define ADD_HOTPLUG_VAR(fmt, val...)					\
@@ -158,7 +214,7 @@ static ssize_t show_supported_protocols(struct device *d,
 			return err;					\
 	} while (0)
 
-static int ir_dev_uevent(struct device *device, struct kobj_uevent_env *env)
+static int rc_dev_uevent(struct device *device, struct kobj_uevent_env *env)
 {
 	struct ir_input_dev *ir_dev = dev_get_drvdata(device);
 
@@ -173,34 +229,26 @@ static int ir_dev_uevent(struct device *device, struct kobj_uevent_env *env)
 /*
  * Static device attribute struct with the sysfs attributes for IR's
  */
-static DEVICE_ATTR(protocol, S_IRUGO | S_IWUSR,
-		   show_protocol, store_protocol);
+static DEVICE_ATTR(protocols, S_IRUGO | S_IWUSR,
+		   show_protocols, store_protocols);
 
-static DEVICE_ATTR(supported_protocols, S_IRUGO | S_IWUSR,
-		   show_supported_protocols, NULL);
-
-static struct attribute *ir_hw_dev_attrs[] = {
-	&dev_attr_protocol.attr,
-	&dev_attr_supported_protocols.attr,
+static struct attribute *rc_dev_attrs[] = {
+	&dev_attr_protocols.attr,
 	NULL,
 };
 
-static struct attribute_group ir_hw_dev_attr_grp = {
-	.attrs	= ir_hw_dev_attrs,
+static struct attribute_group rc_dev_attr_grp = {
+	.attrs	= rc_dev_attrs,
 };
 
-static const struct attribute_group *ir_hw_dev_attr_groups[] = {
-	&ir_hw_dev_attr_grp,
+static const struct attribute_group *rc_dev_attr_groups[] = {
+	&rc_dev_attr_grp,
 	NULL
 };
 
 static struct device_type rc_dev_type = {
-	.groups		= ir_hw_dev_attr_groups,
-	.uevent		= ir_dev_uevent,
-};
-
-static struct device_type ir_raw_dev_type = {
-	.uevent		= ir_dev_uevent,
+	.groups		= rc_dev_attr_groups,
+	.uevent		= rc_dev_uevent,
 };
 
 /**
@@ -220,11 +268,7 @@ int ir_register_class(struct input_dev *input_dev)
 	if (unlikely(devno < 0))
 		return devno;
 
-	if (ir_dev->props->driver_type == RC_DRIVER_SCANCODE)
-		ir_dev->dev.type = &rc_dev_type;
-	else
-		ir_dev->dev.type = &ir_raw_dev_type;
-
+	ir_dev->dev.type = &rc_dev_type;
 	ir_dev->dev.class = &ir_input_class;
 	ir_dev->dev.parent = input_dev->dev.parent;
 	dev_set_name(&ir_dev->dev, "rc%d", devno);


^ permalink raw reply related	[flat|nested] 21+ messages in thread
* [PATCH 2/2] ir-core: move decoding state to ir_raw_event_ctrl
  2010-06-13 20:29 ` David Härdeman
@ 2010-06-13 20:29   ` David Härdeman
  -1 siblings, 0 replies; 21+ messages in thread
From: David Härdeman @ 2010-06-13 20:29 UTC (permalink / raw)
  To: jarod; +Cc: jarod, linux-media, mchehab, linux-input

This patch moves the state from each raw decoder into the
ir_raw_event_ctrl struct.

This allows the removal of code like this:

        spin_lock(&decoder_lock);
        list_for_each_entry(data, &decoder_list, list) {
                if (data->ir_dev == ir_dev)
                        break;
        }
        spin_unlock(&decoder_lock);
        return data;

which is currently run for each decoder on each event in order
to get the client-specific decoding state data.

In addition, ir decoding modules and ir driver module load
order is now independent. Centralizing the data also allows
for a nice code reduction of about 30% per raw decoder as
client lists and client registration callbacks are no longer
necessary (but still kept around for the benefit of the lirc
decoder).

Out-of-tree modules can still use a similar trick to what
the raw decoders did before this patch until they are merged.

Signed-off-by: David Härdeman <david@hardeman.nu>
---
 drivers/media/IR/ir-core-priv.h    |   38 +++++++++++++
 drivers/media/IR/ir-jvc-decoder.c  |   90 ++-----------------------------
 drivers/media/IR/ir-nec-decoder.c  |   89 +++----------------------------
 drivers/media/IR/ir-raw-event.c    |   74 +++++++++++++-------------
 drivers/media/IR/ir-rc5-decoder.c  |  103 +++++-------------------------------
 drivers/media/IR/ir-rc6-decoder.c  |   92 ++------------------------------
 drivers/media/IR/ir-sony-decoder.c |   93 +++------------------------------
 7 files changed, 118 insertions(+), 461 deletions(-)

diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h
index 3072e55..9994af4 100644
--- a/drivers/media/IR/ir-core-priv.h
+++ b/drivers/media/IR/ir-core-priv.h
@@ -23,17 +23,55 @@ struct ir_raw_handler {
 
 	u64 protocols; /* which are handled by this handler */
 	int (*decode)(struct input_dev *input_dev, struct ir_raw_event event);
+
+	/* These two should only be used by the lirc decoder */
 	int (*raw_register)(struct input_dev *input_dev);
 	int (*raw_unregister)(struct input_dev *input_dev);
 };
 
 struct ir_raw_event_ctrl {
+	struct list_head		list;		/* to keep track of raw clients */
 	struct work_struct		rx_work;	/* for the rx decoding workqueue */
 	struct kfifo			kfifo;		/* fifo for the pulse/space durations */
 	ktime_t				last_event;	/* when last event occurred */
 	enum raw_event_type		last_type;	/* last event type */
 	struct input_dev		*input_dev;	/* pointer to the parent input_dev */
 	u64				enabled_protocols; /* enabled raw protocol decoders */
+
+	/* raw decoder state follows */
+	struct ir_raw_event prev_ev;
+	struct nec_dec {
+		int state;
+		unsigned count;
+		u32 bits;
+	} nec;
+	struct rc5_dec {
+		int state;
+		u32 bits;
+		unsigned count;
+		unsigned wanted_bits;
+	} rc5;
+	struct rc6_dec {
+		int state;
+		u8 header;
+		u32 body;
+		bool toggle;
+		unsigned count;
+		unsigned wanted_bits;
+	} rc6;
+	struct sony_dec {
+		int state;
+		u32 bits;
+		unsigned count;
+	} sony;
+	struct jvc_dec {
+		int state;
+		u16 bits;
+		u16 old_bits;
+		unsigned count;
+		bool first;
+		bool toggle;
+	} jvc;
 };
 
 /* macros for IR decoders */
diff --git a/drivers/media/IR/ir-jvc-decoder.c b/drivers/media/IR/ir-jvc-decoder.c
index 1055de4..8894d8b 100644
--- a/drivers/media/IR/ir-jvc-decoder.c
+++ b/drivers/media/IR/ir-jvc-decoder.c
@@ -25,10 +25,6 @@
 #define JVC_TRAILER_PULSE	(1  * JVC_UNIT)
 #define	JVC_TRAILER_SPACE	(35 * JVC_UNIT)
 
-/* Used to register jvc_decoder clients */
-static LIST_HEAD(decoder_list);
-DEFINE_SPINLOCK(decoder_lock);
-
 enum jvc_state {
 	STATE_INACTIVE,
 	STATE_HEADER_SPACE,
@@ -38,39 +34,6 @@ enum jvc_state {
 	STATE_TRAILER_SPACE,
 };
 
-struct decoder_data {
-	struct list_head	list;
-	struct ir_input_dev	*ir_dev;
-
-	/* State machine control */
-	enum jvc_state		state;
-	u16			jvc_bits;
-	u16			jvc_old_bits;
-	unsigned		count;
-	bool			first;
-	bool			toggle;
-};
-
-
-/**
- * get_decoder_data()	- gets decoder data
- * @input_dev:	input device
- *
- * Returns the struct decoder_data that corresponds to a device
- */
-static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
-{
-	struct decoder_data *data = NULL;
-
-	spin_lock(&decoder_lock);
-	list_for_each_entry(data, &decoder_list, list) {
-		if (data->ir_dev == ir_dev)
-			break;
-	}
-	spin_unlock(&decoder_lock);
-	return data;
-}
-
 /**
  * ir_jvc_decode() - Decode one JVC pulse or space
  * @input_dev:	the struct input_dev descriptor of the device
@@ -80,12 +43,8 @@ static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
  */
 static int ir_jvc_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 {
-	struct decoder_data *data;
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return -EINVAL;
+	struct jvc_dec *data = &ir_dev->raw->jvc;
 
 	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_JVC))
 		return 0;
@@ -140,9 +99,9 @@ static int ir_jvc_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 		if (ev.pulse)
 			break;
 
-		data->jvc_bits <<= 1;
+		data->bits <<= 1;
 		if (eq_margin(ev.duration, JVC_BIT_1_SPACE, JVC_UNIT / 2)) {
-			data->jvc_bits |= 1;
+			data->bits |= 1;
 			decrease_duration(&ev, JVC_BIT_1_SPACE);
 		} else if (eq_margin(ev.duration, JVC_BIT_0_SPACE, JVC_UNIT / 2))
 			decrease_duration(&ev, JVC_BIT_0_SPACE);
@@ -175,13 +134,13 @@ static int ir_jvc_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 
 		if (data->first) {
 			u32 scancode;
-			scancode = (bitrev8((data->jvc_bits >> 8) & 0xff) << 8) |
-				   (bitrev8((data->jvc_bits >> 0) & 0xff) << 0);
+			scancode = (bitrev8((data->bits >> 8) & 0xff) << 8) |
+				   (bitrev8((data->bits >> 0) & 0xff) << 0);
 			IR_dprintk(1, "JVC scancode 0x%04x\n", scancode);
 			ir_keydown(input_dev, scancode, data->toggle);
 			data->first = false;
-			data->jvc_old_bits = data->jvc_bits;
-		} else if (data->jvc_bits == data->jvc_old_bits) {
+			data->old_bits = data->bits;
+		} else if (data->bits == data->old_bits) {
 			IR_dprintk(1, "JVC repeat\n");
 			ir_repeat(input_dev);
 		} else {
@@ -201,44 +160,9 @@ out:
 	return -EINVAL;
 }
 
-static int ir_jvc_register(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	struct decoder_data *data;
-
-	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	data->ir_dev = ir_dev;
-	spin_lock(&decoder_lock);
-	list_add_tail(&data->list, &decoder_list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
-static int ir_jvc_unregister(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	static struct decoder_data *data;
-
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return 0;
-
-	spin_lock(&decoder_lock);
-	list_del(&data->list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
 static struct ir_raw_handler jvc_handler = {
 	.protocols	= IR_TYPE_JVC,
 	.decode		= ir_jvc_decode,
-	.raw_register	= ir_jvc_register,
-	.raw_unregister	= ir_jvc_unregister,
 };
 
 static int __init ir_jvc_decode_init(void)
diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c
index 2cc2b92..52e0f37 100644
--- a/drivers/media/IR/ir-nec-decoder.c
+++ b/drivers/media/IR/ir-nec-decoder.c
@@ -27,10 +27,6 @@
 #define	NEC_TRAILER_PULSE	(1  * NEC_UNIT)
 #define	NEC_TRAILER_SPACE	(10 * NEC_UNIT) /* even longer in reality */
 
-/* Used to register nec_decoder clients */
-static LIST_HEAD(decoder_list);
-static DEFINE_SPINLOCK(decoder_lock);
-
 enum nec_state {
 	STATE_INACTIVE,
 	STATE_HEADER_SPACE,
@@ -40,36 +36,6 @@ enum nec_state {
 	STATE_TRAILER_SPACE,
 };
 
-struct decoder_data {
-	struct list_head	list;
-	struct ir_input_dev	*ir_dev;
-
-	/* State machine control */
-	enum nec_state		state;
-	u32			nec_bits;
-	unsigned		count;
-};
-
-
-/**
- * get_decoder_data()	- gets decoder data
- * @input_dev:	input device
- *
- * Returns the struct decoder_data that corresponds to a device
- */
-static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
-{
-	struct decoder_data *data = NULL;
-
-	spin_lock(&decoder_lock);
-	list_for_each_entry(data, &decoder_list, list) {
-		if (data->ir_dev == ir_dev)
-			break;
-	}
-	spin_unlock(&decoder_lock);
-	return data;
-}
-
 /**
  * ir_nec_decode() - Decode one NEC pulse or space
  * @input_dev:	the struct input_dev descriptor of the device
@@ -79,15 +45,11 @@ static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
  */
 static int ir_nec_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 {
-	struct decoder_data *data;
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
+	struct nec_dec *data = &ir_dev->raw->nec;
 	u32 scancode;
 	u8 address, not_address, command, not_command;
 
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return -EINVAL;
-
 	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_NEC))
 		return 0;
 
@@ -143,9 +105,9 @@ static int ir_nec_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 		if (ev.pulse)
 			break;
 
-		data->nec_bits <<= 1;
+		data->bits <<= 1;
 		if (eq_margin(ev.duration, NEC_BIT_1_SPACE, NEC_UNIT / 2))
-			data->nec_bits |= 1;
+			data->bits |= 1;
 		else if (!eq_margin(ev.duration, NEC_BIT_0_SPACE, NEC_UNIT / 2))
 			break;
 		data->count++;
@@ -174,14 +136,14 @@ static int ir_nec_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 		if (!geq_margin(ev.duration, NEC_TRAILER_SPACE, NEC_UNIT / 2))
 			break;
 
-		address     = bitrev8((data->nec_bits >> 24) & 0xff);
-		not_address = bitrev8((data->nec_bits >> 16) & 0xff);
-		command	    = bitrev8((data->nec_bits >>  8) & 0xff);
-		not_command = bitrev8((data->nec_bits >>  0) & 0xff);
+		address     = bitrev8((data->bits >> 24) & 0xff);
+		not_address = bitrev8((data->bits >> 16) & 0xff);
+		command	    = bitrev8((data->bits >>  8) & 0xff);
+		not_command = bitrev8((data->bits >>  0) & 0xff);
 
 		if ((command ^ not_command) != 0xff) {
 			IR_dprintk(1, "NEC checksum error: received 0x%08x\n",
-				   data->nec_bits);
+				   data->bits);
 			break;
 		}
 
@@ -208,44 +170,9 @@ static int ir_nec_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 	return -EINVAL;
 }
 
-static int ir_nec_register(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	struct decoder_data *data;
-
-	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	data->ir_dev = ir_dev;
-	spin_lock(&decoder_lock);
-	list_add_tail(&data->list, &decoder_list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
-static int ir_nec_unregister(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	static struct decoder_data *data;
-
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return 0;
-
-	spin_lock(&decoder_lock);
-	list_del(&data->list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
 static struct ir_raw_handler nec_handler = {
 	.protocols	= IR_TYPE_NEC,
 	.decode		= ir_nec_decode,
-	.raw_register	= ir_nec_register,
-	.raw_unregister	= ir_nec_unregister,
 };
 
 static int __init ir_nec_decode_init(void)
diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c
index eeca8a8..5f98ab8 100644
--- a/drivers/media/IR/ir-raw-event.c
+++ b/drivers/media/IR/ir-raw-event.c
@@ -20,37 +20,14 @@
 /* Define the max number of pulse/space transitions to buffer */
 #define MAX_IR_EVENT_SIZE      512
 
+/* Used to keep track of IR raw clients, protected by ir_raw_handler_lock */
+static LIST_HEAD(ir_raw_client_list);
+
 /* Used to handle IR raw handler extensions */
 static DEFINE_SPINLOCK(ir_raw_handler_lock);
 static LIST_HEAD(ir_raw_handler_list);
 static u64 available_protocols;
 
-/**
- * RUN_DECODER()	- runs an operation on all IR decoders
- * @ops:	IR raw handler operation to be called
- * @arg:	arguments to be passed to the callback
- *
- * Calls ir_raw_handler::ops for all registered IR handlers. It prevents
- * new decode addition/removal while running, by locking ir_raw_handler_lock
- * mutex. If an error occurs, it stops the ops. Otherwise, it returns a sum
- * of the return codes.
- */
-#define RUN_DECODER(ops, ...) ({					    \
-	struct ir_raw_handler		*_ir_raw_handler;		    \
-	int _sumrc = 0, _rc;						    \
-	spin_lock(&ir_raw_handler_lock);				    \
-	list_for_each_entry(_ir_raw_handler, &ir_raw_handler_list, list) {  \
-		if (_ir_raw_handler->ops) {				    \
-			_rc = _ir_raw_handler->ops(__VA_ARGS__);	    \
-			if (_rc < 0)					    \
-				break;					    \
-			_sumrc += _rc;					    \
-		}							    \
-	}								    \
-	spin_unlock(&ir_raw_handler_lock);				    \
-	_sumrc;								    \
-})
-
 #ifdef MODULE
 /* Used to load the decoders */
 static struct work_struct wq_load;
@@ -59,11 +36,17 @@ static struct work_struct wq_load;
 static void ir_raw_event_work(struct work_struct *work)
 {
 	struct ir_raw_event ev;
+	struct ir_raw_handler *handler;
 	struct ir_raw_event_ctrl *raw =
 		container_of(work, struct ir_raw_event_ctrl, rx_work);
 
-	while (kfifo_out(&raw->kfifo, &ev, sizeof(ev)) == sizeof(ev))
-		RUN_DECODER(decode, raw->input_dev, ev);
+	while (kfifo_out(&raw->kfifo, &ev, sizeof(ev)) == sizeof(ev)) {
+		spin_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);
+		raw->prev_ev = ev;
+	}
 }
 
 /**
@@ -177,6 +160,7 @@ int ir_raw_event_register(struct input_dev *input_dev)
 {
 	struct ir_input_dev *ir = input_get_drvdata(input_dev);
 	int rc;
+	struct ir_raw_handler *handler;
 
 	ir->raw = kzalloc(sizeof(*ir->raw), GFP_KERNEL);
 	if (!ir->raw)
@@ -193,26 +177,32 @@ int ir_raw_event_register(struct input_dev *input_dev)
 		return rc;
 	}
 
-	rc = RUN_DECODER(raw_register, input_dev);
-	if (rc < 0) {
-		kfifo_free(&ir->raw->kfifo);
-		kfree(ir->raw);
-		ir->raw = NULL;
-		return rc;
-	}
+	spin_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);
 
-	return rc;
+	return 0;
 }
 
 void ir_raw_event_unregister(struct input_dev *input_dev)
 {
 	struct ir_input_dev *ir = input_get_drvdata(input_dev);
+	struct ir_raw_handler *handler;
 
 	if (!ir->raw)
 		return;
 
 	cancel_work_sync(&ir->raw->rx_work);
-	RUN_DECODER(raw_unregister, input_dev);
+
+	spin_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);
 
 	kfifo_free(&ir->raw->kfifo);
 	kfree(ir->raw);
@@ -225,8 +215,13 @@ void ir_raw_event_unregister(struct input_dev *input_dev)
 
 int ir_raw_handler_register(struct ir_raw_handler *ir_raw_handler)
 {
+	struct ir_raw_event_ctrl *raw;
+
 	spin_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);
 
@@ -236,8 +231,13 @@ EXPORT_SYMBOL(ir_raw_handler_register);
 
 void ir_raw_handler_unregister(struct ir_raw_handler *ir_raw_handler)
 {
+	struct ir_raw_event_ctrl *raw;
+
 	spin_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);
 }
diff --git a/drivers/media/IR/ir-rc5-decoder.c b/drivers/media/IR/ir-rc5-decoder.c
index 1be8981..7af656d 100644
--- a/drivers/media/IR/ir-rc5-decoder.c
+++ b/drivers/media/IR/ir-rc5-decoder.c
@@ -30,10 +30,6 @@
 #define RC5_BIT_END		(1 * RC5_UNIT)
 #define RC5X_SPACE		(4 * RC5_UNIT)
 
-/* Used to register rc5_decoder clients */
-static LIST_HEAD(decoder_list);
-static DEFINE_SPINLOCK(decoder_lock);
-
 enum rc5_state {
 	STATE_INACTIVE,
 	STATE_BIT_START,
@@ -42,39 +38,6 @@ enum rc5_state {
 	STATE_FINISHED,
 };
 
-struct decoder_data {
-	struct list_head	list;
-	struct ir_input_dev	*ir_dev;
-
-	/* State machine control */
-	enum rc5_state		state;
-	u32			rc5_bits;
-	struct ir_raw_event	prev_ev;
-	unsigned		count;
-	unsigned		wanted_bits;
-};
-
-
-/**
- * get_decoder_data()	- gets decoder data
- * @input_dev:	input device
- *
- * Returns the struct decoder_data that corresponds to a device
- */
-
-static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
-{
-	struct decoder_data *data = NULL;
-
-	spin_lock(&decoder_lock);
-	list_for_each_entry(data, &decoder_list, list) {
-		if (data->ir_dev == ir_dev)
-			break;
-	}
-	spin_unlock(&decoder_lock);
-	return data;
-}
-
 /**
  * ir_rc5_decode() - Decode one RC-5 pulse or space
  * @input_dev:	the struct input_dev descriptor of the device
@@ -84,15 +47,11 @@ static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
  */
 static int ir_rc5_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 {
-	struct decoder_data *data;
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
+	struct rc5_dec *data = &ir_dev->raw->rc5;
 	u8 toggle;
 	u32 scancode;
 
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return -EINVAL;
-
 	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_RC5))
 		return 0;
 
@@ -128,16 +87,15 @@ again:
 		if (!eq_margin(ev.duration, RC5_BIT_START, RC5_UNIT / 2))
 			break;
 
-		data->rc5_bits <<= 1;
+		data->bits <<= 1;
 		if (!ev.pulse)
-			data->rc5_bits |= 1;
+			data->bits |= 1;
 		data->count++;
-		data->prev_ev = ev;
 		data->state = STATE_BIT_END;
 		return 0;
 
 	case STATE_BIT_END:
-		if (!is_transition(&ev, &data->prev_ev))
+		if (!is_transition(&ev, &ir_dev->raw->prev_ev))
 			break;
 
 		if (data->count == data->wanted_bits)
@@ -169,11 +127,11 @@ again:
 		if (data->wanted_bits == RC5X_NBITS) {
 			/* RC5X */
 			u8 xdata, command, system;
-			xdata    = (data->rc5_bits & 0x0003F) >> 0;
-			command  = (data->rc5_bits & 0x00FC0) >> 6;
-			system   = (data->rc5_bits & 0x1F000) >> 12;
-			toggle   = (data->rc5_bits & 0x20000) ? 1 : 0;
-			command += (data->rc5_bits & 0x01000) ? 0 : 0x40;
+			xdata    = (data->bits & 0x0003F) >> 0;
+			command  = (data->bits & 0x00FC0) >> 6;
+			system   = (data->bits & 0x1F000) >> 12;
+			toggle   = (data->bits & 0x20000) ? 1 : 0;
+			command += (data->bits & 0x01000) ? 0 : 0x40;
 			scancode = system << 16 | command << 8 | xdata;
 
 			IR_dprintk(1, "RC5X scancode 0x%06x (toggle: %u)\n",
@@ -182,10 +140,10 @@ again:
 		} else {
 			/* RC5 */
 			u8 command, system;
-			command  = (data->rc5_bits & 0x0003F) >> 0;
-			system   = (data->rc5_bits & 0x007C0) >> 6;
-			toggle   = (data->rc5_bits & 0x00800) ? 1 : 0;
-			command += (data->rc5_bits & 0x01000) ? 0 : 0x40;
+			command  = (data->bits & 0x0003F) >> 0;
+			system   = (data->bits & 0x007C0) >> 6;
+			toggle   = (data->bits & 0x00800) ? 1 : 0;
+			command += (data->bits & 0x01000) ? 0 : 0x40;
 			scancode = system << 8 | command;
 
 			IR_dprintk(1, "RC5 scancode 0x%04x (toggle: %u)\n",
@@ -204,44 +162,9 @@ out:
 	return -EINVAL;
 }
 
-static int ir_rc5_register(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	struct decoder_data *data;
-
-	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	data->ir_dev = ir_dev;
-	spin_lock(&decoder_lock);
-	list_add_tail(&data->list, &decoder_list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
-static int ir_rc5_unregister(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	static struct decoder_data *data;
-
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return 0;
-
-	spin_lock(&decoder_lock);
-	list_del(&data->list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
 static struct ir_raw_handler rc5_handler = {
 	.protocols	= IR_TYPE_RC5,
 	.decode		= ir_rc5_decode,
-	.raw_register	= ir_rc5_register,
-	.raw_unregister	= ir_rc5_unregister,
 };
 
 static int __init ir_rc5_decode_init(void)
diff --git a/drivers/media/IR/ir-rc6-decoder.c b/drivers/media/IR/ir-rc6-decoder.c
index 5e940a8..a562952 100644
--- a/drivers/media/IR/ir-rc6-decoder.c
+++ b/drivers/media/IR/ir-rc6-decoder.c
@@ -36,10 +36,6 @@
 #define RC6_STARTBIT_MASK	0x08	/* for the header bits */
 #define RC6_6A_MCE_TOGGLE_MASK	0x8000	/* for the body bits */
 
-/* Used to register rc6_decoder clients */
-static LIST_HEAD(decoder_list);
-static DEFINE_SPINLOCK(decoder_lock);
-
 enum rc6_mode {
 	RC6_MODE_0,
 	RC6_MODE_6A,
@@ -58,41 +54,7 @@ enum rc6_state {
 	STATE_FINISHED,
 };
 
-struct decoder_data {
-	struct list_head	list;
-	struct ir_input_dev	*ir_dev;
-
-	/* State machine control */
-	enum rc6_state		state;
-	u8			header;
-	u32			body;
-	struct ir_raw_event	prev_ev;
-	bool			toggle;
-	unsigned		count;
-	unsigned		wanted_bits;
-};
-
-
-/**
- * get_decoder_data()	- gets decoder data
- * @input_dev:	input device
- *
- * Returns the struct decoder_data that corresponds to a device
- */
-static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
-{
-	struct decoder_data *data = NULL;
-
-	spin_lock(&decoder_lock);
-	list_for_each_entry(data, &decoder_list, list) {
-		if (data->ir_dev == ir_dev)
-			break;
-	}
-	spin_unlock(&decoder_lock);
-	return data;
-}
-
-static enum rc6_mode rc6_mode(struct decoder_data *data) {
+static enum rc6_mode rc6_mode(struct rc6_dec *data) {
 	switch (data->header & RC6_MODE_MASK) {
 	case 0:
 		return RC6_MODE_0;
@@ -114,15 +76,11 @@ static enum rc6_mode rc6_mode(struct decoder_data *data) {
  */
 static int ir_rc6_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 {
-	struct decoder_data *data;
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
+	struct rc6_dec *data = &ir_dev->raw->rc6;
 	u32 scancode;
 	u8 toggle;
 
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return -EINVAL;
-
 	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_RC6))
 		return 0;
 
@@ -175,12 +133,11 @@ again:
 		if (ev.pulse)
 			data->header |= 1;
 		data->count++;
-		data->prev_ev = ev;
 		data->state = STATE_HEADER_BIT_END;
 		return 0;
 
 	case STATE_HEADER_BIT_END:
-		if (!is_transition(&ev, &data->prev_ev))
+		if (!is_transition(&ev, &ir_dev->raw->prev_ev))
 			break;
 
 		if (data->count == RC6_HEADER_NBITS)
@@ -196,12 +153,11 @@ again:
 			break;
 
 		data->toggle = ev.pulse;
-		data->prev_ev = ev;
 		data->state = STATE_TOGGLE_END;
 		return 0;
 
 	case STATE_TOGGLE_END:
-		if (!is_transition(&ev, &data->prev_ev) ||
+		if (!is_transition(&ev, &ir_dev->raw->prev_ev) ||
 		    !geq_margin(ev.duration, RC6_TOGGLE_END, RC6_UNIT / 2))
 			break;
 
@@ -211,7 +167,6 @@ again:
 		}
 
 		data->state = STATE_BODY_BIT_START;
-		data->prev_ev = ev;
 		decrease_duration(&ev, RC6_TOGGLE_END);
 		data->count = 0;
 
@@ -243,13 +198,11 @@ again:
 		if (ev.pulse)
 			data->body |= 1;
 		data->count++;
-		data->prev_ev = ev;
-
 		data->state = STATE_BODY_BIT_END;
 		return 0;
 
 	case STATE_BODY_BIT_END:
-		if (!is_transition(&ev, &data->prev_ev))
+		if (!is_transition(&ev, &ir_dev->raw->prev_ev))
 			break;
 
 		if (data->count == data->wanted_bits)
@@ -300,44 +253,9 @@ out:
 	return -EINVAL;
 }
 
-static int ir_rc6_register(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	struct decoder_data *data;
-
-	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	data->ir_dev = ir_dev;
-	spin_lock(&decoder_lock);
-	list_add_tail(&data->list, &decoder_list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
-static int ir_rc6_unregister(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	static struct decoder_data *data;
-
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return 0;
-
-	spin_lock(&decoder_lock);
-	list_del(&data->list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
 static struct ir_raw_handler rc6_handler = {
 	.protocols	= IR_TYPE_RC6,
 	.decode		= ir_rc6_decode,
-	.raw_register	= ir_rc6_register,
-	.raw_unregister	= ir_rc6_unregister,
 };
 
 static int __init ir_rc6_decode_init(void)
diff --git a/drivers/media/IR/ir-sony-decoder.c b/drivers/media/IR/ir-sony-decoder.c
index 8afd16a..b9074f0 100644
--- a/drivers/media/IR/ir-sony-decoder.c
+++ b/drivers/media/IR/ir-sony-decoder.c
@@ -23,10 +23,6 @@
 #define SONY_BIT_SPACE		(1 * SONY_UNIT)
 #define SONY_TRAILER_SPACE	(10 * SONY_UNIT) /* minimum */
 
-/* Used to register sony_decoder clients */
-static LIST_HEAD(decoder_list);
-static DEFINE_SPINLOCK(decoder_lock);
-
 enum sony_state {
 	STATE_INACTIVE,
 	STATE_HEADER_SPACE,
@@ -35,36 +31,6 @@ enum sony_state {
 	STATE_FINISHED,
 };
 
-struct decoder_data {
-	struct list_head	list;
-	struct ir_input_dev	*ir_dev;
-
-	/* State machine control */
-	enum sony_state		state;
-	u32			sony_bits;
-	unsigned		count;
-};
-
-
-/**
- * get_decoder_data()	- gets decoder data
- * @input_dev:	input device
- *
- * Returns the struct decoder_data that corresponds to a device
- */
-static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
-{
-	struct decoder_data *data = NULL;
-
-	spin_lock(&decoder_lock);
-	list_for_each_entry(data, &decoder_list, list) {
-		if (data->ir_dev == ir_dev)
-			break;
-	}
-	spin_unlock(&decoder_lock);
-	return data;
-}
-
 /**
  * ir_sony_decode() - Decode one Sony pulse or space
  * @input_dev:	the struct input_dev descriptor of the device
@@ -74,15 +40,11 @@ static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
  */
 static int ir_sony_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 {
-	struct decoder_data *data;
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
+	struct sony_dec *data = &ir_dev->raw->sony;
 	u32 scancode;
 	u8 device, subdevice, function;
 
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return -EINVAL;
-
 	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_SONY))
 		return 0;
 
@@ -124,9 +86,9 @@ static int ir_sony_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 		if (!ev.pulse)
 			break;
 
-		data->sony_bits <<= 1;
+		data->bits <<= 1;
 		if (eq_margin(ev.duration, SONY_BIT_1_PULSE, SONY_UNIT / 2))
-			data->sony_bits |= 1;
+			data->bits |= 1;
 		else if (!eq_margin(ev.duration, SONY_BIT_0_PULSE, SONY_UNIT / 2))
 			break;
 
@@ -160,19 +122,19 @@ static int ir_sony_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 
 		switch (data->count) {
 		case 12:
-			device    = bitrev8((data->sony_bits <<  3) & 0xF8);
+			device    = bitrev8((data->bits <<  3) & 0xF8);
 			subdevice = 0;
-			function  = bitrev8((data->sony_bits >>  4) & 0xFE);
+			function  = bitrev8((data->bits >>  4) & 0xFE);
 			break;
 		case 15:
-			device    = bitrev8((data->sony_bits >>  0) & 0xFF);
+			device    = bitrev8((data->bits >>  0) & 0xFF);
 			subdevice = 0;
-			function  = bitrev8((data->sony_bits >>  7) & 0xFD);
+			function  = bitrev8((data->bits >>  7) & 0xFD);
 			break;
 		case 20:
-			device    = bitrev8((data->sony_bits >>  5) & 0xF8);
-			subdevice = bitrev8((data->sony_bits >>  0) & 0xFF);
-			function  = bitrev8((data->sony_bits >> 12) & 0xFE);
+			device    = bitrev8((data->bits >>  5) & 0xF8);
+			subdevice = bitrev8((data->bits >>  0) & 0xFF);
+			function  = bitrev8((data->bits >> 12) & 0xFE);
 			break;
 		default:
 			IR_dprintk(1, "Sony invalid bitcount %u\n", data->count);
@@ -193,44 +155,9 @@ out:
 	return -EINVAL;
 }
 
-static int ir_sony_register(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	struct decoder_data *data;
-
-	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	data->ir_dev = ir_dev;
-	spin_lock(&decoder_lock);
-	list_add_tail(&data->list, &decoder_list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
-static int ir_sony_unregister(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	static struct decoder_data *data;
-
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return 0;
-
-	spin_lock(&decoder_lock);
-	list_del(&data->list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
 static struct ir_raw_handler sony_handler = {
 	.protocols	= IR_TYPE_SONY,
 	.decode		= ir_sony_decode,
-	.raw_register	= ir_sony_register,
-	.raw_unregister	= ir_sony_unregister,
 };
 
 static int __init ir_sony_decode_init(void)


^ permalink raw reply related	[flat|nested] 21+ messages in thread
* [PATCH 2/2] ir-core: move decoding state to ir_raw_event_ctrl
@ 2010-06-13 20:29   ` David Härdeman
  0 siblings, 0 replies; 21+ messages in thread
From: David Härdeman @ 2010-06-13 20:29 UTC (permalink / raw)
  To: jarod; +Cc: jarod, linux-media, mchehab, linux-input

This patch moves the state from each raw decoder into the
ir_raw_event_ctrl struct.

This allows the removal of code like this:

        spin_lock(&decoder_lock);
        list_for_each_entry(data, &decoder_list, list) {
                if (data->ir_dev == ir_dev)
                        break;
        }
        spin_unlock(&decoder_lock);
        return data;

which is currently run for each decoder on each event in order
to get the client-specific decoding state data.

In addition, ir decoding modules and ir driver module load
order is now independent. Centralizing the data also allows
for a nice code reduction of about 30% per raw decoder as
client lists and client registration callbacks are no longer
necessary (but still kept around for the benefit of the lirc
decoder).

Out-of-tree modules can still use a similar trick to what
the raw decoders did before this patch until they are merged.

Signed-off-by: David Härdeman <david@hardeman.nu>
---
 drivers/media/IR/ir-core-priv.h    |   38 +++++++++++++
 drivers/media/IR/ir-jvc-decoder.c  |   90 ++-----------------------------
 drivers/media/IR/ir-nec-decoder.c  |   89 +++----------------------------
 drivers/media/IR/ir-raw-event.c    |   74 +++++++++++++-------------
 drivers/media/IR/ir-rc5-decoder.c  |  103 +++++-------------------------------
 drivers/media/IR/ir-rc6-decoder.c  |   92 ++------------------------------
 drivers/media/IR/ir-sony-decoder.c |   93 +++------------------------------
 7 files changed, 118 insertions(+), 461 deletions(-)

diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h
index 3072e55..9994af4 100644
--- a/drivers/media/IR/ir-core-priv.h
+++ b/drivers/media/IR/ir-core-priv.h
@@ -23,17 +23,55 @@ struct ir_raw_handler {
 
 	u64 protocols; /* which are handled by this handler */
 	int (*decode)(struct input_dev *input_dev, struct ir_raw_event event);
+
+	/* These two should only be used by the lirc decoder */
 	int (*raw_register)(struct input_dev *input_dev);
 	int (*raw_unregister)(struct input_dev *input_dev);
 };
 
 struct ir_raw_event_ctrl {
+	struct list_head		list;		/* to keep track of raw clients */
 	struct work_struct		rx_work;	/* for the rx decoding workqueue */
 	struct kfifo			kfifo;		/* fifo for the pulse/space durations */
 	ktime_t				last_event;	/* when last event occurred */
 	enum raw_event_type		last_type;	/* last event type */
 	struct input_dev		*input_dev;	/* pointer to the parent input_dev */
 	u64				enabled_protocols; /* enabled raw protocol decoders */
+
+	/* raw decoder state follows */
+	struct ir_raw_event prev_ev;
+	struct nec_dec {
+		int state;
+		unsigned count;
+		u32 bits;
+	} nec;
+	struct rc5_dec {
+		int state;
+		u32 bits;
+		unsigned count;
+		unsigned wanted_bits;
+	} rc5;
+	struct rc6_dec {
+		int state;
+		u8 header;
+		u32 body;
+		bool toggle;
+		unsigned count;
+		unsigned wanted_bits;
+	} rc6;
+	struct sony_dec {
+		int state;
+		u32 bits;
+		unsigned count;
+	} sony;
+	struct jvc_dec {
+		int state;
+		u16 bits;
+		u16 old_bits;
+		unsigned count;
+		bool first;
+		bool toggle;
+	} jvc;
 };
 
 /* macros for IR decoders */
diff --git a/drivers/media/IR/ir-jvc-decoder.c b/drivers/media/IR/ir-jvc-decoder.c
index 1055de4..8894d8b 100644
--- a/drivers/media/IR/ir-jvc-decoder.c
+++ b/drivers/media/IR/ir-jvc-decoder.c
@@ -25,10 +25,6 @@
 #define JVC_TRAILER_PULSE	(1  * JVC_UNIT)
 #define	JVC_TRAILER_SPACE	(35 * JVC_UNIT)
 
-/* Used to register jvc_decoder clients */
-static LIST_HEAD(decoder_list);
-DEFINE_SPINLOCK(decoder_lock);
-
 enum jvc_state {
 	STATE_INACTIVE,
 	STATE_HEADER_SPACE,
@@ -38,39 +34,6 @@ enum jvc_state {
 	STATE_TRAILER_SPACE,
 };
 
-struct decoder_data {
-	struct list_head	list;
-	struct ir_input_dev	*ir_dev;
-
-	/* State machine control */
-	enum jvc_state		state;
-	u16			jvc_bits;
-	u16			jvc_old_bits;
-	unsigned		count;
-	bool			first;
-	bool			toggle;
-};
-
-
-/**
- * get_decoder_data()	- gets decoder data
- * @input_dev:	input device
- *
- * Returns the struct decoder_data that corresponds to a device
- */
-static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
-{
-	struct decoder_data *data = NULL;
-
-	spin_lock(&decoder_lock);
-	list_for_each_entry(data, &decoder_list, list) {
-		if (data->ir_dev == ir_dev)
-			break;
-	}
-	spin_unlock(&decoder_lock);
-	return data;
-}
-
 /**
  * ir_jvc_decode() - Decode one JVC pulse or space
  * @input_dev:	the struct input_dev descriptor of the device
@@ -80,12 +43,8 @@ static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
  */
 static int ir_jvc_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 {
-	struct decoder_data *data;
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return -EINVAL;
+	struct jvc_dec *data = &ir_dev->raw->jvc;
 
 	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_JVC))
 		return 0;
@@ -140,9 +99,9 @@ static int ir_jvc_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 		if (ev.pulse)
 			break;
 
-		data->jvc_bits <<= 1;
+		data->bits <<= 1;
 		if (eq_margin(ev.duration, JVC_BIT_1_SPACE, JVC_UNIT / 2)) {
-			data->jvc_bits |= 1;
+			data->bits |= 1;
 			decrease_duration(&ev, JVC_BIT_1_SPACE);
 		} else if (eq_margin(ev.duration, JVC_BIT_0_SPACE, JVC_UNIT / 2))
 			decrease_duration(&ev, JVC_BIT_0_SPACE);
@@ -175,13 +134,13 @@ static int ir_jvc_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 
 		if (data->first) {
 			u32 scancode;
-			scancode = (bitrev8((data->jvc_bits >> 8) & 0xff) << 8) |
-				   (bitrev8((data->jvc_bits >> 0) & 0xff) << 0);
+			scancode = (bitrev8((data->bits >> 8) & 0xff) << 8) |
+				   (bitrev8((data->bits >> 0) & 0xff) << 0);
 			IR_dprintk(1, "JVC scancode 0x%04x\n", scancode);
 			ir_keydown(input_dev, scancode, data->toggle);
 			data->first = false;
-			data->jvc_old_bits = data->jvc_bits;
-		} else if (data->jvc_bits == data->jvc_old_bits) {
+			data->old_bits = data->bits;
+		} else if (data->bits == data->old_bits) {
 			IR_dprintk(1, "JVC repeat\n");
 			ir_repeat(input_dev);
 		} else {
@@ -201,44 +160,9 @@ out:
 	return -EINVAL;
 }
 
-static int ir_jvc_register(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	struct decoder_data *data;
-
-	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	data->ir_dev = ir_dev;
-	spin_lock(&decoder_lock);
-	list_add_tail(&data->list, &decoder_list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
-static int ir_jvc_unregister(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	static struct decoder_data *data;
-
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return 0;
-
-	spin_lock(&decoder_lock);
-	list_del(&data->list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
 static struct ir_raw_handler jvc_handler = {
 	.protocols	= IR_TYPE_JVC,
 	.decode		= ir_jvc_decode,
-	.raw_register	= ir_jvc_register,
-	.raw_unregister	= ir_jvc_unregister,
 };
 
 static int __init ir_jvc_decode_init(void)
diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c
index 2cc2b92..52e0f37 100644
--- a/drivers/media/IR/ir-nec-decoder.c
+++ b/drivers/media/IR/ir-nec-decoder.c
@@ -27,10 +27,6 @@
 #define	NEC_TRAILER_PULSE	(1  * NEC_UNIT)
 #define	NEC_TRAILER_SPACE	(10 * NEC_UNIT) /* even longer in reality */
 
-/* Used to register nec_decoder clients */
-static LIST_HEAD(decoder_list);
-static DEFINE_SPINLOCK(decoder_lock);
-
 enum nec_state {
 	STATE_INACTIVE,
 	STATE_HEADER_SPACE,
@@ -40,36 +36,6 @@ enum nec_state {
 	STATE_TRAILER_SPACE,
 };
 
-struct decoder_data {
-	struct list_head	list;
-	struct ir_input_dev	*ir_dev;
-
-	/* State machine control */
-	enum nec_state		state;
-	u32			nec_bits;
-	unsigned		count;
-};
-
-
-/**
- * get_decoder_data()	- gets decoder data
- * @input_dev:	input device
- *
- * Returns the struct decoder_data that corresponds to a device
- */
-static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
-{
-	struct decoder_data *data = NULL;
-
-	spin_lock(&decoder_lock);
-	list_for_each_entry(data, &decoder_list, list) {
-		if (data->ir_dev == ir_dev)
-			break;
-	}
-	spin_unlock(&decoder_lock);
-	return data;
-}
-
 /**
  * ir_nec_decode() - Decode one NEC pulse or space
  * @input_dev:	the struct input_dev descriptor of the device
@@ -79,15 +45,11 @@ static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
  */
 static int ir_nec_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 {
-	struct decoder_data *data;
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
+	struct nec_dec *data = &ir_dev->raw->nec;
 	u32 scancode;
 	u8 address, not_address, command, not_command;
 
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return -EINVAL;
-
 	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_NEC))
 		return 0;
 
@@ -143,9 +105,9 @@ static int ir_nec_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 		if (ev.pulse)
 			break;
 
-		data->nec_bits <<= 1;
+		data->bits <<= 1;
 		if (eq_margin(ev.duration, NEC_BIT_1_SPACE, NEC_UNIT / 2))
-			data->nec_bits |= 1;
+			data->bits |= 1;
 		else if (!eq_margin(ev.duration, NEC_BIT_0_SPACE, NEC_UNIT / 2))
 			break;
 		data->count++;
@@ -174,14 +136,14 @@ static int ir_nec_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 		if (!geq_margin(ev.duration, NEC_TRAILER_SPACE, NEC_UNIT / 2))
 			break;
 
-		address     = bitrev8((data->nec_bits >> 24) & 0xff);
-		not_address = bitrev8((data->nec_bits >> 16) & 0xff);
-		command	    = bitrev8((data->nec_bits >>  8) & 0xff);
-		not_command = bitrev8((data->nec_bits >>  0) & 0xff);
+		address     = bitrev8((data->bits >> 24) & 0xff);
+		not_address = bitrev8((data->bits >> 16) & 0xff);
+		command	    = bitrev8((data->bits >>  8) & 0xff);
+		not_command = bitrev8((data->bits >>  0) & 0xff);
 
 		if ((command ^ not_command) != 0xff) {
 			IR_dprintk(1, "NEC checksum error: received 0x%08x\n",
-				   data->nec_bits);
+				   data->bits);
 			break;
 		}
 
@@ -208,44 +170,9 @@ static int ir_nec_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 	return -EINVAL;
 }
 
-static int ir_nec_register(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	struct decoder_data *data;
-
-	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	data->ir_dev = ir_dev;
-	spin_lock(&decoder_lock);
-	list_add_tail(&data->list, &decoder_list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
-static int ir_nec_unregister(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	static struct decoder_data *data;
-
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return 0;
-
-	spin_lock(&decoder_lock);
-	list_del(&data->list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
 static struct ir_raw_handler nec_handler = {
 	.protocols	= IR_TYPE_NEC,
 	.decode		= ir_nec_decode,
-	.raw_register	= ir_nec_register,
-	.raw_unregister	= ir_nec_unregister,
 };
 
 static int __init ir_nec_decode_init(void)
diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c
index eeca8a8..5f98ab8 100644
--- a/drivers/media/IR/ir-raw-event.c
+++ b/drivers/media/IR/ir-raw-event.c
@@ -20,37 +20,14 @@
 /* Define the max number of pulse/space transitions to buffer */
 #define MAX_IR_EVENT_SIZE      512
 
+/* Used to keep track of IR raw clients, protected by ir_raw_handler_lock */
+static LIST_HEAD(ir_raw_client_list);
+
 /* Used to handle IR raw handler extensions */
 static DEFINE_SPINLOCK(ir_raw_handler_lock);
 static LIST_HEAD(ir_raw_handler_list);
 static u64 available_protocols;
 
-/**
- * RUN_DECODER()	- runs an operation on all IR decoders
- * @ops:	IR raw handler operation to be called
- * @arg:	arguments to be passed to the callback
- *
- * Calls ir_raw_handler::ops for all registered IR handlers. It prevents
- * new decode addition/removal while running, by locking ir_raw_handler_lock
- * mutex. If an error occurs, it stops the ops. Otherwise, it returns a sum
- * of the return codes.
- */
-#define RUN_DECODER(ops, ...) ({					    \
-	struct ir_raw_handler		*_ir_raw_handler;		    \
-	int _sumrc = 0, _rc;						    \
-	spin_lock(&ir_raw_handler_lock);				    \
-	list_for_each_entry(_ir_raw_handler, &ir_raw_handler_list, list) {  \
-		if (_ir_raw_handler->ops) {				    \
-			_rc = _ir_raw_handler->ops(__VA_ARGS__);	    \
-			if (_rc < 0)					    \
-				break;					    \
-			_sumrc += _rc;					    \
-		}							    \
-	}								    \
-	spin_unlock(&ir_raw_handler_lock);				    \
-	_sumrc;								    \
-})
-
 #ifdef MODULE
 /* Used to load the decoders */
 static struct work_struct wq_load;
@@ -59,11 +36,17 @@ static struct work_struct wq_load;
 static void ir_raw_event_work(struct work_struct *work)
 {
 	struct ir_raw_event ev;
+	struct ir_raw_handler *handler;
 	struct ir_raw_event_ctrl *raw =
 		container_of(work, struct ir_raw_event_ctrl, rx_work);
 
-	while (kfifo_out(&raw->kfifo, &ev, sizeof(ev)) == sizeof(ev))
-		RUN_DECODER(decode, raw->input_dev, ev);
+	while (kfifo_out(&raw->kfifo, &ev, sizeof(ev)) == sizeof(ev)) {
+		spin_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);
+		raw->prev_ev = ev;
+	}
 }
 
 /**
@@ -177,6 +160,7 @@ int ir_raw_event_register(struct input_dev *input_dev)
 {
 	struct ir_input_dev *ir = input_get_drvdata(input_dev);
 	int rc;
+	struct ir_raw_handler *handler;
 
 	ir->raw = kzalloc(sizeof(*ir->raw), GFP_KERNEL);
 	if (!ir->raw)
@@ -193,26 +177,32 @@ int ir_raw_event_register(struct input_dev *input_dev)
 		return rc;
 	}
 
-	rc = RUN_DECODER(raw_register, input_dev);
-	if (rc < 0) {
-		kfifo_free(&ir->raw->kfifo);
-		kfree(ir->raw);
-		ir->raw = NULL;
-		return rc;
-	}
+	spin_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);
 
-	return rc;
+	return 0;
 }
 
 void ir_raw_event_unregister(struct input_dev *input_dev)
 {
 	struct ir_input_dev *ir = input_get_drvdata(input_dev);
+	struct ir_raw_handler *handler;
 
 	if (!ir->raw)
 		return;
 
 	cancel_work_sync(&ir->raw->rx_work);
-	RUN_DECODER(raw_unregister, input_dev);
+
+	spin_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);
 
 	kfifo_free(&ir->raw->kfifo);
 	kfree(ir->raw);
@@ -225,8 +215,13 @@ void ir_raw_event_unregister(struct input_dev *input_dev)
 
 int ir_raw_handler_register(struct ir_raw_handler *ir_raw_handler)
 {
+	struct ir_raw_event_ctrl *raw;
+
 	spin_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);
 
@@ -236,8 +231,13 @@ EXPORT_SYMBOL(ir_raw_handler_register);
 
 void ir_raw_handler_unregister(struct ir_raw_handler *ir_raw_handler)
 {
+	struct ir_raw_event_ctrl *raw;
+
 	spin_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);
 }
diff --git a/drivers/media/IR/ir-rc5-decoder.c b/drivers/media/IR/ir-rc5-decoder.c
index 1be8981..7af656d 100644
--- a/drivers/media/IR/ir-rc5-decoder.c
+++ b/drivers/media/IR/ir-rc5-decoder.c
@@ -30,10 +30,6 @@
 #define RC5_BIT_END		(1 * RC5_UNIT)
 #define RC5X_SPACE		(4 * RC5_UNIT)
 
-/* Used to register rc5_decoder clients */
-static LIST_HEAD(decoder_list);
-static DEFINE_SPINLOCK(decoder_lock);
-
 enum rc5_state {
 	STATE_INACTIVE,
 	STATE_BIT_START,
@@ -42,39 +38,6 @@ enum rc5_state {
 	STATE_FINISHED,
 };
 
-struct decoder_data {
-	struct list_head	list;
-	struct ir_input_dev	*ir_dev;
-
-	/* State machine control */
-	enum rc5_state		state;
-	u32			rc5_bits;
-	struct ir_raw_event	prev_ev;
-	unsigned		count;
-	unsigned		wanted_bits;
-};
-
-
-/**
- * get_decoder_data()	- gets decoder data
- * @input_dev:	input device
- *
- * Returns the struct decoder_data that corresponds to a device
- */
-
-static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
-{
-	struct decoder_data *data = NULL;
-
-	spin_lock(&decoder_lock);
-	list_for_each_entry(data, &decoder_list, list) {
-		if (data->ir_dev == ir_dev)
-			break;
-	}
-	spin_unlock(&decoder_lock);
-	return data;
-}
-
 /**
  * ir_rc5_decode() - Decode one RC-5 pulse or space
  * @input_dev:	the struct input_dev descriptor of the device
@@ -84,15 +47,11 @@ static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
  */
 static int ir_rc5_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 {
-	struct decoder_data *data;
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
+	struct rc5_dec *data = &ir_dev->raw->rc5;
 	u8 toggle;
 	u32 scancode;
 
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return -EINVAL;
-
 	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_RC5))
 		return 0;
 
@@ -128,16 +87,15 @@ again:
 		if (!eq_margin(ev.duration, RC5_BIT_START, RC5_UNIT / 2))
 			break;
 
-		data->rc5_bits <<= 1;
+		data->bits <<= 1;
 		if (!ev.pulse)
-			data->rc5_bits |= 1;
+			data->bits |= 1;
 		data->count++;
-		data->prev_ev = ev;
 		data->state = STATE_BIT_END;
 		return 0;
 
 	case STATE_BIT_END:
-		if (!is_transition(&ev, &data->prev_ev))
+		if (!is_transition(&ev, &ir_dev->raw->prev_ev))
 			break;
 
 		if (data->count == data->wanted_bits)
@@ -169,11 +127,11 @@ again:
 		if (data->wanted_bits == RC5X_NBITS) {
 			/* RC5X */
 			u8 xdata, command, system;
-			xdata    = (data->rc5_bits & 0x0003F) >> 0;
-			command  = (data->rc5_bits & 0x00FC0) >> 6;
-			system   = (data->rc5_bits & 0x1F000) >> 12;
-			toggle   = (data->rc5_bits & 0x20000) ? 1 : 0;
-			command += (data->rc5_bits & 0x01000) ? 0 : 0x40;
+			xdata    = (data->bits & 0x0003F) >> 0;
+			command  = (data->bits & 0x00FC0) >> 6;
+			system   = (data->bits & 0x1F000) >> 12;
+			toggle   = (data->bits & 0x20000) ? 1 : 0;
+			command += (data->bits & 0x01000) ? 0 : 0x40;
 			scancode = system << 16 | command << 8 | xdata;
 
 			IR_dprintk(1, "RC5X scancode 0x%06x (toggle: %u)\n",
@@ -182,10 +140,10 @@ again:
 		} else {
 			/* RC5 */
 			u8 command, system;
-			command  = (data->rc5_bits & 0x0003F) >> 0;
-			system   = (data->rc5_bits & 0x007C0) >> 6;
-			toggle   = (data->rc5_bits & 0x00800) ? 1 : 0;
-			command += (data->rc5_bits & 0x01000) ? 0 : 0x40;
+			command  = (data->bits & 0x0003F) >> 0;
+			system   = (data->bits & 0x007C0) >> 6;
+			toggle   = (data->bits & 0x00800) ? 1 : 0;
+			command += (data->bits & 0x01000) ? 0 : 0x40;
 			scancode = system << 8 | command;
 
 			IR_dprintk(1, "RC5 scancode 0x%04x (toggle: %u)\n",
@@ -204,44 +162,9 @@ out:
 	return -EINVAL;
 }
 
-static int ir_rc5_register(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	struct decoder_data *data;
-
-	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	data->ir_dev = ir_dev;
-	spin_lock(&decoder_lock);
-	list_add_tail(&data->list, &decoder_list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
-static int ir_rc5_unregister(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	static struct decoder_data *data;
-
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return 0;
-
-	spin_lock(&decoder_lock);
-	list_del(&data->list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
 static struct ir_raw_handler rc5_handler = {
 	.protocols	= IR_TYPE_RC5,
 	.decode		= ir_rc5_decode,
-	.raw_register	= ir_rc5_register,
-	.raw_unregister	= ir_rc5_unregister,
 };
 
 static int __init ir_rc5_decode_init(void)
diff --git a/drivers/media/IR/ir-rc6-decoder.c b/drivers/media/IR/ir-rc6-decoder.c
index 5e940a8..a562952 100644
--- a/drivers/media/IR/ir-rc6-decoder.c
+++ b/drivers/media/IR/ir-rc6-decoder.c
@@ -36,10 +36,6 @@
 #define RC6_STARTBIT_MASK	0x08	/* for the header bits */
 #define RC6_6A_MCE_TOGGLE_MASK	0x8000	/* for the body bits */
 
-/* Used to register rc6_decoder clients */
-static LIST_HEAD(decoder_list);
-static DEFINE_SPINLOCK(decoder_lock);
-
 enum rc6_mode {
 	RC6_MODE_0,
 	RC6_MODE_6A,
@@ -58,41 +54,7 @@ enum rc6_state {
 	STATE_FINISHED,
 };
 
-struct decoder_data {
-	struct list_head	list;
-	struct ir_input_dev	*ir_dev;
-
-	/* State machine control */
-	enum rc6_state		state;
-	u8			header;
-	u32			body;
-	struct ir_raw_event	prev_ev;
-	bool			toggle;
-	unsigned		count;
-	unsigned		wanted_bits;
-};
-
-
-/**
- * get_decoder_data()	- gets decoder data
- * @input_dev:	input device
- *
- * Returns the struct decoder_data that corresponds to a device
- */
-static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
-{
-	struct decoder_data *data = NULL;
-
-	spin_lock(&decoder_lock);
-	list_for_each_entry(data, &decoder_list, list) {
-		if (data->ir_dev == ir_dev)
-			break;
-	}
-	spin_unlock(&decoder_lock);
-	return data;
-}
-
-static enum rc6_mode rc6_mode(struct decoder_data *data) {
+static enum rc6_mode rc6_mode(struct rc6_dec *data) {
 	switch (data->header & RC6_MODE_MASK) {
 	case 0:
 		return RC6_MODE_0;
@@ -114,15 +76,11 @@ static enum rc6_mode rc6_mode(struct decoder_data *data) {
  */
 static int ir_rc6_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 {
-	struct decoder_data *data;
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
+	struct rc6_dec *data = &ir_dev->raw->rc6;
 	u32 scancode;
 	u8 toggle;
 
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return -EINVAL;
-
 	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_RC6))
 		return 0;
 
@@ -175,12 +133,11 @@ again:
 		if (ev.pulse)
 			data->header |= 1;
 		data->count++;
-		data->prev_ev = ev;
 		data->state = STATE_HEADER_BIT_END;
 		return 0;
 
 	case STATE_HEADER_BIT_END:
-		if (!is_transition(&ev, &data->prev_ev))
+		if (!is_transition(&ev, &ir_dev->raw->prev_ev))
 			break;
 
 		if (data->count == RC6_HEADER_NBITS)
@@ -196,12 +153,11 @@ again:
 			break;
 
 		data->toggle = ev.pulse;
-		data->prev_ev = ev;
 		data->state = STATE_TOGGLE_END;
 		return 0;
 
 	case STATE_TOGGLE_END:
-		if (!is_transition(&ev, &data->prev_ev) ||
+		if (!is_transition(&ev, &ir_dev->raw->prev_ev) ||
 		    !geq_margin(ev.duration, RC6_TOGGLE_END, RC6_UNIT / 2))
 			break;
 
@@ -211,7 +167,6 @@ again:
 		}
 
 		data->state = STATE_BODY_BIT_START;
-		data->prev_ev = ev;
 		decrease_duration(&ev, RC6_TOGGLE_END);
 		data->count = 0;
 
@@ -243,13 +198,11 @@ again:
 		if (ev.pulse)
 			data->body |= 1;
 		data->count++;
-		data->prev_ev = ev;
-
 		data->state = STATE_BODY_BIT_END;
 		return 0;
 
 	case STATE_BODY_BIT_END:
-		if (!is_transition(&ev, &data->prev_ev))
+		if (!is_transition(&ev, &ir_dev->raw->prev_ev))
 			break;
 
 		if (data->count == data->wanted_bits)
@@ -300,44 +253,9 @@ out:
 	return -EINVAL;
 }
 
-static int ir_rc6_register(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	struct decoder_data *data;
-
-	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	data->ir_dev = ir_dev;
-	spin_lock(&decoder_lock);
-	list_add_tail(&data->list, &decoder_list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
-static int ir_rc6_unregister(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	static struct decoder_data *data;
-
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return 0;
-
-	spin_lock(&decoder_lock);
-	list_del(&data->list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
 static struct ir_raw_handler rc6_handler = {
 	.protocols	= IR_TYPE_RC6,
 	.decode		= ir_rc6_decode,
-	.raw_register	= ir_rc6_register,
-	.raw_unregister	= ir_rc6_unregister,
 };
 
 static int __init ir_rc6_decode_init(void)
diff --git a/drivers/media/IR/ir-sony-decoder.c b/drivers/media/IR/ir-sony-decoder.c
index 8afd16a..b9074f0 100644
--- a/drivers/media/IR/ir-sony-decoder.c
+++ b/drivers/media/IR/ir-sony-decoder.c
@@ -23,10 +23,6 @@
 #define SONY_BIT_SPACE		(1 * SONY_UNIT)
 #define SONY_TRAILER_SPACE	(10 * SONY_UNIT) /* minimum */
 
-/* Used to register sony_decoder clients */
-static LIST_HEAD(decoder_list);
-static DEFINE_SPINLOCK(decoder_lock);
-
 enum sony_state {
 	STATE_INACTIVE,
 	STATE_HEADER_SPACE,
@@ -35,36 +31,6 @@ enum sony_state {
 	STATE_FINISHED,
 };
 
-struct decoder_data {
-	struct list_head	list;
-	struct ir_input_dev	*ir_dev;
-
-	/* State machine control */
-	enum sony_state		state;
-	u32			sony_bits;
-	unsigned		count;
-};
-
-
-/**
- * get_decoder_data()	- gets decoder data
- * @input_dev:	input device
- *
- * Returns the struct decoder_data that corresponds to a device
- */
-static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
-{
-	struct decoder_data *data = NULL;
-
-	spin_lock(&decoder_lock);
-	list_for_each_entry(data, &decoder_list, list) {
-		if (data->ir_dev == ir_dev)
-			break;
-	}
-	spin_unlock(&decoder_lock);
-	return data;
-}
-
 /**
  * ir_sony_decode() - Decode one Sony pulse or space
  * @input_dev:	the struct input_dev descriptor of the device
@@ -74,15 +40,11 @@ static struct decoder_data *get_decoder_data(struct  ir_input_dev *ir_dev)
  */
 static int ir_sony_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 {
-	struct decoder_data *data;
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
+	struct sony_dec *data = &ir_dev->raw->sony;
 	u32 scancode;
 	u8 device, subdevice, function;
 
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return -EINVAL;
-
 	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_SONY))
 		return 0;
 
@@ -124,9 +86,9 @@ static int ir_sony_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 		if (!ev.pulse)
 			break;
 
-		data->sony_bits <<= 1;
+		data->bits <<= 1;
 		if (eq_margin(ev.duration, SONY_BIT_1_PULSE, SONY_UNIT / 2))
-			data->sony_bits |= 1;
+			data->bits |= 1;
 		else if (!eq_margin(ev.duration, SONY_BIT_0_PULSE, SONY_UNIT / 2))
 			break;
 
@@ -160,19 +122,19 @@ static int ir_sony_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 
 		switch (data->count) {
 		case 12:
-			device    = bitrev8((data->sony_bits <<  3) & 0xF8);
+			device    = bitrev8((data->bits <<  3) & 0xF8);
 			subdevice = 0;
-			function  = bitrev8((data->sony_bits >>  4) & 0xFE);
+			function  = bitrev8((data->bits >>  4) & 0xFE);
 			break;
 		case 15:
-			device    = bitrev8((data->sony_bits >>  0) & 0xFF);
+			device    = bitrev8((data->bits >>  0) & 0xFF);
 			subdevice = 0;
-			function  = bitrev8((data->sony_bits >>  7) & 0xFD);
+			function  = bitrev8((data->bits >>  7) & 0xFD);
 			break;
 		case 20:
-			device    = bitrev8((data->sony_bits >>  5) & 0xF8);
-			subdevice = bitrev8((data->sony_bits >>  0) & 0xFF);
-			function  = bitrev8((data->sony_bits >> 12) & 0xFE);
+			device    = bitrev8((data->bits >>  5) & 0xF8);
+			subdevice = bitrev8((data->bits >>  0) & 0xFF);
+			function  = bitrev8((data->bits >> 12) & 0xFE);
 			break;
 		default:
 			IR_dprintk(1, "Sony invalid bitcount %u\n", data->count);
@@ -193,44 +155,9 @@ out:
 	return -EINVAL;
 }
 
-static int ir_sony_register(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	struct decoder_data *data;
-
-	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	data->ir_dev = ir_dev;
-	spin_lock(&decoder_lock);
-	list_add_tail(&data->list, &decoder_list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
-static int ir_sony_unregister(struct input_dev *input_dev)
-{
-	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
-	static struct decoder_data *data;
-
-	data = get_decoder_data(ir_dev);
-	if (!data)
-		return 0;
-
-	spin_lock(&decoder_lock);
-	list_del(&data->list);
-	spin_unlock(&decoder_lock);
-
-	return 0;
-}
-
 static struct ir_raw_handler sony_handler = {
 	.protocols	= IR_TYPE_SONY,
 	.decode		= ir_sony_decode,
-	.raw_register	= ir_sony_register,
-	.raw_unregister	= ir_sony_unregister,
 };
 
 static int __init ir_sony_decode_init(void)

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] ir-core: centralize sysfs raw decoder enabling/disabling
  2010-06-13 20:29 ` [PATCH 1/2] ir-core: centralize sysfs raw decoder enabling/disabling David Härdeman
@ 2010-06-16 20:05     ` Jarod Wilson
  2010-06-28 16:56     ` Mauro Carvalho Chehab
  2010-09-08 14:04   ` Brian Rogers
  2 siblings, 0 replies; 21+ messages in thread
From: Jarod Wilson @ 2010-06-16 20:05 UTC (permalink / raw)
  To: David Härdeman; +Cc: jarod, linux-media, mchehab, linux-input

On Sun, Jun 13, 2010 at 4:29 PM, David Härdeman <david@hardeman.nu> wrote:
> With the current logic, each raw decoder needs to add a copy of the exact
> same sysfs code. This is both unnecessary and also means that (re)loading
> an IR driver after raw decoder modules have been loaded won't work as
> expected.
>
> This patch moves that logic into ir-raw-event and adds a single sysfs
> file per device.
>
> Reading that file returns something like:
>
>        "rc5 [rc6] nec jvc [sony]"
>
> (with enabled protocols in [] brackets)
>
> Writing either "+protocol" or "-protocol" to that file will
> enable or disable the according protocol decoder.
>
> An additional benefit is that the disabling of a decoder will be
> remembered across module removal/insertion so a previously
> disabled decoder won't suddenly be activated again. The default
> setting is to enable all decoders.
>
> This is also necessary for the next patch which moves even more decoder
> state into the central raw decoding structs.
>
> Signed-off-by: David Härdeman <david@hardeman.nu>

Acked-by: Jarod Wilson <jarod@redhat.com>
Tested-by: Jarod Wilson <jarod@redhat.com>

Note that I was running a version rebased atop the linuxtv staging/rc
branch though.

-- 
Jarod Wilson
jarod@wilsonet.com

^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] ir-core: centralize sysfs raw decoder enabling/disabling
@ 2010-06-16 20:05     ` Jarod Wilson
  0 siblings, 0 replies; 21+ messages in thread
From: Jarod Wilson @ 2010-06-16 20:05 UTC (permalink / raw)
  To: David Härdeman; +Cc: jarod, linux-media, mchehab, linux-input

On Sun, Jun 13, 2010 at 4:29 PM, David Härdeman <david@hardeman.nu> wrote:
> With the current logic, each raw decoder needs to add a copy of the exact
> same sysfs code. This is both unnecessary and also means that (re)loading
> an IR driver after raw decoder modules have been loaded won't work as
> expected.
>
> This patch moves that logic into ir-raw-event and adds a single sysfs
> file per device.
>
> Reading that file returns something like:
>
>        "rc5 [rc6] nec jvc [sony]"
>
> (with enabled protocols in [] brackets)
>
> Writing either "+protocol" or "-protocol" to that file will
> enable or disable the according protocol decoder.
>
> An additional benefit is that the disabling of a decoder will be
> remembered across module removal/insertion so a previously
> disabled decoder won't suddenly be activated again. The default
> setting is to enable all decoders.
>
> This is also necessary for the next patch which moves even more decoder
> state into the central raw decoding structs.
>
> Signed-off-by: David Härdeman <david@hardeman.nu>

Acked-by: Jarod Wilson <jarod@redhat.com>
Tested-by: Jarod Wilson <jarod@redhat.com>

Note that I was running a version rebased atop the linuxtv staging/rc
branch though.

-- 
Jarod Wilson
jarod@wilsonet.com
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] ir-core: move decoding state to ir_raw_event_ctrl
  2010-06-13 20:29   ` David Härdeman
@ 2010-06-16 20:06     ` Jarod Wilson
  -1 siblings, 0 replies; 21+ messages in thread
From: Jarod Wilson @ 2010-06-16 20:06 UTC (permalink / raw)
  To: David Härdeman; +Cc: jarod, linux-media, mchehab, linux-input

On Sun, Jun 13, 2010 at 4:29 PM, David Härdeman <david@hardeman.nu> wrote:
> This patch moves the state from each raw decoder into the
> ir_raw_event_ctrl struct.
>
> This allows the removal of code like this:
>
>        spin_lock(&decoder_lock);
>        list_for_each_entry(data, &decoder_list, list) {
>                if (data->ir_dev == ir_dev)
>                        break;
>        }
>        spin_unlock(&decoder_lock);
>        return data;
>
> which is currently run for each decoder on each event in order
> to get the client-specific decoding state data.
>
> In addition, ir decoding modules and ir driver module load
> order is now independent. Centralizing the data also allows
> for a nice code reduction of about 30% per raw decoder as
> client lists and client registration callbacks are no longer
> necessary (but still kept around for the benefit of the lirc
> decoder).
>
> Out-of-tree modules can still use a similar trick to what
> the raw decoders did before this patch until they are merged.
>
> Signed-off-by: David Härdeman <david@hardeman.nu>

Acked-by: Jarod Wilson <jarod@redhat.com>
Tested-by: Jarod Wilson <jarod@redhat.com>

Note that I was running a version rebased atop the linuxtv staging/rc
branch though.

-- 
Jarod Wilson
jarod@wilsonet.com

^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] ir-core: move decoding state to ir_raw_event_ctrl
@ 2010-06-16 20:06     ` Jarod Wilson
  0 siblings, 0 replies; 21+ messages in thread
From: Jarod Wilson @ 2010-06-16 20:06 UTC (permalink / raw)
  To: David Härdeman; +Cc: jarod, linux-media, mchehab, linux-input

On Sun, Jun 13, 2010 at 4:29 PM, David Härdeman <david@hardeman.nu> wrote:
> This patch moves the state from each raw decoder into the
> ir_raw_event_ctrl struct.
>
> This allows the removal of code like this:
>
>        spin_lock(&decoder_lock);
>        list_for_each_entry(data, &decoder_list, list) {
>                if (data->ir_dev == ir_dev)
>                        break;
>        }
>        spin_unlock(&decoder_lock);
>        return data;
>
> which is currently run for each decoder on each event in order
> to get the client-specific decoding state data.
>
> In addition, ir decoding modules and ir driver module load
> order is now independent. Centralizing the data also allows
> for a nice code reduction of about 30% per raw decoder as
> client lists and client registration callbacks are no longer
> necessary (but still kept around for the benefit of the lirc
> decoder).
>
> Out-of-tree modules can still use a similar trick to what
> the raw decoders did before this patch until they are merged.
>
> Signed-off-by: David Härdeman <david@hardeman.nu>

Acked-by: Jarod Wilson <jarod@redhat.com>
Tested-by: Jarod Wilson <jarod@redhat.com>

Note that I was running a version rebased atop the linuxtv staging/rc
branch though.

-- 
Jarod Wilson
jarod@wilsonet.com
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] ir-core: centralize sysfs raw decoder enabling/disabling
  2010-06-16 20:05     ` Jarod Wilson
@ 2010-06-16 20:39       ` Jarod Wilson
  -1 siblings, 0 replies; 21+ messages in thread
From: Jarod Wilson @ 2010-06-16 20:39 UTC (permalink / raw)
  To: David Härdeman; +Cc: jarod, linux-media, mchehab, linux-input

On Wed, Jun 16, 2010 at 4:05 PM, Jarod Wilson <jarod@wilsonet.com> wrote:
> On Sun, Jun 13, 2010 at 4:29 PM, David Härdeman <david@hardeman.nu> wrote:
>> With the current logic, each raw decoder needs to add a copy of the exact
>> same sysfs code. This is both unnecessary and also means that (re)loading
>> an IR driver after raw decoder modules have been loaded won't work as
>> expected.
>>
>> This patch moves that logic into ir-raw-event and adds a single sysfs
>> file per device.
>>
>> Reading that file returns something like:
>>
>>        "rc5 [rc6] nec jvc [sony]"
>>
>> (with enabled protocols in [] brackets)
>>
>> Writing either "+protocol" or "-protocol" to that file will
>> enable or disable the according protocol decoder.
>>
>> An additional benefit is that the disabling of a decoder will be
>> remembered across module removal/insertion so a previously
>> disabled decoder won't suddenly be activated again. The default
>> setting is to enable all decoders.
>>
>> This is also necessary for the next patch which moves even more decoder
>> state into the central raw decoding structs.
>>
>> Signed-off-by: David Härdeman <david@hardeman.nu>
>
> Acked-by: Jarod Wilson <jarod@redhat.com>
> Tested-by: Jarod Wilson <jarod@redhat.com>
>
> Note that I was running a version rebased atop the linuxtv staging/rc
> branch though.

Found here:

http://git.wilsonet.com//linux-2.6-ir-wip.git?a=commitdiff;h=e4166a6c70c4f9a06644ce8b4e9beeea7c336d90


-- 
Jarod Wilson
jarod@wilsonet.com

^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] ir-core: centralize sysfs raw decoder enabling/disabling
@ 2010-06-16 20:39       ` Jarod Wilson
  0 siblings, 0 replies; 21+ messages in thread
From: Jarod Wilson @ 2010-06-16 20:39 UTC (permalink / raw)
  To: David Härdeman; +Cc: jarod, linux-media, mchehab, linux-input

On Wed, Jun 16, 2010 at 4:05 PM, Jarod Wilson <jarod@wilsonet.com> wrote:
> On Sun, Jun 13, 2010 at 4:29 PM, David Härdeman <david@hardeman.nu> wrote:
>> With the current logic, each raw decoder needs to add a copy of the exact
>> same sysfs code. This is both unnecessary and also means that (re)loading
>> an IR driver after raw decoder modules have been loaded won't work as
>> expected.
>>
>> This patch moves that logic into ir-raw-event and adds a single sysfs
>> file per device.
>>
>> Reading that file returns something like:
>>
>>        "rc5 [rc6] nec jvc [sony]"
>>
>> (with enabled protocols in [] brackets)
>>
>> Writing either "+protocol" or "-protocol" to that file will
>> enable or disable the according protocol decoder.
>>
>> An additional benefit is that the disabling of a decoder will be
>> remembered across module removal/insertion so a previously
>> disabled decoder won't suddenly be activated again. The default
>> setting is to enable all decoders.
>>
>> This is also necessary for the next patch which moves even more decoder
>> state into the central raw decoding structs.
>>
>> Signed-off-by: David Härdeman <david@hardeman.nu>
>
> Acked-by: Jarod Wilson <jarod@redhat.com>
> Tested-by: Jarod Wilson <jarod@redhat.com>
>
> Note that I was running a version rebased atop the linuxtv staging/rc
> branch though.

Found here:

http://git.wilsonet.com//linux-2.6-ir-wip.git?a=commitdiff;h=e4166a6c70c4f9a06644ce8b4e9beeea7c336d90


-- 
Jarod Wilson
jarod@wilsonet.com
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] ir-core: move decoding state to ir_raw_event_ctrl
  2010-06-16 20:06     ` Jarod Wilson
@ 2010-06-16 20:39       ` Jarod Wilson
  -1 siblings, 0 replies; 21+ messages in thread
From: Jarod Wilson @ 2010-06-16 20:39 UTC (permalink / raw)
  To: David Härdeman; +Cc: jarod, linux-media, mchehab, linux-input

On Wed, Jun 16, 2010 at 4:06 PM, Jarod Wilson <jarod@wilsonet.com> wrote:
> On Sun, Jun 13, 2010 at 4:29 PM, David Härdeman <david@hardeman.nu> wrote:
>> This patch moves the state from each raw decoder into the
>> ir_raw_event_ctrl struct.
>>
>> This allows the removal of code like this:
>>
>>        spin_lock(&decoder_lock);
>>        list_for_each_entry(data, &decoder_list, list) {
>>                if (data->ir_dev == ir_dev)
>>                        break;
>>        }
>>        spin_unlock(&decoder_lock);
>>        return data;
>>
>> which is currently run for each decoder on each event in order
>> to get the client-specific decoding state data.
>>
>> In addition, ir decoding modules and ir driver module load
>> order is now independent. Centralizing the data also allows
>> for a nice code reduction of about 30% per raw decoder as
>> client lists and client registration callbacks are no longer
>> necessary (but still kept around for the benefit of the lirc
>> decoder).
>>
>> Out-of-tree modules can still use a similar trick to what
>> the raw decoders did before this patch until they are merged.
>>
>> Signed-off-by: David Härdeman <david@hardeman.nu>
>
> Acked-by: Jarod Wilson <jarod@redhat.com>
> Tested-by: Jarod Wilson <jarod@redhat.com>
>
> Note that I was running a version rebased atop the linuxtv staging/rc
> branch though.

Which can be seen here:

http://git.wilsonet.com//linux-2.6-ir-wip.git?a=commitdiff;h=faee132179abe884a6e995acb1f5106cb2d5afa2


-- 
Jarod Wilson
jarod@wilsonet.com

^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] ir-core: move decoding state to ir_raw_event_ctrl
@ 2010-06-16 20:39       ` Jarod Wilson
  0 siblings, 0 replies; 21+ messages in thread
From: Jarod Wilson @ 2010-06-16 20:39 UTC (permalink / raw)
  To: David Härdeman; +Cc: jarod, linux-media, mchehab, linux-input

On Wed, Jun 16, 2010 at 4:06 PM, Jarod Wilson <jarod@wilsonet.com> wrote:
> On Sun, Jun 13, 2010 at 4:29 PM, David Härdeman <david@hardeman.nu> wrote:
>> This patch moves the state from each raw decoder into the
>> ir_raw_event_ctrl struct.
>>
>> This allows the removal of code like this:
>>
>>        spin_lock(&decoder_lock);
>>        list_for_each_entry(data, &decoder_list, list) {
>>                if (data->ir_dev == ir_dev)
>>                        break;
>>        }
>>        spin_unlock(&decoder_lock);
>>        return data;
>>
>> which is currently run for each decoder on each event in order
>> to get the client-specific decoding state data.
>>
>> In addition, ir decoding modules and ir driver module load
>> order is now independent. Centralizing the data also allows
>> for a nice code reduction of about 30% per raw decoder as
>> client lists and client registration callbacks are no longer
>> necessary (but still kept around for the benefit of the lirc
>> decoder).
>>
>> Out-of-tree modules can still use a similar trick to what
>> the raw decoders did before this patch until they are merged.
>>
>> Signed-off-by: David Härdeman <david@hardeman.nu>
>
> Acked-by: Jarod Wilson <jarod@redhat.com>
> Tested-by: Jarod Wilson <jarod@redhat.com>
>
> Note that I was running a version rebased atop the linuxtv staging/rc
> branch though.

Which can be seen here:

http://git.wilsonet.com//linux-2.6-ir-wip.git?a=commitdiff;h=faee132179abe884a6e995acb1f5106cb2d5afa2


-- 
Jarod Wilson
jarod@wilsonet.com
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] ir-core: centralize sysfs raw decoder enabling/disabling
  2010-06-13 20:29 ` [PATCH 1/2] ir-core: centralize sysfs raw decoder enabling/disabling David Härdeman
@ 2010-06-28 16:56     ` Mauro Carvalho Chehab
  2010-06-28 16:56     ` Mauro Carvalho Chehab
  2010-09-08 14:04   ` Brian Rogers
  2 siblings, 0 replies; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2010-06-28 16:56 UTC (permalink / raw)
  To: David Härdeman; +Cc: jarod, jarod, linux-media, linux-input

Em 13-06-2010 17:29, David Härdeman escreveu:
> With the current logic, each raw decoder needs to add a copy of the exact
> same sysfs code. This is both unnecessary and also means that (re)loading
> an IR driver after raw decoder modules have been loaded won't work as
> expected.
> 
> This patch moves that logic into ir-raw-event and adds a single sysfs
> file per device.
> 
> Reading that file returns something like:
> 
> 	"rc5 [rc6] nec jvc [sony]"
> 
> (with enabled protocols in [] brackets)
> 
> Writing either "+protocol" or "-protocol" to that file will
> enable or disable the according protocol decoder.
> 
> An additional benefit is that the disabling of a decoder will be
> remembered across module removal/insertion so a previously
> disabled decoder won't suddenly be activated again. The default
> setting is to enable all decoders.
> 
> This is also necessary for the next patch which moves even more decoder
> state into the central raw decoding structs.
> 
> Signed-off-by: David Härdeman <david@hardeman.nu>

> +	if (!strncasecmp(tmp, "unknown", 7)) {
> +		tmp += 7;
> +		mask = IR_TYPE_UNKNOWN;
> +	} else if (!strncasecmp(tmp, "rc5", 3)) {
> +		tmp += 3;
> +		mask = IR_TYPE_RC5;
> +	} else if (!strncasecmp(tmp, "nec", 3)) {
> +		tmp += 3;
> +		mask = IR_TYPE_NEC;
> +	} else if (!strncasecmp(tmp, "rc6", 3)) {
> +		tmp += 3;
> +		mask = IR_TYPE_RC6;
> +	} else if (!strncasecmp(tmp, "jvc", 3)) {
> +		tmp += 3;
> +		mask = IR_TYPE_JVC;
> +	} else if (!strncasecmp(tmp, "sony", 4)) {
> +		tmp += 4;
> +		mask = IR_TYPE_SONY;
> +	} else {
>  		IR_dprintk(1, "Unknown protocol\n");
>  		return -EINVAL;
>  	}

Those "magic" sizes at the strcasecmp are ugly. Also, you didn't send
a patch for the ir-keytable userspace tool (part of v4l-utils.git tree).

Also, this allows some undocumented (and problematic) ways to set protocols. 
For example, if someone writes: something like "rc5necjvcsony"
(on this exact order and without spaces), it will accept it as just "sony",
without complaining.

As I found some time during the weekend to add support for the new /protocols way at
the userspace tool, I'll be applying this patch as-is, plus a few patches after it 
fixing the problems I found and adding some additional features that will be needed,
in order to make userspace stuff easier, like supporting multiple protocols specs
without needing to close/open the sysfs node for each protocol.

Btw, we should start writing some docs about the sysfs interface at the media
specs (at Documentation/DocBoook).

Cheers,
Mauro

^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] ir-core: centralize sysfs raw decoder enabling/disabling
@ 2010-06-28 16:56     ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2010-06-28 16:56 UTC (permalink / raw)
  To: David Härdeman; +Cc: jarod, jarod, linux-media, linux-input

Em 13-06-2010 17:29, David Härdeman escreveu:
> With the current logic, each raw decoder needs to add a copy of the exact
> same sysfs code. This is both unnecessary and also means that (re)loading
> an IR driver after raw decoder modules have been loaded won't work as
> expected.
> 
> This patch moves that logic into ir-raw-event and adds a single sysfs
> file per device.
> 
> Reading that file returns something like:
> 
> 	"rc5 [rc6] nec jvc [sony]"
> 
> (with enabled protocols in [] brackets)
> 
> Writing either "+protocol" or "-protocol" to that file will
> enable or disable the according protocol decoder.
> 
> An additional benefit is that the disabling of a decoder will be
> remembered across module removal/insertion so a previously
> disabled decoder won't suddenly be activated again. The default
> setting is to enable all decoders.
> 
> This is also necessary for the next patch which moves even more decoder
> state into the central raw decoding structs.
> 
> Signed-off-by: David Härdeman <david@hardeman.nu>

> +	if (!strncasecmp(tmp, "unknown", 7)) {
> +		tmp += 7;
> +		mask = IR_TYPE_UNKNOWN;
> +	} else if (!strncasecmp(tmp, "rc5", 3)) {
> +		tmp += 3;
> +		mask = IR_TYPE_RC5;
> +	} else if (!strncasecmp(tmp, "nec", 3)) {
> +		tmp += 3;
> +		mask = IR_TYPE_NEC;
> +	} else if (!strncasecmp(tmp, "rc6", 3)) {
> +		tmp += 3;
> +		mask = IR_TYPE_RC6;
> +	} else if (!strncasecmp(tmp, "jvc", 3)) {
> +		tmp += 3;
> +		mask = IR_TYPE_JVC;
> +	} else if (!strncasecmp(tmp, "sony", 4)) {
> +		tmp += 4;
> +		mask = IR_TYPE_SONY;
> +	} else {
>  		IR_dprintk(1, "Unknown protocol\n");
>  		return -EINVAL;
>  	}

Those "magic" sizes at the strcasecmp are ugly. Also, you didn't send
a patch for the ir-keytable userspace tool (part of v4l-utils.git tree).

Also, this allows some undocumented (and problematic) ways to set protocols. 
For example, if someone writes: something like "rc5necjvcsony"
(on this exact order and without spaces), it will accept it as just "sony",
without complaining.

As I found some time during the weekend to add support for the new /protocols way at
the userspace tool, I'll be applying this patch as-is, plus a few patches after it 
fixing the problems I found and adding some additional features that will be needed,
in order to make userspace stuff easier, like supporting multiple protocols specs
without needing to close/open the sysfs node for each protocol.

Btw, we should start writing some docs about the sysfs interface at the media
specs (at Documentation/DocBoook).

Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] ir-core: centralize sysfs raw decoder enabling/disabling
  2010-06-13 20:29 ` [PATCH 1/2] ir-core: centralize sysfs raw decoder enabling/disabling David Härdeman
  2010-06-16 20:05     ` Jarod Wilson
  2010-06-28 16:56     ` Mauro Carvalho Chehab
@ 2010-09-08 14:04   ` Brian Rogers
  2010-09-08 14:16     ` Jarod Wilson
  2 siblings, 1 reply; 21+ messages in thread
From: Brian Rogers @ 2010-09-08 14:04 UTC (permalink / raw)
  To: David Härdeman; +Cc: jarod, jarod, linux-media, mchehab, linux-input

  On 06/13/2010 01:29 PM, David Härdeman wrote:
> diff --git a/drivers/media/IR/ir-sysfs.c b/drivers/media/IR/ir-sysfs.c
> index daf33c1..7ae5662 100644
> --- a/drivers/media/IR/ir-sysfs.c
> +++ b/drivers/media/IR/ir-sysfs.c
> @@ -33,122 +33,178 @@ static struct class ir_input_class = {
>   };
>
>   /**
> - * show_protocol() - shows the current IR protocol
> + * show_protocols() - shows the current IR protocol(s)
>    * @d:		the device descriptor
>    * @mattr:	the device attribute struct (unused)
>    * @buf:	a pointer to the output buffer
>    *
> - * This routine is a callback routine for input read the IR protocol type.
> - * it is trigged by reading /sys/class/rc/rc?/current_protocol.
> - * It returns the protocol name, as understood by the driver.
> + * This routine is a callback routine for input read the IR protocol type(s).
> + * it is trigged by reading /sys/class/rc/rc?/protocols.
> + * It returns the protocol names of supported protocols.
> + * Enabled protocols are printed in brackets.
>    */
> -static ssize_t show_protocol(struct device *d,
> -			     struct device_attribute *mattr, char *buf)
> +static ssize_t show_protocols(struct device *d,
> +			      struct device_attribute *mattr, char *buf)
>   {
> -	char *s;
>   	struct ir_input_dev *ir_dev = dev_get_drvdata(d);
> -	u64 ir_type = ir_dev->rc_tab.ir_type;
> -
> -	IR_dprintk(1, "Current protocol is %lld\n", (long long)ir_type);
> -
> -	/* FIXME: doesn't support multiple protocols at the same time */
> -	if (ir_type == IR_TYPE_UNKNOWN)
> -		s = "Unknown";
> -	else if (ir_type == IR_TYPE_RC5)
> -		s = "rc-5";
> -	else if (ir_type == IR_TYPE_NEC)
> -		s = "nec";
> -	else if (ir_type == IR_TYPE_RC6)
> -		s = "rc6";
> -	else if (ir_type == IR_TYPE_JVC)
> -		s = "jvc";
> -	else if (ir_type == IR_TYPE_SONY)
> -		s = "sony";
> -	else
> -		s = "other";
> +	u64 allowed, enabled;
> +	char *tmp = buf;
> +
> +	if (ir_dev->props->driver_type == RC_DRIVER_SCANCODE) {

This change introduced an oops for me. On my saa7134 MSI TV Anywhere 
Plus, ir_dev->props is null, so dereferencing it causes the following 
while attempting to read /sys/devices/virtual/rc/rc0/protocols:

[  601.632041] BUG: unable to handle kernel NULL pointer dereference at 
(null)
[  601.632061] IP: [<ffffffffa00f43b5>] show_protocols+0x25/0x120 [ir_core]
[  601.632079] PGD 7b181067 PUD 7bafe067 PMD 0
[  601.632093] Oops: 0000 [#1] SMP
[  601.632103] last sysfs file: /sys/devices/virtual/rc/rc0/protocols
[  601.632111] CPU 0
[  601.632115] Modules linked in: binfmt_misc rfcomm parport_pc ppdev 
sco bnep l2cap saa7134_alsa arc4 rc_msi_tvanywhere_plus ir_kbd_i2c 
advantechwdt tda827x rt2500pci rt2x00pci rt2x00lib snd_intel8x0 tda8290 
snd_ac97_codec led_class ac97_bus tuner snd_seq_midi snd_rawmidi 
ir_lirc_codec lirc_dev mac80211 cfg80211 snd_seq_midi_event 
ir_sony_decoder ir_jvc_decoder saa7134 v4l2_common videodev v4l1_compat 
v4l2_compat_ioctl32 psmouse serio_raw ir_rc6_decoder snd_pcm snd_seq lp 
shpchp eeprom_93cx6 btusb snd_timer snd_seq_device videobuf_dma_sg 
ir_rc5_decoder snd videobuf_core ir_nec_decoder ir_common ir_core 
tveeprom soundcore edac_core bluetooth snd_page_alloc parport 
i2c_nforce2 k8temp edac_mce_amd usbhid hid btrfs zlib_deflate crc32c 
libcrc32c sata_nv forcedeth pata_amd
[  601.632319]
[  601.632335] Pid: 2928, comm: cat Not tainted 
2.6.36-rc3-00185-gd56557a #2 KN9 Series(NF-CK804)/Unknow
[  601.632368] RIP: 0010:[<ffffffffa00f43b5>]  [<ffffffffa00f43b5>] 
show_protocols+0x25/0x120 [ir_core]
[  601.632404] RSP: 0018:ffff88007bb93e38  EFLAGS: 00010282
[  601.632423] RAX: ffff88007bb6e000 RBX: ffffffffa00f5ee0 RCX: 
ffffffffa00f4390
[  601.632444] RDX: 0000000000000000 RSI: ffffffffa00f5ee0 RDI: 
ffff88007bb6e000
[  601.632465] RBP: ffff88007bb93e68 R08: ffff88007bb6e010 R09: 
ffffffff8164ab40
[  601.632486] R10: 0000000000000000 R11: 0000000000000246 R12: 
ffff88007c928000
[  601.632507] R13: 0000000000008000 R14: 000000000246d000 R15: 
ffff88007c8798a0
[  601.632529] FS:  00007fe7aa26c700(0000) GS:ffff880001e00000(0000) 
knlGS:0000000000000000
[  601.632561] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  601.632580] CR2: 0000000000000000 CR3: 000000007b19f000 CR4: 
00000000000006f0
[  601.632601] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 
0000000000000000
[  601.632623] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 
0000000000000400
[  601.632644] Process cat (pid: 2928, threadinfo ffff88007bb92000, task 
ffff88007bcd2dc0)
[  601.632675] Stack:
[  601.632689]  0000000000000000 ffffffffa00f5ee0 ffff88007bb93f48 
0000000000008000
[  601.632713] <0> 000000000246d000 ffff88007c8798a0 ffff88007bb93e98 
ffffffff813816e7
[  601.632750] <0> ffff88007bb93e88 ffffffff8110681e ffff88007bb93e98 
ffff88007c8798c0
[  601.632797] Call Trace:
[  601.632819]  [<ffffffff813816e7>] dev_attr_show+0x27/0x50
[  601.632842]  [<ffffffff8110681e>] ? __get_free_pages+0xe/0x50
[  601.632864]  [<ffffffff811bf5b1>] sysfs_read_file+0xd1/0x1c0
[  601.632886]  [<ffffffff81152f15>] vfs_read+0xc5/0x190
[  601.632906]  [<ffffffff81153551>] sys_read+0x51/0x90
[  601.632928]  [<ffffffff8100b072>] system_call_fastpath+0x16/0x1b
[  601.632947] Code: eb 04 90 90 90 90 55 48 89 e5 41 57 41 56 41 55 41 
54 53 48 83 ec 08 0f 1f 44 00 00 49 89 d4 e8 12 0e 29 e1 48 8b 90 90 02 
00 00 <44> 8b 0a 45 85 c9 0f 85 af 00 00 00 4c 8b a8 70 02 00 00 48 8b
[  601.633112] RIP  [<ffffffffa00f43b5>] show_protocols+0x25/0x120 [ir_core]
[  601.633136]  RSP <ffff88007bb93e38>
[  601.633152] CR2: 0000000000000000
[  601.633448] ---[ end trace 0dbd0f2ee839a90b ]---

A similar problem exists in store_protocols and makes lircd oops and die 
at startup.

> +		enabled = ir_dev->rc_tab.ir_type;
> +		allowed = ir_dev->props->allowed_protos;
> +	} else {
> +		enabled = ir_dev->raw->enabled_protocols;

ir_dev->raw is also null. If I check these pointers before using them, 
and bail out if both are null, then I get a working lircd, but of course 
the file /sys/devices/virtual/rc/rc0/protocols no longer does anything. 
On 2.6.35.4, the system never creates the 
/sys/class/rc/rc0/current_protocol file. Is it the case that the 
'protocols' file should never appear, because my card can't support this 
feature?

Brian


^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] ir-core: centralize sysfs raw decoder enabling/disabling
  2010-09-08 14:04   ` Brian Rogers
@ 2010-09-08 14:16     ` Jarod Wilson
  2010-09-08 21:22         ` David Härdeman
  2010-09-15 12:57       ` [PATCH] ir-core: Fix null dereferences in the protocols sysfs interface Brian Rogers
  0 siblings, 2 replies; 21+ messages in thread
From: Jarod Wilson @ 2010-09-08 14:16 UTC (permalink / raw)
  To: Brian Rogers
  Cc: David Härdeman, jarod, linux-media, mchehab, linux-input

On Wed, Sep 08, 2010 at 07:04:03AM -0700, Brian Rogers wrote:
>  On 06/13/2010 01:29 PM, David Härdeman wrote:
> >diff --git a/drivers/media/IR/ir-sysfs.c b/drivers/media/IR/ir-sysfs.c
> >index daf33c1..7ae5662 100644
> >--- a/drivers/media/IR/ir-sysfs.c
> >+++ b/drivers/media/IR/ir-sysfs.c
> >@@ -33,122 +33,178 @@ static struct class ir_input_class = {
> >  };
> >
> >  /**
> >- * show_protocol() - shows the current IR protocol
> >+ * show_protocols() - shows the current IR protocol(s)
> >   * @d:		the device descriptor
> >   * @mattr:	the device attribute struct (unused)
> >   * @buf:	a pointer to the output buffer
> >   *
> >- * This routine is a callback routine for input read the IR protocol type.
> >- * it is trigged by reading /sys/class/rc/rc?/current_protocol.
> >- * It returns the protocol name, as understood by the driver.
> >+ * This routine is a callback routine for input read the IR protocol type(s).
> >+ * it is trigged by reading /sys/class/rc/rc?/protocols.
> >+ * It returns the protocol names of supported protocols.
> >+ * Enabled protocols are printed in brackets.
> >   */
> >-static ssize_t show_protocol(struct device *d,
> >-			     struct device_attribute *mattr, char *buf)
> >+static ssize_t show_protocols(struct device *d,
> >+			      struct device_attribute *mattr, char *buf)
> >  {
> >-	char *s;
> >  	struct ir_input_dev *ir_dev = dev_get_drvdata(d);
> >-	u64 ir_type = ir_dev->rc_tab.ir_type;
> >-
> >-	IR_dprintk(1, "Current protocol is %lld\n", (long long)ir_type);
> >-
> >-	/* FIXME: doesn't support multiple protocols at the same time */
> >-	if (ir_type == IR_TYPE_UNKNOWN)
> >-		s = "Unknown";
> >-	else if (ir_type == IR_TYPE_RC5)
> >-		s = "rc-5";
> >-	else if (ir_type == IR_TYPE_NEC)
> >-		s = "nec";
> >-	else if (ir_type == IR_TYPE_RC6)
> >-		s = "rc6";
> >-	else if (ir_type == IR_TYPE_JVC)
> >-		s = "jvc";
> >-	else if (ir_type == IR_TYPE_SONY)
> >-		s = "sony";
> >-	else
> >-		s = "other";
> >+	u64 allowed, enabled;
> >+	char *tmp = buf;
> >+
> >+	if (ir_dev->props->driver_type == RC_DRIVER_SCANCODE) {
> 
> This change introduced an oops for me. On my saa7134 MSI TV Anywhere
> Plus, ir_dev->props is null, so dereferencing it causes the
> following while attempting to read
> /sys/devices/virtual/rc/rc0/protocols:
> 
> [  601.632041] BUG: unable to handle kernel NULL pointer dereference
> at (null)
> [  601.632061] IP: [<ffffffffa00f43b5>] show_protocols+0x25/0x120 [ir_core]
> [  601.632079] PGD 7b181067 PUD 7bafe067 PMD 0
> [  601.632093] Oops: 0000 [#1] SMP
> [  601.632103] last sysfs file: /sys/devices/virtual/rc/rc0/protocols
> [  601.632111] CPU 0
> [  601.632115] Modules linked in: binfmt_misc rfcomm parport_pc
> ppdev sco bnep l2cap saa7134_alsa arc4 rc_msi_tvanywhere_plus
> ir_kbd_i2c advantechwdt tda827x rt2500pci rt2x00pci rt2x00lib
> snd_intel8x0 tda8290 snd_ac97_codec led_class ac97_bus tuner
> snd_seq_midi snd_rawmidi ir_lirc_codec lirc_dev mac80211 cfg80211
> snd_seq_midi_event ir_sony_decoder ir_jvc_decoder saa7134
> v4l2_common videodev v4l1_compat v4l2_compat_ioctl32 psmouse
> serio_raw ir_rc6_decoder snd_pcm snd_seq lp shpchp eeprom_93cx6
> btusb snd_timer snd_seq_device videobuf_dma_sg ir_rc5_decoder snd
> videobuf_core ir_nec_decoder ir_common ir_core tveeprom soundcore
> edac_core bluetooth snd_page_alloc parport i2c_nforce2 k8temp
> edac_mce_amd usbhid hid btrfs zlib_deflate crc32c libcrc32c sata_nv
> forcedeth pata_amd
> [  601.632319]
> [  601.632335] Pid: 2928, comm: cat Not tainted
> 2.6.36-rc3-00185-gd56557a #2 KN9 Series(NF-CK804)/Unknow
> [  601.632368] RIP: 0010:[<ffffffffa00f43b5>]  [<ffffffffa00f43b5>]
> show_protocols+0x25/0x120 [ir_core]
> [  601.632404] RSP: 0018:ffff88007bb93e38  EFLAGS: 00010282
> [  601.632423] RAX: ffff88007bb6e000 RBX: ffffffffa00f5ee0 RCX:
> ffffffffa00f4390
> [  601.632444] RDX: 0000000000000000 RSI: ffffffffa00f5ee0 RDI:
> ffff88007bb6e000
> [  601.632465] RBP: ffff88007bb93e68 R08: ffff88007bb6e010 R09:
> ffffffff8164ab40
> [  601.632486] R10: 0000000000000000 R11: 0000000000000246 R12:
> ffff88007c928000
> [  601.632507] R13: 0000000000008000 R14: 000000000246d000 R15:
> ffff88007c8798a0
> [  601.632529] FS:  00007fe7aa26c700(0000) GS:ffff880001e00000(0000)
> knlGS:0000000000000000
> [  601.632561] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  601.632580] CR2: 0000000000000000 CR3: 000000007b19f000 CR4:
> 00000000000006f0
> [  601.632601] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [  601.632623] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
> 0000000000000400
> [  601.632644] Process cat (pid: 2928, threadinfo ffff88007bb92000,
> task ffff88007bcd2dc0)
> [  601.632675] Stack:
> [  601.632689]  0000000000000000 ffffffffa00f5ee0 ffff88007bb93f48
> 0000000000008000
> [  601.632713] <0> 000000000246d000 ffff88007c8798a0
> ffff88007bb93e98 ffffffff813816e7
> [  601.632750] <0> ffff88007bb93e88 ffffffff8110681e
> ffff88007bb93e98 ffff88007c8798c0
> [  601.632797] Call Trace:
> [  601.632819]  [<ffffffff813816e7>] dev_attr_show+0x27/0x50
> [  601.632842]  [<ffffffff8110681e>] ? __get_free_pages+0xe/0x50
> [  601.632864]  [<ffffffff811bf5b1>] sysfs_read_file+0xd1/0x1c0
> [  601.632886]  [<ffffffff81152f15>] vfs_read+0xc5/0x190
> [  601.632906]  [<ffffffff81153551>] sys_read+0x51/0x90
> [  601.632928]  [<ffffffff8100b072>] system_call_fastpath+0x16/0x1b
> [  601.632947] Code: eb 04 90 90 90 90 55 48 89 e5 41 57 41 56 41 55
> 41 54 53 48 83 ec 08 0f 1f 44 00 00 49 89 d4 e8 12 0e 29 e1 48 8b 90
> 90 02 00 00 <44> 8b 0a 45 85 c9 0f 85 af 00 00 00 4c 8b a8 70 02 00
> 00 48 8b
> [  601.633112] RIP  [<ffffffffa00f43b5>] show_protocols+0x25/0x120 [ir_core]
> [  601.633136]  RSP <ffff88007bb93e38>
> [  601.633152] CR2: 0000000000000000
> [  601.633448] ---[ end trace 0dbd0f2ee839a90b ]---
> 
> A similar problem exists in store_protocols and makes lircd oops and
> die at startup.
> 
> >+		enabled = ir_dev->rc_tab.ir_type;
> >+		allowed = ir_dev->props->allowed_protos;
> >+	} else {
> >+		enabled = ir_dev->raw->enabled_protocols;
> 
> ir_dev->raw is also null. If I check these pointers before using
> them, and bail out if both are null, then I get a working lircd, but
> of course the file /sys/devices/virtual/rc/rc0/protocols no longer
> does anything. On 2.6.35.4, the system never creates the
> /sys/class/rc/rc0/current_protocol file. Is it the case that the
> 'protocols' file should never appear, because my card can't support
> this feature?

Hm... So protocols is indeed intended for hardware that handles raw IR, as
its a list of raw IR decoders available/enabled/disabled for the receiver.
But some devices that do onboard decoding and deal with scancodes still
need to support changing protocols, as they can be told "decode rc5" or
"decode nec", etc... My memory is currently foggy on how it was exactly
that it was supposed to be donee though. :) (Yet another reason I really
need to poke at the imon driver code again).


-- 
Jarod Wilson
jarod@redhat.com


^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] ir-core: centralize sysfs raw decoder enabling/disabling
  2010-09-08 14:16     ` Jarod Wilson
@ 2010-09-08 21:22         ` David Härdeman
  2010-09-15 12:57       ` [PATCH] ir-core: Fix null dereferences in the protocols sysfs interface Brian Rogers
  1 sibling, 0 replies; 21+ messages in thread
From: David Härdeman @ 2010-09-08 21:22 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: Brian Rogers, jarod, linux-media, mchehab, linux-input

On Wed, Sep 08, 2010 at 10:16:13AM -0400, Jarod Wilson wrote:
> On Wed, Sep 08, 2010 at 07:04:03AM -0700, Brian Rogers wrote:
> > ir_dev->raw is also null. If I check these pointers before using
> > them, and bail out if both are null, then I get a working lircd, but
> > of course the file /sys/devices/virtual/rc/rc0/protocols no longer
> > does anything. On 2.6.35.4, the system never creates the
> > /sys/class/rc/rc0/current_protocol file. Is it the case that the
> > 'protocols' file should never appear, because my card can't support
> > this feature?
> 
> Hm... So protocols is indeed intended for hardware that handles raw IR, as
> its a list of raw IR decoders available/enabled/disabled for the receiver.
> But some devices that do onboard decoding and deal with scancodes still
> need to support changing protocols, as they can be told "decode rc5" or
> "decode nec", etc... My memory is currently foggy on how it was exactly
> that it was supposed to be donee though. :) (Yet another reason I really
> need to poke at the imon driver code again).

This, and a raft of similar bugreports was one of the reasons I wrote 
the rc_dev patch (which gets rid of ir_dev->props, the source of many 
oopses by now).

Hardware decoders should work with the same sysfs file, the driver 
should set ir_dev->props->change_protocol (current) or 
rc->change_protocol (future) and it'll get notified when userspace 
interacts with the sysfs file and the hardware can then react 
accordingly. So the answer is yes - all hardware should have the file.

-- 
David Härdeman

^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] ir-core: centralize sysfs raw decoder enabling/disabling
@ 2010-09-08 21:22         ` David Härdeman
  0 siblings, 0 replies; 21+ messages in thread
From: David Härdeman @ 2010-09-08 21:22 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: Brian Rogers, jarod, linux-media, mchehab, linux-input

On Wed, Sep 08, 2010 at 10:16:13AM -0400, Jarod Wilson wrote:
> On Wed, Sep 08, 2010 at 07:04:03AM -0700, Brian Rogers wrote:
> > ir_dev->raw is also null. If I check these pointers before using
> > them, and bail out if both are null, then I get a working lircd, but
> > of course the file /sys/devices/virtual/rc/rc0/protocols no longer
> > does anything. On 2.6.35.4, the system never creates the
> > /sys/class/rc/rc0/current_protocol file. Is it the case that the
> > 'protocols' file should never appear, because my card can't support
> > this feature?
> 
> Hm... So protocols is indeed intended for hardware that handles raw IR, as
> its a list of raw IR decoders available/enabled/disabled for the receiver.
> But some devices that do onboard decoding and deal with scancodes still
> need to support changing protocols, as they can be told "decode rc5" or
> "decode nec", etc... My memory is currently foggy on how it was exactly
> that it was supposed to be donee though. :) (Yet another reason I really
> need to poke at the imon driver code again).

This, and a raft of similar bugreports was one of the reasons I wrote 
the rc_dev patch (which gets rid of ir_dev->props, the source of many 
oopses by now).

Hardware decoders should work with the same sysfs file, the driver 
should set ir_dev->props->change_protocol (current) or 
rc->change_protocol (future) and it'll get notified when userspace 
interacts with the sysfs file and the hardware can then react 
accordingly. So the answer is yes - all hardware should have the file.

-- 
David Härdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread
* [PATCH] ir-core: Fix null dereferences in the protocols sysfs interface
  2010-09-08 14:16     ` Jarod Wilson
  2010-09-08 21:22         ` David Härdeman
@ 2010-09-15 12:57       ` Brian Rogers
  2010-09-15 14:41         ` Jarod Wilson
  1 sibling, 1 reply; 21+ messages in thread
From: Brian Rogers @ 2010-09-15 12:57 UTC (permalink / raw)
  To: Jarod Wilson
  Cc: David Härdeman, jarod, linux-media, mchehab, linux-input

[-- Attachment #1: Type: text/plain, Size: 1128 bytes --]

  On 09/08/2010 07:16 AM, Jarod Wilson wrote:
> On Wed, Sep 08, 2010 at 07:04:03AM -0700, Brian Rogers wrote:
>> ir_dev->raw is also null. If I check these pointers before using
>> them, and bail out if both are null, then I get a working lircd, but
>> of course the file /sys/devices/virtual/rc/rc0/protocols no longer
>> does anything. On 2.6.35.4, the system never creates the
>> /sys/class/rc/rc0/current_protocol file. Is it the case that the
>> 'protocols' file should never appear, because my card can't support
>> this feature?
> Hm... So protocols is indeed intended for hardware that handles raw IR, as
> its a list of raw IR decoders available/enabled/disabled for the receiver.
> But some devices that do onboard decoding and deal with scancodes still
> need to support changing protocols, as they can be told "decode rc5" or
> "decode nec", etc... My memory is currently foggy on how it was exactly
> that it was supposed to be donee though. :) (Yet another reason I really
> need to poke at the imon driver code again).

How about the attached patch? Does this look like a reasonable solution 
for 2.6.36?

Brian


[-- Attachment #2: 0001-ir-core-Fix-null-dereferences-in-the-protocols-sysfs.patch --]
[-- Type: text/x-patch, Size: 2391 bytes --]

>From 7937051c5e2c8b5b0410172d48e62d54bd1906ee Mon Sep 17 00:00:00 2001
From: Brian Rogers <brian@xyzw.org>
Date: Wed, 8 Sep 2010 05:33:34 -0700
Subject: [PATCH] ir-core: Fix null dereferences in the protocols sysfs interface

For some cards, ir_dev->props and ir_dev->raw are both NULL. These cards are
using built-in IR decoding instead of raw, and can't easily be made to switch
protocols.

So upon reading /sys/class/rc/rc?/protocols on such a card, return 'builtin' as
the supported and enabled protocol. Return -EINVAL on any attempts to change
the protocol. And most important of all, don't crash.

Signed-off-by: Brian Rogers <brian@xyzw.org>
---
 drivers/media/IR/ir-sysfs.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/media/IR/ir-sysfs.c b/drivers/media/IR/ir-sysfs.c
index 96dafc4..46d4246 100644
--- a/drivers/media/IR/ir-sysfs.c
+++ b/drivers/media/IR/ir-sysfs.c
@@ -67,13 +67,14 @@ static ssize_t show_protocols(struct device *d,
 	char *tmp = buf;
 	int i;
 
-	if (ir_dev->props->driver_type == RC_DRIVER_SCANCODE) {
+	if (ir_dev->props && ir_dev->props->driver_type == RC_DRIVER_SCANCODE) {
 		enabled = ir_dev->rc_tab.ir_type;
 		allowed = ir_dev->props->allowed_protos;
-	} else {
+	} else if (ir_dev->raw) {
 		enabled = ir_dev->raw->enabled_protocols;
 		allowed = ir_raw_get_allowed_protocols();
-	}
+	} else
+		return sprintf(tmp, "[builtin]\n");
 
 	IR_dprintk(1, "allowed - 0x%llx, enabled - 0x%llx\n",
 		   (long long)allowed,
@@ -121,10 +122,14 @@ static ssize_t store_protocols(struct device *d,
 	int rc, i, count = 0;
 	unsigned long flags;
 
-	if (ir_dev->props->driver_type == RC_DRIVER_SCANCODE)
+	if (ir_dev->props && ir_dev->props->driver_type == RC_DRIVER_SCANCODE)
 		type = ir_dev->rc_tab.ir_type;
-	else
+	else if (ir_dev->raw)
 		type = ir_dev->raw->enabled_protocols;
+	else {
+		IR_dprintk(1, "Protocol switching not supported\n");
+		return -EINVAL;
+	}
 
 	while ((tmp = strsep((char **) &data, " \n")) != NULL) {
 		if (!*tmp)
@@ -185,7 +190,7 @@ static ssize_t store_protocols(struct device *d,
 		}
 	}
 
-	if (ir_dev->props->driver_type == RC_DRIVER_SCANCODE) {
+	if (ir_dev->props && ir_dev->props->driver_type == RC_DRIVER_SCANCODE) {
 		spin_lock_irqsave(&ir_dev->rc_tab.lock, flags);
 		ir_dev->rc_tab.ir_type = type;
 		spin_unlock_irqrestore(&ir_dev->rc_tab.lock, flags);
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread
* Re: [PATCH] ir-core: Fix null dereferences in the protocols sysfs interface
  2010-09-15 12:57       ` [PATCH] ir-core: Fix null dereferences in the protocols sysfs interface Brian Rogers
@ 2010-09-15 14:41         ` Jarod Wilson
  0 siblings, 0 replies; 21+ messages in thread
From: Jarod Wilson @ 2010-09-15 14:41 UTC (permalink / raw)
  To: Brian Rogers
  Cc: David Härdeman, jarod, linux-media, mchehab, linux-input

On Wed, Sep 15, 2010 at 05:57:10AM -0700, Brian Rogers wrote:
>  On 09/08/2010 07:16 AM, Jarod Wilson wrote:
> >On Wed, Sep 08, 2010 at 07:04:03AM -0700, Brian Rogers wrote:
> >>ir_dev->raw is also null. If I check these pointers before using
> >>them, and bail out if both are null, then I get a working lircd, but
> >>of course the file /sys/devices/virtual/rc/rc0/protocols no longer
> >>does anything. On 2.6.35.4, the system never creates the
> >>/sys/class/rc/rc0/current_protocol file. Is it the case that the
> >>'protocols' file should never appear, because my card can't support
> >>this feature?
> >Hm... So protocols is indeed intended for hardware that handles raw IR, as
> >its a list of raw IR decoders available/enabled/disabled for the receiver.
> >But some devices that do onboard decoding and deal with scancodes still
> >need to support changing protocols, as they can be told "decode rc5" or
> >"decode nec", etc... My memory is currently foggy on how it was exactly
> >that it was supposed to be donee though. :) (Yet another reason I really
> >need to poke at the imon driver code again).
> 
> How about the attached patch? Does this look like a reasonable
> solution for 2.6.36?
> 
> Brian
> 

> From 7937051c5e2c8b5b0410172d48e62d54bd1906ee Mon Sep 17 00:00:00 2001
> From: Brian Rogers <brian@xyzw.org>
> Date: Wed, 8 Sep 2010 05:33:34 -0700
> Subject: [PATCH] ir-core: Fix null dereferences in the protocols sysfs interface
> 
> For some cards, ir_dev->props and ir_dev->raw are both NULL. These cards are
> using built-in IR decoding instead of raw, and can't easily be made to switch
> protocols.
> 
> So upon reading /sys/class/rc/rc?/protocols on such a card, return 'builtin' as
> the supported and enabled protocol. Return -EINVAL on any attempts to change
> the protocol. And most important of all, don't crash.
> 
> Signed-off-by: Brian Rogers <brian@xyzw.org>
> ---
>  drivers/media/IR/ir-sysfs.c |   17 +++++++++++------
>  1 files changed, 11 insertions(+), 6 deletions(-)

Yeah, this looks pretty sane for 2.6.36, would just be a short-lived panic
preventer until David's interface changes get merged after 2.6.37-rc1.

Acked-by: Jarod Wilson <jarod@redhat.com>


-- 
Jarod Wilson
jarod@redhat.com


^ permalink raw reply	[flat|nested] 21+ messages in thread
end of thread, other threads:[~2010-09-15 14:41 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-13 20:29 [PATCH 0/2] ir-core: raw decoder framework changes David Härdeman
2010-06-13 20:29 ` David Härdeman
2010-06-13 20:29 ` [PATCH 1/2] ir-core: centralize sysfs raw decoder enabling/disabling David Härdeman
2010-06-16 20:05   ` Jarod Wilson
2010-06-16 20:05     ` Jarod Wilson
2010-06-16 20:39     ` Jarod Wilson
2010-06-16 20:39       ` Jarod Wilson
2010-06-28 16:56   ` Mauro Carvalho Chehab
2010-06-28 16:56     ` Mauro Carvalho Chehab
2010-09-08 14:04   ` Brian Rogers
2010-09-08 14:16     ` Jarod Wilson
2010-09-08 21:22       ` David Härdeman
2010-09-08 21:22         ` David Härdeman
2010-09-15 12:57       ` [PATCH] ir-core: Fix null dereferences in the protocols sysfs interface Brian Rogers
2010-09-15 14:41         ` Jarod Wilson
2010-06-13 20:29 ` [PATCH 2/2] ir-core: move decoding state to ir_raw_event_ctrl David Härdeman
2010-06-13 20:29   ` David Härdeman
2010-06-16 20:06   ` Jarod Wilson
2010-06-16 20:06     ` Jarod Wilson
2010-06-16 20:39     ` Jarod Wilson
2010-06-16 20:39       ` Jarod Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.