[PATCH 0/5] rc: scancode filtering improvements

[PATCH 0/5] rc: scancode filtering improvements

[James Hogan]

These patches make some improvements relating to the recently added RC
scancode filtering interface:
- Patch 1 adds generic scancode filtering. This allows filtering to also
  work for raw rc drivers and scancode drivers without filtering
  capabilities.
- Patches 2-4 future proof the sysfs API to allow a different wakeup
  filter protocol to be set than the current protocol. A new
  wakeup_protocols sysfs file is added which behaves similarly to the
  protocols sysfs file but applies only to wakeup filters.
- Finally patch 5 improves the driver interface so that changing either
  the normal or wakeup protocol automatically causes the corresponding
  filter to be refreshed to the driver, or failing that cleared. It also
  ensures that the filter is turned off (and for wakeup that means
  wakeup is disabled) if the protocol is set to none. This avoids the
  driver having to maintain the filters, or even need a
  change_wakeup_protocol() callback if there is only one wakeup protocol
  allowed at a time.

The patch "rc-main: store_filter: pass errors to userland" should
probably be applied first.

An updated img-ir v4 patchset which depends on this one will follow
soon.

Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: "Bruno Prémont" <bonbons@linux-vserver.org>
Cc: Maxim Levitsky <maximlevitsky@gmail.com>
Cc: Sean Young <sean@mess.org>
Cc: "David Härdeman" <david@hardeman.nu>
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: "Antti Seppälä" <a.seppala@gmail.com>

James Hogan (5):
  rc-main: add generic scancode filtering
  rc: abstract access to allowed/enabled protocols
  rc: add allowed/enabled wakeup protocol masks
  rc: add wakeup_protocols sysfs file
  rc-main: automatically refresh filter on protocol change

 Documentation/ABI/testing/sysfs-class-rc           |  23 +++-
 .../DocBook/media/v4l/remote_controllers.xml       |  20 ++-
 drivers/hid/hid-picolcd_cir.c                      |   2 +-
 drivers/media/common/siano/smsir.c                 |   2 +-
 drivers/media/i2c/ir-kbd-i2c.c                     |   4 +-
 drivers/media/pci/cx23885/cx23885-input.c          |   2 +-
 drivers/media/pci/cx88/cx88-input.c                |   2 +-
 drivers/media/rc/ati_remote.c                      |   2 +-
 drivers/media/rc/ene_ir.c                          |   2 +-
 drivers/media/rc/fintek-cir.c                      |   2 +-
 drivers/media/rc/gpio-ir-recv.c                    |   4 +-
 drivers/media/rc/iguanair.c                        |   2 +-
 drivers/media/rc/imon.c                            |   7 +-
 drivers/media/rc/ir-jvc-decoder.c                  |   2 +-
 drivers/media/rc/ir-lirc-codec.c                   |   2 +-
 drivers/media/rc/ir-mce_kbd-decoder.c              |   2 +-
 drivers/media/rc/ir-nec-decoder.c                  |   2 +-
 drivers/media/rc/ir-raw.c                          |   2 +-
 drivers/media/rc/ir-rc5-decoder.c                  |   6 +-
 drivers/media/rc/ir-rc5-sz-decoder.c               |   2 +-
 drivers/media/rc/ir-rc6-decoder.c                  |   6 +-
 drivers/media/rc/ir-sanyo-decoder.c                |   2 +-
 drivers/media/rc/ir-sharp-decoder.c                |   2 +-
 drivers/media/rc/ir-sony-decoder.c                 |  10 +-
 drivers/media/rc/ite-cir.c                         |   2 +-
 drivers/media/rc/mceusb.c                          |   2 +-
 drivers/media/rc/nuvoton-cir.c                     |   2 +-
 drivers/media/rc/rc-loopback.c                     |   2 +-
 drivers/media/rc/rc-main.c                         | 141 +++++++++++++++------
 drivers/media/rc/redrat3.c                         |   2 +-
 drivers/media/rc/st_rc.c                           |   2 +-
 drivers/media/rc/streamzap.c                       |   2 +-
 drivers/media/rc/ttusbir.c                         |   2 +-
 drivers/media/rc/winbond-cir.c                     |   2 +-
 drivers/media/usb/dvb-usb-v2/dvb_usb_core.c        |   2 +-
 drivers/media/usb/dvb-usb/dvb-usb-remote.c         |   2 +-
 drivers/media/usb/em28xx/em28xx-input.c            |   8 +-
 drivers/media/usb/tm6000/tm6000-input.c            |   2 +-
 include/media/rc-core.h                            |  49 ++++++-
 39 files changed, 234 insertions(+), 100 deletions(-)

-- 
1.8.3.2

[PATCH 1/5] rc-main: add generic scancode filtering

[James Hogan]

Add generic scancode filtering of RC input events, and fall back to
permitting any RC_FILTER_NORMAL scancode filter to be set if no s_filter
callback exists. This allows raw IR decoder events to be filtered, and
potentially allows hardware decoders to set looser filters and rely on
generic code to filter out the corner cases.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: Antti Seppälä <a.seppala@gmail.com>
Cc: linux-media@vger.kernel.org
---
 drivers/media/rc/rc-main.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 6448128..0a4f680 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -633,6 +633,7 @@ 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;
@@ -640,6 +641,11 @@ static void ir_do_keydown(struct rc_dev *dev, int scancode,
 	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) {
@@ -1019,9 +1025,7 @@ static ssize_t show_filter(struct device *device,
 		return -EINVAL;
 
 	mutex_lock(&dev->lock);
-	if (!dev->s_filter)
-		val = 0;
-	else if (fattr->mask)
+	if (fattr->mask)
 		val = dev->scancode_filters[fattr->type].mask;
 	else
 		val = dev->scancode_filters[fattr->type].data;
@@ -1069,7 +1073,7 @@ static ssize_t store_filter(struct device *device,
 		return ret;
 
 	/* Scancode filter not supported (but still accept 0) */
-	if (!dev->s_filter)
+	if (!dev->s_filter && fattr->type != RC_FILTER_NORMAL)
 		return val ? -EINVAL : count;
 
 	mutex_lock(&dev->lock);
@@ -1081,9 +1085,11 @@ static ssize_t store_filter(struct device *device,
 		local_filter.mask = val;
 	else
 		local_filter.data = val;
-	ret = dev->s_filter(dev, fattr->type, &local_filter);
-	if (ret < 0)
-		goto unlock;
+	if (dev->s_filter) {
+		ret = dev->s_filter(dev, fattr->type, &local_filter);
+		if (ret < 0)
+			goto unlock;
+	}
 
 	/* Success, commit the new filter */
 	*filter = local_filter;
-- 
1.8.3.2

[PATCH 2/5] rc: abstract access to allowed/enabled protocols

[James Hogan]

The allowed and enabled protocol masks need to be expanded to be per
filter type in order to support wakeup filter protocol selection. To
ease that process abstract access to the rc_dev::allowed_protos and
rc_dev::enabled_protocols members with inline functions.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: "Bruno Prémont" <bonbons@linux-vserver.org>
Cc: Maxim Levitsky <maximlevitsky@gmail.com>
Cc: Sean Young <sean@mess.org>
Cc: "David Härdeman" <david@hardeman.nu>
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: "Antti Seppälä" <a.seppala@gmail.com>
Cc: linux-media@vger.kernel.org
---
 drivers/hid/hid-picolcd_cir.c               |  2 +-
 drivers/media/common/siano/smsir.c          |  2 +-
 drivers/media/i2c/ir-kbd-i2c.c              |  4 ++--
 drivers/media/pci/cx23885/cx23885-input.c   |  2 +-
 drivers/media/pci/cx88/cx88-input.c         |  2 +-
 drivers/media/rc/ati_remote.c               |  2 +-
 drivers/media/rc/ene_ir.c                   |  2 +-
 drivers/media/rc/fintek-cir.c               |  2 +-
 drivers/media/rc/gpio-ir-recv.c             |  4 ++--
 drivers/media/rc/iguanair.c                 |  2 +-
 drivers/media/rc/imon.c                     |  7 ++++---
 drivers/media/rc/ir-jvc-decoder.c           |  2 +-
 drivers/media/rc/ir-lirc-codec.c            |  2 +-
 drivers/media/rc/ir-mce_kbd-decoder.c       |  2 +-
 drivers/media/rc/ir-nec-decoder.c           |  2 +-
 drivers/media/rc/ir-raw.c                   |  2 +-
 drivers/media/rc/ir-rc5-decoder.c           |  6 +++---
 drivers/media/rc/ir-rc5-sz-decoder.c        |  2 +-
 drivers/media/rc/ir-rc6-decoder.c           |  6 +++---
 drivers/media/rc/ir-sanyo-decoder.c         |  2 +-
 drivers/media/rc/ir-sharp-decoder.c         |  2 +-
 drivers/media/rc/ir-sony-decoder.c          | 10 +++++-----
 drivers/media/rc/ite-cir.c                  |  2 +-
 drivers/media/rc/mceusb.c                   |  2 +-
 drivers/media/rc/nuvoton-cir.c              |  2 +-
 drivers/media/rc/rc-loopback.c              |  2 +-
 drivers/media/rc/redrat3.c                  |  2 +-
 drivers/media/rc/st_rc.c                    |  2 +-
 drivers/media/rc/streamzap.c                |  2 +-
 drivers/media/rc/ttusbir.c                  |  2 +-
 drivers/media/rc/winbond-cir.c              |  2 +-
 drivers/media/usb/dvb-usb-v2/dvb_usb_core.c |  2 +-
 drivers/media/usb/dvb-usb/dvb-usb-remote.c  |  2 +-
 drivers/media/usb/em28xx/em28xx-input.c     |  8 ++++----
 drivers/media/usb/tm6000/tm6000-input.c     |  2 +-
 include/media/rc-core.h                     | 22 ++++++++++++++++++++++
 36 files changed, 73 insertions(+), 50 deletions(-)

diff --git a/drivers/hid/hid-picolcd_cir.c b/drivers/hid/hid-picolcd_cir.c
index 59d5eb1..cf1a9f1 100644
--- a/drivers/hid/hid-picolcd_cir.c
+++ b/drivers/hid/hid-picolcd_cir.c
@@ -114,7 +114,7 @@ int picolcd_init_cir(struct picolcd_data *data, struct hid_report *report)
 
 	rdev->priv             = data;
 	rdev->driver_type      = RC_DRIVER_IR_RAW;
-	rdev->allowed_protos   = RC_BIT_ALL;
+	rc_set_allowed_protocols(rdev, RC_BIT_ALL);
 	rdev->open             = picolcd_cir_open;
 	rdev->close            = picolcd_cir_close;
 	rdev->input_name       = data->hdev->name;
diff --git a/drivers/media/common/siano/smsir.c b/drivers/media/common/siano/smsir.c
index b8c5cad..6d7c0c8 100644
--- a/drivers/media/common/siano/smsir.c
+++ b/drivers/media/common/siano/smsir.c
@@ -88,7 +88,7 @@ int sms_ir_init(struct smscore_device_t *coredev)
 
 	dev->priv = coredev;
 	dev->driver_type = RC_DRIVER_IR_RAW;
-	dev->allowed_protos = RC_BIT_ALL;
+	rc_set_allowed_protocols(dev, RC_BIT_ALL);
 	dev->map_name = sms_get_board(board_id)->rc_codes;
 	dev->driver_name = MODULE_NAME;
 
diff --git a/drivers/media/i2c/ir-kbd-i2c.c b/drivers/media/i2c/ir-kbd-i2c.c
index 99ee456..c8fe135 100644
--- a/drivers/media/i2c/ir-kbd-i2c.c
+++ b/drivers/media/i2c/ir-kbd-i2c.c
@@ -431,8 +431,8 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	 * Initialize the other fields of rc_dev
 	 */
 	rc->map_name       = ir->ir_codes;
-	rc->allowed_protos = rc_type;
-	rc->enabled_protocols = rc_type;
+	rc_set_allowed_protocols(rc, rc_type);
+	rc_set_enabled_protocols(rc, rc_type);
 	if (!rc->driver_name)
 		rc->driver_name = MODULE_NAME;
 
diff --git a/drivers/media/pci/cx23885/cx23885-input.c b/drivers/media/pci/cx23885/cx23885-input.c
index 8a49e7c..097d0a0 100644
--- a/drivers/media/pci/cx23885/cx23885-input.c
+++ b/drivers/media/pci/cx23885/cx23885-input.c
@@ -346,7 +346,7 @@ int cx23885_input_init(struct cx23885_dev *dev)
 	}
 	rc->dev.parent = &dev->pci->dev;
 	rc->driver_type = driver_type;
-	rc->allowed_protos = allowed_protos;
+	rc_set_allowed_protocols(rc, allowed_protos);
 	rc->priv = kernel_ir;
 	rc->open = cx23885_input_ir_open;
 	rc->close = cx23885_input_ir_close;
diff --git a/drivers/media/pci/cx88/cx88-input.c b/drivers/media/pci/cx88/cx88-input.c
index f29e18c..f991696 100644
--- a/drivers/media/pci/cx88/cx88-input.c
+++ b/drivers/media/pci/cx88/cx88-input.c
@@ -469,7 +469,7 @@ int cx88_ir_init(struct cx88_core *core, struct pci_dev *pci)
 		dev->timeout = 10 * 1000 * 1000; /* 10 ms */
 	} else {
 		dev->driver_type = RC_DRIVER_SCANCODE;
-		dev->allowed_protos = rc_type;
+		rc_set_allowed_protocols(dev, rc_type);
 	}
 
 	ir->core = core;
diff --git a/drivers/media/rc/ati_remote.c b/drivers/media/rc/ati_remote.c
index 4d6a63f..2df7c55 100644
--- a/drivers/media/rc/ati_remote.c
+++ b/drivers/media/rc/ati_remote.c
@@ -784,7 +784,7 @@ static void ati_remote_rc_init(struct ati_remote *ati_remote)
 
 	rdev->priv = ati_remote;
 	rdev->driver_type = RC_DRIVER_SCANCODE;
-	rdev->allowed_protos = RC_BIT_OTHER;
+	rc_set_allowed_protocols(rdev, RC_BIT_OTHER);
 	rdev->driver_name = "ati_remote";
 
 	rdev->open = ati_remote_rc_open;
diff --git a/drivers/media/rc/ene_ir.c b/drivers/media/rc/ene_ir.c
index c1444f8..fc9d23f 100644
--- a/drivers/media/rc/ene_ir.c
+++ b/drivers/media/rc/ene_ir.c
@@ -1059,7 +1059,7 @@ static int ene_probe(struct pnp_dev *pnp_dev, const struct pnp_device_id *id)
 		learning_mode_force = false;
 
 	rdev->driver_type = RC_DRIVER_IR_RAW;
-	rdev->allowed_protos = RC_BIT_ALL;
+	rc_set_allowed_protocols(rdev, RC_BIT_ALL);
 	rdev->priv = dev;
 	rdev->open = ene_open;
 	rdev->close = ene_close;
diff --git a/drivers/media/rc/fintek-cir.c b/drivers/media/rc/fintek-cir.c
index d6fa441..46b66e5 100644
--- a/drivers/media/rc/fintek-cir.c
+++ b/drivers/media/rc/fintek-cir.c
@@ -541,7 +541,7 @@ static int fintek_probe(struct pnp_dev *pdev, const struct pnp_device_id *dev_id
 	/* Set up the rc device */
 	rdev->priv = fintek;
 	rdev->driver_type = RC_DRIVER_IR_RAW;
-	rdev->allowed_protos = RC_BIT_ALL;
+	rc_set_allowed_protocols(rdev, RC_BIT_ALL);
 	rdev->open = fintek_open;
 	rdev->close = fintek_close;
 	rdev->input_name = FINTEK_DESCRIPTION;
diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 80c611c..29b5f89 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -145,9 +145,9 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 	rcdev->dev.parent = &pdev->dev;
 	rcdev->driver_name = GPIO_IR_DRIVER_NAME;
 	if (pdata->allowed_protos)
-		rcdev->allowed_protos = pdata->allowed_protos;
+		rc_set_allowed_protocols(rcdev, pdata->allowed_protos);
 	else
-		rcdev->allowed_protos = RC_BIT_ALL;
+		rc_set_allowed_protocols(rcdev, RC_BIT_ALL);
 	rcdev->map_name = pdata->map_name ?: RC_MAP_EMPTY;
 
 	gpio_dev->rcdev = rcdev;
diff --git a/drivers/media/rc/iguanair.c b/drivers/media/rc/iguanair.c
index a83519a..627ddfd 100644
--- a/drivers/media/rc/iguanair.c
+++ b/drivers/media/rc/iguanair.c
@@ -495,7 +495,7 @@ static int iguanair_probe(struct usb_interface *intf,
 	usb_to_input_id(ir->udev, &rc->input_id);
 	rc->dev.parent = &intf->dev;
 	rc->driver_type = RC_DRIVER_IR_RAW;
-	rc->allowed_protos = RC_BIT_ALL;
+	rc_set_allowed_protocols(rc, RC_BIT_ALL);
 	rc->priv = ir;
 	rc->open = iguanair_open;
 	rc->close = iguanair_close;
diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c
index 822b9f4..6f24e77 100644
--- a/drivers/media/rc/imon.c
+++ b/drivers/media/rc/imon.c
@@ -1017,7 +1017,7 @@ static int imon_ir_change_protocol(struct rc_dev *rc, u64 *rc_type)
 	unsigned char ir_proto_packet[] = {
 		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x86 };
 
-	if (*rc_type && !(*rc_type & rc->allowed_protos))
+	if (*rc_type && !rc_protocols_allowed(rc, *rc_type))
 		dev_warn(dev, "Looks like you're trying to use an IR protocol "
 			 "this device does not support\n");
 
@@ -1867,7 +1867,8 @@ static struct rc_dev *imon_init_rdev(struct imon_context *ictx)
 
 	rdev->priv = ictx;
 	rdev->driver_type = RC_DRIVER_SCANCODE;
-	rdev->allowed_protos = RC_BIT_OTHER | RC_BIT_RC6_MCE; /* iMON PAD or MCE */
+					/* iMON PAD or MCE */
+	rc_set_allowed_protocols(rdev, RC_BIT_OTHER | RC_BIT_RC6_MCE);
 	rdev->change_protocol = imon_ir_change_protocol;
 	rdev->driver_name = MOD_NAME;
 
@@ -1880,7 +1881,7 @@ static struct rc_dev *imon_init_rdev(struct imon_context *ictx)
 
 	if (ictx->product == 0xffdc) {
 		imon_get_ffdc_type(ictx);
-		rdev->allowed_protos = ictx->rc_type;
+		rc_set_allowed_protocols(rdev, ictx->rc_type);
 	}
 
 	imon_set_display_type(ictx);
diff --git a/drivers/media/rc/ir-jvc-decoder.c b/drivers/media/rc/ir-jvc-decoder.c
index 3948138..4ea62a1 100644
--- a/drivers/media/rc/ir-jvc-decoder.c
+++ b/drivers/media/rc/ir-jvc-decoder.c
@@ -47,7 +47,7 @@ static int ir_jvc_decode(struct rc_dev *dev, struct ir_raw_event ev)
 {
 	struct jvc_dec *data = &dev->raw->jvc;
 
-	if (!(dev->enabled_protocols & RC_BIT_JVC))
+	if (!rc_protocols_enabled(dev, RC_BIT_JVC))
 		return 0;
 
 	if (!is_timing_event(ev)) {
diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index ed2c8a1..d731da6 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -35,7 +35,7 @@ static int ir_lirc_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	struct lirc_codec *lirc = &dev->raw->lirc;
 	int sample;
 
-	if (!(dev->enabled_protocols & RC_BIT_LIRC))
+	if (!rc_protocols_enabled(dev, RC_BIT_LIRC))
 		return 0;
 
 	if (!dev->raw->lirc.drv || !dev->raw->lirc.drv->rbuf)
diff --git a/drivers/media/rc/ir-mce_kbd-decoder.c b/drivers/media/rc/ir-mce_kbd-decoder.c
index 9f3c9b5..0c55f79 100644
--- a/drivers/media/rc/ir-mce_kbd-decoder.c
+++ b/drivers/media/rc/ir-mce_kbd-decoder.c
@@ -216,7 +216,7 @@ static int ir_mce_kbd_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	u32 scancode;
 	unsigned long delay;
 
-	if (!(dev->enabled_protocols & RC_BIT_MCE_KBD))
+	if (!rc_protocols_enabled(dev, RC_BIT_MCE_KBD))
 		return 0;
 
 	if (!is_timing_event(ev)) {
diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c
index e687a42..9de1791 100644
--- a/drivers/media/rc/ir-nec-decoder.c
+++ b/drivers/media/rc/ir-nec-decoder.c
@@ -52,7 +52,7 @@ static int ir_nec_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	u8 address, not_address, command, not_command;
 	bool send_32bits = false;
 
-	if (!(dev->enabled_protocols & RC_BIT_NEC))
+	if (!rc_protocols_enabled(dev, RC_BIT_NEC))
 		return 0;
 
 	if (!is_timing_event(ev)) {
diff --git a/drivers/media/rc/ir-raw.c b/drivers/media/rc/ir-raw.c
index f0656fa..763c9d1 100644
--- a/drivers/media/rc/ir-raw.c
+++ b/drivers/media/rc/ir-raw.c
@@ -256,7 +256,7 @@ int ir_raw_event_register(struct rc_dev *dev)
 		return -ENOMEM;
 
 	dev->raw->dev = dev;
-	dev->enabled_protocols = ~0;
+	rc_set_enabled_protocols(dev, ~0);
 	rc = kfifo_alloc(&dev->raw->kfifo,
 			 sizeof(struct ir_raw_event) * MAX_IR_EVENT_SIZE,
 			 GFP_KERNEL);
diff --git a/drivers/media/rc/ir-rc5-decoder.c b/drivers/media/rc/ir-rc5-decoder.c
index 1085e17..4295d9b 100644
--- a/drivers/media/rc/ir-rc5-decoder.c
+++ b/drivers/media/rc/ir-rc5-decoder.c
@@ -52,7 +52,7 @@ static int ir_rc5_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	u8 toggle;
 	u32 scancode;
 
-	if (!(dev->enabled_protocols & (RC_BIT_RC5 | RC_BIT_RC5X)))
+	if (!rc_protocols_enabled(dev, RC_BIT_RC5 | RC_BIT_RC5X))
 		return 0;
 
 	if (!is_timing_event(ev)) {
@@ -128,7 +128,7 @@ again:
 		if (data->wanted_bits == RC5X_NBITS) {
 			/* RC5X */
 			u8 xdata, command, system;
-			if (!(dev->enabled_protocols & RC_BIT_RC5X)) {
+			if (!rc_protocols_enabled(dev, RC_BIT_RC5X)) {
 				data->state = STATE_INACTIVE;
 				return 0;
 			}
@@ -145,7 +145,7 @@ again:
 		} else {
 			/* RC5 */
 			u8 command, system;
-			if (!(dev->enabled_protocols & RC_BIT_RC5)) {
+			if (!rc_protocols_enabled(dev, RC_BIT_RC5)) {
 				data->state = STATE_INACTIVE;
 				return 0;
 			}
diff --git a/drivers/media/rc/ir-rc5-sz-decoder.c b/drivers/media/rc/ir-rc5-sz-decoder.c
index 984e5b9..dc18b74 100644
--- a/drivers/media/rc/ir-rc5-sz-decoder.c
+++ b/drivers/media/rc/ir-rc5-sz-decoder.c
@@ -48,7 +48,7 @@ static int ir_rc5_sz_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	u8 toggle, command, system;
 	u32 scancode;
 
-	if (!(dev->enabled_protocols & RC_BIT_RC5_SZ))
+	if (!rc_protocols_enabled(dev, RC_BIT_RC5_SZ))
 		return 0;
 
 	if (!is_timing_event(ev)) {
diff --git a/drivers/media/rc/ir-rc6-decoder.c b/drivers/media/rc/ir-rc6-decoder.c
index 7cba7d3..cfbd64e 100644
--- a/drivers/media/rc/ir-rc6-decoder.c
+++ b/drivers/media/rc/ir-rc6-decoder.c
@@ -89,9 +89,9 @@ static int ir_rc6_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	u32 scancode;
 	u8 toggle;
 
-	if (!(dev->enabled_protocols &
-	      (RC_BIT_RC6_0 | RC_BIT_RC6_6A_20 | RC_BIT_RC6_6A_24 |
-	       RC_BIT_RC6_6A_32 | RC_BIT_RC6_MCE)))
+	if (!rc_protocols_enabled(dev, RC_BIT_RC6_0 | RC_BIT_RC6_6A_20 |
+				  RC_BIT_RC6_6A_24 | RC_BIT_RC6_6A_32 |
+				  RC_BIT_RC6_MCE))
 		return 0;
 
 	if (!is_timing_event(ev)) {
diff --git a/drivers/media/rc/ir-sanyo-decoder.c b/drivers/media/rc/ir-sanyo-decoder.c
index e1351ed..eb715f0 100644
--- a/drivers/media/rc/ir-sanyo-decoder.c
+++ b/drivers/media/rc/ir-sanyo-decoder.c
@@ -58,7 +58,7 @@ static int ir_sanyo_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	u32 scancode;
 	u8 address, command, not_command;
 
-	if (!(dev->enabled_protocols & RC_BIT_SANYO))
+	if (!rc_protocols_enabled(dev, RC_BIT_SANYO))
 		return 0;
 
 	if (!is_timing_event(ev)) {
diff --git a/drivers/media/rc/ir-sharp-decoder.c b/drivers/media/rc/ir-sharp-decoder.c
index 4895bc7..66d2039 100644
--- a/drivers/media/rc/ir-sharp-decoder.c
+++ b/drivers/media/rc/ir-sharp-decoder.c
@@ -48,7 +48,7 @@ static int ir_sharp_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	struct sharp_dec *data = &dev->raw->sharp;
 	u32 msg, echo, address, command, scancode;
 
-	if (!(dev->enabled_protocols & RC_BIT_SHARP))
+	if (!rc_protocols_enabled(dev, RC_BIT_SHARP))
 		return 0;
 
 	if (!is_timing_event(ev)) {
diff --git a/drivers/media/rc/ir-sony-decoder.c b/drivers/media/rc/ir-sony-decoder.c
index 29ab9c2..599c19a 100644
--- a/drivers/media/rc/ir-sony-decoder.c
+++ b/drivers/media/rc/ir-sony-decoder.c
@@ -45,8 +45,8 @@ static int ir_sony_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	u32 scancode;
 	u8 device, subdevice, function;
 
-	if (!(dev->enabled_protocols &
-	      (RC_BIT_SONY12 | RC_BIT_SONY15 | RC_BIT_SONY20)))
+	if (!rc_protocols_enabled(dev, RC_BIT_SONY12 | RC_BIT_SONY15 |
+				  RC_BIT_SONY20))
 		return 0;
 
 	if (!is_timing_event(ev)) {
@@ -124,7 +124,7 @@ static int ir_sony_decode(struct rc_dev *dev, struct ir_raw_event ev)
 
 		switch (data->count) {
 		case 12:
-			if (!(dev->enabled_protocols & RC_BIT_SONY12)) {
+			if (!rc_protocols_enabled(dev, RC_BIT_SONY12)) {
 				data->state = STATE_INACTIVE;
 				return 0;
 			}
@@ -133,7 +133,7 @@ static int ir_sony_decode(struct rc_dev *dev, struct ir_raw_event ev)
 			function  = bitrev8((data->bits >>  4) & 0xFE);
 			break;
 		case 15:
-			if (!(dev->enabled_protocols & RC_BIT_SONY15)) {
+			if (!rc_protocols_enabled(dev, RC_BIT_SONY15)) {
 				data->state = STATE_INACTIVE;
 				return 0;
 			}
@@ -142,7 +142,7 @@ static int ir_sony_decode(struct rc_dev *dev, struct ir_raw_event ev)
 			function  = bitrev8((data->bits >>  7) & 0xFE);
 			break;
 		case 20:
-			if (!(dev->enabled_protocols & RC_BIT_SONY20)) {
+			if (!rc_protocols_enabled(dev, RC_BIT_SONY20)) {
 				data->state = STATE_INACTIVE;
 				return 0;
 			}
diff --git a/drivers/media/rc/ite-cir.c b/drivers/media/rc/ite-cir.c
index 63b4225..ab24cc6 100644
--- a/drivers/media/rc/ite-cir.c
+++ b/drivers/media/rc/ite-cir.c
@@ -1563,7 +1563,7 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
 	/* set up ir-core props */
 	rdev->priv = itdev;
 	rdev->driver_type = RC_DRIVER_IR_RAW;
-	rdev->allowed_protos = RC_BIT_ALL;
+	rc_set_allowed_protocols(rdev, RC_BIT_ALL);
 	rdev->open = ite_open;
 	rdev->close = ite_close;
 	rdev->s_idle = ite_s_idle;
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index c01b4c1..5d8f3d4 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -1211,7 +1211,7 @@ static struct rc_dev *mceusb_init_rc_dev(struct mceusb_dev *ir)
 	rc->dev.parent = dev;
 	rc->priv = ir;
 	rc->driver_type = RC_DRIVER_IR_RAW;
-	rc->allowed_protos = RC_BIT_ALL;
+	rc_set_allowed_protocols(rc, RC_BIT_ALL);
 	rc->timeout = MS_TO_NS(100);
 	if (!ir->flags.no_tx) {
 		rc->s_tx_mask = mceusb_set_tx_mask;
diff --git a/drivers/media/rc/nuvoton-cir.c b/drivers/media/rc/nuvoton-cir.c
index b41e52e..bedf784 100644
--- a/drivers/media/rc/nuvoton-cir.c
+++ b/drivers/media/rc/nuvoton-cir.c
@@ -1038,7 +1038,7 @@ static int nvt_probe(struct pnp_dev *pdev, const struct pnp_device_id *dev_id)
 	/* Set up the rc device */
 	rdev->priv = nvt;
 	rdev->driver_type = RC_DRIVER_IR_RAW;
-	rdev->allowed_protos = RC_BIT_ALL;
+	rc_set_allowed_protocols(rdev, RC_BIT_ALL);
 	rdev->open = nvt_open;
 	rdev->close = nvt_close;
 	rdev->tx_ir = nvt_tx_ir;
diff --git a/drivers/media/rc/rc-loopback.c b/drivers/media/rc/rc-loopback.c
index 53d0282..0a88e0c 100644
--- a/drivers/media/rc/rc-loopback.c
+++ b/drivers/media/rc/rc-loopback.c
@@ -195,7 +195,7 @@ static int __init loop_init(void)
 	rc->map_name		= RC_MAP_EMPTY;
 	rc->priv		= &loopdev;
 	rc->driver_type		= RC_DRIVER_IR_RAW;
-	rc->allowed_protos	= RC_BIT_ALL;
+	rc_set_allowed_protocols(rc, RC_BIT_ALL);
 	rc->timeout		= 100 * 1000 * 1000; /* 100 ms */
 	rc->min_timeout		= 1;
 	rc->max_timeout		= UINT_MAX;
diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c
index a5d4f88..47cd373 100644
--- a/drivers/media/rc/redrat3.c
+++ b/drivers/media/rc/redrat3.c
@@ -922,7 +922,7 @@ static struct rc_dev *redrat3_init_rc_dev(struct redrat3_dev *rr3)
 	rc->dev.parent = dev;
 	rc->priv = rr3;
 	rc->driver_type = RC_DRIVER_IR_RAW;
-	rc->allowed_protos = RC_BIT_ALL;
+	rc_set_allowed_protocols(rc, RC_BIT_ALL);
 	rc->timeout = US_TO_NS(2750);
 	rc->tx_ir = redrat3_transmit_ir;
 	rc->s_tx_carrier = redrat3_set_tx_carrier;
diff --git a/drivers/media/rc/st_rc.c b/drivers/media/rc/st_rc.c
index 8f0cddb..22e4c1f 100644
--- a/drivers/media/rc/st_rc.c
+++ b/drivers/media/rc/st_rc.c
@@ -287,7 +287,7 @@ static int st_rc_probe(struct platform_device *pdev)
 	st_rc_hardware_init(rc_dev);
 
 	rdev->driver_type = RC_DRIVER_IR_RAW;
-	rdev->allowed_protos = RC_BIT_ALL;
+	rc_set_allowed_protocols(rdev, RC_BIT_ALL);
 	/* rx sampling rate is 10Mhz */
 	rdev->rx_resolution = 100;
 	rdev->timeout = US_TO_NS(MAX_SYMB_TIME);
diff --git a/drivers/media/rc/streamzap.c b/drivers/media/rc/streamzap.c
index d7b11e6..f4e0bc3 100644
--- a/drivers/media/rc/streamzap.c
+++ b/drivers/media/rc/streamzap.c
@@ -322,7 +322,7 @@ static struct rc_dev *streamzap_init_rc_dev(struct streamzap_ir *sz)
 	rdev->dev.parent = dev;
 	rdev->priv = sz;
 	rdev->driver_type = RC_DRIVER_IR_RAW;
-	rdev->allowed_protos = RC_BIT_ALL;
+	rc_set_allowed_protocols(rdev, RC_BIT_ALL);
 	rdev->driver_name = DRIVER_NAME;
 	rdev->map_name = RC_MAP_STREAMZAP;
 
diff --git a/drivers/media/rc/ttusbir.c b/drivers/media/rc/ttusbir.c
index d8de205..c5be38e 100644
--- a/drivers/media/rc/ttusbir.c
+++ b/drivers/media/rc/ttusbir.c
@@ -318,7 +318,7 @@ static int ttusbir_probe(struct usb_interface *intf,
 	usb_to_input_id(tt->udev, &rc->input_id);
 	rc->dev.parent = &intf->dev;
 	rc->driver_type = RC_DRIVER_IR_RAW;
-	rc->allowed_protos = RC_BIT_ALL;
+	rc_set_allowed_protocols(rc, RC_BIT_ALL);
 	rc->priv = tt;
 	rc->driver_name = DRIVER_NAME;
 	rc->map_name = RC_MAP_TT_1500;
diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
index 904baf4..a8b981f 100644
--- a/drivers/media/rc/winbond-cir.c
+++ b/drivers/media/rc/winbond-cir.c
@@ -1082,7 +1082,7 @@ wbcir_probe(struct pnp_dev *device, const struct pnp_device_id *dev_id)
 	data->dev->dev.parent = &device->dev;
 	data->dev->timeout = MS_TO_NS(100);
 	data->dev->rx_resolution = US_TO_NS(2);
-	data->dev->allowed_protos = RC_BIT_ALL;
+	rc_set_allowed_protocols(data->dev, RC_BIT_ALL);
 
 	err = rc_register_device(data->dev);
 	if (err)
diff --git a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
index 8a054d6..de02db8 100644
--- a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
+++ b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
@@ -164,7 +164,7 @@ static int dvb_usbv2_remote_init(struct dvb_usb_device *d)
 	dev->driver_name = (char *) d->props->driver_name;
 	dev->map_name = d->rc.map_name;
 	dev->driver_type = d->rc.driver_type;
-	dev->allowed_protos = d->rc.allowed_protos;
+	rc_set_allowed_protocols(dev, d->rc.allowed_protos);
 	dev->change_protocol = d->rc.change_protocol;
 	dev->priv = d;
 
diff --git a/drivers/media/usb/dvb-usb/dvb-usb-remote.c b/drivers/media/usb/dvb-usb/dvb-usb-remote.c
index 41bacff..4058aea 100644
--- a/drivers/media/usb/dvb-usb/dvb-usb-remote.c
+++ b/drivers/media/usb/dvb-usb/dvb-usb-remote.c
@@ -272,7 +272,7 @@ static int rc_core_dvb_usb_remote_init(struct dvb_usb_device *d)
 	dev->driver_name = d->props.rc.core.module_name;
 	dev->map_name = d->props.rc.core.rc_codes;
 	dev->change_protocol = d->props.rc.core.change_protocol;
-	dev->allowed_protos = d->props.rc.core.allowed_protos;
+	rc_set_allowed_protocols(dev, d->props.rc.core.allowed_protos);
 	dev->driver_type = d->props.rc.core.driver_type;
 	usb_to_input_id(d->udev, &dev->input_id);
 	dev->input_name = "IR-receiver inside an USB DVB receiver";
diff --git a/drivers/media/usb/em28xx/em28xx-input.c b/drivers/media/usb/em28xx/em28xx-input.c
index 048e5b6..3543c12 100644
--- a/drivers/media/usb/em28xx/em28xx-input.c
+++ b/drivers/media/usb/em28xx/em28xx-input.c
@@ -725,7 +725,7 @@ static int em28xx_ir_init(struct em28xx *dev)
 		case EM2820_BOARD_HAUPPAUGE_WINTV_USB_2:
 			rc->map_name = RC_MAP_HAUPPAUGE;
 			ir->get_key_i2c = em28xx_get_key_em_haup;
-			rc->allowed_protos = RC_BIT_RC5;
+			rc_set_allowed_protocols(rc, RC_BIT_RC5);
 			break;
 		case EM2820_BOARD_LEADTEK_WINFAST_USBII_DELUXE:
 			rc->map_name = RC_MAP_WINFAST_USBII_DELUXE;
@@ -741,7 +741,7 @@ static int em28xx_ir_init(struct em28xx *dev)
 		switch (dev->chip_id) {
 		case CHIP_ID_EM2860:
 		case CHIP_ID_EM2883:
-			rc->allowed_protos = RC_BIT_RC5 | RC_BIT_NEC;
+			rc_set_allowed_protocols(rc, RC_BIT_RC5 | RC_BIT_NEC);
 			ir->get_key = default_polling_getkey;
 			break;
 		case CHIP_ID_EM2884:
@@ -749,8 +749,8 @@ static int em28xx_ir_init(struct em28xx *dev)
 		case CHIP_ID_EM28174:
 		case CHIP_ID_EM28178:
 			ir->get_key = em2874_polling_getkey;
-			rc->allowed_protos = RC_BIT_RC5 | RC_BIT_NEC |
-					     RC_BIT_RC6_0;
+			rc_set_allowed_protocols(rc, RC_BIT_RC5 | RC_BIT_NEC |
+						 RC_BIT_RC6_0);
 			break;
 		default:
 			err = -ENODEV;
diff --git a/drivers/media/usb/tm6000/tm6000-input.c b/drivers/media/usb/tm6000/tm6000-input.c
index 8a6bbf1..d1af543 100644
--- a/drivers/media/usb/tm6000/tm6000-input.c
+++ b/drivers/media/usb/tm6000/tm6000-input.c
@@ -422,7 +422,7 @@ int tm6000_ir_init(struct tm6000_core *dev)
 	ir->rc = rc;
 
 	/* input setup */
-	rc->allowed_protos = RC_BIT_RC5 | RC_BIT_NEC;
+	rc_set_allowed_protocols(rc, RC_BIT_RC5 | RC_BIT_NEC);
 	/* Neded, in order to support NEC remotes with 24 or 32 bits */
 	rc->scanmask = 0xffff;
 	rc->priv = ir;
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 5e7197e..6f3c3d9 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -160,6 +160,28 @@ struct rc_dev {
 
 #define to_rc_dev(d) container_of(d, struct rc_dev, dev)
 
+static inline bool rc_protocols_allowed(struct rc_dev *rdev, u64 protos)
+{
+	return rdev->allowed_protos & protos;
+}
+
+/* should be called prior to registration or with mutex held */
+static inline void rc_set_allowed_protocols(struct rc_dev *rdev, u64 protos)
+{
+	rdev->allowed_protos = protos;
+}
+
+static inline bool rc_protocols_enabled(struct rc_dev *rdev, u64 protos)
+{
+	return rdev->enabled_protocols & protos;
+}
+
+/* should be called prior to registration or with mutex held */
+static inline void rc_set_enabled_protocols(struct rc_dev *rdev, u64 protos)
+{
+	rdev->enabled_protocols = protos;
+}
+
 /*
  * From rc-main.c
  * Those functions can be used on any type of Remote Controller. They
-- 
1.8.3.2

[PATCH 3/5] rc: add allowed/enabled wakeup protocol masks

[James Hogan]

Only a single allowed and enabled protocol mask currently exists in
struct rc_dev, however to support a separate wakeup filter protocol two
of each are needed, ideally as an array.

Therefore make both rc_dev::allowed_protos and rc_dev::enabled_protocols
arrays, update all users to reference the first element
(RC_FILTER_NORMAL), and add a couple more helper functions for drivers
to use for setting the allowed and enabled wakeup protocols.

We also rename allowed_protos to allowed_protocols while we're at it,
which is more consistent with enabled_protocols.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: Antti Seppälä <a.seppala@gmail.com>
Cc: linux-media@vger.kernel.org
---
 drivers/media/rc/rc-main.c | 10 +++++-----
 include/media/rc-core.h    | 32 ++++++++++++++++++++++++--------
 2 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 0a4f680..309d791 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -830,9 +830,9 @@ static ssize_t show_protocols(struct device *device,
 
 	mutex_lock(&dev->lock);
 
-	enabled = dev->enabled_protocols;
+	enabled = dev->enabled_protocols[RC_FILTER_NORMAL];
 	if (dev->driver_type == RC_DRIVER_SCANCODE)
-		allowed = dev->allowed_protos;
+		allowed = dev->allowed_protocols[RC_FILTER_NORMAL];
 	else if (dev->raw)
 		allowed = ir_raw_get_allowed_protocols();
 	else {
@@ -906,7 +906,7 @@ static ssize_t store_protocols(struct device *device,
 		ret = -EINVAL;
 		goto out;
 	}
-	type = dev->enabled_protocols;
+	type = dev->enabled_protocols[RC_FILTER_NORMAL];
 
 	while ((tmp = strsep((char **) &data, " \n")) != NULL) {
 		if (!*tmp)
@@ -964,7 +964,7 @@ static ssize_t store_protocols(struct device *device,
 		}
 	}
 
-	dev->enabled_protocols = type;
+	dev->enabled_protocols[RC_FILTER_NORMAL] = type;
 	IR_dprintk(1, "Current protocol(s): 0x%llx\n",
 		   (long long)type);
 
@@ -1316,7 +1316,7 @@ int rc_register_device(struct rc_dev *dev)
 		rc = dev->change_protocol(dev, &rc_type);
 		if (rc < 0)
 			goto out_raw;
-		dev->enabled_protocols = rc_type;
+		dev->enabled_protocols[RC_FILTER_NORMAL] = rc_type;
 	}
 
 	mutex_unlock(&dev->lock);
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 6f3c3d9..f165115 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -73,8 +73,10 @@ enum rc_filter_type {
  * @input_dev: the input child device used to communicate events to userspace
  * @driver_type: specifies if protocol decoding is done in hardware or software
  * @idle: used to keep track of RX state
- * @allowed_protos: bitmask with the supported RC_BIT_* protocols
- * @enabled_protocols: bitmask with the enabled RC_BIT_* protocols
+ * @allowed_protocols: bitmask with the supported RC_BIT_* protocols for each
+ *	filter type
+ * @enabled_protocols: bitmask with the enabled RC_BIT_* protocols for each
+ *	filter type
  * @scanmask: some hardware decoders are not capable of providing the full
  *	scancode to the application. As this is a hardware limit, we can't do
  *	anything with it. Yet, as the same keycode table can be used with other
@@ -124,8 +126,8 @@ struct rc_dev {
 	struct input_dev		*input_dev;
 	enum rc_driver_type		driver_type;
 	bool				idle;
-	u64				allowed_protos;
-	u64				enabled_protocols;
+	u64				allowed_protocols[RC_FILTER_MAX];
+	u64				enabled_protocols[RC_FILTER_MAX];
 	u32				users;
 	u32				scanmask;
 	void				*priv;
@@ -162,24 +164,38 @@ struct rc_dev {
 
 static inline bool rc_protocols_allowed(struct rc_dev *rdev, u64 protos)
 {
-	return rdev->allowed_protos & protos;
+	return rdev->allowed_protocols[RC_FILTER_NORMAL] & protos;
 }
 
 /* should be called prior to registration or with mutex held */
 static inline void rc_set_allowed_protocols(struct rc_dev *rdev, u64 protos)
 {
-	rdev->allowed_protos = protos;
+	rdev->allowed_protocols[RC_FILTER_NORMAL] = protos;
 }
 
 static inline bool rc_protocols_enabled(struct rc_dev *rdev, u64 protos)
 {
-	return rdev->enabled_protocols & protos;
+	return rdev->enabled_protocols[RC_FILTER_NORMAL] & protos;
 }
 
 /* should be called prior to registration or with mutex held */
 static inline void rc_set_enabled_protocols(struct rc_dev *rdev, u64 protos)
 {
-	rdev->enabled_protocols = protos;
+	rdev->enabled_protocols[RC_FILTER_NORMAL] = protos;
+}
+
+/* should be called prior to registration or with mutex held */
+static inline void rc_set_allowed_wakeup_protocols(struct rc_dev *rdev,
+						   u64 protos)
+{
+	rdev->allowed_protocols[RC_FILTER_WAKEUP] = protos;
+}
+
+/* should be called prior to registration or with mutex held */
+static inline void rc_set_enabled_wakeup_protocols(struct rc_dev *rdev,
+						   u64 protos)
+{
+	rdev->enabled_protocols[RC_FILTER_WAKEUP] = protos;
 }
 
 /*
-- 
1.8.3.2

[PATCH 4/5] rc: add wakeup_protocols sysfs file

[James Hogan]

Add a wakeup_protocols sysfs file which controls the new
rc_dev::enabled_protocols[RC_FILTER_WAKEUP], which is the mask of
protocols that are used for the wakeup filter.

A new RC driver callback change_wakeup_protocol() is called to change
the wakeup protocol mask.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: Antti Seppälä <a.seppala@gmail.com>
Cc: Rob Landley <rob@landley.net>
Cc: linux-media@vger.kernel.org
Cc: linux-doc@vger.kernel.org
---
 Documentation/ABI/testing/sysfs-class-rc           | 23 +++++-
 .../DocBook/media/v4l/remote_controllers.xml       | 20 +++++-
 drivers/media/rc/rc-main.c                         | 82 +++++++++++++---------
 include/media/rc-core.h                            |  3 +
 4 files changed, 90 insertions(+), 38 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-rc b/Documentation/ABI/testing/sysfs-class-rc
index c0e1d14..b65674d 100644
--- a/Documentation/ABI/testing/sysfs-class-rc
+++ b/Documentation/ABI/testing/sysfs-class-rc
@@ -61,6 +61,25 @@ Description:
 		an error.
 		This value may be reset to 0 if the current protocol is altered.
 
+What:		/sys/class/rc/rcN/wakeup_protocols
+Date:		Feb 2014
+KernelVersion:	3.15
+Contact:	Mauro Carvalho Chehab <m.chehab@samsung.com>
+Description:
+		Reading this file returns a list of available protocols to use
+		for the wakeup filter, something like:
+		    "rc5 rc6 nec jvc [sony]"
+		The enabled wakeup protocol is shown in [] brackets.
+		Writing "+proto" will add a protocol to the list of enabled
+		wakeup protocols.
+		Writing "-proto" will remove a protocol from the list of enabled
+		wakeup protocols.
+		Writing "proto" will use "proto" for wakeup events.
+		Writing "none" will disable wakeup.
+		Write fails with EINVAL if an invalid protocol combination or
+		unknown protocol name is used, or if wakeup is not supported by
+		the hardware.
+
 What:		/sys/class/rc/rcN/wakeup_filter
 Date:		Jan 2014
 KernelVersion:	3.15
@@ -74,7 +93,7 @@ Description:
 		scancodes which match the filter will wake the system from e.g.
 		suspend to RAM or power off.
 		Otherwise the write will fail with an error.
-		This value may be reset to 0 if the current protocol is altered.
+		This value may be reset to 0 if the wakeup protocol is altered.
 
 What:		/sys/class/rc/rcN/wakeup_filter_mask
 Date:		Jan 2014
@@ -89,4 +108,4 @@ Description:
 		scancodes which match the filter will wake the system from e.g.
 		suspend to RAM or power off.
 		Otherwise the write will fail with an error.
-		This value may be reset to 0 if the current protocol is altered.
+		This value may be reset to 0 if the wakeup protocol is altered.
diff --git a/Documentation/DocBook/media/v4l/remote_controllers.xml b/Documentation/DocBook/media/v4l/remote_controllers.xml
index c440a81..5124a6c 100644
--- a/Documentation/DocBook/media/v4l/remote_controllers.xml
+++ b/Documentation/DocBook/media/v4l/remote_controllers.xml
@@ -102,6 +102,22 @@ an error.</para>
 <para>This value may be reset to 0 if the current protocol is altered.</para>
 
 </section>
+<section id="sys_class_rc_rcN_wakeup_protocols">
+<title>/sys/class/rc/rcN/wakeup_protocols</title>
+<para>Reading this file returns a list of available protocols to use for the
+wakeup filter, something like:</para>
+<para><constant>rc5 rc6 nec jvc [sony]</constant></para>
+<para>The enabled wakeup protocol is shown in [] brackets.</para>
+<para>Writing "+proto" will add a protocol to the list of enabled wakeup
+protocols.</para>
+<para>Writing "-proto" will remove a protocol from the list of enabled wakeup
+protocols.</para>
+<para>Writing "proto" will use "proto" for wakeup events.</para>
+<para>Writing "none" will disable wakeup.</para>
+<para>Write fails with EINVAL if an invalid protocol combination or unknown
+protocol name is used, or if wakeup is not supported by the hardware.</para>
+
+</section>
 <section id="sys_class_rc_rcN_wakeup_filter">
 <title>/sys/class/rc/rcN/wakeup_filter</title>
 <para>Sets the scancode wakeup filter expected value.
@@ -112,7 +128,7 @@ to trigger a system wake event.</para>
 scancodes which match the filter will wake the system from e.g.
 suspend to RAM or power off.
 Otherwise the write will fail with an error.</para>
-<para>This value may be reset to 0 if the current protocol is altered.</para>
+<para>This value may be reset to 0 if the wakeup protocol is altered.</para>
 
 </section>
 <section id="sys_class_rc_rcN_wakeup_filter_mask">
@@ -125,7 +141,7 @@ expected value to trigger a system wake event.</para>
 scancodes which match the filter will wake the system from e.g.
 suspend to RAM or power off.
 Otherwise the write will fail with an error.</para>
-<para>This value may be reset to 0 if the current protocol is altered.</para>
+<para>This value may be reset to 0 if the wakeup protocol is altered.</para>
 </section>
 </section>
 
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 309d791..e6e3ec7 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -803,13 +803,38 @@ static struct {
 };
 
 /**
- * show_protocols() - shows the current IR protocol(s)
+ * struct rc_filter_attribute - Device attribute relating to a filter type.
+ * @attr:	Device attribute.
+ * @type:	Filter type.
+ * @mask:	false for filter value, true for filter mask.
+ */
+struct rc_filter_attribute {
+	struct device_attribute		attr;
+	enum rc_filter_type		type;
+	bool				mask;
+};
+#define to_rc_filter_attr(a) container_of(a, struct rc_filter_attribute, attr)
+
+#define RC_PROTO_ATTR(_name, _mode, _show, _store, _type)		\
+	struct rc_filter_attribute dev_attr_##_name = {			\
+		.attr = __ATTR(_name, _mode, _show, _store),		\
+		.type = (_type),					\
+	}
+#define RC_FILTER_ATTR(_name, _mode, _show, _store, _type, _mask)	\
+	struct rc_filter_attribute dev_attr_##_name = {			\
+		.attr = __ATTR(_name, _mode, _show, _store),		\
+		.type = (_type),					\
+		.mask = (_mask),					\
+	}
+
+/**
+ * show_protocols() - shows the current/wakeup IR protocol(s)
  * @device:	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(s).
- * it is trigged by reading /sys/class/rc/rc?/protocols.
+ * it is trigged by reading /sys/class/rc/rc?/[wakeup_]protocols.
  * It returns the protocol names of supported protocols.
  * Enabled protocols are printed in brackets.
  *
@@ -820,6 +845,7 @@ static ssize_t show_protocols(struct device *device,
 			      struct device_attribute *mattr, char *buf)
 {
 	struct rc_dev *dev = to_rc_dev(device);
+	struct rc_filter_attribute *fattr = to_rc_filter_attr(mattr);
 	u64 allowed, enabled;
 	char *tmp = buf;
 	int i;
@@ -830,9 +856,10 @@ static ssize_t show_protocols(struct device *device,
 
 	mutex_lock(&dev->lock);
 
-	enabled = dev->enabled_protocols[RC_FILTER_NORMAL];
-	if (dev->driver_type == RC_DRIVER_SCANCODE)
-		allowed = dev->allowed_protocols[RC_FILTER_NORMAL];
+	enabled = dev->enabled_protocols[fattr->type];
+	if (dev->driver_type == RC_DRIVER_SCANCODE ||
+	    fattr->type == RC_FILTER_WAKEUP)
+		allowed = dev->allowed_protocols[fattr->type];
 	else if (dev->raw)
 		allowed = ir_raw_get_allowed_protocols();
 	else {
@@ -864,14 +891,14 @@ static ssize_t show_protocols(struct device *device,
 }
 
 /**
- * store_protocols() - changes the current IR protocol(s)
+ * store_protocols() - changes the current/wakeup IR protocol(s)
  * @device:	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 for changing the IR protocol type.
- * It is trigged by writing to /sys/class/rc/rc?/protocols.
+ * It is trigged by writing to /sys/class/rc/rc?/[wakeup_]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".
@@ -888,12 +915,14 @@ static ssize_t store_protocols(struct device *device,
 			       size_t len)
 {
 	struct rc_dev *dev = to_rc_dev(device);
+	struct rc_filter_attribute *fattr = to_rc_filter_attr(mattr);
 	bool enable, disable;
 	const char *tmp;
 	u64 type;
 	u64 mask;
 	int rc, i, count = 0;
 	ssize_t ret;
+	int (*change_protocol)(struct rc_dev *dev, u64 *rc_type);
 
 	/* Device is being removed */
 	if (!dev)
@@ -906,7 +935,7 @@ static ssize_t store_protocols(struct device *device,
 		ret = -EINVAL;
 		goto out;
 	}
-	type = dev->enabled_protocols[RC_FILTER_NORMAL];
+	type = dev->enabled_protocols[fattr->type];
 
 	while ((tmp = strsep((char **) &data, " \n")) != NULL) {
 		if (!*tmp)
@@ -954,8 +983,10 @@ static ssize_t store_protocols(struct device *device,
 		goto out;
 	}
 
-	if (dev->change_protocol) {
-		rc = dev->change_protocol(dev, &type);
+	change_protocol = (fattr->type == RC_FILTER_NORMAL)
+		? dev->change_protocol : dev->change_wakeup_protocol;
+	if (change_protocol) {
+		rc = change_protocol(dev, &type);
 		if (rc < 0) {
 			IR_dprintk(1, "Error setting protocols to 0x%llx\n",
 				   (long long)type);
@@ -964,7 +995,7 @@ static ssize_t store_protocols(struct device *device,
 		}
 	}
 
-	dev->enabled_protocols[RC_FILTER_NORMAL] = type;
+	dev->enabled_protocols[fattr->type] = type;
 	IR_dprintk(1, "Current protocol(s): 0x%llx\n",
 		   (long long)type);
 
@@ -976,26 +1007,6 @@ out:
 }
 
 /**
- * struct rc_filter_attribute - Device attribute relating to a filter type.
- * @attr:	Device attribute.
- * @type:	Filter type.
- * @mask:	false for filter value, true for filter mask.
- */
-struct rc_filter_attribute {
-	struct device_attribute		attr;
-	enum rc_filter_type		type;
-	bool				mask;
-};
-#define to_rc_filter_attr(a) container_of(a, struct rc_filter_attribute, attr)
-
-#define RC_FILTER_ATTR(_name, _mode, _show, _store, _type, _mask)	\
-	struct rc_filter_attribute dev_attr_##_name = {			\
-		.attr = __ATTR(_name, _mode, _show, _store),		\
-		.type = (_type),					\
-		.mask = (_mask),					\
-	}
-
-/**
  * show_filter() - shows the current scancode filter value or mask
  * @device:	the device descriptor
  * @attr:	the device attribute struct
@@ -1128,8 +1139,10 @@ static int rc_dev_uevent(struct device *device, struct kobj_uevent_env *env)
 /*
  * Static device attribute struct with the sysfs attributes for IR's
  */
-static DEVICE_ATTR(protocols, S_IRUGO | S_IWUSR,
-		   show_protocols, store_protocols);
+static RC_PROTO_ATTR(protocols, S_IRUGO | S_IWUSR,
+		     show_protocols, store_protocols, RC_FILTER_NORMAL);
+static RC_PROTO_ATTR(wakeup_protocols, S_IRUGO | S_IWUSR,
+		     show_protocols, store_protocols, RC_FILTER_WAKEUP);
 static RC_FILTER_ATTR(filter, S_IRUGO|S_IWUSR,
 		      show_filter, store_filter, RC_FILTER_NORMAL, false);
 static RC_FILTER_ATTR(filter_mask, S_IRUGO|S_IWUSR,
@@ -1140,7 +1153,8 @@ 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[] = {
-	&dev_attr_protocols.attr,
+	&dev_attr_protocols.attr.attr,
+	&dev_attr_wakeup_protocols.attr.attr,
 	&dev_attr_filter.attr.attr,
 	&dev_attr_filter_mask.attr.attr,
 	&dev_attr_wakeup_filter.attr.attr,
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index f165115..0b9f890 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -97,6 +97,8 @@ enum rc_filter_type {
  * @tx_resolution: resolution (in ns) of output sampler
  * @scancode_filters: scancode filters (indexed by enum rc_filter_type)
  * @change_protocol: allow changing the protocol used on hardware decoders
+ * @change_wakeup_protocol: allow changing the protocol used for wakeup
+ *	filtering
  * @open: callback to allow drivers to enable polling/irq when IR input device
  *	is opened.
  * @close: callback to allow drivers to disable polling/irq when IR input device
@@ -145,6 +147,7 @@ struct rc_dev {
 	u32				tx_resolution;
 	struct rc_scancode_filter	scancode_filters[RC_FILTER_MAX];
 	int				(*change_protocol)(struct rc_dev *dev, u64 *rc_type);
+	int				(*change_wakeup_protocol)(struct rc_dev *dev, u64 *rc_type);
 	int				(*open)(struct rc_dev *dev);
 	void				(*close)(struct rc_dev *dev);
 	int				(*s_tx_mask)(struct rc_dev *dev, u32 mask);
-- 
1.8.3.2

[PATCH 5/5] rc-main: automatically refresh filter on protocol change

[James Hogan]

When either of the normal or wakeup filter protocols are changed,
refresh the corresponding scancode filter, i.e. try and set the same
scancode filter with the new protocol. If that fails clear the filter
instead.

If no protocol was selected the filter is just cleared, and if no
s_filter callback exists the filter is left unmodified.

Similarly clear the filter mask when the filter is set if no protocol is
currently selected.

This simplifies driver code which no longer has to explicitly worry
about modifying the filter on a protocol change. This also allows the
change_wakeup_protocol callback to be omitted entirely if there is only
a single available wakeup protocol at a time, since selecting no
protocol will automatically clear the wakeup filter, disabling wakeup.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: Antti Seppälä <a.seppala@gmail.com>
Cc: linux-media@vger.kernel.org
---
 drivers/media/rc/rc-main.c | 41 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index e6e3ec7..b1a6900 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -918,11 +918,12 @@ static ssize_t store_protocols(struct device *device,
 	struct rc_filter_attribute *fattr = to_rc_filter_attr(mattr);
 	bool enable, disable;
 	const char *tmp;
-	u64 type;
+	u64 old_type, type;
 	u64 mask;
 	int rc, i, count = 0;
 	ssize_t ret;
 	int (*change_protocol)(struct rc_dev *dev, u64 *rc_type);
+	struct rc_scancode_filter local_filter, *filter;
 
 	/* Device is being removed */
 	if (!dev)
@@ -935,7 +936,8 @@ static ssize_t store_protocols(struct device *device,
 		ret = -EINVAL;
 		goto out;
 	}
-	type = dev->enabled_protocols[fattr->type];
+	old_type = dev->enabled_protocols[fattr->type];
+	type = old_type;
 
 	while ((tmp = strsep((char **) &data, " \n")) != NULL) {
 		if (!*tmp)
@@ -999,6 +1001,36 @@ static ssize_t store_protocols(struct device *device,
 	IR_dprintk(1, "Current protocol(s): 0x%llx\n",
 		   (long long)type);
 
+	/*
+	 * If the protocol is changed the filter needs updating.
+	 * Try setting the same filter with the new protocol (if any).
+	 * Fall back to clearing the filter.
+	 */
+	filter = &dev->scancode_filters[fattr->type];
+	if (old_type != type && filter->mask) {
+		local_filter = *filter;
+		if (!type) {
+			/* no protocol => clear filter */
+			ret = -1;
+		} else if (!dev->s_filter) {
+			/* generic filtering => accept any filter */
+			ret = 0;
+		} else {
+			/* hardware filtering => try setting, otherwise clear */
+			ret = dev->s_filter(dev, fattr->type, &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);
+		}
+
+		/* commit the new filter */
+		*filter = local_filter;
+	}
+
 	ret = len;
 
 out:
@@ -1096,6 +1128,11 @@ 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 (ret < 0)
-- 
1.8.3.2

Re: [PATCH 0/5] rc: scancode filtering improvements

[Antti Seppälä]

On 1 March 2014 01:17, James Hogan <james.hogan@imgtec.com> wrote:
> These patches make some improvements relating to the recently added RC
> scancode filtering interface:
> - Patch 1 adds generic scancode filtering. This allows filtering to also
>   work for raw rc drivers and scancode drivers without filtering
>   capabilities.
> - Patches 2-4 future proof the sysfs API to allow a different wakeup
>   filter protocol to be set than the current protocol. A new
>   wakeup_protocols sysfs file is added which behaves similarly to the
>   protocols sysfs file but applies only to wakeup filters.
> - Finally patch 5 improves the driver interface so that changing either
>   the normal or wakeup protocol automatically causes the corresponding
>   filter to be refreshed to the driver, or failing that cleared. It also
>   ensures that the filter is turned off (and for wakeup that means
>   wakeup is disabled) if the protocol is set to none. This avoids the
>   driver having to maintain the filters, or even need a
>   change_wakeup_protocol() callback if there is only one wakeup protocol
>   allowed at a time.
>
> The patch "rc-main: store_filter: pass errors to userland" should
> probably be applied first.
>
> An updated img-ir v4 patchset which depends on this one will follow
> soon.
>
> Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
> Cc: "Bruno Prémont" <bonbons@linux-vserver.org>
> Cc: Maxim Levitsky <maximlevitsky@gmail.com>
> Cc: Sean Young <sean@mess.org>
> Cc: "David Härdeman" <david@hardeman.nu>
> Cc: Jiri Kosina <jkosina@suse.cz>
> Cc: "Antti Seppälä" <a.seppala@gmail.com>
>

After reviewing the series and porting my nuvoton changes to it I
haven't noticed any errors worth mentioning.
In fact I think this series is very well written and should be merged.

James, I hope you also have the time to submit the ir encoder series
for inclusion. :)

Reviewed-by: Antti Seppälä <a.seppala@gmail.com>

Re: [PATCH 0/5] rc: scancode filtering improvements

[James Hogan]

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

Hi Antti,

On Wednesday 05 March 2014 20:12:15 Antti Seppälä wrote:
> After reviewing the series and porting my nuvoton changes to it I
> haven't noticed any errors worth mentioning.
> In fact I think this series is very well written and should be merged.

Thanks for reviewing!

> James, I hope you also have the time to submit the ir encoder series
> for inclusion. :)

I did a little work on it the other day, including adding encode+loopback on 
filter change to loopback driver, fixing a few bugs and adding RC-5/RC-5X 
encode. I'll probably see what I can do to add partial encode to be sure the 
API doesn't fall short and then send a combined patchset.

Cheers
James

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

Re: [PATCH 1/5] rc-main: add generic scancode filtering

[David Härdeman]

On Fri, Feb 28, 2014 at 11:17:02PM +0000, James Hogan wrote:
>Add generic scancode filtering of RC input events, and fall back to
>permitting any RC_FILTER_NORMAL scancode filter to be set if no s_filter
>callback exists. This allows raw IR decoder events to be filtered, and
>potentially allows hardware decoders to set looser filters and rely on
>generic code to filter out the corner cases.

Hi James,

What's the purpose of providing the sw scancode filtering in the case where
there's no hardware filtering support at all?

(sorry that I'm replying so late...busy schedule :))

>
>Signed-off-by: James Hogan <james.hogan@imgtec.com>
>Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
>Cc: Antti Seppälä <a.seppala@gmail.com>
>Cc: linux-media@vger.kernel.org
>---
> drivers/media/rc/rc-main.c | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
>index 6448128..0a4f680 100644
>--- a/drivers/media/rc/rc-main.c
>+++ b/drivers/media/rc/rc-main.c
>@@ -633,6 +633,7 @@ 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;
>@@ -640,6 +641,11 @@ static void ir_do_keydown(struct rc_dev *dev, int scancode,
> 	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) {
>@@ -1019,9 +1025,7 @@ static ssize_t show_filter(struct device *device,
> 		return -EINVAL;
> 
> 	mutex_lock(&dev->lock);
>-	if (!dev->s_filter)
>-		val = 0;
>-	else if (fattr->mask)
>+	if (fattr->mask)
> 		val = dev->scancode_filters[fattr->type].mask;
> 	else
> 		val = dev->scancode_filters[fattr->type].data;
>@@ -1069,7 +1073,7 @@ static ssize_t store_filter(struct device *device,
> 		return ret;
> 
> 	/* Scancode filter not supported (but still accept 0) */
>-	if (!dev->s_filter)
>+	if (!dev->s_filter && fattr->type != RC_FILTER_NORMAL)
> 		return val ? -EINVAL : count;
> 
> 	mutex_lock(&dev->lock);
>@@ -1081,9 +1085,11 @@ static ssize_t store_filter(struct device *device,
> 		local_filter.mask = val;
> 	else
> 		local_filter.data = val;
>-	ret = dev->s_filter(dev, fattr->type, &local_filter);
>-	if (ret < 0)
>-		goto unlock;
>+	if (dev->s_filter) {
>+		ret = dev->s_filter(dev, fattr->type, &local_filter);
>+		if (ret < 0)
>+			goto unlock;
>+	}
> 
> 	/* Success, commit the new filter */
> 	*filter = local_filter;
>-- 
>1.8.3.2
>
>--
>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
>

-- 
David Härdeman

Re: [PATCH 1/5] rc-main: add generic scancode filtering

[James Hogan]

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

On Tuesday 25 March 2014 00:51:46 David Härdeman wrote:
> On Fri, Feb 28, 2014 at 11:17:02PM +0000, James Hogan wrote:
> >Add generic scancode filtering of RC input events, and fall back to
> >permitting any RC_FILTER_NORMAL scancode filter to be set if no s_filter
> >callback exists. This allows raw IR decoder events to be filtered, and
> >potentially allows hardware decoders to set looser filters and rely on
> >generic code to filter out the corner cases.
> 
> Hi James,
> 
> What's the purpose of providing the sw scancode filtering in the case where
> there's no hardware filtering support at all?

Consistency is probably the main reason, but I'll admit it's not perfectly 
consistent between generic/hardware filtering (mostly thanks to NEC scancode 
complexities), and I have no particular objection to dropping it if that isn't 
considered a good enough reason.

Here's the original discussion:
On Monday 10 February 2014 21:45:30 Antti Seppälä wrote:
> On 10 February 2014 11:58, James Hogan <james.hogan@imgtec.com> wrote:
> > On Saturday 08 February 2014 13:30:01 Antti Seppälä wrote:
> > > Also adding the scancode filter to it would
> > > demonstrate its usage.
> > 
> > To actually add filtering support to loopback would require either:
> > * raw-decoder/rc-core level scancode filtering for raw ir drivers
> > * OR loopback driver to encode like nuvoton and fuzzy match the IR
> > signals.
> 
> Rc-core level scancode filtering shouldn't be too hard to do right? If
> such would exist then it would provide a software fallback to other rc
> devices where hardware filtering isn't available. I'd love to see the
> sysfs filter and filter_mask files to have an effect on my nuvoton too


> (sorry that I'm replying so late...busy schedule :))

No problem :)

Cheers
James

> >Signed-off-by: James Hogan <james.hogan@imgtec.com>
> >Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
> >Cc: Antti Seppälä <a.seppala@gmail.com>
> >Cc: linux-media@vger.kernel.org
> >---
> >
> > drivers/media/rc/rc-main.c | 20 +++++++++++++-------
> > 1 file changed, 13 insertions(+), 7 deletions(-)
> >
> >diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> >index 6448128..0a4f680 100644
> >--- a/drivers/media/rc/rc-main.c
> >+++ b/drivers/media/rc/rc-main.c
> >@@ -633,6 +633,7 @@ 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;
> >
> >@@ -640,6 +641,11 @@ static void ir_do_keydown(struct rc_dev *dev, int
> >scancode,>
> > 	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) {
> >
> >@@ -1019,9 +1025,7 @@ static ssize_t show_filter(struct device *device,
> >
> > 		return -EINVAL;
> > 	
> > 	mutex_lock(&dev->lock);
> >
> >-	if (!dev->s_filter)
> >-		val = 0;
> >-	else if (fattr->mask)
> >+	if (fattr->mask)
> >
> > 		val = dev->scancode_filters[fattr->type].mask;
> > 	
> > 	else
> > 	
> > 		val = dev->scancode_filters[fattr->type].data;
> >
> >@@ -1069,7 +1073,7 @@ static ssize_t store_filter(struct device *device,
> >
> > 		return ret;
> > 	
> > 	/* Scancode filter not supported (but still accept 0) */
> >
> >-	if (!dev->s_filter)
> >+	if (!dev->s_filter && fattr->type != RC_FILTER_NORMAL)
> >
> > 		return val ? -EINVAL : count;
> > 	
> > 	mutex_lock(&dev->lock);
> >
> >@@ -1081,9 +1085,11 @@ static ssize_t store_filter(struct device *device,
> >
> > 		local_filter.mask = val;
> > 	
> > 	else
> > 	
> > 		local_filter.data = val;
> >
> >-	ret = dev->s_filter(dev, fattr->type, &local_filter);
> >-	if (ret < 0)
> >-		goto unlock;
> >+	if (dev->s_filter) {
> >+		ret = dev->s_filter(dev, fattr->type, &local_filter);
> >+		if (ret < 0)
> >+			goto unlock;
> >+	}
> >
> > 	/* Success, commit the new filter */
> > 	*filter = local_filter;

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

Re: [PATCH 1/5] rc-main: add generic scancode filtering

[David Härdeman]

On Tue, Mar 25, 2014 at 09:12:11AM +0000, James Hogan wrote:
>On Tuesday 25 March 2014 00:51:46 David Härdeman wrote:
>> On Fri, Feb 28, 2014 at 11:17:02PM +0000, James Hogan wrote:
>> >Add generic scancode filtering of RC input events, and fall back to
>> >permitting any RC_FILTER_NORMAL scancode filter to be set if no s_filter
>> >callback exists. This allows raw IR decoder events to be filtered, and
>> >potentially allows hardware decoders to set looser filters and rely on
>> >generic code to filter out the corner cases.
>> 
>> Hi James,
>> 
>> What's the purpose of providing the sw scancode filtering in the case where
>> there's no hardware filtering support at all?
>
>Consistency is probably the main reason, but I'll admit it's not perfectly 
>consistent between generic/hardware filtering (mostly thanks to NEC scancode 
>complexities), and I have no particular objection to dropping it if that isn't 
>considered a good enough reason.

I'm kind of sceptical...and given how difficult it is to remove
functionality that is in a released kernel...I think that particular
part (i.e. the software filtering) should be removed until it has had
further discussion...

>Here's the original discussion:
>On Monday 10 February 2014 21:45:30 Antti Seppälä wrote:
>> On 10 February 2014 11:58, James Hogan <james.hogan@imgtec.com> wrote:
>> > On Saturday 08 February 2014 13:30:01 Antti Seppälä wrote:
>> > > Also adding the scancode filter to it would
>> > > demonstrate its usage.
>> > 
>> > To actually add filtering support to loopback would require either:
>> > * raw-decoder/rc-core level scancode filtering for raw ir drivers
>> > * OR loopback driver to encode like nuvoton and fuzzy match the IR
>> > signals.
>> 
>> Rc-core level scancode filtering shouldn't be too hard to do right? If
>> such would exist then it would provide a software fallback to other rc
>> devices where hardware filtering isn't available. I'd love to see the
>> sysfs filter and filter_mask files to have an effect on my nuvoton too

I don't understand. What's the purpose of a "software fallback" for
scancode filtering? Antti?

-- 
David Härdeman

Re: [PATCH 1/5] rc-main: add generic scancode filtering

[Antti Seppälä]

On 26 March 2014 01:21, David Härdeman <david@hardeman.nu> wrote:
> On Tue, Mar 25, 2014 at 09:12:11AM +0000, James Hogan wrote:
>>On Tuesday 25 March 2014 00:51:46 David Härdeman wrote:
>>> On Fri, Feb 28, 2014 at 11:17:02PM +0000, James Hogan wrote:
>>> >Add generic scancode filtering of RC input events, and fall back to
>>> >permitting any RC_FILTER_NORMAL scancode filter to be set if no s_filter
>>> >callback exists. This allows raw IR decoder events to be filtered, and
>>> >potentially allows hardware decoders to set looser filters and rely on
>>> >generic code to filter out the corner cases.
>>>
>>> Hi James,
>>>
>>> What's the purpose of providing the sw scancode filtering in the case where
>>> there's no hardware filtering support at all?
>>
>>Consistency is probably the main reason, but I'll admit it's not perfectly
>>consistent between generic/hardware filtering (mostly thanks to NEC scancode
>>complexities), and I have no particular objection to dropping it if that isn't
>>considered a good enough reason.
>
> I'm kind of sceptical...and given how difficult it is to remove
> functionality that is in a released kernel...I think that particular
> part (i.e. the software filtering) should be removed until it has had
> further discussion...
>
>>Here's the original discussion:
>>On Monday 10 February 2014 21:45:30 Antti Seppälä wrote:
>>> On 10 February 2014 11:58, James Hogan <james.hogan@imgtec.com> wrote:
>>> > On Saturday 08 February 2014 13:30:01 Antti Seppälä wrote:
>>> > > Also adding the scancode filter to it would
>>> > > demonstrate its usage.
>>> >
>>> > To actually add filtering support to loopback would require either:
>>> > * raw-decoder/rc-core level scancode filtering for raw ir drivers
>>> > * OR loopback driver to encode like nuvoton and fuzzy match the IR
>>> > signals.
>>>
>>> Rc-core level scancode filtering shouldn't be too hard to do right? If
>>> such would exist then it would provide a software fallback to other rc
>>> devices where hardware filtering isn't available. I'd love to see the
>>> sysfs filter and filter_mask files to have an effect on my nuvoton too
>
> I don't understand. What's the purpose of a "software fallback" for
> scancode filtering? Antti?
>

Well since the ImgTec patches will create a new sysfs interface for
the HW scancode filtering I figured that it would be nice for it to
also function on devices which lack the hardware filtering
capabilities. Especially since it's only three lines of code. :)

Therefore I suggested the software fallback. At the time I had no clue
that there might be added complexities with nec scancodes.

So like James said it exists mainly for api consistency reasons.

-Antti

Re: [PATCH 1/5] rc-main: add generic scancode filtering

[David Härdeman]

On 2014-03-26 08:08, Antti Seppälä wrote:
> On 26 March 2014 01:21, David Härdeman <david@hardeman.nu> wrote:
>> On Tue, Mar 25, 2014 at 09:12:11AM +0000, James Hogan wrote:
>>> On Tuesday 25 March 2014 00:51:46 David Härdeman wrote:
>>>> What's the purpose of providing the sw scancode filtering in the 
>>>> case where
>>>> there's no hardware filtering support at all?
>>> 
>>> Consistency is probably the main reason, but I'll admit it's not 
>>> perfectly
>>> consistent between generic/hardware filtering (mostly thanks to NEC 
>>> scancode
>>> complexities), and I have no particular objection to dropping it if 
>>> that isn't
>>> considered a good enough reason.
>> 
>> I'm kind of sceptical...and given how difficult it is to remove
>> functionality that is in a released kernel...I think that particular
>> part (i.e. the software filtering) should be removed until it has had
>> further discussion...
...
>> I don't understand. What's the purpose of a "software fallback" for
>> scancode filtering? Antti?
>> 
> 
> Well since the ImgTec patches will create a new sysfs interface for
> the HW scancode filtering I figured that it would be nice for it to
> also function on devices which lack the hardware filtering
> capabilities. Especially since it's only three lines of code. :)
> 
> Therefore I suggested the software fallback. At the time I had no clue
> that there might be added complexities with nec scancodes.

It's not only NEC scancodes, the sw scancode filter is state that is 
changeable from user-space and which will require reader/writer 
synchronization during the RX path (which is the "hottest" path in 
rc-core). I've posted patches before which make the RX path lockless, 
this change makes complicates such changes.

Additionally, the provision of the sw fallback means that userspace has 
no idea if there is an actual hardware filter present or not, meaning 
that a userspace program that is aware of the scancode filter will 
always enable it.

So, I still think the SW part should be reverted, at least for now (i.e. 
the sysfs file should only be present if there is hardware support).

Mauro?

//David

Re: [PATCH 1/5] rc-main: add generic scancode filtering

[James Hogan]

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

On 26/03/14 13:44, David Härdeman wrote:
> On 2014-03-26 08:08, Antti Seppälä wrote:
>> On 26 March 2014 01:21, David Härdeman <david@hardeman.nu> wrote:
>>> On Tue, Mar 25, 2014 at 09:12:11AM +0000, James Hogan wrote:
>>>> On Tuesday 25 March 2014 00:51:46 David Härdeman wrote:
>>>>> What's the purpose of providing the sw scancode filtering in the
>>>>> case where
>>>>> there's no hardware filtering support at all?
>>>>
>>>> Consistency is probably the main reason, but I'll admit it's not
>>>> perfectly
>>>> consistent between generic/hardware filtering (mostly thanks to NEC
>>>> scancode
>>>> complexities), and I have no particular objection to dropping it if
>>>> that isn't
>>>> considered a good enough reason.
>>>
>>> I'm kind of sceptical...and given how difficult it is to remove
>>> functionality that is in a released kernel...I think that particular
>>> part (i.e. the software filtering) should be removed until it has had
>>> further discussion...
> ...
>>> I don't understand. What's the purpose of a "software fallback" for
>>> scancode filtering? Antti?
>>>
>>
>> Well since the ImgTec patches will create a new sysfs interface for
>> the HW scancode filtering I figured that it would be nice for it to
>> also function on devices which lack the hardware filtering
>> capabilities. Especially since it's only three lines of code. :)
>>
>> Therefore I suggested the software fallback. At the time I had no clue
>> that there might be added complexities with nec scancodes.
> 
> It's not only NEC scancodes, the sw scancode filter is state that is
> changeable from user-space and which will require reader/writer
> synchronization during the RX path (which is the "hottest" path in
> rc-core). I've posted patches before which make the RX path lockless,
> this change makes complicates such changes.
> 
> Additionally, the provision of the sw fallback means that userspace has
> no idea if there is an actual hardware filter present or not, meaning
> that a userspace program that is aware of the scancode filter will
> always enable it.
> 
> So, I still think the SW part should be reverted, at least for now (i.e.
> the sysfs file should only be present if there is hardware support).

I've prepared a revert patch (which also reverts a part of a later patch
which takes SW filtering into account when updating the filter on a
protocol change). I'll double check it works right later and submit.

Note that this still leaves the sysfs nodes there though, but if
!s_filter then they read as 0 and only accept the value 0 to be written
(mask == 0 represents no filtering).

Cheers
James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]