mbox series

[0/5] Touch Bar and ALS support for MacBook Pro's

Message ID 20210228012643.69944-1-ronald@innovation.ch
Headers show
Series Touch Bar and ALS support for MacBook Pro's | expand

Message

Ronald Tschalär Feb. 28, 2021, 1:26 a.m. UTC
This patch set provides Touch Bar and ALS support on MacBook Pro's
13,*, 14,*, and 15,*.

Some time a go an earlier version of these were posted to the list;
all code comments from there have been incorporated. In addition the
approach has been cleaned up, especially given that we now know how
the 15,* models are implemented, so that the ibridge driver is only
needed for the pre-15,* models and the ALS and Touch Bar drivers work
unchanged for all models.

Ronald Tschalär (5):
  HID: Recognize sensors with application collections too.
  iio: hid-sensor-als: Support change sensitivity in illuminance too.
  HID: core: Export some report item parsing functions.
  HID: apple-ibridge: Add Apple iBridge HID driver for T1 chip.
  HID: apple-touchbar - Add driver for the Touch Bar on MacBook Pro's.

 drivers/hid/Kconfig                |   26 +
 drivers/hid/Makefile               |    2 +
 drivers/hid/apple-ibridge.c        |  682 +++++++++++++
 drivers/hid/apple-ibridge.h        |   15 +
 drivers/hid/apple-touchbar.c       | 1523 ++++++++++++++++++++++++++++
 drivers/hid/hid-core.c             |   57 +-
 drivers/hid/hid-ids.h              |    1 +
 drivers/hid/hid-quirks.c           |    3 +
 drivers/hid/hid-sensor-hub.c       |    6 +-
 drivers/iio/light/hid-sensor-als.c |    8 +
 include/linux/hid.h                |    4 +
 11 files changed, 2302 insertions(+), 25 deletions(-)
 create mode 100644 drivers/hid/apple-ibridge.c
 create mode 100644 drivers/hid/apple-ibridge.h
 create mode 100644 drivers/hid/apple-touchbar.c

Comments

Andy Shevchenko March 1, 2021, 2:18 p.m. UTC | #1
On Sun, Feb 28, 2021 at 3:30 AM Ronald Tschalär <ronald@innovation.ch> wrote:
>

> These are useful to drivers that need to scan or parse reports

> themselves.


...

> -       while ((start = fetch_item(start, end, &item)) != NULL)

> +       while ((start = hid_fetch_item(start, end, &item)) != NULL)

>                 dispatch_type[item.type](parser, &item);


> -       while ((next = fetch_item(start, end, &item)) != NULL) {

> +       while ((next = hid_fetch_item(start, end, &item)) != NULL) {

>                 start = next;


I don't see the full picture, but perhaps you may also introduce
for_each_hid_item() or so.

-- 
With Best Regards,
Andy Shevchenko
Benjamin Tissoires March 1, 2021, 2:27 p.m. UTC | #2
On Mon, Mar 1, 2021 at 3:18 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>

> On Sun, Feb 28, 2021 at 3:30 AM Ronald Tschalär <ronald@innovation.ch> wrote:

> >

> > These are useful to drivers that need to scan or parse reports

> > themselves.

>

> ...

>

> > -       while ((start = fetch_item(start, end, &item)) != NULL)

> > +       while ((start = hid_fetch_item(start, end, &item)) != NULL)

> >                 dispatch_type[item.type](parser, &item);

>

> > -       while ((next = fetch_item(start, end, &item)) != NULL) {

> > +       while ((next = hid_fetch_item(start, end, &item)) != NULL) {

> >                 start = next;

>

> I don't see the full picture, but perhaps you may also introduce

> for_each_hid_item() or so.


Same here, I don't see the full picture, but I would suggest to not
export those functions at all.

From 4/5, I can see that you are using them in
appleib_find_collection(), which is only called by
appleib_add_device(), which in turn is always called with a parsed and
started HID device. Why can you not rely on the core parsing and
iterate over the already parsed hid_field?

Cheers,
Benjamin