| Message ID | 20190114071922.25957-1-philipp.zabel@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 1950f462916edc9581168ca8d5882a8101e8bbcf |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | [v2] HID: core: simplify active collection tracking | expand |
Hi Philipp, On Mon, Jan 14, 2019 at 8:19 AM Philipp Zabel <philipp.zabel@gmail.com> wrote: > > Manually tracking an active collection to set collection parents is not > necessary, we just have to look one step back into the collection stack > to find the correct parent. Looks good to me. I'd better have Peter confirming this is OK from his point of view before applying it however. Cheers, Benjamin > > Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com> > --- > Changes since v1: > - Rebased onto commit ee46967fc6e ("HID: core: replace the collection tree > pointers with indices"). > --- > drivers/hid/hid-core.c | 13 ++----------- > include/linux/hid.h | 1 - > 2 files changed, 2 insertions(+), 12 deletions(-) > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index f9093dedf647..9993b692598f 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -173,8 +173,8 @@ static int open_collection(struct hid_parser *parser, unsigned type) > collection->type = type; > collection->usage = usage; > collection->level = parser->collection_stack_ptr - 1; > - collection->parent_idx = parser->active_collection_idx; > - parser->active_collection_idx = collection_index; > + collection->parent_idx = (collection->level == 0) ? -1 : > + parser->collection_stack[collection->level - 1]; > > if (type == HID_COLLECTION_APPLICATION) > parser->device->maxapplication++; > @@ -193,13 +193,6 @@ static int close_collection(struct hid_parser *parser) > return -EINVAL; > } > parser->collection_stack_ptr--; > - if (parser->active_collection_idx != -1) { > - struct hid_device *device = parser->device; > - struct hid_collection *c; > - > - c = &device->collection[parser->active_collection_idx]; > - parser->active_collection_idx = c->parent_idx; > - } > return 0; > } > > @@ -825,7 +818,6 @@ static int hid_scan_report(struct hid_device *hid) > return -ENOMEM; > > parser->device = hid; > - parser->active_collection_idx = -1; > hid->group = HID_GROUP_GENERIC; > > /* > @@ -1179,7 +1171,6 @@ int hid_open_report(struct hid_device *device) > } > > parser->device = device; > - parser->active_collection_idx = -1; > > end = start + size; > > diff --git a/include/linux/hid.h b/include/linux/hid.h > index 992bbb7196df..f9707d1dcb58 100644 > --- a/include/linux/hid.h > +++ b/include/linux/hid.h > @@ -658,7 +658,6 @@ struct hid_parser { > unsigned int *collection_stack; > unsigned int collection_stack_ptr; > unsigned int collection_stack_size; > - int active_collection_idx; /* device->collection */ > struct hid_device *device; > unsigned int scan_flags; > }; > -- > 2.20.1 >
On Mon, Jan 14, 2019 at 08:19:22AM +0100, Philipp Zabel wrote: > Manually tracking an active collection to set collection parents is not > necessary, we just have to look one step back into the collection stack > to find the correct parent. > > Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com> > --- > Changes since v1: > - Rebased onto commit ee46967fc6e ("HID: core: replace the collection tree > pointers with indices"). yep, much simpler, and it passes the test suite. Thanks! Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> Cheers, Peter > --- > drivers/hid/hid-core.c | 13 ++----------- > include/linux/hid.h | 1 - > 2 files changed, 2 insertions(+), 12 deletions(-) > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index f9093dedf647..9993b692598f 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -173,8 +173,8 @@ static int open_collection(struct hid_parser *parser, unsigned type) > collection->type = type; > collection->usage = usage; > collection->level = parser->collection_stack_ptr - 1; > - collection->parent_idx = parser->active_collection_idx; > - parser->active_collection_idx = collection_index; > + collection->parent_idx = (collection->level == 0) ? -1 : > + parser->collection_stack[collection->level - 1]; > > if (type == HID_COLLECTION_APPLICATION) > parser->device->maxapplication++; > @@ -193,13 +193,6 @@ static int close_collection(struct hid_parser *parser) > return -EINVAL; > } > parser->collection_stack_ptr--; > - if (parser->active_collection_idx != -1) { > - struct hid_device *device = parser->device; > - struct hid_collection *c; > - > - c = &device->collection[parser->active_collection_idx]; > - parser->active_collection_idx = c->parent_idx; > - } > return 0; > } > > @@ -825,7 +818,6 @@ static int hid_scan_report(struct hid_device *hid) > return -ENOMEM; > > parser->device = hid; > - parser->active_collection_idx = -1; > hid->group = HID_GROUP_GENERIC; > > /* > @@ -1179,7 +1171,6 @@ int hid_open_report(struct hid_device *device) > } > > parser->device = device; > - parser->active_collection_idx = -1; > > end = start + size; > > diff --git a/include/linux/hid.h b/include/linux/hid.h > index 992bbb7196df..f9707d1dcb58 100644 > --- a/include/linux/hid.h > +++ b/include/linux/hid.h > @@ -658,7 +658,6 @@ struct hid_parser { > unsigned int *collection_stack; > unsigned int collection_stack_ptr; > unsigned int collection_stack_size; > - int active_collection_idx; /* device->collection */ > struct hid_device *device; > unsigned int scan_flags; > }; > -- > 2.20.1 >
On Wed, Jan 16, 2019 at 4:13 AM Peter Hutterer <peter.hutterer@who-t.net> wrote: > > On Mon, Jan 14, 2019 at 08:19:22AM +0100, Philipp Zabel wrote: > > Manually tracking an active collection to set collection parents is not > > necessary, we just have to look one step back into the collection stack > > to find the correct parent. > > > > Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com> > > --- > > Changes since v1: > > - Rebased onto commit ee46967fc6e ("HID: core: replace the collection tree > > pointers with indices"). > > > yep, much simpler, and it passes the test suite. Thanks! > > Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> Thanks both of you. Applied to for-5.0/upstream-fixes Cheers, Benjamin > > Cheers, > Peter > > > > --- > > drivers/hid/hid-core.c | 13 ++----------- > > include/linux/hid.h | 1 - > > 2 files changed, 2 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > > index f9093dedf647..9993b692598f 100644 > > --- a/drivers/hid/hid-core.c > > +++ b/drivers/hid/hid-core.c > > @@ -173,8 +173,8 @@ static int open_collection(struct hid_parser *parser, unsigned type) > > collection->type = type; > > collection->usage = usage; > > collection->level = parser->collection_stack_ptr - 1; > > - collection->parent_idx = parser->active_collection_idx; > > - parser->active_collection_idx = collection_index; > > + collection->parent_idx = (collection->level == 0) ? -1 : > > + parser->collection_stack[collection->level - 1]; > > > > if (type == HID_COLLECTION_APPLICATION) > > parser->device->maxapplication++; > > @@ -193,13 +193,6 @@ static int close_collection(struct hid_parser *parser) > > return -EINVAL; > > } > > parser->collection_stack_ptr--; > > - if (parser->active_collection_idx != -1) { > > - struct hid_device *device = parser->device; > > - struct hid_collection *c; > > - > > - c = &device->collection[parser->active_collection_idx]; > > - parser->active_collection_idx = c->parent_idx; > > - } > > return 0; > > } > > > > @@ -825,7 +818,6 @@ static int hid_scan_report(struct hid_device *hid) > > return -ENOMEM; > > > > parser->device = hid; > > - parser->active_collection_idx = -1; > > hid->group = HID_GROUP_GENERIC; > > > > /* > > @@ -1179,7 +1171,6 @@ int hid_open_report(struct hid_device *device) > > } > > > > parser->device = device; > > - parser->active_collection_idx = -1; > > > > end = start + size; > > > > diff --git a/include/linux/hid.h b/include/linux/hid.h > > index 992bbb7196df..f9707d1dcb58 100644 > > --- a/include/linux/hid.h > > +++ b/include/linux/hid.h > > @@ -658,7 +658,6 @@ struct hid_parser { > > unsigned int *collection_stack; > > unsigned int collection_stack_ptr; > > unsigned int collection_stack_size; > > - int active_collection_idx; /* device->collection */ > > struct hid_device *device; > > unsigned int scan_flags; > > }; > > -- > > 2.20.1 > >
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index f9093dedf647..9993b692598f 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -173,8 +173,8 @@ static int open_collection(struct hid_parser *parser, unsigned type) collection->type = type; collection->usage = usage; collection->level = parser->collection_stack_ptr - 1; - collection->parent_idx = parser->active_collection_idx; - parser->active_collection_idx = collection_index; + collection->parent_idx = (collection->level == 0) ? -1 : + parser->collection_stack[collection->level - 1]; if (type == HID_COLLECTION_APPLICATION) parser->device->maxapplication++; @@ -193,13 +193,6 @@ static int close_collection(struct hid_parser *parser) return -EINVAL; } parser->collection_stack_ptr--; - if (parser->active_collection_idx != -1) { - struct hid_device *device = parser->device; - struct hid_collection *c; - - c = &device->collection[parser->active_collection_idx]; - parser->active_collection_idx = c->parent_idx; - } return 0; } @@ -825,7 +818,6 @@ static int hid_scan_report(struct hid_device *hid) return -ENOMEM; parser->device = hid; - parser->active_collection_idx = -1; hid->group = HID_GROUP_GENERIC; /* @@ -1179,7 +1171,6 @@ int hid_open_report(struct hid_device *device) } parser->device = device; - parser->active_collection_idx = -1; end = start + size; diff --git a/include/linux/hid.h b/include/linux/hid.h index 992bbb7196df..f9707d1dcb58 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -658,7 +658,6 @@ struct hid_parser { unsigned int *collection_stack; unsigned int collection_stack_ptr; unsigned int collection_stack_size; - int active_collection_idx; /* device->collection */ struct hid_device *device; unsigned int scan_flags; };
Manually tracking an active collection to set collection parents is not necessary, we just have to look one step back into the collection stack to find the correct parent. Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com> --- Changes since v1: - Rebased onto commit ee46967fc6e ("HID: core: replace the collection tree pointers with indices"). --- drivers/hid/hid-core.c | 13 ++----------- include/linux/hid.h | 1 - 2 files changed, 2 insertions(+), 12 deletions(-)