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@xxxxxxxxxxx>
> ---
> @@ -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
JamesAttachment:
signature.asc
Description: This is a digitally signed message part.
