All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] 3.15 fixes
@ 2014-04-04 22:05 David Härdeman
  2014-04-04 22:05 ` [PATCH 1/3] rc-core: do not change 32bit NEC scancode format for now David Härdeman
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: David Härdeman @ 2014-04-04 22:05 UTC (permalink / raw)
  To: linux-media; +Cc: james.hogan, m.chehab

The following three patches are the quickfixes that we (Mauro, James, me)
seemed to agree to. I've updated the patches to apply cleanly without 
the preceeding patches from my previous patchbomb and I've hopefully fixed
the comments that James provided (I'll let him reply with separate Acked-by
lines if he agrees to each patch).

---

David Härdeman (3):
      rc-core: do not change 32bit NEC scancode format for now
      rc-core: split dev->s_filter
      rc-core: remove generic scancode filter


 drivers/media/rc/img-ir/img-ir-hw.c  |   15 +++++
 drivers/media/rc/img-ir/img-ir-nec.c |   27 +++++----
 drivers/media/rc/ir-nec-decoder.c    |    5 --
 drivers/media/rc/keymaps/rc-tivo.c   |   86 +++++++++++++++---------------
 drivers/media/rc/rc-main.c           |   98 ++++++++++++++++++++++------------
 include/media/rc-core.h              |    8 ++-
 6 files changed, 142 insertions(+), 97 deletions(-)

--
David Härdeman

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] rc-core: do not change 32bit NEC scancode format for now
  2014-04-04 22:05 [PATCH 0/3] 3.15 fixes David Härdeman
@ 2014-04-04 22:05 ` David Härdeman
  2014-04-04 23:34   ` James Hogan
  2014-04-04 22:06 ` [PATCH 2/3] rc-core: split dev->s_filter David Härdeman
  2014-04-04 22:06 ` [PATCH 3/3] rc-core: remove generic scancode filter David Härdeman
  2 siblings, 1 reply; 7+ messages in thread
From: David Härdeman @ 2014-04-04 22:05 UTC (permalink / raw)
  To: linux-media; +Cc: james.hogan, m.chehab

This reverts 18bc17448147e93f31cc9b1a83be49f1224657b2

The patch ignores the fact that NEC32 scancodes are generated not only in the
NEC raw decoder but also directly in some drivers. Whichever approach is chosen
it should be consistent across drivers and this patch needs more discussion.

Furthermore, I'm convinced that we have to stop playing games trying to
decipher the "meaning" of NEC scancodes (what's the customer/vendor/address,
which byte is the MSB, etc).

This patch is in preparation for the next few patches in this series.

v2: make sure img-ir scancodes are bitrev8():ed as well

v3: update comments

Signed-off-by: David Härdeman <david@hardeman.nu>
---
 drivers/media/rc/img-ir/img-ir-nec.c |   27 ++++++-----
 drivers/media/rc/ir-nec-decoder.c    |    5 --
 drivers/media/rc/keymaps/rc-tivo.c   |   86 +++++++++++++++++-----------------
 3 files changed, 59 insertions(+), 59 deletions(-)

diff --git a/drivers/media/rc/img-ir/img-ir-nec.c b/drivers/media/rc/img-ir/img-ir-nec.c
index e7a731b..751d9d9 100644
--- a/drivers/media/rc/img-ir/img-ir-nec.c
+++ b/drivers/media/rc/img-ir/img-ir-nec.c
@@ -5,6 +5,7 @@
  */
 
 #include "img-ir-hw.h"
+#include <linux/bitrev.h>
 
 /* Convert NEC data to a scancode */
 static int img_ir_nec_scancode(int len, u64 raw, int *scancode, u64 protocols)
@@ -22,11 +23,11 @@ static int img_ir_nec_scancode(int len, u64 raw, int *scancode, u64 protocols)
 	data_inv = (raw >> 24) & 0xff;
 	if ((data_inv ^ data) != 0xff) {
 		/* 32-bit NEC (used by Apple and TiVo remotes) */
-		/* scan encoding: aaAAddDD */
-		*scancode = addr_inv << 24 |
-			    addr     << 16 |
-			    data_inv <<  8 |
-			    data;
+		/* scan encoding: as transmitted, MSBit = first received bit */
+		*scancode = bitrev8(addr)     << 24 |
+			    bitrev8(addr_inv) << 16 |
+			    bitrev8(data)     <<  8 |
+			    bitrev8(data_inv);
 	} else if ((addr_inv ^ addr) != 0xff) {
 		/* Extended NEC */
 		/* scan encoding: AAaaDD */
@@ -54,13 +55,15 @@ static int img_ir_nec_filter(const struct rc_scancode_filter *in,
 
 	if ((in->data | in->mask) & 0xff000000) {
 		/* 32-bit NEC (used by Apple and TiVo remotes) */
-		/* scan encoding: aaAAddDD */
-		addr_inv   = (in->data >> 24) & 0xff;
-		addr_inv_m = (in->mask >> 24) & 0xff;
-		addr       = (in->data >> 16) & 0xff;
-		addr_m     = (in->mask >> 16) & 0xff;
-		data_inv   = (in->data >>  8) & 0xff;
-		data_inv_m = (in->mask >>  8) & 0xff;
+		/* scan encoding: as transmitted, MSBit = first received bit */
+		addr       = bitrev8(in->data >> 24);
+		addr_m     = bitrev8(in->mask >> 24);
+		addr_inv   = bitrev8(in->data >> 16);
+		addr_inv_m = bitrev8(in->mask >> 16);
+		data       = bitrev8(in->data >>  8);
+		data_m     = bitrev8(in->mask >>  8);
+		data_inv   = bitrev8(in->data >>  0);
+		data_inv_m = bitrev8(in->mask >>  0);
 	} else if ((in->data | in->mask) & 0x00ff0000) {
 		/* Extended NEC */
 		/* scan encoding AAaaDD */
diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c
index 9de1791..35c42e5 100644
--- a/drivers/media/rc/ir-nec-decoder.c
+++ b/drivers/media/rc/ir-nec-decoder.c
@@ -172,10 +172,7 @@ static int ir_nec_decode(struct rc_dev *dev, struct ir_raw_event ev)
 		if (send_32bits) {
 			/* NEC transport, but modified protocol, used by at
 			 * least Apple and TiVo remotes */
-			scancode = not_address << 24 |
-				   address     << 16 |
-				   not_command <<  8 |
-				   command;
+			scancode = data->bits;
 			IR_dprintk(1, "NEC (modified) scancode 0x%08x\n", scancode);
 		} else if ((address ^ not_address) != 0xff) {
 			/* Extended NEC */
diff --git a/drivers/media/rc/keymaps/rc-tivo.c b/drivers/media/rc/keymaps/rc-tivo.c
index 5cc1b45..454e062 100644
--- a/drivers/media/rc/keymaps/rc-tivo.c
+++ b/drivers/media/rc/keymaps/rc-tivo.c
@@ -15,62 +15,62 @@
  * Initial mapping is for the TiVo remote included in the Nero LiquidTV bundle,
  * which also ships with a TiVo-branded IR transceiver, supported by the mceusb
  * driver. Note that the remote uses an NEC-ish protocol, but instead of having
- * a command/not_command pair, it has a vendor ID of 0x3085, but some keys, the
+ * a command/not_command pair, it has a vendor ID of 0xa10c, but some keys, the
  * NEC extended checksums do pass, so the table presently has the intended
  * values and the checksum-passed versions for those keys.
  */
 static struct rc_map_table tivo[] = {
-	{ 0x3085f009, KEY_MEDIA },	/* TiVo Button */
-	{ 0x3085e010, KEY_POWER2 },	/* TV Power */
-	{ 0x3085e011, KEY_TV },		/* Live TV/Swap */
-	{ 0x3085c034, KEY_VIDEO_NEXT },	/* TV Input */
-	{ 0x3085e013, KEY_INFO },
-	{ 0x3085a05f, KEY_CYCLEWINDOWS }, /* Window */
+	{ 0xa10c900f, KEY_MEDIA },	/* TiVo Button */
+	{ 0xa10c0807, KEY_POWER2 },	/* TV Power */
+	{ 0xa10c8807, KEY_TV },		/* Live TV/Swap */
+	{ 0xa10c2c03, KEY_VIDEO_NEXT },	/* TV Input */
+	{ 0xa10cc807, KEY_INFO },
+	{ 0xa10cfa05, KEY_CYCLEWINDOWS }, /* Window */
 	{ 0x0085305f, KEY_CYCLEWINDOWS },
-	{ 0x3085c036, KEY_EPG },	/* Guide */
+	{ 0xa10c6c03, KEY_EPG },	/* Guide */
 
-	{ 0x3085e014, KEY_UP },
-	{ 0x3085e016, KEY_DOWN },
-	{ 0x3085e017, KEY_LEFT },
-	{ 0x3085e015, KEY_RIGHT },
+	{ 0xa10c2807, KEY_UP },
+	{ 0xa10c6807, KEY_DOWN },
+	{ 0xa10ce807, KEY_LEFT },
+	{ 0xa10ca807, KEY_RIGHT },
 
-	{ 0x3085e018, KEY_SCROLLDOWN },	/* Red Thumbs Down */
-	{ 0x3085e019, KEY_SELECT },
-	{ 0x3085e01a, KEY_SCROLLUP },	/* Green Thumbs Up */
+	{ 0xa10c1807, KEY_SCROLLDOWN },	/* Red Thumbs Down */
+	{ 0xa10c9807, KEY_SELECT },
+	{ 0xa10c5807, KEY_SCROLLUP },	/* Green Thumbs Up */
 
-	{ 0x3085e01c, KEY_VOLUMEUP },
-	{ 0x3085e01d, KEY_VOLUMEDOWN },
-	{ 0x3085e01b, KEY_MUTE },
-	{ 0x3085d020, KEY_RECORD },
-	{ 0x3085e01e, KEY_CHANNELUP },
-	{ 0x3085e01f, KEY_CHANNELDOWN },
+	{ 0xa10c3807, KEY_VOLUMEUP },
+	{ 0xa10cb807, KEY_VOLUMEDOWN },
+	{ 0xa10cd807, KEY_MUTE },
+	{ 0xa10c040b, KEY_RECORD },
+	{ 0xa10c7807, KEY_CHANNELUP },
+	{ 0xa10cf807, KEY_CHANNELDOWN },
 	{ 0x0085301f, KEY_CHANNELDOWN },
 
-	{ 0x3085d021, KEY_PLAY },
-	{ 0x3085d023, KEY_PAUSE },
-	{ 0x3085d025, KEY_SLOW },
-	{ 0x3085d022, KEY_REWIND },
-	{ 0x3085d024, KEY_FASTFORWARD },
-	{ 0x3085d026, KEY_PREVIOUS },
-	{ 0x3085d027, KEY_NEXT },	/* ->| */
+	{ 0xa10c840b, KEY_PLAY },
+	{ 0xa10cc40b, KEY_PAUSE },
+	{ 0xa10ca40b, KEY_SLOW },
+	{ 0xa10c440b, KEY_REWIND },
+	{ 0xa10c240b, KEY_FASTFORWARD },
+	{ 0xa10c640b, KEY_PREVIOUS },
+	{ 0xa10ce40b, KEY_NEXT },	/* ->| */
 
-	{ 0x3085b044, KEY_ZOOM },	/* Aspect */
-	{ 0x3085b048, KEY_STOP },
-	{ 0x3085b04a, KEY_DVD },	/* DVD Menu */
+	{ 0xa10c220d, KEY_ZOOM },	/* Aspect */
+	{ 0xa10c120d, KEY_STOP },
+	{ 0xa10c520d, KEY_DVD },	/* DVD Menu */
 
-	{ 0x3085d028, KEY_NUMERIC_1 },
-	{ 0x3085d029, KEY_NUMERIC_2 },
-	{ 0x3085d02a, KEY_NUMERIC_3 },
-	{ 0x3085d02b, KEY_NUMERIC_4 },
-	{ 0x3085d02c, KEY_NUMERIC_5 },
-	{ 0x3085d02d, KEY_NUMERIC_6 },
-	{ 0x3085d02e, KEY_NUMERIC_7 },
-	{ 0x3085d02f, KEY_NUMERIC_8 },
+	{ 0xa10c140b, KEY_NUMERIC_1 },
+	{ 0xa10c940b, KEY_NUMERIC_2 },
+	{ 0xa10c540b, KEY_NUMERIC_3 },
+	{ 0xa10cd40b, KEY_NUMERIC_4 },
+	{ 0xa10c340b, KEY_NUMERIC_5 },
+	{ 0xa10cb40b, KEY_NUMERIC_6 },
+	{ 0xa10c740b, KEY_NUMERIC_7 },
+	{ 0xa10cf40b, KEY_NUMERIC_8 },
 	{ 0x0085302f, KEY_NUMERIC_8 },
-	{ 0x3085c030, KEY_NUMERIC_9 },
-	{ 0x3085c031, KEY_NUMERIC_0 },
-	{ 0x3085c033, KEY_ENTER },
-	{ 0x3085c032, KEY_CLEAR },
+	{ 0xa10c0c03, KEY_NUMERIC_9 },
+	{ 0xa10c8c03, KEY_NUMERIC_0 },
+	{ 0xa10ccc03, KEY_ENTER },
+	{ 0xa10c4c03, KEY_CLEAR },
 };
 
 static struct rc_map_list tivo_map = {


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] rc-core: split dev->s_filter
  2014-04-04 22:05 [PATCH 0/3] 3.15 fixes David Härdeman
  2014-04-04 22:05 ` [PATCH 1/3] rc-core: do not change 32bit NEC scancode format for now David Härdeman
@ 2014-04-04 22:06 ` David Härdeman
  2014-04-04 23:36   ` James Hogan
  2014-04-04 22:06 ` [PATCH 3/3] rc-core: remove generic scancode filter David Härdeman
  2 siblings, 1 reply; 7+ messages in thread
From: David Härdeman @ 2014-04-04 22:06 UTC (permalink / raw)
  To: linux-media; +Cc: james.hogan, m.chehab

Overloading dev->s_filter to do two different functions (set wakeup filters
and generic hardware filters) makes it impossible to tell what the
hardware actually supports, so create a separate dev->s_wakeup_filter and
make the distinction explicit.

v2: hopefully address James' comments on what should be moved from this to the
next patch.

Signed-off-by: David Härdeman <david@hardeman.nu>
---
 drivers/media/rc/img-ir/img-ir-hw.c |   15 ++++++++++++++-
 drivers/media/rc/rc-main.c          |   24 +++++++++++++++++-------
 include/media/rc-core.h             |    6 ++++--
 3 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/drivers/media/rc/img-ir/img-ir-hw.c b/drivers/media/rc/img-ir/img-ir-hw.c
index 579a52b..0127dd2 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -504,6 +504,18 @@ unlock:
 	return ret;
 }
 
+static int img_ir_set_normal_filter(struct rc_dev *dev,
+				    struct rc_scancode_filter *sc_filter)
+{
+	return img_ir_set_filter(dev, RC_FILTER_NORMAL, sc_filter); 
+}
+
+static int img_ir_set_wakeup_filter(struct rc_dev *dev,
+				    struct rc_scancode_filter *sc_filter)
+{
+	return img_ir_set_filter(dev, RC_FILTER_WAKEUP, sc_filter);
+}
+
 /**
  * img_ir_set_decoder() - Set the current decoder.
  * @priv:	IR private data.
@@ -986,7 +998,8 @@ int img_ir_probe_hw(struct img_ir_priv *priv)
 	rdev->map_name = RC_MAP_EMPTY;
 	rc_set_allowed_protocols(rdev, img_ir_allowed_protos(priv));
 	rdev->input_name = "IMG Infrared Decoder";
-	rdev->s_filter = img_ir_set_filter;
+	rdev->s_filter = img_ir_set_normal_filter;
+	rdev->s_wakeup_filter = img_ir_set_wakeup_filter;
 
 	/* Register hardware decoder */
 	error = rc_register_device(rdev);
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 99697aa..ecbc20c 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -923,6 +923,7 @@ static ssize_t store_protocols(struct device *device,
 	int rc, i, count = 0;
 	ssize_t ret;
 	int (*change_protocol)(struct rc_dev *dev, u64 *rc_type);
+	int (*set_filter)(struct rc_dev *dev, struct rc_scancode_filter *filter);
 	struct rc_scancode_filter local_filter, *filter;
 
 	/* Device is being removed */
@@ -1007,24 +1008,27 @@ static ssize_t store_protocols(struct device *device,
 	 * Fall back to clearing the filter.
 	 */
 	filter = &dev->scancode_filters[fattr->type];
+	set_filter = (fattr->type == RC_FILTER_NORMAL)
+		? dev->s_filter : dev->s_wakeup_filter;
+
 	if (old_type != type && filter->mask) {
 		local_filter = *filter;
 		if (!type) {
 			/* no protocol => clear filter */
 			ret = -1;
-		} else if (!dev->s_filter) {
+		} else if (!set_filter) {
 			/* generic filtering => accept any filter */
 			ret = 0;
 		} else {
 			/* hardware filtering => try setting, otherwise clear */
-			ret = dev->s_filter(dev, fattr->type, &local_filter);
+			ret = set_filter(dev, &local_filter);
 		}
 		if (ret < 0) {
 			/* clear the filter */
 			local_filter.data = 0;
 			local_filter.mask = 0;
-			if (dev->s_filter)
-				dev->s_filter(dev, fattr->type, &local_filter);
+			if (set_filter)
+				set_filter(dev, &local_filter);
 		}
 
 		/* commit the new filter */
@@ -1106,6 +1110,7 @@ static ssize_t store_filter(struct device *device,
 	struct rc_scancode_filter local_filter, *filter;
 	int ret;
 	unsigned long val;
+	int (*set_filter)(struct rc_dev *dev, struct rc_scancode_filter *filter);
 
 	/* Device is being removed */
 	if (!dev)
@@ -1115,8 +1120,11 @@ static ssize_t store_filter(struct device *device,
 	if (ret < 0)
 		return ret;
 
+	set_filter = (fattr->type == RC_FILTER_NORMAL) ? dev->s_filter :
+							 dev->s_wakeup_filter;
+
 	/* Scancode filter not supported (but still accept 0) */
-	if (!dev->s_filter && fattr->type != RC_FILTER_NORMAL)
+	if (!set_filter && fattr->type == RC_FILTER_WAKEUP)
 		return val ? -EINVAL : count;
 
 	mutex_lock(&dev->lock);
@@ -1128,13 +1136,15 @@ static ssize_t store_filter(struct device *device,
 		local_filter.mask = val;
 	else
 		local_filter.data = val;
+
 	if (!dev->enabled_protocols[fattr->type] && local_filter.mask) {
 		/* refuse to set a filter unless a protocol is enabled */
 		ret = -EINVAL;
 		goto unlock;
 	}
-	if (dev->s_filter) {
-		ret = dev->s_filter(dev, fattr->type, &local_filter);
+
+	if (set_filter) {
+		ret = set_filter(dev, &local_filter);
 		if (ret < 0)
 			goto unlock;
 	}
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 0b9f890..6dbc7c1 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -112,7 +112,8 @@ enum rc_filter_type {
  *	device doesn't interrupt host until it sees IR pulses
  * @s_learning_mode: enable wide band receiver used for learning
  * @s_carrier_report: enable carrier reports
- * @s_filter: set the scancode filter of a given type
+ * @s_filter: set the scancode filter 
+ * @s_wakeup_filter: set the wakeup scancode filter
  */
 struct rc_dev {
 	struct device			dev;
@@ -159,8 +160,9 @@ struct rc_dev {
 	int				(*s_learning_mode)(struct rc_dev *dev, int enable);
 	int				(*s_carrier_report) (struct rc_dev *dev, int enable);
 	int				(*s_filter)(struct rc_dev *dev,
-						    enum rc_filter_type type,
 						    struct rc_scancode_filter *filter);
+	int				(*s_wakeup_filter)(struct rc_dev *dev,
+							   struct rc_scancode_filter *filter);
 };
 
 #define to_rc_dev(d) container_of(d, struct rc_dev, dev)


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] rc-core: remove generic scancode filter
  2014-04-04 22:05 [PATCH 0/3] 3.15 fixes David Härdeman
  2014-04-04 22:05 ` [PATCH 1/3] rc-core: do not change 32bit NEC scancode format for now David Härdeman
  2014-04-04 22:06 ` [PATCH 2/3] rc-core: split dev->s_filter David Härdeman
@ 2014-04-04 22:06 ` David Härdeman
  2014-04-04 23:38   ` James Hogan
  2 siblings, 1 reply; 7+ messages in thread
From: David Härdeman @ 2014-04-04 22:06 UTC (permalink / raw)
  To: linux-media; +Cc: james.hogan, m.chehab

The generic scancode filtering has questionable value and makes it
impossible to determine from userspace if there is an actual
scancode hw filter present or not.

So revert the generic parts.

Based on a patch from James Hogan <james.hogan@imgtec.com>, but this
version also makes sure that only the valid sysfs files are created
in the first place.

v2: correct dev->s_filter check

v3: move some parts over from the previous patch

Signed-off-by: David Härdeman <david@hardeman.nu>
---
 drivers/media/rc/rc-main.c |   88 +++++++++++++++++++++++++++-----------------
 include/media/rc-core.h    |    2 +
 2 files changed, 55 insertions(+), 35 deletions(-)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index ecbc20c..970b93d 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -633,19 +633,13 @@ EXPORT_SYMBOL_GPL(rc_repeat);
 static void ir_do_keydown(struct rc_dev *dev, int scancode,
 			  u32 keycode, u8 toggle)
 {
-	struct rc_scancode_filter *filter;
-	bool new_event = !dev->keypressed ||
-			 dev->last_scancode != scancode ||
-			 dev->last_toggle != toggle;
+	bool new_event = (!dev->keypressed		 ||
+			  dev->last_scancode != scancode ||
+			  dev->last_toggle != toggle);
 
 	if (new_event && dev->keypressed)
 		ir_do_keyup(dev, false);
 
-	/* Generic scancode filtering */
-	filter = &dev->scancode_filters[RC_FILTER_NORMAL];
-	if (filter->mask && ((scancode ^ filter->data) & filter->mask))
-		return;
-
 	input_event(dev->input_dev, EV_MSC, MSC_SCAN, scancode);
 
 	if (new_event && keycode != KEY_RESERVED) {
@@ -1011,14 +1005,11 @@ static ssize_t store_protocols(struct device *device,
 	set_filter = (fattr->type == RC_FILTER_NORMAL)
 		? dev->s_filter : dev->s_wakeup_filter;
 
-	if (old_type != type && filter->mask) {
+	if (set_filter && old_type != type && filter->mask) {
 		local_filter = *filter;
 		if (!type) {
 			/* no protocol => clear filter */
 			ret = -1;
-		} else if (!set_filter) {
-			/* generic filtering => accept any filter */
-			ret = 0;
 		} else {
 			/* hardware filtering => try setting, otherwise clear */
 			ret = set_filter(dev, &local_filter);
@@ -1027,8 +1018,7 @@ static ssize_t store_protocols(struct device *device,
 			/* clear the filter */
 			local_filter.data = 0;
 			local_filter.mask = 0;
-			if (set_filter)
-				set_filter(dev, &local_filter);
+			set_filter(dev, &local_filter);
 		}
 
 		/* commit the new filter */
@@ -1072,7 +1062,10 @@ static ssize_t show_filter(struct device *device,
 		return -EINVAL;
 
 	mutex_lock(&dev->lock);
-	if (fattr->mask)
+	if ((fattr->type == RC_FILTER_NORMAL && !dev->s_filter) ||
+	    (fattr->type == RC_FILTER_WAKEUP && !dev->s_wakeup_filter))
+		val = 0;
+	else if (fattr->mask)
 		val = dev->scancode_filters[fattr->type].mask;
 	else
 		val = dev->scancode_filters[fattr->type].data;
@@ -1120,12 +1113,11 @@ static ssize_t store_filter(struct device *device,
 	if (ret < 0)
 		return ret;
 
+	/* Can the scancode filter be set? */
 	set_filter = (fattr->type == RC_FILTER_NORMAL) ? dev->s_filter :
 							 dev->s_wakeup_filter;
-
-	/* Scancode filter not supported (but still accept 0) */
-	if (!set_filter && fattr->type == RC_FILTER_WAKEUP)
-		return val ? -EINVAL : count;
+	if (!set_filter)
+		return -EINVAL;
 
 	mutex_lock(&dev->lock);
 
@@ -1143,11 +1135,9 @@ static ssize_t store_filter(struct device *device,
 		goto unlock;
 	}
 
-	if (set_filter) {
-		ret = set_filter(dev, &local_filter);
-		if (ret < 0)
-			goto unlock;
-	}
+	ret = set_filter(dev, &local_filter);
+	if (ret < 0)
+		goto unlock;
 
 	/* Success, commit the new filter */
 	*filter = local_filter;
@@ -1199,27 +1189,45 @@ static RC_FILTER_ATTR(wakeup_filter, S_IRUGO|S_IWUSR,
 static RC_FILTER_ATTR(wakeup_filter_mask, S_IRUGO|S_IWUSR,
 		      show_filter, store_filter, RC_FILTER_WAKEUP, true);
 
-static struct attribute *rc_dev_attrs[] = {
+static struct attribute *rc_dev_protocol_attrs[] = {
 	&dev_attr_protocols.attr.attr,
+	NULL,
+};
+
+static struct attribute_group rc_dev_protocol_attr_grp = {
+	.attrs	= rc_dev_protocol_attrs,
+};
+
+static struct attribute *rc_dev_wakeup_protocol_attrs[] = {
 	&dev_attr_wakeup_protocols.attr.attr,
+	NULL,
+};
+
+static struct attribute_group rc_dev_wakeup_protocol_attr_grp = {
+	.attrs	= rc_dev_wakeup_protocol_attrs,
+};
+
+static struct attribute *rc_dev_filter_attrs[] = {
 	&dev_attr_filter.attr.attr,
 	&dev_attr_filter_mask.attr.attr,
-	&dev_attr_wakeup_filter.attr.attr,
-	&dev_attr_wakeup_filter_mask.attr.attr,
 	NULL,
 };
 
-static struct attribute_group rc_dev_attr_grp = {
-	.attrs	= rc_dev_attrs,
+static struct attribute_group rc_dev_filter_attr_grp = {
+	.attrs	= rc_dev_filter_attrs,
 };
 
-static const struct attribute_group *rc_dev_attr_groups[] = {
-	&rc_dev_attr_grp,
-	NULL
+static struct attribute *rc_dev_wakeup_filter_attrs[] = {
+	&dev_attr_wakeup_filter.attr.attr,
+	&dev_attr_wakeup_filter_mask.attr.attr,
+	NULL,
+};
+
+static struct attribute_group rc_dev_wakeup_filter_attr_grp = {
+	.attrs	= rc_dev_wakeup_filter_attrs,
 };
 
 static struct device_type rc_dev_type = {
-	.groups		= rc_dev_attr_groups,
 	.release	= rc_dev_release,
 	.uevent		= rc_dev_uevent,
 };
@@ -1276,7 +1284,7 @@ int rc_register_device(struct rc_dev *dev)
 	static bool raw_init = false; /* raw decoders loaded? */
 	struct rc_map *rc_map;
 	const char *path;
-	int rc, devno;
+	int rc, devno, attr = 0;
 
 	if (!dev || !dev->map_name)
 		return -EINVAL;
@@ -1304,6 +1312,16 @@ int rc_register_device(struct rc_dev *dev)
 			return -ENOMEM;
 	} while (test_and_set_bit(devno, ir_core_dev_number));
 
+	dev->dev.groups = dev->sysfs_groups;
+	dev->sysfs_groups[attr++] = &rc_dev_protocol_attr_grp;
+	if (dev->s_filter)
+		dev->sysfs_groups[attr++] = &rc_dev_filter_attr_grp;	
+	if (dev->s_wakeup_filter)
+		dev->sysfs_groups[attr++] = &rc_dev_wakeup_filter_attr_grp;
+	if (dev->change_wakeup_protocol)
+		dev->sysfs_groups[attr++] = &rc_dev_wakeup_protocol_attr_grp;
+	dev->sysfs_groups[attr++] = NULL;
+
 	/*
 	 * Take the lock here, as the device sysfs node will appear
 	 * when device_add() is called, which may trigger an ir-keytable udev
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 6dbc7c1..fde142e 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -60,6 +60,7 @@ enum rc_filter_type {
 /**
  * struct rc_dev - represents a remote control device
  * @dev: driver model's view of this device
+ * @sysfs_groups: sysfs attribute groups
  * @input_name: name of the input child device
  * @input_phys: physical path to the input child device
  * @input_id: id of the input child device (struct input_id)
@@ -117,6 +118,7 @@ enum rc_filter_type {
  */
 struct rc_dev {
 	struct device			dev;
+	const struct attribute_group	*sysfs_groups[5];
 	const char			*input_name;
 	const char			*input_phys;
 	struct input_id			input_id;


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] rc-core: do not change 32bit NEC scancode format for now
  2014-04-04 22:05 ` [PATCH 1/3] rc-core: do not change 32bit NEC scancode format for now David Härdeman
@ 2014-04-04 23:34   ` James Hogan
  0 siblings, 0 replies; 7+ messages in thread
From: James Hogan @ 2014-04-04 23:34 UTC (permalink / raw)
  To: David Härdeman; +Cc: linux-media, m.chehab

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

On Saturday 05 April 2014 00:05:56 David Härdeman wrote:
> This reverts 18bc17448147e93f31cc9b1a83be49f1224657b2
> 
> The patch ignores the fact that NEC32 scancodes are generated not only in
> the NEC raw decoder but also directly in some drivers. Whichever approach
> is chosen it should be consistent across drivers and this patch needs more
> discussion.
> 
> Furthermore, I'm convinced that we have to stop playing games trying to
> decipher the "meaning" of NEC scancodes (what's the customer/vendor/address,
> which byte is the MSB, etc).
> 
> This patch is in preparation for the next few patches in this series.
> 
> v2: make sure img-ir scancodes are bitrev8():ed as well
> 
> v3: update comments
> 
> Signed-off-by: David Härdeman <david@hardeman.nu>

Acked-by: James Hogan <james.hogan@imgtec.com>

Thanks
James

> ---
>  drivers/media/rc/img-ir/img-ir-nec.c |   27 ++++++-----
>  drivers/media/rc/ir-nec-decoder.c    |    5 --
>  drivers/media/rc/keymaps/rc-tivo.c   |   86
> +++++++++++++++++----------------- 3 files changed, 59 insertions(+), 59
> deletions(-)
> 
> diff --git a/drivers/media/rc/img-ir/img-ir-nec.c
> b/drivers/media/rc/img-ir/img-ir-nec.c index e7a731b..751d9d9 100644
> --- a/drivers/media/rc/img-ir/img-ir-nec.c
> +++ b/drivers/media/rc/img-ir/img-ir-nec.c
> @@ -5,6 +5,7 @@
>   */
> 
>  #include "img-ir-hw.h"
> +#include <linux/bitrev.h>
> 
>  /* Convert NEC data to a scancode */
>  static int img_ir_nec_scancode(int len, u64 raw, int *scancode, u64
> protocols) @@ -22,11 +23,11 @@ static int img_ir_nec_scancode(int len, u64
> raw, int *scancode, u64 protocols) data_inv = (raw >> 24) & 0xff;
>  	if ((data_inv ^ data) != 0xff) {
>  		/* 32-bit NEC (used by Apple and TiVo remotes) */
> -		/* scan encoding: aaAAddDD */
> -		*scancode = addr_inv << 24 |
> -			    addr     << 16 |
> -			    data_inv <<  8 |
> -			    data;
> +		/* scan encoding: as transmitted, MSBit = first received bit */
> +		*scancode = bitrev8(addr)     << 24 |
> +			    bitrev8(addr_inv) << 16 |
> +			    bitrev8(data)     <<  8 |
> +			    bitrev8(data_inv);
>  	} else if ((addr_inv ^ addr) != 0xff) {
>  		/* Extended NEC */
>  		/* scan encoding: AAaaDD */
> @@ -54,13 +55,15 @@ static int img_ir_nec_filter(const struct
> rc_scancode_filter *in,
> 
>  	if ((in->data | in->mask) & 0xff000000) {
>  		/* 32-bit NEC (used by Apple and TiVo remotes) */
> -		/* scan encoding: aaAAddDD */
> -		addr_inv   = (in->data >> 24) & 0xff;
> -		addr_inv_m = (in->mask >> 24) & 0xff;
> -		addr       = (in->data >> 16) & 0xff;
> -		addr_m     = (in->mask >> 16) & 0xff;
> -		data_inv   = (in->data >>  8) & 0xff;
> -		data_inv_m = (in->mask >>  8) & 0xff;
> +		/* scan encoding: as transmitted, MSBit = first received bit */
> +		addr       = bitrev8(in->data >> 24);
> +		addr_m     = bitrev8(in->mask >> 24);
> +		addr_inv   = bitrev8(in->data >> 16);
> +		addr_inv_m = bitrev8(in->mask >> 16);
> +		data       = bitrev8(in->data >>  8);
> +		data_m     = bitrev8(in->mask >>  8);
> +		data_inv   = bitrev8(in->data >>  0);
> +		data_inv_m = bitrev8(in->mask >>  0);
>  	} else if ((in->data | in->mask) & 0x00ff0000) {
>  		/* Extended NEC */
>  		/* scan encoding AAaaDD */
> diff --git a/drivers/media/rc/ir-nec-decoder.c
> b/drivers/media/rc/ir-nec-decoder.c index 9de1791..35c42e5 100644
> --- a/drivers/media/rc/ir-nec-decoder.c
> +++ b/drivers/media/rc/ir-nec-decoder.c
> @@ -172,10 +172,7 @@ static int ir_nec_decode(struct rc_dev *dev, struct
> ir_raw_event ev) if (send_32bits) {
>  			/* NEC transport, but modified protocol, used by at
>  			 * least Apple and TiVo remotes */
> -			scancode = not_address << 24 |
> -				   address     << 16 |
> -				   not_command <<  8 |
> -				   command;
> +			scancode = data->bits;
>  			IR_dprintk(1, "NEC (modified) scancode 0x%08x\n", scancode);
>  		} else if ((address ^ not_address) != 0xff) {
>  			/* Extended NEC */
> diff --git a/drivers/media/rc/keymaps/rc-tivo.c
> b/drivers/media/rc/keymaps/rc-tivo.c index 5cc1b45..454e062 100644
> --- a/drivers/media/rc/keymaps/rc-tivo.c
> +++ b/drivers/media/rc/keymaps/rc-tivo.c
> @@ -15,62 +15,62 @@
>   * Initial mapping is for the TiVo remote included in the Nero LiquidTV
> bundle, * which also ships with a TiVo-branded IR transceiver, supported by
> the mceusb * driver. Note that the remote uses an NEC-ish protocol, but
> instead of having - * a command/not_command pair, it has a vendor ID of
> 0x3085, but some keys, the + * a command/not_command pair, it has a vendor
> ID of 0xa10c, but some keys, the * NEC extended checksums do pass, so the
> table presently has the intended * values and the checksum-passed versions
> for those keys.
>   */
>  static struct rc_map_table tivo[] = {
> -	{ 0x3085f009, KEY_MEDIA },	/* TiVo Button */
> -	{ 0x3085e010, KEY_POWER2 },	/* TV Power */
> -	{ 0x3085e011, KEY_TV },		/* Live TV/Swap */
> -	{ 0x3085c034, KEY_VIDEO_NEXT },	/* TV Input */
> -	{ 0x3085e013, KEY_INFO },
> -	{ 0x3085a05f, KEY_CYCLEWINDOWS }, /* Window */
> +	{ 0xa10c900f, KEY_MEDIA },	/* TiVo Button */
> +	{ 0xa10c0807, KEY_POWER2 },	/* TV Power */
> +	{ 0xa10c8807, KEY_TV },		/* Live TV/Swap */
> +	{ 0xa10c2c03, KEY_VIDEO_NEXT },	/* TV Input */
> +	{ 0xa10cc807, KEY_INFO },
> +	{ 0xa10cfa05, KEY_CYCLEWINDOWS }, /* Window */
>  	{ 0x0085305f, KEY_CYCLEWINDOWS },
> -	{ 0x3085c036, KEY_EPG },	/* Guide */
> +	{ 0xa10c6c03, KEY_EPG },	/* Guide */
> 
> -	{ 0x3085e014, KEY_UP },
> -	{ 0x3085e016, KEY_DOWN },
> -	{ 0x3085e017, KEY_LEFT },
> -	{ 0x3085e015, KEY_RIGHT },
> +	{ 0xa10c2807, KEY_UP },
> +	{ 0xa10c6807, KEY_DOWN },
> +	{ 0xa10ce807, KEY_LEFT },
> +	{ 0xa10ca807, KEY_RIGHT },
> 
> -	{ 0x3085e018, KEY_SCROLLDOWN },	/* Red Thumbs Down */
> -	{ 0x3085e019, KEY_SELECT },
> -	{ 0x3085e01a, KEY_SCROLLUP },	/* Green Thumbs Up */
> +	{ 0xa10c1807, KEY_SCROLLDOWN },	/* Red Thumbs Down */
> +	{ 0xa10c9807, KEY_SELECT },
> +	{ 0xa10c5807, KEY_SCROLLUP },	/* Green Thumbs Up */
> 
> -	{ 0x3085e01c, KEY_VOLUMEUP },
> -	{ 0x3085e01d, KEY_VOLUMEDOWN },
> -	{ 0x3085e01b, KEY_MUTE },
> -	{ 0x3085d020, KEY_RECORD },
> -	{ 0x3085e01e, KEY_CHANNELUP },
> -	{ 0x3085e01f, KEY_CHANNELDOWN },
> +	{ 0xa10c3807, KEY_VOLUMEUP },
> +	{ 0xa10cb807, KEY_VOLUMEDOWN },
> +	{ 0xa10cd807, KEY_MUTE },
> +	{ 0xa10c040b, KEY_RECORD },
> +	{ 0xa10c7807, KEY_CHANNELUP },
> +	{ 0xa10cf807, KEY_CHANNELDOWN },
>  	{ 0x0085301f, KEY_CHANNELDOWN },
> 
> -	{ 0x3085d021, KEY_PLAY },
> -	{ 0x3085d023, KEY_PAUSE },
> -	{ 0x3085d025, KEY_SLOW },
> -	{ 0x3085d022, KEY_REWIND },
> -	{ 0x3085d024, KEY_FASTFORWARD },
> -	{ 0x3085d026, KEY_PREVIOUS },
> -	{ 0x3085d027, KEY_NEXT },	/* ->| */
> +	{ 0xa10c840b, KEY_PLAY },
> +	{ 0xa10cc40b, KEY_PAUSE },
> +	{ 0xa10ca40b, KEY_SLOW },
> +	{ 0xa10c440b, KEY_REWIND },
> +	{ 0xa10c240b, KEY_FASTFORWARD },
> +	{ 0xa10c640b, KEY_PREVIOUS },
> +	{ 0xa10ce40b, KEY_NEXT },	/* ->| */
> 
> -	{ 0x3085b044, KEY_ZOOM },	/* Aspect */
> -	{ 0x3085b048, KEY_STOP },
> -	{ 0x3085b04a, KEY_DVD },	/* DVD Menu */
> +	{ 0xa10c220d, KEY_ZOOM },	/* Aspect */
> +	{ 0xa10c120d, KEY_STOP },
> +	{ 0xa10c520d, KEY_DVD },	/* DVD Menu */
> 
> -	{ 0x3085d028, KEY_NUMERIC_1 },
> -	{ 0x3085d029, KEY_NUMERIC_2 },
> -	{ 0x3085d02a, KEY_NUMERIC_3 },
> -	{ 0x3085d02b, KEY_NUMERIC_4 },
> -	{ 0x3085d02c, KEY_NUMERIC_5 },
> -	{ 0x3085d02d, KEY_NUMERIC_6 },
> -	{ 0x3085d02e, KEY_NUMERIC_7 },
> -	{ 0x3085d02f, KEY_NUMERIC_8 },
> +	{ 0xa10c140b, KEY_NUMERIC_1 },
> +	{ 0xa10c940b, KEY_NUMERIC_2 },
> +	{ 0xa10c540b, KEY_NUMERIC_3 },
> +	{ 0xa10cd40b, KEY_NUMERIC_4 },
> +	{ 0xa10c340b, KEY_NUMERIC_5 },
> +	{ 0xa10cb40b, KEY_NUMERIC_6 },
> +	{ 0xa10c740b, KEY_NUMERIC_7 },
> +	{ 0xa10cf40b, KEY_NUMERIC_8 },
>  	{ 0x0085302f, KEY_NUMERIC_8 },
> -	{ 0x3085c030, KEY_NUMERIC_9 },
> -	{ 0x3085c031, KEY_NUMERIC_0 },
> -	{ 0x3085c033, KEY_ENTER },
> -	{ 0x3085c032, KEY_CLEAR },
> +	{ 0xa10c0c03, KEY_NUMERIC_9 },
> +	{ 0xa10c8c03, KEY_NUMERIC_0 },
> +	{ 0xa10ccc03, KEY_ENTER },
> +	{ 0xa10c4c03, KEY_CLEAR },
>  };
> 
>  static struct rc_map_list tivo_map = {
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] rc-core: split dev->s_filter
  2014-04-04 22:06 ` [PATCH 2/3] rc-core: split dev->s_filter David Härdeman
@ 2014-04-04 23:36   ` James Hogan
  0 siblings, 0 replies; 7+ messages in thread
From: James Hogan @ 2014-04-04 23:36 UTC (permalink / raw)
  To: David Härdeman; +Cc: linux-media, m.chehab

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

On Saturday 05 April 2014 00:06:01 David Härdeman wrote:
> Overloading dev->s_filter to do two different functions (set wakeup filters
> and generic hardware filters) makes it impossible to tell what the
> hardware actually supports, so create a separate dev->s_wakeup_filter and
> make the distinction explicit.
> 
> v2: hopefully address James' comments on what should be moved from this to
> the next patch.
> 
> Signed-off-by: David Härdeman <david@hardeman.nu>

Acked-by: James Hogan <james.hogan@imgtec.com>

Thanks
James

> ---
>  drivers/media/rc/img-ir/img-ir-hw.c |   15 ++++++++++++++-
>  drivers/media/rc/rc-main.c          |   24 +++++++++++++++++-------
>  include/media/rc-core.h             |    6 ++++--
>  3 files changed, 35 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/rc/img-ir/img-ir-hw.c
> b/drivers/media/rc/img-ir/img-ir-hw.c index 579a52b..0127dd2 100644
> --- a/drivers/media/rc/img-ir/img-ir-hw.c
> +++ b/drivers/media/rc/img-ir/img-ir-hw.c
> @@ -504,6 +504,18 @@ unlock:
>  	return ret;
>  }
> 
> +static int img_ir_set_normal_filter(struct rc_dev *dev,
> +				    struct rc_scancode_filter *sc_filter)
> +{
> +	return img_ir_set_filter(dev, RC_FILTER_NORMAL, sc_filter);
> +}
> +
> +static int img_ir_set_wakeup_filter(struct rc_dev *dev,
> +				    struct rc_scancode_filter *sc_filter)
> +{
> +	return img_ir_set_filter(dev, RC_FILTER_WAKEUP, sc_filter);
> +}
> +
>  /**
>   * img_ir_set_decoder() - Set the current decoder.
>   * @priv:	IR private data.
> @@ -986,7 +998,8 @@ int img_ir_probe_hw(struct img_ir_priv *priv)
>  	rdev->map_name = RC_MAP_EMPTY;
>  	rc_set_allowed_protocols(rdev, img_ir_allowed_protos(priv));
>  	rdev->input_name = "IMG Infrared Decoder";
> -	rdev->s_filter = img_ir_set_filter;
> +	rdev->s_filter = img_ir_set_normal_filter;
> +	rdev->s_wakeup_filter = img_ir_set_wakeup_filter;
> 
>  	/* Register hardware decoder */
>  	error = rc_register_device(rdev);
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index 99697aa..ecbc20c 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -923,6 +923,7 @@ static ssize_t store_protocols(struct device *device,
>  	int rc, i, count = 0;
>  	ssize_t ret;
>  	int (*change_protocol)(struct rc_dev *dev, u64 *rc_type);
> +	int (*set_filter)(struct rc_dev *dev, struct rc_scancode_filter *filter);
>  	struct rc_scancode_filter local_filter, *filter;
> 
>  	/* Device is being removed */
> @@ -1007,24 +1008,27 @@ static ssize_t store_protocols(struct device
> *device, * Fall back to clearing the filter.
>  	 */
>  	filter = &dev->scancode_filters[fattr->type];
> +	set_filter = (fattr->type == RC_FILTER_NORMAL)
> +		? dev->s_filter : dev->s_wakeup_filter;
> +
>  	if (old_type != type && filter->mask) {
>  		local_filter = *filter;
>  		if (!type) {
>  			/* no protocol => clear filter */
>  			ret = -1;
> -		} else if (!dev->s_filter) {
> +		} else if (!set_filter) {
>  			/* generic filtering => accept any filter */
>  			ret = 0;
>  		} else {
>  			/* hardware filtering => try setting, otherwise clear */
> -			ret = dev->s_filter(dev, fattr->type, &local_filter);
> +			ret = set_filter(dev, &local_filter);
>  		}
>  		if (ret < 0) {
>  			/* clear the filter */
>  			local_filter.data = 0;
>  			local_filter.mask = 0;
> -			if (dev->s_filter)
> -				dev->s_filter(dev, fattr->type, &local_filter);
> +			if (set_filter)
> +				set_filter(dev, &local_filter);
>  		}
> 
>  		/* commit the new filter */
> @@ -1106,6 +1110,7 @@ static ssize_t store_filter(struct device *device,
>  	struct rc_scancode_filter local_filter, *filter;
>  	int ret;
>  	unsigned long val;
> +	int (*set_filter)(struct rc_dev *dev, struct rc_scancode_filter *filter);
> 
>  	/* Device is being removed */
>  	if (!dev)
> @@ -1115,8 +1120,11 @@ static ssize_t store_filter(struct device *device,
>  	if (ret < 0)
>  		return ret;
> 
> +	set_filter = (fattr->type == RC_FILTER_NORMAL) ? dev->s_filter :
> +							 dev->s_wakeup_filter;
> +
>  	/* Scancode filter not supported (but still accept 0) */
> -	if (!dev->s_filter && fattr->type != RC_FILTER_NORMAL)
> +	if (!set_filter && fattr->type == RC_FILTER_WAKEUP)
>  		return val ? -EINVAL : count;
> 
>  	mutex_lock(&dev->lock);
> @@ -1128,13 +1136,15 @@ static ssize_t store_filter(struct device *device,
>  		local_filter.mask = val;
>  	else
>  		local_filter.data = val;
> +
>  	if (!dev->enabled_protocols[fattr->type] && local_filter.mask) {
>  		/* refuse to set a filter unless a protocol is enabled */
>  		ret = -EINVAL;
>  		goto unlock;
>  	}
> -	if (dev->s_filter) {
> -		ret = dev->s_filter(dev, fattr->type, &local_filter);
> +
> +	if (set_filter) {
> +		ret = set_filter(dev, &local_filter);
>  		if (ret < 0)
>  			goto unlock;
>  	}
> diff --git a/include/media/rc-core.h b/include/media/rc-core.h
> index 0b9f890..6dbc7c1 100644
> --- a/include/media/rc-core.h
> +++ b/include/media/rc-core.h
> @@ -112,7 +112,8 @@ enum rc_filter_type {
>   *	device doesn't interrupt host until it sees IR pulses
>   * @s_learning_mode: enable wide band receiver used for learning
>   * @s_carrier_report: enable carrier reports
> - * @s_filter: set the scancode filter of a given type
> + * @s_filter: set the scancode filter
> + * @s_wakeup_filter: set the wakeup scancode filter
>   */
>  struct rc_dev {
>  	struct device			dev;
> @@ -159,8 +160,9 @@ struct rc_dev {
>  	int				(*s_learning_mode)(struct rc_dev *dev, int enable);
>  	int				(*s_carrier_report) (struct rc_dev *dev, int enable);
>  	int				(*s_filter)(struct rc_dev *dev,
> -						    enum rc_filter_type type,
>  						    struct rc_scancode_filter *filter);
> +	int				(*s_wakeup_filter)(struct rc_dev *dev,
> +							   struct rc_scancode_filter *filter);
>  };
> 
>  #define to_rc_dev(d) container_of(d, struct rc_dev, dev)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] rc-core: remove generic scancode filter
  2014-04-04 22:06 ` [PATCH 3/3] rc-core: remove generic scancode filter David Härdeman
@ 2014-04-04 23:38   ` James Hogan
  0 siblings, 0 replies; 7+ messages in thread
From: James Hogan @ 2014-04-04 23:38 UTC (permalink / raw)
  To: David Härdeman; +Cc: linux-media, m.chehab

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

On Saturday 05 April 2014 00:06:06 David Härdeman wrote:
> The generic scancode filtering has questionable value and makes it
> impossible to determine from userspace if there is an actual
> scancode hw filter present or not.
> 
> So revert the generic parts.
> 
> Based on a patch from James Hogan <james.hogan@imgtec.com>, but this
> version also makes sure that only the valid sysfs files are created
> in the first place.
> 
> v2: correct dev->s_filter check
> 
> v3: move some parts over from the previous patch
> 
> Signed-off-by: David Härdeman <david@hardeman.nu>

Acked-by: James Hogan <james.hogan@imgtec.com>

Thanks
James

> ---
>  drivers/media/rc/rc-main.c |   88
> +++++++++++++++++++++++++++----------------- include/media/rc-core.h    |  
>  2 +
>  2 files changed, 55 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index ecbc20c..970b93d 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -633,19 +633,13 @@ EXPORT_SYMBOL_GPL(rc_repeat);
>  static void ir_do_keydown(struct rc_dev *dev, int scancode,
>  			  u32 keycode, u8 toggle)
>  {
> -	struct rc_scancode_filter *filter;
> -	bool new_event = !dev->keypressed ||
> -			 dev->last_scancode != scancode ||
> -			 dev->last_toggle != toggle;
> +	bool new_event = (!dev->keypressed		 ||
> +			  dev->last_scancode != scancode ||
> +			  dev->last_toggle != toggle);
> 
>  	if (new_event && dev->keypressed)
>  		ir_do_keyup(dev, false);
> 
> -	/* Generic scancode filtering */
> -	filter = &dev->scancode_filters[RC_FILTER_NORMAL];
> -	if (filter->mask && ((scancode ^ filter->data) & filter->mask))
> -		return;
> -
>  	input_event(dev->input_dev, EV_MSC, MSC_SCAN, scancode);
> 
>  	if (new_event && keycode != KEY_RESERVED) {
> @@ -1011,14 +1005,11 @@ static ssize_t store_protocols(struct device
> *device, set_filter = (fattr->type == RC_FILTER_NORMAL)
>  		? dev->s_filter : dev->s_wakeup_filter;
> 
> -	if (old_type != type && filter->mask) {
> +	if (set_filter && old_type != type && filter->mask) {
>  		local_filter = *filter;
>  		if (!type) {
>  			/* no protocol => clear filter */
>  			ret = -1;
> -		} else if (!set_filter) {
> -			/* generic filtering => accept any filter */
> -			ret = 0;
>  		} else {
>  			/* hardware filtering => try setting, otherwise clear */
>  			ret = set_filter(dev, &local_filter);
> @@ -1027,8 +1018,7 @@ static ssize_t store_protocols(struct device *device,
>  			/* clear the filter */
>  			local_filter.data = 0;
>  			local_filter.mask = 0;
> -			if (set_filter)
> -				set_filter(dev, &local_filter);
> +			set_filter(dev, &local_filter);
>  		}
> 
>  		/* commit the new filter */
> @@ -1072,7 +1062,10 @@ static ssize_t show_filter(struct device *device,
>  		return -EINVAL;
> 
>  	mutex_lock(&dev->lock);
> -	if (fattr->mask)
> +	if ((fattr->type == RC_FILTER_NORMAL && !dev->s_filter) ||
> +	    (fattr->type == RC_FILTER_WAKEUP && !dev->s_wakeup_filter))
> +		val = 0;
> +	else if (fattr->mask)
>  		val = dev->scancode_filters[fattr->type].mask;
>  	else
>  		val = dev->scancode_filters[fattr->type].data;
> @@ -1120,12 +1113,11 @@ static ssize_t store_filter(struct device *device,
>  	if (ret < 0)
>  		return ret;
> 
> +	/* Can the scancode filter be set? */
>  	set_filter = (fattr->type == RC_FILTER_NORMAL) ? dev->s_filter :
>  							 dev->s_wakeup_filter;
> -
> -	/* Scancode filter not supported (but still accept 0) */
> -	if (!set_filter && fattr->type == RC_FILTER_WAKEUP)
> -		return val ? -EINVAL : count;
> +	if (!set_filter)
> +		return -EINVAL;
> 
>  	mutex_lock(&dev->lock);
> 
> @@ -1143,11 +1135,9 @@ static ssize_t store_filter(struct device *device,
>  		goto unlock;
>  	}
> 
> -	if (set_filter) {
> -		ret = set_filter(dev, &local_filter);
> -		if (ret < 0)
> -			goto unlock;
> -	}
> +	ret = set_filter(dev, &local_filter);
> +	if (ret < 0)
> +		goto unlock;
> 
>  	/* Success, commit the new filter */
>  	*filter = local_filter;
> @@ -1199,27 +1189,45 @@ static RC_FILTER_ATTR(wakeup_filter,
> S_IRUGO|S_IWUSR, static RC_FILTER_ATTR(wakeup_filter_mask, S_IRUGO|S_IWUSR,
>  		      show_filter, store_filter, RC_FILTER_WAKEUP, true);
> 
> -static struct attribute *rc_dev_attrs[] = {
> +static struct attribute *rc_dev_protocol_attrs[] = {
>  	&dev_attr_protocols.attr.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group rc_dev_protocol_attr_grp = {
> +	.attrs	= rc_dev_protocol_attrs,
> +};
> +
> +static struct attribute *rc_dev_wakeup_protocol_attrs[] = {
>  	&dev_attr_wakeup_protocols.attr.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group rc_dev_wakeup_protocol_attr_grp = {
> +	.attrs	= rc_dev_wakeup_protocol_attrs,
> +};
> +
> +static struct attribute *rc_dev_filter_attrs[] = {
>  	&dev_attr_filter.attr.attr,
>  	&dev_attr_filter_mask.attr.attr,
> -	&dev_attr_wakeup_filter.attr.attr,
> -	&dev_attr_wakeup_filter_mask.attr.attr,
>  	NULL,
>  };
> 
> -static struct attribute_group rc_dev_attr_grp = {
> -	.attrs	= rc_dev_attrs,
> +static struct attribute_group rc_dev_filter_attr_grp = {
> +	.attrs	= rc_dev_filter_attrs,
>  };
> 
> -static const struct attribute_group *rc_dev_attr_groups[] = {
> -	&rc_dev_attr_grp,
> -	NULL
> +static struct attribute *rc_dev_wakeup_filter_attrs[] = {
> +	&dev_attr_wakeup_filter.attr.attr,
> +	&dev_attr_wakeup_filter_mask.attr.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group rc_dev_wakeup_filter_attr_grp = {
> +	.attrs	= rc_dev_wakeup_filter_attrs,
>  };
> 
>  static struct device_type rc_dev_type = {
> -	.groups		= rc_dev_attr_groups,
>  	.release	= rc_dev_release,
>  	.uevent		= rc_dev_uevent,
>  };
> @@ -1276,7 +1284,7 @@ int rc_register_device(struct rc_dev *dev)
>  	static bool raw_init = false; /* raw decoders loaded? */
>  	struct rc_map *rc_map;
>  	const char *path;
> -	int rc, devno;
> +	int rc, devno, attr = 0;
> 
>  	if (!dev || !dev->map_name)
>  		return -EINVAL;
> @@ -1304,6 +1312,16 @@ int rc_register_device(struct rc_dev *dev)
>  			return -ENOMEM;
>  	} while (test_and_set_bit(devno, ir_core_dev_number));
> 
> +	dev->dev.groups = dev->sysfs_groups;
> +	dev->sysfs_groups[attr++] = &rc_dev_protocol_attr_grp;
> +	if (dev->s_filter)
> +		dev->sysfs_groups[attr++] = &rc_dev_filter_attr_grp;
> +	if (dev->s_wakeup_filter)
> +		dev->sysfs_groups[attr++] = &rc_dev_wakeup_filter_attr_grp;
> +	if (dev->change_wakeup_protocol)
> +		dev->sysfs_groups[attr++] = &rc_dev_wakeup_protocol_attr_grp;
> +	dev->sysfs_groups[attr++] = NULL;
> +
>  	/*
>  	 * Take the lock here, as the device sysfs node will appear
>  	 * when device_add() is called, which may trigger an ir-keytable udev
> diff --git a/include/media/rc-core.h b/include/media/rc-core.h
> index 6dbc7c1..fde142e 100644
> --- a/include/media/rc-core.h
> +++ b/include/media/rc-core.h
> @@ -60,6 +60,7 @@ enum rc_filter_type {
>  /**
>   * struct rc_dev - represents a remote control device
>   * @dev: driver model's view of this device
> + * @sysfs_groups: sysfs attribute groups
>   * @input_name: name of the input child device
>   * @input_phys: physical path to the input child device
>   * @input_id: id of the input child device (struct input_id)
> @@ -117,6 +118,7 @@ enum rc_filter_type {
>   */
>  struct rc_dev {
>  	struct device			dev;
> +	const struct attribute_group	*sysfs_groups[5];
>  	const char			*input_name;
>  	const char			*input_phys;
>  	struct input_id			input_id;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-04-04 23:38 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-04 22:05 [PATCH 0/3] 3.15 fixes David Härdeman
2014-04-04 22:05 ` [PATCH 1/3] rc-core: do not change 32bit NEC scancode format for now David Härdeman
2014-04-04 23:34   ` James Hogan
2014-04-04 22:06 ` [PATCH 2/3] rc-core: split dev->s_filter David Härdeman
2014-04-04 23:36   ` James Hogan
2014-04-04 22:06 ` [PATCH 3/3] rc-core: remove generic scancode filter David Härdeman
2014-04-04 23:38   ` James Hogan

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.