Re: [RFC PATCH 2/3] ir-rc5-sz: Add ir encoding support

[James Hogan <james.hogan@imgtec.com>]

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

On Tuesday 11 February 2014 20:14:19 Antti Seppälä wrote:
> On 10 February 2014 22:50, James Hogan <james.hogan@imgtec.com> wrote:
> >> > I suspect it needs some more space at the end too, to be sure that no
> >> > more bits afterwards are accepted.
> >> 
> >> I'm sorry but I'm not sure I completely understood what you meant
> >> here. For RC-5-SZ the entire scancode gets encoded and nothing more.
> >> Do you mean that the encoder should append some ir silence to the end
> >> result to make sure the ir sample has ended?
> > 
> > Yeh something like that. Certainly the raw decoders I've looked at expect
> > a
> > certain amount of space at the end to avoid decoding part of a longer
> > protocol (it's in the pulse distance helper as the trailer space timing).
> > Similarly the IMG hardware decoder has register fields for the free-time
> > to require at the end of the message.
> > 
> > In fact it becomes a bit awkward for the raw IR driver for the IMG
> > hardware
> > which uses edge interrupts, as it has to have a timeout to emit a final
> > repeat event after 150ms of inactivity, in order for the raw decoders to
> > accept it (unless you hold the button down in which case the repeat code
> > edges result in the long space).
> 
> Ok, I understand now.
> 
> I suppose I can append some IR silence to the encoded result. The
> trailer space timing seems like a good way to do it. I'll create new
> version of my patches sometime later.
> 
> Are you working on the wakeup protocol selector sysfs interface?

I gave it a try yesterday, but it's a bit of a work in progress at the moment.
It's also a bit more effort for img-ir to work properly with it, so I'd
probably just limit the allowed wakeup protocols to the enabled normal
protocol at first in img-ir.

Here's what I have (hopefully kmail won't corrupt it), feel free to take and
improve/fix it. I'm not keen on the invasiveness of the
allowed_protos/enabled_protocols change (which isn't complete), but it
should probably be abstracted at some point anyway.

Cheers
James

diff --git a/Documentation/ABI/testing/sysfs-class-rc b/Documentation/ABI/testing/sysfs-class-rc
index c0e1d14..1e4ecc8 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_protocol
+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 more than one protocol would be
+		enabled, an 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/drivers/media/rc/ir-jvc-decoder.c b/drivers/media/rc/ir-jvc-decoder.c
index 3948138..7fb9467 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 (!(dev->enabled_protocols[RC_FILTER_NORMAL] & RC_BIT_JVC))
 		return 0;
 
 	if (!is_timing_event(ev)) {
diff --git a/drivers/media/rc/ir-mce_kbd-decoder.c b/drivers/media/rc/ir-mce_kbd-decoder.c
index 9f3c9b5..bc93e11 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 (!(dev->enabled_protocols[RC_FILTER_NORMAL] & 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 5083ed6..f08010d 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 (!(dev->enabled_protocols[RC_FILTER_NORMAL] & 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 ae7b445..553937e 100644
--- a/drivers/media/rc/ir-raw.c
+++ b/drivers/media/rc/ir-raw.c
@@ -326,7 +326,7 @@ int ir_raw_event_register(struct rc_dev *dev)
 		return -ENOMEM;
 
 	dev->raw->dev = dev;
-	dev->enabled_protocols = ~0;
+	dev->enabled_protocols[RC_FILTER_NORMAL] = ~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 4e53a31..e0e5118 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 (!(dev->enabled_protocols[RC_FILTER_NORMAL] & (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 (!(dev->enabled_protocols[RC_FILTER_NORMAL] & 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 (!(dev->enabled_protocols[RC_FILTER_NORMAL] & 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 865fe84..47de4b3 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 (!(dev->enabled_protocols[RC_FILTER_NORMAL] & 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..e2cee1d 100644
--- a/drivers/media/rc/ir-rc6-decoder.c
+++ b/drivers/media/rc/ir-rc6-decoder.c
@@ -89,7 +89,7 @@ static int ir_rc6_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	u32 scancode;
 	u8 toggle;
 
-	if (!(dev->enabled_protocols &
+	if (!(dev->enabled_protocols[RC_FILTER_NORMAL] &
 	      (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;
diff --git a/drivers/media/rc/ir-sanyo-decoder.c b/drivers/media/rc/ir-sanyo-decoder.c
index 0a06205..352bd37 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 (!(dev->enabled_protocols[RC_FILTER_NORMAL] & 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 4c17be5..e4e66f6 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 (!(dev->enabled_protocols[RC_FILTER_NORMAL] & 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..1c586603 100644
--- a/drivers/media/rc/ir-sony-decoder.c
+++ b/drivers/media/rc/ir-sony-decoder.c
@@ -45,7 +45,7 @@ static int ir_sony_decode(struct rc_dev *dev, struct ir_raw_event ev)
 	u32 scancode;
 	u8 device, subdevice, function;
 
-	if (!(dev->enabled_protocols &
+	if (!(dev->enabled_protocols[RC_FILTER_NORMAL] &
 	      (RC_BIT_SONY12 | RC_BIT_SONY15 | RC_BIT_SONY20)))
 		return 0;
 
@@ -124,7 +124,8 @@ 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 (!(dev->enabled_protocols[RC_FILTER_NORMAL] &
+							RC_BIT_SONY12)) {
 				data->state = STATE_INACTIVE;
 				return 0;
 			}
@@ -133,7 +134,8 @@ 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 (!(dev->enabled_protocols[RC_FILTER_NORMAL] &
+							RC_BIT_SONY15)) {
 				data->state = STATE_INACTIVE;
 				return 0;
 			}
@@ -142,7 +144,8 @@ 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 (!(dev->enabled_protocols[RC_FILTER_NORMAL] &
+							RC_BIT_SONY20)) {
 				data->state = STATE_INACTIVE;
 				return 0;
 			}
diff --git a/drivers/media/rc/rc-loopback.c b/drivers/media/rc/rc-loopback.c
index 53d0282..b447045 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->allowed_protos[RC_FILTER_NORMAL]	= 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/rc-main.c b/drivers/media/rc/rc-main.c
index 25b3f8f..4594b61 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -793,13 +793,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.
  *
@@ -810,6 +835,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;
@@ -820,11 +846,13 @@ static ssize_t show_protocols(struct device *device,
 
 	mutex_lock(&dev->lock);
 
-	enabled = dev->enabled_protocols;
+	enabled = dev->enabled_protocols[fattr->type];
 	if (dev->driver_type == RC_DRIVER_SCANCODE)
-		allowed = dev->allowed_protos;
+		allowed = dev->allowed_protos[fattr->type];
 	else if (dev->raw)
-		allowed = ir_raw_get_allowed_protocols();
+		allowed = (fattr->type == RC_FILTER_NORMAL)
+			? ir_raw_get_allowed_protocols()
+			: 0;
 	else {
 		mutex_unlock(&dev->lock);
 		return -ENODEV;
@@ -854,14 +882,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".
@@ -878,6 +906,7 @@ 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;
@@ -896,7 +925,7 @@ static ssize_t store_protocols(struct device *device,
 		ret = -EINVAL;
 		goto out;
 	}
-	type = dev->enabled_protocols;
+	type = dev->enabled_protocols[fattr->type];
 
 	while ((tmp = strsep((char **) &data, " \n")) != NULL) {
 		if (!*tmp)
@@ -945,7 +974,7 @@ static ssize_t store_protocols(struct device *device,
 	}
 
 	if (dev->change_protocol) {
-		rc = dev->change_protocol(dev, &type);
+		rc = dev->change_protocol(dev, fattr->type, &type);
 		if (rc < 0) {
 			IR_dprintk(1, "Error setting protocols to 0x%llx\n",
 				   (long long)type);
@@ -954,7 +983,7 @@ static ssize_t store_protocols(struct device *device,
 		}
 	}
 
-	dev->enabled_protocols = type;
+	dev->enabled_protocols[fattr->type] = type;
 	IR_dprintk(1, "Current protocol(s): 0x%llx\n",
 		   (long long)type);
 
@@ -966,26 +995,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
@@ -1118,8 +1127,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_protocol, 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,
@@ -1130,7 +1141,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_protocol.attr.attr,
 	&dev_attr_filter.attr.attr,
 	&dev_attr_filter_mask.attr.attr,
 	&dev_attr_wakeup_filter.attr.attr,
@@ -1296,10 +1308,10 @@ int rc_register_device(struct rc_dev *dev)
 
 	if (dev->change_protocol) {
 		u64 rc_type = (1 << rc_map->rc_type);
-		rc = dev->change_protocol(dev, &rc_type);
+		rc = dev->change_protocol(dev, RC_FILTER_NORMAL, &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 7bd66be..eb47584 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -124,8 +124,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_protos[RC_FILTER_MAX];
+	u64				enabled_protocols[RC_FILTER_MAX];
 	u32				users;
 	u32				scanmask;
 	void				*priv;
@@ -142,7 +142,9 @@ struct rc_dev {
 	u32				rx_resolution;
 	u32				tx_resolution;
 	struct rc_scancode_filter	scancode_filters[RC_FILTER_MAX];
-	int				(*change_protocol)(struct rc_dev *dev, u64 *rc_type);
+	int				(*change_protocol)(struct rc_dev *dev,
+							   enum rc_filter_type type,
+							   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);

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