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

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

[-- 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 --]