On Saturday 05 April 2014 00:06:06 David Härdeman wrote:
> The generic scancode filtering has questionable value and makes it
> impossible to determine from userspace if there is an actual
> scancode hw filter present or not.
>
> So revert the generic parts.
>
> Based on a patch from James Hogan <james.hogan@xxxxxxxxxx>, but this
> version also makes sure that only the valid sysfs files are created
> in the first place.
>
> v2: correct dev->s_filter check
>
> v3: move some parts over from the previous patch
>
> Signed-off-by: David Härdeman <david@xxxxxxxxxxx>
Acked-by: James Hogan <james.hogan@xxxxxxxxxx>
Thanks
James
> ---
> drivers/media/rc/rc-main.c | 88
> +++++++++++++++++++++++++++----------------- include/media/rc-core.h |
> 2 +
> 2 files changed, 55 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index ecbc20c..970b93d 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -633,19 +633,13 @@ 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;
> + bool new_event = (!dev->keypressed ||
> + dev->last_scancode != scancode ||
> + dev->last_toggle != toggle);
>
> 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) {
> @@ -1011,14 +1005,11 @@ static ssize_t store_protocols(struct device
> *device, set_filter = (fattr->type == RC_FILTER_NORMAL)
> ? dev->s_filter : dev->s_wakeup_filter;
>
> - if (old_type != type && filter->mask) {
> + if (set_filter && old_type != type && filter->mask) {
> local_filter = *filter;
> if (!type) {
> /* no protocol => clear filter */
> ret = -1;
> - } else if (!set_filter) {
> - /* generic filtering => accept any filter */
> - ret = 0;
> } else {
> /* hardware filtering => try setting, otherwise clear */
> ret = set_filter(dev, &local_filter);
> @@ -1027,8 +1018,7 @@ static ssize_t store_protocols(struct device *device,
> /* clear the filter */
> local_filter.data = 0;
> local_filter.mask = 0;
> - if (set_filter)
> - set_filter(dev, &local_filter);
> + set_filter(dev, &local_filter);
> }
>
> /* commit the new filter */
> @@ -1072,7 +1062,10 @@ static ssize_t show_filter(struct device *device,
> return -EINVAL;
>
> mutex_lock(&dev->lock);
> - if (fattr->mask)
> + if ((fattr->type == RC_FILTER_NORMAL && !dev->s_filter) ||
> + (fattr->type == RC_FILTER_WAKEUP && !dev->s_wakeup_filter))
> + val = 0;
> + else if (fattr->mask)
> val = dev->scancode_filters[fattr->type].mask;
> else
> val = dev->scancode_filters[fattr->type].data;
> @@ -1120,12 +1113,11 @@ static ssize_t store_filter(struct device *device,
> if (ret < 0)
> return ret;
>
> + /* Can the scancode filter be set? */
> set_filter = (fattr->type == RC_FILTER_NORMAL) ? dev->s_filter :
> dev->s_wakeup_filter;
> -
> - /* Scancode filter not supported (but still accept 0) */
> - if (!set_filter && fattr->type == RC_FILTER_WAKEUP)
> - return val ? -EINVAL : count;
> + if (!set_filter)
> + return -EINVAL;
>
> mutex_lock(&dev->lock);
>
> @@ -1143,11 +1135,9 @@ static ssize_t store_filter(struct device *device,
> goto unlock;
> }
>
> - if (set_filter) {
> - ret = set_filter(dev, &local_filter);
> - if (ret < 0)
> - goto unlock;
> - }
> + ret = set_filter(dev, &local_filter);
> + if (ret < 0)
> + goto unlock;
>
> /* Success, commit the new filter */
> *filter = local_filter;
> @@ -1199,27 +1189,45 @@ static RC_FILTER_ATTR(wakeup_filter,
> S_IRUGO|S_IWUSR, 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[] = {
> +static struct attribute *rc_dev_protocol_attrs[] = {
> &dev_attr_protocols.attr.attr,
> + NULL,
> +};
> +
> +static struct attribute_group rc_dev_protocol_attr_grp = {
> + .attrs = rc_dev_protocol_attrs,
> +};
> +
> +static struct attribute *rc_dev_wakeup_protocol_attrs[] = {
> &dev_attr_wakeup_protocols.attr.attr,
> + NULL,
> +};
> +
> +static struct attribute_group rc_dev_wakeup_protocol_attr_grp = {
> + .attrs = rc_dev_wakeup_protocol_attrs,
> +};
> +
> +static struct attribute *rc_dev_filter_attrs[] = {
> &dev_attr_filter.attr.attr,
> &dev_attr_filter_mask.attr.attr,
> - &dev_attr_wakeup_filter.attr.attr,
> - &dev_attr_wakeup_filter_mask.attr.attr,
> NULL,
> };
>
> -static struct attribute_group rc_dev_attr_grp = {
> - .attrs = rc_dev_attrs,
> +static struct attribute_group rc_dev_filter_attr_grp = {
> + .attrs = rc_dev_filter_attrs,
> };
>
> -static const struct attribute_group *rc_dev_attr_groups[] = {
> - &rc_dev_attr_grp,
> - NULL
> +static struct attribute *rc_dev_wakeup_filter_attrs[] = {
> + &dev_attr_wakeup_filter.attr.attr,
> + &dev_attr_wakeup_filter_mask.attr.attr,
> + NULL,
> +};
> +
> +static struct attribute_group rc_dev_wakeup_filter_attr_grp = {
> + .attrs = rc_dev_wakeup_filter_attrs,
> };
>
> static struct device_type rc_dev_type = {
> - .groups = rc_dev_attr_groups,
> .release = rc_dev_release,
> .uevent = rc_dev_uevent,
> };
> @@ -1276,7 +1284,7 @@ int rc_register_device(struct rc_dev *dev)
> static bool raw_init = false; /* raw decoders loaded? */
> struct rc_map *rc_map;
> const char *path;
> - int rc, devno;
> + int rc, devno, attr = 0;
>
> if (!dev || !dev->map_name)
> return -EINVAL;
> @@ -1304,6 +1312,16 @@ int rc_register_device(struct rc_dev *dev)
> return -ENOMEM;
> } while (test_and_set_bit(devno, ir_core_dev_number));
>
> + dev->dev.groups = dev->sysfs_groups;
> + dev->sysfs_groups[attr++] = &rc_dev_protocol_attr_grp;
> + if (dev->s_filter)
> + dev->sysfs_groups[attr++] = &rc_dev_filter_attr_grp;
> + if (dev->s_wakeup_filter)
> + dev->sysfs_groups[attr++] = &rc_dev_wakeup_filter_attr_grp;
> + if (dev->change_wakeup_protocol)
> + dev->sysfs_groups[attr++] = &rc_dev_wakeup_protocol_attr_grp;
> + dev->sysfs_groups[attr++] = NULL;
> +
> /*
> * Take the lock here, as the device sysfs node will appear
> * when device_add() is called, which may trigger an ir-keytable udev
> diff --git a/include/media/rc-core.h b/include/media/rc-core.h
> index 6dbc7c1..fde142e 100644
> --- a/include/media/rc-core.h
> +++ b/include/media/rc-core.h
> @@ -60,6 +60,7 @@ enum rc_filter_type {
> /**
> * struct rc_dev - represents a remote control device
> * @dev: driver model's view of this device
> + * @sysfs_groups: sysfs attribute groups
> * @input_name: name of the input child device
> * @input_phys: physical path to the input child device
> * @input_id: id of the input child device (struct input_id)
> @@ -117,6 +118,7 @@ enum rc_filter_type {
> */
> struct rc_dev {
> struct device dev;
> + const struct attribute_group *sysfs_groups[5];
> const char *input_name;
> const char *input_phys;
> struct input_id input_id;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
signature.asc
Description: This is a digitally signed message part.
