Re: [PATCH 05/11] rc-core: split dev->s_filter

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

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

Hi David,

On Saturday 29 March 2014 17:11:11 David Härdeman wrote:
> Overloading dev->s_filter to do two different functions (set wakeup filters
> and generic hardware filters) makes it impossible to tell what the
> hardware actually supports, so create a separate dev->s_wakeup_filter and
> make the distinction explicit.
> 
> Signed-off-by: David Härdeman <david@hardeman.nu>
> ---

> @@ -1121,9 +1126,11 @@ static ssize_t store_filter(struct device *device,
>  	if (ret < 0)
>  		return ret;
> 
> -	/* Scancode filter not supported (but still accept 0) */
> -	if (!dev->s_filter && fattr->type != RC_FILTER_NORMAL)
> -		return val ? -EINVAL : count;
> +	/* Can the scancode filter be set? */
> +	set_filter = (fattr->type == RC_FILTER_NORMAL)
> +		? dev->s_filter : dev->s_wakeup_filter;
> +	if (!set_filter)
> +		return -EINVAL;

Technically the removal of the "fattr->type != RC_FILTER_NORMAL" condition and 
returning -EINVAL rather than "val ? -EINVAL : count" should be in patch 6 
since it's for generic scancode filter support.


> -	if (dev->s_filter) {
> -		ret = dev->s_filter(dev, fattr->type, &local_filter);
> -		if (ret < 0)
> -			goto unlock;
> -	}
> +
> +	ret = set_filter(dev, &local_filter);
> +	if (ret < 0)
> +		goto unlock;

same here for removing the if condition.

Otherwise this patch looks okay to me.

Cheers
James

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