Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH 1/3] HID: apple-ibridge: Add Apple iBridge HID driver for T1 chip.
From: Aditya Garg @ 2023-02-10 13:07 UTC (permalink / raw)
  To: Orlando Chamberlain
  Cc: Thomas Weißschuh, Jiri Kosina, jkosina@suse.cz,
	benjamin.tissoires@redhat.com, Andy Shevchenko,
	andy.shevchenko@gmail.com, LKML, linux-input@vger.kernel.org,
	ronald@innovation.ch, kekrby@gmail.com
In-Reply-To: <20230210232043.18483401@redecorated-mbp>


> Were the changes needed for these structs to be const in the
> linux-input tree for 6.3? If so then if you're applying the patches
> onto linus' tree that might be why there are errors about consts.

I’d want the maintainers comment on this. Imo, these 2 structures needn’t be constantified.

Also, it would be nice if we could get a review on the other 2 patches, so that a v2 can be prepared.

^ permalink raw reply

* Re: [PATCH 1/3] HID: apple-ibridge: Add Apple iBridge HID driver for T1 chip.
From: Orlando Chamberlain @ 2023-02-10 12:20 UTC (permalink / raw)
  To: Aditya Garg
  Cc: Thomas Weißschuh, Jiri Kosina, jkosina@suse.cz,
	benjamin.tissoires@redhat.com, Andy Shevchenko,
	andy.shevchenko@gmail.com, LKML, linux-input@vger.kernel.org,
	ronald@innovation.ch, kekrby@gmail.com
In-Reply-To: <B9E319F8-6047-40E5-BD9F-D90D6504AA9E@live.com>

On Fri, 10 Feb 2023 12:05:13 +0000
Aditya Garg <gargaditya08@live.com> wrote:

> > On 10-Feb-2023, at 10:26 AM, Thomas Weißschuh <thomas@t-8ch.de>
> > wrote:
> > 
> > Hi,
> > 
> > some comments inline.
> > 
> > On Fri, Feb 10, 2023 at 03:43:24AM +0000, Aditya Garg wrote:
> >   
> >> +
> >> +static struct {
> >> + unsigned int usage;
> >> + struct hid_device_id *dev_id;
> >> +} appleib_usage_map[] = {
> >> + /* Default iBridge configuration, key inputs and mode settings */
> >> + { 0x00010006, &appleib_sub_hid_ids[0] },
> >> + /* OS X iBridge configuration, digitizer inputs */
> >> + { 0x000D0005, &appleib_sub_hid_ids[0] },
> >> + /* All iBridge configurations, display/DFR settings */
> >> + { 0xFF120001, &appleib_sub_hid_ids[0] },
> >> + /* All iBridge configurations, ALS */
> >> + { 0x00200041, &appleib_sub_hid_ids[1] },
> >> +};  
> > 
> > const
> >   
> 
> Constantifying this results in compiler giving warnings
> 
> drivers/hid/apple-ibridge.c:78:23: warning: initialization discards
> 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
> 78 |         { 0x00200041, &appleib_sub_hid_ids[1] }, |
>         ^ drivers/hid/apple-ibridge.c: In function
> 'appleib_add_sub_dev': drivers/hid/apple-ibridge.c:363:29: warning:
> assignment discards 'const' qualifier from pointer target type
> [-Wdiscarded-qualifiers] 363 |         sub_hdev->ll_driver =
> &appleib_ll_driver; |                             ^
> drivers/hid/apple-ibridge.c: In function 'appleib_hid_probe':
> drivers/hid/apple-ibridge.c:436:12: error: expected '(' before
> 'hid_is_usb' 436 |         if hid_is_usb(hdev) |            ^~~~~~~~~~
>       |            (
> In file included from drivers/hid/apple-ibridge.c:48:
> drivers/hid/apple-ibridge.c: In function 'appleib_probe':
> drivers/hid/apple-ibridge.c:544:35: warning: passing argument 1 of
> '__hid_register_driver' discards 'const' qualifier from pointer
> target type [-Wdiscarded-qualifiers] 544 |         ret =
> hid_register_driver(&appleib_hid_driver); |
>         ^~~~~~~~~~~~~~~~~~~ ./include/linux/hid.h:898:31: note: in
> definition of macro 'hid_register_driver' 898 |
> __hid_register_driver(driver, THIS_MODULE, KBUILD_MODNAME) |
>                      ^~~~~~ ./include/linux/hid.h:893:47: note:
> expected 'struct hid_driver *' but argument is of type 'const struct
> hid_driver *' 893 | extern int __must_check
> __hid_register_driver(struct hid_driver *, |
>                      ^~~~~~~~~~~~~~~~~~~ drivers/hid/apple-ibridge.c:
> In function 'appleib_remove': drivers/hid/apple-ibridge.c:558:31:
> warning: passing argument 1 of 'hid_unregister_driver' discards
> 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
> 558 |         hid_unregister_driver(&appleib_hid_driver); |
>                     ^~~~~~~~~~~~~~~~~~~ ./include/linux/hid.h:900:35:
> note: expected 'struct hid_driver *' but argument is of type 'const
> struct hid_driver *' 900 | extern void hid_unregister_driver(struct
> hid_driver *); |
> ^~~~~~~~~~~~~~~~~~~ make[6]: *** [scripts/Makefile.build:250:
> drivers/hid/apple-ibridge.o] Error 1 make[5]: ***
> [scripts/Makefile.build:500: drivers/hid] Error 2 make[5]: ***
> Waiting for unfinished jobs….
> 
> Some warnings are also due to a typo in if and constantifying `static
> struct hid_driver`, although they probably can be fixed.
> 
> In short, Thomas, do you really want me to constantify the structure
> I am talking about in this email, as well `static struct hid_driver`?
> 

Were the changes needed for these structs to be const in the
linux-input tree for 6.3? If so then if you're applying the patches
onto linus' tree that might be why there are errors about consts.

^ permalink raw reply

* Re: [PATCH 1/3] HID: apple-ibridge: Add Apple iBridge HID driver for T1 chip.
From: Aditya Garg @ 2023-02-10 12:05 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Jiri Kosina, jkosina@suse.cz, benjamin.tissoires@redhat.com,
	Andy Shevchenko, andy.shevchenko@gmail.com, LKML,
	linux-input@vger.kernel.org, ronald@innovation.ch,
	kekrby@gmail.com, Orlando Chamberlain
In-Reply-To: <20230210045624.cjxroikmmvm3liij@t-8ch.de>



> On 10-Feb-2023, at 10:26 AM, Thomas Weißschuh <thomas@t-8ch.de> wrote:
> 
> Hi,
> 
> some comments inline.
> 
> On Fri, Feb 10, 2023 at 03:43:24AM +0000, Aditya Garg wrote:
> 
>> +
>> +static struct {
>> + unsigned int usage;
>> + struct hid_device_id *dev_id;
>> +} appleib_usage_map[] = {
>> + /* Default iBridge configuration, key inputs and mode settings */
>> + { 0x00010006, &appleib_sub_hid_ids[0] },
>> + /* OS X iBridge configuration, digitizer inputs */
>> + { 0x000D0005, &appleib_sub_hid_ids[0] },
>> + /* All iBridge configurations, display/DFR settings */
>> + { 0xFF120001, &appleib_sub_hid_ids[0] },
>> + /* All iBridge configurations, ALS */
>> + { 0x00200041, &appleib_sub_hid_ids[1] },
>> +};
> 
> const
> 

Constantifying this results in compiler giving warnings

drivers/hid/apple-ibridge.c:78:23: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
   78 |         { 0x00200041, &appleib_sub_hid_ids[1] },
      |                       ^
drivers/hid/apple-ibridge.c: In function 'appleib_add_sub_dev':
drivers/hid/apple-ibridge.c:363:29: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  363 |         sub_hdev->ll_driver = &appleib_ll_driver;
      |                             ^
drivers/hid/apple-ibridge.c: In function 'appleib_hid_probe':
drivers/hid/apple-ibridge.c:436:12: error: expected '(' before 'hid_is_usb'
  436 |         if hid_is_usb(hdev)
      |            ^~~~~~~~~~
      |            (
In file included from drivers/hid/apple-ibridge.c:48:
drivers/hid/apple-ibridge.c: In function 'appleib_probe':
drivers/hid/apple-ibridge.c:544:35: warning: passing argument 1 of '__hid_register_driver' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  544 |         ret = hid_register_driver(&appleib_hid_driver);
      |                                   ^~~~~~~~~~~~~~~~~~~
./include/linux/hid.h:898:31: note: in definition of macro 'hid_register_driver'
  898 |         __hid_register_driver(driver, THIS_MODULE, KBUILD_MODNAME)
      |                               ^~~~~~
./include/linux/hid.h:893:47: note: expected 'struct hid_driver *' but argument is of type 'const struct hid_driver *'
  893 | extern int __must_check __hid_register_driver(struct hid_driver *,
      |                                               ^~~~~~~~~~~~~~~~~~~
drivers/hid/apple-ibridge.c: In function 'appleib_remove':
drivers/hid/apple-ibridge.c:558:31: warning: passing argument 1 of 'hid_unregister_driver' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  558 |         hid_unregister_driver(&appleib_hid_driver);
      |                               ^~~~~~~~~~~~~~~~~~~
./include/linux/hid.h:900:35: note: expected 'struct hid_driver *' but argument is of type 'const struct hid_driver *'
  900 | extern void hid_unregister_driver(struct hid_driver *);
      |                                   ^~~~~~~~~~~~~~~~~~~
make[6]: *** [scripts/Makefile.build:250: drivers/hid/apple-ibridge.o] Error 1
make[5]: *** [scripts/Makefile.build:500: drivers/hid] Error 2
make[5]: *** Waiting for unfinished jobs….

Some warnings are also due to a typo in if and constantifying `static struct hid_driver`, although they probably can
be fixed.

In short, Thomas, do you really want me to constantify the structure I am talking about in this email, as well `static struct hid_driver`?


^ permalink raw reply

* Re: [PATCH v2 0/3] Add tri-state-key for oneplus6
From: Krzysztof Kozlowski @ 2023-02-10 11:34 UTC (permalink / raw)
  To: Gergo Koteles, Dmitry Torokhov, Andy Gross, Bjorn Andersson,
	Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Jiri Kosina,
	Benjamin Tissoires
  Cc: linux-input, linux-kernel, linux-arm-msm, devicetree,
	Caleb Connolly
In-Reply-To: <20230209232510.91431-1-soyer@irl.hu>

On 10/02/2023 00:25, Gergo Koteles wrote:
> V1 -> V2: rebase to qcom/for-next
> 
> Gergo Koteles (3):
>   Input: gpio-keys - add support for linux,input-value dts property
>   Input: add ABS_SND_PROFILE
>   arm64: dts: qcom: sdm845-oneplus: add tri-state-key

OK, here they are. Your mail threading is broken.

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2 3/3] arm64: dts: qcom: sdm845-oneplus: add tri-state-key
From: Krzysztof Kozlowski @ 2023-02-10 11:33 UTC (permalink / raw)
  To: Gergo Koteles, Dmitry Torokhov, Andy Gross, Bjorn Andersson,
	Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Jiri Kosina,
	Benjamin Tissoires
  Cc: linux-input, linux-kernel, linux-arm-msm, devicetree,
	Caleb Connolly
In-Reply-To: <20230209232556.91554-1-soyer@irl.hu>

On 10/02/2023 00:25, Gergo Koteles wrote:
> The tri-state-key is a sound profile switch found on the OnePlus 6,
> Android maps the states to "mute", "vibrate" and "ring". Expose them as
> ABS_SND_PROFILE events.
> The previous GPIO numbers were wrong. Update them to the correct
> ones.
> 
> Co-developed-by: Caleb Connolly <caleb@connolly.tech>
> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
> Signed-off-by: Gergo Koteles <soyer@irl.hu>

Where are other patches? I got only 3/3.

> ---
>  .../boot/dts/qcom/sdm845-oneplus-common.dtsi  | 39 ++++++++++++++++++-
>  1 file changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> index 64638ea94db7..e45d4fdead82 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> @@ -52,6 +52,43 @@ key-vol-up {
>  		};
>  	};
>  
> +	tri-state-key {
> +		compatible = "gpio-keys";
> +		label = "Tri-state key";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&tri_state_key_default>;

Missing blank line.

> +		state-top {

Does not look like you tested the DTS against bindings. Please run `make
dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst
for instructions).


Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH 0/3] Touch Bar and Keyboard backlight driver for Intel Macs
From: Andy Shevchenko @ 2023-02-10 11:12 UTC (permalink / raw)
  To: Orlando Chamberlain
  Cc: Aditya Garg, Jiri Kosina, jkosina@suse.cz,
	benjamin.tissoires@redhat.com, Andy Shevchenko, LKML,
	linux-input@vger.kernel.org, ronald@innovation.ch,
	kekrby@gmail.com, Thomas Weißschuh
In-Reply-To: <20230210214737.2ef942c6@redecorated-mbp>

On Fri, Feb 10, 2023 at 12:47 PM Orlando Chamberlain
<orlandoch.dev@gmail.com> wrote:
> On Fri, 10 Feb 2023 10:41:07 +0000
> Aditya Garg <gargaditya08@live.com> wrote:

...

> > > Quick observation. Do you miss the Co-developed-by: tags in the
> > > patches?
> >
> > Most of the changes are minor in the 1st and 2nd patch, we haven't
> > changed most of the code. The changes were written as per the
> > documentation given in
> > https://www.kernel.org/doc/html/latest/maintainer/modifying-patches.html
> >
> > Do you think a Co-developed-by is still required?
> >
> > The third patch was actually written by 2 people, so there is a
> > Co-developed-by there.
> >
>
> To add onto this, for patches 1 and 2, as we haven't been able to
> contact the original author (Ronald), I think the only ways we are
> allowed to make changes are either doing them in separate patches, or
> with the [name <email>: changes] tags. For the latter I thought you had
> to do a Signed-off-by after it, but given the changes aren't just to
> make the patch apply on a newer version, do you think the
> Co-developed-by tag is also needed?

I'm not insisting, just asking :-) So, if you think it's not needed, fine to me!

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH 0/3] Touch Bar and Keyboard backlight driver for Intel Macs
From: Orlando Chamberlain @ 2023-02-10 10:47 UTC (permalink / raw)
  To: Aditya Garg
  Cc: Andy Shevchenko, Jiri Kosina, jkosina@suse.cz,
	benjamin.tissoires@redhat.com, Andy Shevchenko, LKML,
	linux-input@vger.kernel.org, ronald@innovation.ch,
	kekrby@gmail.com, Thomas Weißschuh
In-Reply-To: <57D52CFB-228D-4071-94CB-D32883BF872A@live.com>

On Fri, 10 Feb 2023 10:41:07 +0000
Aditya Garg <gargaditya08@live.com> wrote:

> > 
> > Quick observation. Do you miss the Co-developed-by: tags in the
> > patches?  
> 
> Most of the changes are minor in the 1st and 2nd patch, we haven't
> changed most of the code. The changes were written as per the
> documentation given in
> https://www.kernel.org/doc/html/latest/maintainer/modifying-patches.html
> 
> Do you think a Co-developed-by is still required?
> 
> The third patch was actually written by 2 people, so there is a
> Co-developed-by there.
> 

To add onto this, for patches 1 and 2, as we haven't been able to
contact the original author (Ronald), I think the only ways we are
allowed to make changes are either doing them in separate patches, or
with the [name <email>: changes] tags. For the latter I thought you had
to do a Signed-off-by after it, but given the changes aren't just to
make the patch apply on a newer version, do you think the
Co-developed-by tag is also needed?

> > 
> > -- 
> > With Best Regards,
> > Andy Shevchenko  
> 


^ permalink raw reply

* Re: [PATCH 0/3] Touch Bar and Keyboard backlight driver for Intel Macs
From: Aditya Garg @ 2023-02-10 10:41 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jiri Kosina, jkosina@suse.cz, benjamin.tissoires@redhat.com,
	Andy Shevchenko, LKML, linux-input@vger.kernel.org,
	ronald@innovation.ch, kekrby@gmail.com, Orlando Chamberlain,
	Thomas Weißschuh
In-Reply-To: <CAHp75VcPPDTmpx9jpu3ZoaVH_xBgtaEbDQcJJdqcaXi1J+_q0A@mail.gmail.com>


> 
> Quick observation. Do you miss the Co-developed-by: tags in the patches?

Most of the changes are minor in the 1st and 2nd patch, we haven't changed
most of the code. The changes were written as per the documentation given
in https://www.kernel.org/doc/html/latest/maintainer/modifying-patches.html

Do you think a Co-developed-by is still required?

The third patch was actually written by 2 people, so there is a Co-developed-by there.

> 
> -- 
> With Best Regards,
> Andy Shevchenko


^ permalink raw reply

* Re: [PATCH 0/3] Touch Bar and Keyboard backlight driver for Intel Macs
From: Andy Shevchenko @ 2023-02-10 10:18 UTC (permalink / raw)
  To: Aditya Garg
  Cc: Jiri Kosina, jkosina@suse.cz, benjamin.tissoires@redhat.com,
	Andy Shevchenko, LKML, linux-input@vger.kernel.org,
	ronald@innovation.ch, kekrby@gmail.com, Orlando Chamberlain
In-Reply-To: <E5D8BEBA-3C5B-460F-BD2C-39470A793CC3@live.com>

On Fri, Feb 10, 2023 at 5:41 AM Aditya Garg <gargaditya08@live.com> wrote:
>
> Greetings from t2linux!
>
> 2 years ago an attempt was made to send the driver for the Touch Bar on
> MacBook Pros by Ronald Tschalär [1].
>
> Due to some issues pointed out by the maintainers upstream, unfortunately
> it didn't make it upstream. Now we at t2linux [2] have addressed those
> issues in this patchset and also improved the touchbar driver for the T2
> Macs. We also have added a new driver for keyboard backlight support on
> T2 MacBooks with Magic Keyboard.
>
> The first 2 patches of this patchset are the ones sent by Ronald before,
> with the issues addressed as pointed out in [1].
>
> The third patch introduces a new driver, apple-magic-backlight, which adds
> support for keyboard backlight on T2 MacBooks with the Magic Keyboard.
>
> Note: Apart from these drivers, for the T2 Macs, an additional driver
> shall be required to communicate with the T2 Security Chip, as the Touch
> Bar, the internal keyboard, trackpad, camera, ambient light sensor,
> headphone controls, and NCM Ethernet are accessed through a USB VHCI
> created with DMA channels to the "T2 Bridge Controller" 106b:1801 PCI
> device. A work in progress linux driver called apple-bce [3], or USB
> device passthrough to a Linux VM guest on a Windows host with Apple
> Bootcamp drivers can be used to get Linux these USB devices working and
> test these patches.
>
> [1] https://lore.kernel.org/lkml/20210228012643.69944-1-ronald@innovation.ch/
> [2] https://t2linux.org/
> [3] https://github.com/t2linux/apple-bce-drv

Quick observation. Do you miss the Co-developed-by: tags in the patches?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH 1/3] HID: apple-ibridge: Add Apple iBridge HID driver for T1 chip.
From: Benjamin Tissoires @ 2023-02-10  9:21 UTC (permalink / raw)
  To: Aditya Garg
  Cc: Thomas Weißschuh, Jiri Kosina, jkosina@suse.cz,
	Andy Shevchenko, andy.shevchenko@gmail.com, LKML,
	linux-input@vger.kernel.org, ronald@innovation.ch,
	kekrby@gmail.com, Orlando Chamberlain
In-Reply-To: <B472CABB-5D94-4E15-AE6D-EF8B8BF4CBF3@live.com>

On Fri, Feb 10, 2023 at 9:54 AM Aditya Garg <gargaditya08@live.com> wrote:
>
>
>
> > On 10-Feb-2023, at 2:09 PM, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
> >
> > On Fri, Feb 10, 2023 at 9:30 AM Aditya Garg <gargaditya08@live.com> wrote:
> >>
> >>
> >>
> >>> On 10-Feb-2023, at 10:26 AM, Thomas Weißschuh <thomas@t-8ch.de> wrote:
> >>>
> >>> Hi,
> >>>
> >>> some comments inline.
> >>>
> >>> On Fri, Feb 10, 2023 at 03:43:24AM +0000, Aditya Garg wrote:
> >>>> From: Ronald Tschalär <ronald@innovation.ch>
> >>>>
> >>>> The iBridge device provides access to several devices, including:
> >>>> - the Touch Bar
> >>>> - the iSight webcam
> >>>> - the light sensor
> >>>> - the fingerprint sensor
> >>>>
> >>>> This driver provides the core support for managing the iBridge device
> >>>> and the access to the underlying devices. In particular, the
> >>>> functionality for the touch bar and light sensor is exposed via USB HID
> >>>> interfaces, and on devices with the T1 chip one of the HID devices is
> >>>> used for both functions. So this driver creates virtual HID devices, one
> >>>> per top-level report collection on each HID device (for a total of 3
> >>>> virtual HID devices). The sub-drivers then bind to these virtual HID
> >>>> devices.
> >>>>
> >>>> This way the Touch Bar and ALS drivers can be kept in their own modules,
> >>>> while at the same time making them look very much like as if they were
> >>>> connected to the real HID devices. And those drivers then work (mostly)
> >>>> without further changes on MacBooks with the T2 chip that don't need
> >>>> this driver.
> >>>>
> >>>> Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
> >>>> [Kerem Karabay: convert to a platform driver]
> >>>> [Kerem Karabay: fix appleib_forward_int_op]
> >>>> [Kerem Karabay: rely on HID core's parsing in appleib_add_device]
> >>>> Signed-off-by: Kerem Karabay <kekrby@gmail.com>
> >>>> Signed-off-by: Aditya Garg <gargaditya08@live.com>
> >>>> ---
> >>>> drivers/hid/Kconfig         |  15 +
> >>>> drivers/hid/Makefile        |   1 +
> >>>> drivers/hid/apple-ibridge.c | 610 ++++++++++++++++++++++++++++++++++++
> >>>> drivers/hid/apple-ibridge.h |  15 +
> >>>> drivers/hid/hid-ids.h       |   1 +
> >>>> drivers/hid/hid-quirks.c    |   3 +
> >>>> 6 files changed, 645 insertions(+)
> >>>> create mode 100644 drivers/hid/apple-ibridge.c
> >>>> create mode 100644 drivers/hid/apple-ibridge.h
> >>>>
> >>>
> >>>>
> >>>> +}
> >>>> +
> >>>> +static int appleib_ll_raw_request(struct hid_device *hdev,
> >>>> +   unsigned char reportnum, __u8 *buf,
> >>>> +   size_t len, unsigned char rtype, int reqtype)
> >>>> +{
> >>>> + struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
> >>>> +
> >>>> + return hid_hw_raw_request(hdev_info->hdev, reportnum, buf, len, rtype,
> >>>> +   reqtype);
> >>>> +}
> >>>> +
> >>>> +static int appleib_ll_output_report(struct hid_device *hdev, __u8 *buf,
> >>>> +     size_t len)
> >>>> +{
> >>>> + struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
> >>>> +
> >>>> + return hid_hw_output_report(hdev_info->hdev, buf, len);
> >>>> +}
> >>>> +
> >>>> +static struct hid_ll_driver appleib_ll_driver = {
> >>>> + .start = appleib_ll_start,
> >>>> + .stop = appleib_ll_stop,
> >>>> + .open = appleib_ll_open,
> >>>> + .close = appleib_ll_close,
> >>>> + .power = appleib_ll_power,
> >>>> + .parse = appleib_ll_parse,
> >>>> + .request = appleib_ll_request,
> >>>> + .wait = appleib_ll_wait,
> >>>> + .raw_request = appleib_ll_raw_request,
> >>>> + .output_report = appleib_ll_output_report,
> >>>> +};
> >>>
> >>> const
> >>
> >> Are you sure about const here?
> >>
> >>>
> >>>> +
> >>>> + if (udev->actconfig->desc.bConfigurationValue != APPLEIB_BASIC_CONFIG) {
> >>>> + rc = usb_driver_set_configuration(udev, APPLEIB_BASIC_CONFIG);
> >>>> + return rc ? rc : -ENODEV;
> >>>> + }
> >>>> +
> >>>> + rc = hid_parse(hdev);
> >>>> + if (rc) {
> >>>> + hid_err(hdev, "ib: hid parse failed (%d)\n", rc);
> >>>> + goto error;
> >>>> + }
> >>>> +
> >>>> + rc = hid_hw_start(hdev, HID_CONNECT_DRIVER);
> >>>> + if (rc) {
> >>>> + hid_err(hdev, "ib: hw start failed (%d)\n", rc);
> >>>> + goto error;
> >>>> + }
> >>>> +
> >>>> + hdev_info = appleib_add_device(hdev);
> >>>> + if (IS_ERR(hdev_info)) {
> >>>> + rc = PTR_ERR(hdev_info);
> >>>> + goto stop_hw;
> >>>> + }
> >>>> +
> >>>> + hid_set_drvdata(hdev, hdev_info);
> >>>> +
> >>>> + rc = hid_hw_open(hdev);
> >>>> + if (rc) {
> >>>> + hid_err(hdev, "ib: failed to open hid: %d\n", rc);
> >>>> + goto remove_dev;
> >>>> + }
> >>>> +
> >>>> + return 0;
> >>>> +
> >>>> +remove_dev:
> >>>> + appleib_remove_device(hdev);
> >>>> +stop_hw:
> >>>> + hid_hw_stop(hdev);
> >>>> +error:
> >>>> + return rc;
> >>>> +}
> >>>> +
> >>>> +static void appleib_hid_remove(struct hid_device *hdev)
> >>>> +{
> >>>> + hid_hw_close(hdev);
> >>>> + appleib_remove_device(hdev);
> >>>> + hid_hw_stop(hdev);
> >>>> +}
> >>>> +
> >>>> +static const struct hid_device_id appleib_hid_ids[] = {
> >>>> + { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IBRIDGE) },
> >>>> + { },
> >>>> +};
> >>>> +
> >>>> +static struct hid_driver appleib_hid_driver = {
> >>>> + .name = "apple-ibridge-hid",
> >>>> + .id_table = appleib_hid_ids,
> >>>> + .probe = appleib_hid_probe,
> >>>> + .remove = appleib_hid_remove,
> >>>> + .raw_event = appleib_hid_raw_event,
> >>>> + .report_fixup = appleib_report_fixup,
> >>>> +#ifdef CONFIG_PM
> >>>> + .suspend = appleib_hid_suspend,
> >>>> + .resume = appleib_hid_resume,
> >>>> + .reset_resume = appleib_hid_reset_resume,
> >>>> +#endif
> >>>> +};
> >>>
> >>> const
> >>
> >> Are you sure you want to do const here, cause other hid-drivers are NOT using const.
> >
> > It is scheduled for 6.3:
> > https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-6.3/hid-core
> >
> > So yes, please make them const.
>
> Makes sense for `hid_ll_driver` as per the tree you send me, but I don’t see such a thing for
> `static struct hid_driver`. Is that scheduled as well?

Good point. I don't recall having seen such series now that you pinpoint this.

Thomas, is this something you have planned to submit?

Cheers,
Benjamin

>
> >
> > Cheers,
> > Benjamin
> >
> >>
> >>>> +
> >>>> +static struct appleib_device *appleib_alloc_device(struct platform_device *pdev)
> >>>> +{
> >>>> + struct appleib_device *ib_dev;
> >>>> + acpi_status sts;
>
>


^ permalink raw reply

* Re: [PATCH 1/3] HID: apple-ibridge: Add Apple iBridge HID driver for T1 chip.
From: Aditya Garg @ 2023-02-10  8:54 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Thomas Weißschuh, Jiri Kosina, jkosina@suse.cz,
	Andy Shevchenko, andy.shevchenko@gmail.com, LKML,
	linux-input@vger.kernel.org, ronald@innovation.ch,
	kekrby@gmail.com, Orlando Chamberlain
In-Reply-To: <CAO-hwJK=-9n885crMM8sHFGdNCZfjhqwuQXb2OtgMQiqaoZ66g@mail.gmail.com>



> On 10-Feb-2023, at 2:09 PM, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
> 
> On Fri, Feb 10, 2023 at 9:30 AM Aditya Garg <gargaditya08@live.com> wrote:
>> 
>> 
>> 
>>> On 10-Feb-2023, at 10:26 AM, Thomas Weißschuh <thomas@t-8ch.de> wrote:
>>> 
>>> Hi,
>>> 
>>> some comments inline.
>>> 
>>> On Fri, Feb 10, 2023 at 03:43:24AM +0000, Aditya Garg wrote:
>>>> From: Ronald Tschalär <ronald@innovation.ch>
>>>> 
>>>> The iBridge device provides access to several devices, including:
>>>> - the Touch Bar
>>>> - the iSight webcam
>>>> - the light sensor
>>>> - the fingerprint sensor
>>>> 
>>>> This driver provides the core support for managing the iBridge device
>>>> and the access to the underlying devices. In particular, the
>>>> functionality for the touch bar and light sensor is exposed via USB HID
>>>> interfaces, and on devices with the T1 chip one of the HID devices is
>>>> used for both functions. So this driver creates virtual HID devices, one
>>>> per top-level report collection on each HID device (for a total of 3
>>>> virtual HID devices). The sub-drivers then bind to these virtual HID
>>>> devices.
>>>> 
>>>> This way the Touch Bar and ALS drivers can be kept in their own modules,
>>>> while at the same time making them look very much like as if they were
>>>> connected to the real HID devices. And those drivers then work (mostly)
>>>> without further changes on MacBooks with the T2 chip that don't need
>>>> this driver.
>>>> 
>>>> Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
>>>> [Kerem Karabay: convert to a platform driver]
>>>> [Kerem Karabay: fix appleib_forward_int_op]
>>>> [Kerem Karabay: rely on HID core's parsing in appleib_add_device]
>>>> Signed-off-by: Kerem Karabay <kekrby@gmail.com>
>>>> Signed-off-by: Aditya Garg <gargaditya08@live.com>
>>>> ---
>>>> drivers/hid/Kconfig         |  15 +
>>>> drivers/hid/Makefile        |   1 +
>>>> drivers/hid/apple-ibridge.c | 610 ++++++++++++++++++++++++++++++++++++
>>>> drivers/hid/apple-ibridge.h |  15 +
>>>> drivers/hid/hid-ids.h       |   1 +
>>>> drivers/hid/hid-quirks.c    |   3 +
>>>> 6 files changed, 645 insertions(+)
>>>> create mode 100644 drivers/hid/apple-ibridge.c
>>>> create mode 100644 drivers/hid/apple-ibridge.h
>>>> 
>>> 
>>>> 
>>>> +}
>>>> +
>>>> +static int appleib_ll_raw_request(struct hid_device *hdev,
>>>> +   unsigned char reportnum, __u8 *buf,
>>>> +   size_t len, unsigned char rtype, int reqtype)
>>>> +{
>>>> + struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
>>>> +
>>>> + return hid_hw_raw_request(hdev_info->hdev, reportnum, buf, len, rtype,
>>>> +   reqtype);
>>>> +}
>>>> +
>>>> +static int appleib_ll_output_report(struct hid_device *hdev, __u8 *buf,
>>>> +     size_t len)
>>>> +{
>>>> + struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
>>>> +
>>>> + return hid_hw_output_report(hdev_info->hdev, buf, len);
>>>> +}
>>>> +
>>>> +static struct hid_ll_driver appleib_ll_driver = {
>>>> + .start = appleib_ll_start,
>>>> + .stop = appleib_ll_stop,
>>>> + .open = appleib_ll_open,
>>>> + .close = appleib_ll_close,
>>>> + .power = appleib_ll_power,
>>>> + .parse = appleib_ll_parse,
>>>> + .request = appleib_ll_request,
>>>> + .wait = appleib_ll_wait,
>>>> + .raw_request = appleib_ll_raw_request,
>>>> + .output_report = appleib_ll_output_report,
>>>> +};
>>> 
>>> const
>> 
>> Are you sure about const here?
>> 
>>> 
>>>> +
>>>> + if (udev->actconfig->desc.bConfigurationValue != APPLEIB_BASIC_CONFIG) {
>>>> + rc = usb_driver_set_configuration(udev, APPLEIB_BASIC_CONFIG);
>>>> + return rc ? rc : -ENODEV;
>>>> + }
>>>> +
>>>> + rc = hid_parse(hdev);
>>>> + if (rc) {
>>>> + hid_err(hdev, "ib: hid parse failed (%d)\n", rc);
>>>> + goto error;
>>>> + }
>>>> +
>>>> + rc = hid_hw_start(hdev, HID_CONNECT_DRIVER);
>>>> + if (rc) {
>>>> + hid_err(hdev, "ib: hw start failed (%d)\n", rc);
>>>> + goto error;
>>>> + }
>>>> +
>>>> + hdev_info = appleib_add_device(hdev);
>>>> + if (IS_ERR(hdev_info)) {
>>>> + rc = PTR_ERR(hdev_info);
>>>> + goto stop_hw;
>>>> + }
>>>> +
>>>> + hid_set_drvdata(hdev, hdev_info);
>>>> +
>>>> + rc = hid_hw_open(hdev);
>>>> + if (rc) {
>>>> + hid_err(hdev, "ib: failed to open hid: %d\n", rc);
>>>> + goto remove_dev;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +
>>>> +remove_dev:
>>>> + appleib_remove_device(hdev);
>>>> +stop_hw:
>>>> + hid_hw_stop(hdev);
>>>> +error:
>>>> + return rc;
>>>> +}
>>>> +
>>>> +static void appleib_hid_remove(struct hid_device *hdev)
>>>> +{
>>>> + hid_hw_close(hdev);
>>>> + appleib_remove_device(hdev);
>>>> + hid_hw_stop(hdev);
>>>> +}
>>>> +
>>>> +static const struct hid_device_id appleib_hid_ids[] = {
>>>> + { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IBRIDGE) },
>>>> + { },
>>>> +};
>>>> +
>>>> +static struct hid_driver appleib_hid_driver = {
>>>> + .name = "apple-ibridge-hid",
>>>> + .id_table = appleib_hid_ids,
>>>> + .probe = appleib_hid_probe,
>>>> + .remove = appleib_hid_remove,
>>>> + .raw_event = appleib_hid_raw_event,
>>>> + .report_fixup = appleib_report_fixup,
>>>> +#ifdef CONFIG_PM
>>>> + .suspend = appleib_hid_suspend,
>>>> + .resume = appleib_hid_resume,
>>>> + .reset_resume = appleib_hid_reset_resume,
>>>> +#endif
>>>> +};
>>> 
>>> const
>> 
>> Are you sure you want to do const here, cause other hid-drivers are NOT using const.
> 
> It is scheduled for 6.3:
> https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-6.3/hid-core
> 
> So yes, please make them const.

Makes sense for `hid_ll_driver` as per the tree you send me, but I don’t see such a thing for
`static struct hid_driver`. Is that scheduled as well?

> 
> Cheers,
> Benjamin
> 
>> 
>>>> +
>>>> +static struct appleib_device *appleib_alloc_device(struct platform_device *pdev)
>>>> +{
>>>> + struct appleib_device *ib_dev;
>>>> + acpi_status sts;



^ permalink raw reply

* Re: [PATCH 1/3] HID: apple-ibridge: Add Apple iBridge HID driver for T1 chip.
From: Benjamin Tissoires @ 2023-02-10  8:39 UTC (permalink / raw)
  To: Aditya Garg
  Cc: Thomas Weißschuh, Jiri Kosina, jkosina@suse.cz,
	Andy Shevchenko, andy.shevchenko@gmail.com, LKML,
	linux-input@vger.kernel.org, ronald@innovation.ch,
	kekrby@gmail.com, Orlando Chamberlain
In-Reply-To: <AC5AB6F5-4D65-4362-A8B8-A694D1371188@live.com>

On Fri, Feb 10, 2023 at 9:30 AM Aditya Garg <gargaditya08@live.com> wrote:
>
>
>
> > On 10-Feb-2023, at 10:26 AM, Thomas Weißschuh <thomas@t-8ch.de> wrote:
> >
> > Hi,
> >
> > some comments inline.
> >
> > On Fri, Feb 10, 2023 at 03:43:24AM +0000, Aditya Garg wrote:
> >> From: Ronald Tschalär <ronald@innovation.ch>
> >>
> >> The iBridge device provides access to several devices, including:
> >> - the Touch Bar
> >> - the iSight webcam
> >> - the light sensor
> >> - the fingerprint sensor
> >>
> >> This driver provides the core support for managing the iBridge device
> >> and the access to the underlying devices. In particular, the
> >> functionality for the touch bar and light sensor is exposed via USB HID
> >> interfaces, and on devices with the T1 chip one of the HID devices is
> >> used for both functions. So this driver creates virtual HID devices, one
> >> per top-level report collection on each HID device (for a total of 3
> >> virtual HID devices). The sub-drivers then bind to these virtual HID
> >> devices.
> >>
> >> This way the Touch Bar and ALS drivers can be kept in their own modules,
> >> while at the same time making them look very much like as if they were
> >> connected to the real HID devices. And those drivers then work (mostly)
> >> without further changes on MacBooks with the T2 chip that don't need
> >> this driver.
> >>
> >> Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
> >> [Kerem Karabay: convert to a platform driver]
> >> [Kerem Karabay: fix appleib_forward_int_op]
> >> [Kerem Karabay: rely on HID core's parsing in appleib_add_device]
> >> Signed-off-by: Kerem Karabay <kekrby@gmail.com>
> >> Signed-off-by: Aditya Garg <gargaditya08@live.com>
> >> ---
> >> drivers/hid/Kconfig         |  15 +
> >> drivers/hid/Makefile        |   1 +
> >> drivers/hid/apple-ibridge.c | 610 ++++++++++++++++++++++++++++++++++++
> >> drivers/hid/apple-ibridge.h |  15 +
> >> drivers/hid/hid-ids.h       |   1 +
> >> drivers/hid/hid-quirks.c    |   3 +
> >> 6 files changed, 645 insertions(+)
> >> create mode 100644 drivers/hid/apple-ibridge.c
> >> create mode 100644 drivers/hid/apple-ibridge.h
> >>
> >
> >>
> >> +}
> >> +
> >> +static int appleib_ll_raw_request(struct hid_device *hdev,
> >> +   unsigned char reportnum, __u8 *buf,
> >> +   size_t len, unsigned char rtype, int reqtype)
> >> +{
> >> + struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
> >> +
> >> + return hid_hw_raw_request(hdev_info->hdev, reportnum, buf, len, rtype,
> >> +   reqtype);
> >> +}
> >> +
> >> +static int appleib_ll_output_report(struct hid_device *hdev, __u8 *buf,
> >> +     size_t len)
> >> +{
> >> + struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
> >> +
> >> + return hid_hw_output_report(hdev_info->hdev, buf, len);
> >> +}
> >> +
> >> +static struct hid_ll_driver appleib_ll_driver = {
> >> + .start = appleib_ll_start,
> >> + .stop = appleib_ll_stop,
> >> + .open = appleib_ll_open,
> >> + .close = appleib_ll_close,
> >> + .power = appleib_ll_power,
> >> + .parse = appleib_ll_parse,
> >> + .request = appleib_ll_request,
> >> + .wait = appleib_ll_wait,
> >> + .raw_request = appleib_ll_raw_request,
> >> + .output_report = appleib_ll_output_report,
> >> +};
> >
> > const
>
> Are you sure about const here?
>
> >
> >> +
> >> + if (udev->actconfig->desc.bConfigurationValue != APPLEIB_BASIC_CONFIG) {
> >> + rc = usb_driver_set_configuration(udev, APPLEIB_BASIC_CONFIG);
> >> + return rc ? rc : -ENODEV;
> >> + }
> >> +
> >> + rc = hid_parse(hdev);
> >> + if (rc) {
> >> + hid_err(hdev, "ib: hid parse failed (%d)\n", rc);
> >> + goto error;
> >> + }
> >> +
> >> + rc = hid_hw_start(hdev, HID_CONNECT_DRIVER);
> >> + if (rc) {
> >> + hid_err(hdev, "ib: hw start failed (%d)\n", rc);
> >> + goto error;
> >> + }
> >> +
> >> + hdev_info = appleib_add_device(hdev);
> >> + if (IS_ERR(hdev_info)) {
> >> + rc = PTR_ERR(hdev_info);
> >> + goto stop_hw;
> >> + }
> >> +
> >> + hid_set_drvdata(hdev, hdev_info);
> >> +
> >> + rc = hid_hw_open(hdev);
> >> + if (rc) {
> >> + hid_err(hdev, "ib: failed to open hid: %d\n", rc);
> >> + goto remove_dev;
> >> + }
> >> +
> >> + return 0;
> >> +
> >> +remove_dev:
> >> + appleib_remove_device(hdev);
> >> +stop_hw:
> >> + hid_hw_stop(hdev);
> >> +error:
> >> + return rc;
> >> +}
> >> +
> >> +static void appleib_hid_remove(struct hid_device *hdev)
> >> +{
> >> + hid_hw_close(hdev);
> >> + appleib_remove_device(hdev);
> >> + hid_hw_stop(hdev);
> >> +}
> >> +
> >> +static const struct hid_device_id appleib_hid_ids[] = {
> >> + { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IBRIDGE) },
> >> + { },
> >> +};
> >> +
> >> +static struct hid_driver appleib_hid_driver = {
> >> + .name = "apple-ibridge-hid",
> >> + .id_table = appleib_hid_ids,
> >> + .probe = appleib_hid_probe,
> >> + .remove = appleib_hid_remove,
> >> + .raw_event = appleib_hid_raw_event,
> >> + .report_fixup = appleib_report_fixup,
> >> +#ifdef CONFIG_PM
> >> + .suspend = appleib_hid_suspend,
> >> + .resume = appleib_hid_resume,
> >> + .reset_resume = appleib_hid_reset_resume,
> >> +#endif
> >> +};
> >
> > const
>
> Are you sure you want to do const here, cause other hid-drivers are NOT using const.

It is scheduled for 6.3:
https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-6.3/hid-core

So yes, please make them const.

Cheers,
Benjamin

>
> >> +
> >> +static struct appleib_device *appleib_alloc_device(struct platform_device *pdev)
> >> +{
> >> + struct appleib_device *ib_dev;
> >> + acpi_status sts;
>


^ permalink raw reply

* Re: [PATCH 1/3] HID: apple-ibridge: Add Apple iBridge HID driver for T1 chip.
From: Aditya Garg @ 2023-02-10  8:30 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Jiri Kosina, jkosina@suse.cz, benjamin.tissoires@redhat.com,
	Andy Shevchenko, andy.shevchenko@gmail.com, LKML,
	linux-input@vger.kernel.org, ronald@innovation.ch,
	kekrby@gmail.com, Orlando Chamberlain
In-Reply-To: <20230210045624.cjxroikmmvm3liij@t-8ch.de>



> On 10-Feb-2023, at 10:26 AM, Thomas Weißschuh <thomas@t-8ch.de> wrote:
> 
> Hi,
> 
> some comments inline.
> 
> On Fri, Feb 10, 2023 at 03:43:24AM +0000, Aditya Garg wrote:
>> From: Ronald Tschalär <ronald@innovation.ch>
>> 
>> The iBridge device provides access to several devices, including:
>> - the Touch Bar
>> - the iSight webcam
>> - the light sensor
>> - the fingerprint sensor
>> 
>> This driver provides the core support for managing the iBridge device
>> and the access to the underlying devices. In particular, the
>> functionality for the touch bar and light sensor is exposed via USB HID
>> interfaces, and on devices with the T1 chip one of the HID devices is
>> used for both functions. So this driver creates virtual HID devices, one
>> per top-level report collection on each HID device (for a total of 3
>> virtual HID devices). The sub-drivers then bind to these virtual HID
>> devices.
>> 
>> This way the Touch Bar and ALS drivers can be kept in their own modules,
>> while at the same time making them look very much like as if they were
>> connected to the real HID devices. And those drivers then work (mostly)
>> without further changes on MacBooks with the T2 chip that don't need
>> this driver.
>> 
>> Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
>> [Kerem Karabay: convert to a platform driver]
>> [Kerem Karabay: fix appleib_forward_int_op]
>> [Kerem Karabay: rely on HID core's parsing in appleib_add_device]
>> Signed-off-by: Kerem Karabay <kekrby@gmail.com>
>> Signed-off-by: Aditya Garg <gargaditya08@live.com>
>> ---
>> drivers/hid/Kconfig         |  15 +
>> drivers/hid/Makefile        |   1 +
>> drivers/hid/apple-ibridge.c | 610 ++++++++++++++++++++++++++++++++++++
>> drivers/hid/apple-ibridge.h |  15 +
>> drivers/hid/hid-ids.h       |   1 +
>> drivers/hid/hid-quirks.c    |   3 +
>> 6 files changed, 645 insertions(+)
>> create mode 100644 drivers/hid/apple-ibridge.c
>> create mode 100644 drivers/hid/apple-ibridge.h
>> 
> 
>> 
>> +}
>> +
>> +static int appleib_ll_raw_request(struct hid_device *hdev,
>> +   unsigned char reportnum, __u8 *buf,
>> +   size_t len, unsigned char rtype, int reqtype)
>> +{
>> + struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
>> +
>> + return hid_hw_raw_request(hdev_info->hdev, reportnum, buf, len, rtype,
>> +   reqtype);
>> +}
>> +
>> +static int appleib_ll_output_report(struct hid_device *hdev, __u8 *buf,
>> +     size_t len)
>> +{
>> + struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
>> +
>> + return hid_hw_output_report(hdev_info->hdev, buf, len);
>> +}
>> +
>> +static struct hid_ll_driver appleib_ll_driver = {
>> + .start = appleib_ll_start,
>> + .stop = appleib_ll_stop,
>> + .open = appleib_ll_open,
>> + .close = appleib_ll_close,
>> + .power = appleib_ll_power,
>> + .parse = appleib_ll_parse,
>> + .request = appleib_ll_request,
>> + .wait = appleib_ll_wait,
>> + .raw_request = appleib_ll_raw_request,
>> + .output_report = appleib_ll_output_report,
>> +};
> 
> const

Are you sure about const here?

> 
>> +
>> + if (udev->actconfig->desc.bConfigurationValue != APPLEIB_BASIC_CONFIG) {
>> + rc = usb_driver_set_configuration(udev, APPLEIB_BASIC_CONFIG);
>> + return rc ? rc : -ENODEV;
>> + }
>> +
>> + rc = hid_parse(hdev);
>> + if (rc) {
>> + hid_err(hdev, "ib: hid parse failed (%d)\n", rc);
>> + goto error;
>> + }
>> +
>> + rc = hid_hw_start(hdev, HID_CONNECT_DRIVER);
>> + if (rc) {
>> + hid_err(hdev, "ib: hw start failed (%d)\n", rc);
>> + goto error;
>> + }
>> +
>> + hdev_info = appleib_add_device(hdev);
>> + if (IS_ERR(hdev_info)) {
>> + rc = PTR_ERR(hdev_info);
>> + goto stop_hw;
>> + }
>> +
>> + hid_set_drvdata(hdev, hdev_info);
>> +
>> + rc = hid_hw_open(hdev);
>> + if (rc) {
>> + hid_err(hdev, "ib: failed to open hid: %d\n", rc);
>> + goto remove_dev;
>> + }
>> +
>> + return 0;
>> +
>> +remove_dev:
>> + appleib_remove_device(hdev);
>> +stop_hw:
>> + hid_hw_stop(hdev);
>> +error:
>> + return rc;
>> +}
>> +
>> +static void appleib_hid_remove(struct hid_device *hdev)
>> +{
>> + hid_hw_close(hdev);
>> + appleib_remove_device(hdev);
>> + hid_hw_stop(hdev);
>> +}
>> +
>> +static const struct hid_device_id appleib_hid_ids[] = {
>> + { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IBRIDGE) },
>> + { },
>> +};
>> +
>> +static struct hid_driver appleib_hid_driver = {
>> + .name = "apple-ibridge-hid",
>> + .id_table = appleib_hid_ids,
>> + .probe = appleib_hid_probe,
>> + .remove = appleib_hid_remove,
>> + .raw_event = appleib_hid_raw_event,
>> + .report_fixup = appleib_report_fixup,
>> +#ifdef CONFIG_PM
>> + .suspend = appleib_hid_suspend,
>> + .resume = appleib_hid_resume,
>> + .reset_resume = appleib_hid_reset_resume,
>> +#endif
>> +};
> 
> const

Are you sure you want to do const here, cause other hid-drivers are NOT using const.

>> +
>> +static struct appleib_device *appleib_alloc_device(struct platform_device *pdev)
>> +{
>> + struct appleib_device *ib_dev;
>> + acpi_status sts;


^ permalink raw reply

* RE: [PATCH v4 2/4] input: bbnsm_pwrkey: Add bbnsm power key support
From: Jacky Bai @ 2023-02-10  7:11 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: lee@kernel.org, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, shawnguo@kernel.org,
	s.hauer@pengutronix.de, a.zummo@towertech.it,
	alexandre.belloni@bootlin.com, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-input@vger.kernel.org,
	linux-rtc@vger.kernel.org, kernel@pengutronix.de, dl-linux-imx,
	festevam@gmail.com
In-Reply-To: <Y91qeprAJSDckJ2e@google.com>

> Subject: Re: [PATCH v4 2/4] input: bbnsm_pwrkey: Add bbnsm power key
> support
> 
> Hi Jacky,
> 
> On Sun, Jan 29, 2023 at 03:08:21PM +0800, Jacky Bai wrote:
> > The ON/OFF logic inside the BBNSM allows for connecting directly into
> > a PMIC or other voltage regulator device. The module has an button
> > input signal and a wakeup request input signal. It also has two
> > interrupts (set_pwr_off_irq and set_pwr_on_irq) and an active-low PMIC
> > enable (pmic_en_b) output.
> >
> > Add the power key support for the ON/OFF button function found in
> > BBNSM module.
> >
> > Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> > Reviewed-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >   - v2 changes:
> >     - use device_property_read_u32() to read the property
> >     - clean up the goto return, return directly
> >     - sort the header file alphabetically
> >     - rename the file to add 'nxp' prefix
> >
> >   - v3 changes:
> >     - get the regmap directly from the parent node
> >
> >   - v4 changes:
> >     - update the compatible string to align with binding file
> >     - fix the device_property_read_u32 as stated in v2.
> >     - add back the 'dev_warn' for 'dev_pm_set_wake_irq' return fail.
> > ---
> >  drivers/input/keyboard/Kconfig            |  11 ++
> >  drivers/input/keyboard/Makefile           |   1 +
> >  drivers/input/keyboard/nxp-bbnsm-pwrkey.c | 192
> > ++++++++++++++++++++++
> 
> Majority of pwrkey drivers reside in dribers/input/misc, which
> drivers/input/keyboard intended for more traditional/full keyboards.
> Could you please move this driver there?
> 

Sure, I will move it into misc.

> We should move snvs_pwrkey too...

Will do it in a separate patch.

> 
> 
> >  3 files changed, 204 insertions(+)
> >  create mode 100644 drivers/input/keyboard/nxp-bbnsm-pwrkey.c
> >
> > diff --git a/drivers/input/keyboard/Kconfig
> > b/drivers/input/keyboard/Kconfig index 84490915ae4d..43827e34f276
> > 100644
> > --- a/drivers/input/keyboard/Kconfig
> > +++ b/drivers/input/keyboard/Kconfig
> > @@ -456,6 +456,17 @@ config KEYBOARD_SNVS_PWRKEY
> >  	  To compile this driver as a module, choose M here; the
> >  	  module will be called snvs_pwrkey.
> >
> > +config KEYBOARD_BBNSM_PWRKEY
> > +	tristate "NXP BBNSM Power Key Driver"
> > +	depends on ARCH_MXC || COMPILE_TEST
> > +	depends on OF
> > +	help
> > +	  This is the bbnsm powerkey driver for the NXP i.MX application
> > +	  processors.
> > +
> > +	  To compile this driver as a module, choose M here; the
> > +	  module will be called bbnsm_pwrkey.
> > +
> >  config KEYBOARD_IMX
> >  	tristate "IMX keypad support"
> >  	depends on ARCH_MXC || COMPILE_TEST
> > diff --git a/drivers/input/keyboard/Makefile
> > b/drivers/input/keyboard/Makefile index 5f67196bb2c1..e34dd65a34c3
> > 100644
> > --- a/drivers/input/keyboard/Makefile
> > +++ b/drivers/input/keyboard/Makefile
> > @@ -13,6 +13,7 @@ obj-$(CONFIG_KEYBOARD_AMIGA)		+=
> amikbd.o
> >  obj-$(CONFIG_KEYBOARD_APPLESPI)		+= applespi.o
> >  obj-$(CONFIG_KEYBOARD_ATARI)		+= atakbd.o
> >  obj-$(CONFIG_KEYBOARD_ATKBD)		+= atkbd.o
> > +obj-$(CONFIG_KEYBOARD_BBNSM_PWRKEY)	+= nxp-bbnsm-pwrkey.o
> >  obj-$(CONFIG_KEYBOARD_BCM)		+= bcm-keypad.o
> >  obj-$(CONFIG_KEYBOARD_CAP11XX)		+= cap11xx.o
> >  obj-$(CONFIG_KEYBOARD_CLPS711X)		+= clps711x-keypad.o
> > diff --git a/drivers/input/keyboard/nxp-bbnsm-pwrkey.c
> > b/drivers/input/keyboard/nxp-bbnsm-pwrkey.c
> > new file mode 100644
> > index 000000000000..154ce58b9d5a
> > --- /dev/null
> > +++ b/drivers/input/keyboard/nxp-bbnsm-pwrkey.c
> > @@ -0,0 +1,192 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +//
> > +// Copyright 2022 NXP.
> > +
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/init.h>
> > +#include <linux/input.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/jiffies.h>
> > +#include <linux/kernel.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_wakeirq.h>
> > +#include <linux/regmap.h>
> > +
> > +#define BBNSM_CTRL		0x8
> > +#define BBNSM_INT_EN		0x10
> > +#define BBNSM_EVENTS		0x14
> > +#define BBNSM_PAD_CTRL		0x24
> > +
> > +#define BBNSM_BTN_PRESSED	BIT(7)
> > +#define BBNSM_PWR_ON		BIT(6)
> > +#define BBNSM_BTN_OFF		BIT(5)
> > +#define BBNSM_EMG_OFF		BIT(4)
> > +#define BBNSM_PWRKEY_EVENTS	(BBNSM_PWR_ON |
> BBNSM_BTN_OFF | BBNSM_EMG_OFF)
> > +#define BBNSM_DP_EN		BIT(24)
> > +
> > +#define DEBOUNCE_TIME		30
> > +#define REPEAT_INTERVAL		60
> > +
> > +struct bbnsm_pwrkey {
> > +	struct regmap *regmap;
> > +	int irq;
> > +	int keycode;
> > +	int keystate;  /* 1:pressed */
> > +	struct timer_list check_timer;
> > +	struct input_dev *input;
> > +};
> > +
> > +static void bbnsm_pwrkey_check_for_events(struct timer_list *t) {
> > +	struct bbnsm_pwrkey *bbnsm = from_timer(bbnsm, t, check_timer);
> > +	struct input_dev *input = bbnsm->input;
> > +	u32 state;
> > +
> > +	regmap_read(bbnsm->regmap, BBNSM_EVENTS, &state);
> > +
> > +	state = state & BBNSM_BTN_PRESSED ? 1 : 0;
> > +
> > +	/* only report new event if status changed */
> > +	if (state ^ bbnsm->keystate) {
> > +		bbnsm->keystate = state;
> > +		input_event(input, EV_KEY, bbnsm->keycode, state);
> > +		input_sync(input);
> > +		pm_relax(bbnsm->input->dev.parent);
> > +	}
> > +
> > +	/* repeat check if pressed long */
> > +	if (state) {
> > +		mod_timer(&bbnsm->check_timer,
> > +			  jiffies + msecs_to_jiffies(REPEAT_INTERVAL));
> > +	}
> > +}
> > +
> > +static irqreturn_t bbnsm_pwrkey_interrupt(int irq, void *dev_id) {
> > +	struct platform_device *pdev = dev_id;
> > +	struct bbnsm_pwrkey *bbnsm = platform_get_drvdata(pdev);
> > +	struct input_dev *input = bbnsm->input;
> > +	u32 event;
> > +
> > +	regmap_read(bbnsm->regmap, BBNSM_EVENTS, &event);
> > +	if (event & BBNSM_BTN_OFF)
> > +		mod_timer(&bbnsm->check_timer, jiffies +
> msecs_to_jiffies(DEBOUNCE_TIME));
> > +	else
> > +		return IRQ_NONE;
> > +
> > +	pm_wakeup_event(input->dev.parent, 0);
> > +
> > +	/* clear PWR OFF */
> > +	regmap_write(bbnsm->regmap, BBNSM_EVENTS, BBNSM_BTN_OFF);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static void bbnsm_pwrkey_act(void *pdata) {
> > +	struct bbnsm_pwrkey *bbnsm = pdata;
> > +
> > +	del_timer_sync(&bbnsm->check_timer);
> 
> del_timer()/del_timer_sync() are deprecated. Please use
> timer_shutdown_sync() here.

Ok, will do it in V5.

> 
> > +}
> > +
> > +static int bbnsm_pwrkey_probe(struct platform_device *pdev) {
> > +	struct bbnsm_pwrkey *bbnsm;
> > +	struct input_dev *input;
> > +	struct device_node *np = pdev->dev.of_node;
> > +	int error;
> > +
> > +	bbnsm = devm_kzalloc(&pdev->dev, sizeof(*bbnsm), GFP_KERNEL);
> > +	if (!bbnsm)
> > +		return -ENOMEM;
> > +
> > +	bbnsm->regmap = syscon_node_to_regmap(np->parent);
> > +	if (IS_ERR(bbnsm->regmap)) {
> > +		dev_err(&pdev->dev, "bbnsm pwerkey get regmap failed\n");
> > +		return PTR_ERR(bbnsm->regmap);
> > +	}
> > +
> > +	if (device_property_read_u32(&pdev->dev, "linux,code",
> &bbnsm->keycode)) {
> > +		bbnsm->keycode = KEY_POWER;
> > +		dev_warn(&pdev->dev, "KEY_POWER without setting in dts\n");
> 
> This is a pretty cryptic message IMO. Maybe say "key code is not specified,
> using default KEY_POWER"?

Ok, will refine this log as you suggested.

> 
> > +	}
> > +
> > +	bbnsm->irq = platform_get_irq(pdev, 0);
> > +	if (bbnsm->irq < 0)
> > +		return -EINVAL;
> > +
> > +	/* config the BBNSM power related register */
> > +	regmap_update_bits(bbnsm->regmap, BBNSM_CTRL, BBNSM_DP_EN,
> > +BBNSM_DP_EN);
> > +
> > +	/* clear the unexpected interrupt before driver ready */
> > +	regmap_write_bits(bbnsm->regmap, BBNSM_EVENTS,
> BBNSM_PWRKEY_EVENTS,
> > +BBNSM_PWRKEY_EVENTS);
> > +
> > +	timer_setup(&bbnsm->check_timer, bbnsm_pwrkey_check_for_events,
> 0);
> > +
> > +	input = devm_input_allocate_device(&pdev->dev);
> > +	if (!input) {
> > +		dev_err(&pdev->dev, "failed to allocate the input device\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	input->name = pdev->name;
> > +	input->phys = "bbnsm-pwrkey/input0";
> > +	input->id.bustype = BUS_HOST;
> > +
> > +	input_set_capability(input, EV_KEY, bbnsm->keycode);
> > +
> > +	/* input customer action to cancel release timer */
> > +	error = devm_add_action(&pdev->dev, bbnsm_pwrkey_act, bbnsm);
> > +	if (error) {
> > +		dev_err(&pdev->dev, "failed to register remove action\n");
> > +		return error;
> > +	}
> > +
> > +	bbnsm->input = input;
> > +	platform_set_drvdata(pdev, bbnsm);
> > +
> > +	error = devm_request_irq(&pdev->dev, bbnsm->irq,
> bbnsm_pwrkey_interrupt,
> > +			       IRQF_SHARED, pdev->name, pdev);
> 
> Is this really a shared interrupt?

For current SoC integration, the ON/OFF power key IRQ signal are ORed together with RTC IRQ signal.
So these two functions share the same IRQ line to GIC. I think the IRQF_SHARED flag is necessary, right? ^_^

BR
> 
> > +	if (error) {
> > +		dev_err(&pdev->dev, "interrupt not available.\n");
> > +		return error;
> > +	}
> > +
> > +	error = input_register_device(input);
> > +	if (error < 0) {
> > +		dev_err(&pdev->dev, "failed to register input device\n");
> > +		return error;
> > +	}
> > +
> > +	device_init_wakeup(&pdev->dev, true);
> > +	error = dev_pm_set_wake_irq(&pdev->dev, bbnsm->irq);
> > +	if (error)
> > +		dev_warn(&pdev->dev, "irq wake enable failed.\n");
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id bbnsm_pwrkey_ids[] = {
> > +	{ .compatible = "nxp,imx93-bbnsm-pwrkey" },
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, bbnsm_pwrkey_ids);
> > +
> > +static struct platform_driver bbnsm_pwrkey_driver = {
> > +	.driver = {
> > +		.name = "bbnsm_pwrkey",
> > +		.of_match_table = bbnsm_pwrkey_ids,
> > +	},
> > +	.probe = bbnsm_pwrkey_probe,
> > +};
> > +module_platform_driver(bbnsm_pwrkey_driver);
> > +
> > +MODULE_AUTHOR("Jacky Bai <ping.bai@nxp.com>");
> > +MODULE_DESCRIPTION("NXP bbnsm power key Driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.37.1
> >
> 
> Thanks.
> 
> --
> Dmitry

^ permalink raw reply

* Re: [PATCH 1/3] HID: apple-ibridge: Add Apple iBridge HID driver for T1 chip.
From: Thomas Weißschuh @ 2023-02-10  4:56 UTC (permalink / raw)
  To: Aditya Garg
  Cc: Jiri Kosina, jkosina@suse.cz, benjamin.tissoires@redhat.com,
	Andy Shevchenko, andy.shevchenko@gmail.com, LKML,
	linux-input@vger.kernel.org, ronald@innovation.ch,
	kekrby@gmail.com, Orlando Chamberlain
In-Reply-To: <40274C3D-4F4F-479C-944C-EEBDC78F959C@live.com>

Hi,

some comments inline.

On Fri, Feb 10, 2023 at 03:43:24AM +0000, Aditya Garg wrote:
> From: Ronald Tschalär <ronald@innovation.ch>
> 
> The iBridge device provides access to several devices, including:
> - the Touch Bar
> - the iSight webcam
> - the light sensor
> - the fingerprint sensor
> 
> This driver provides the core support for managing the iBridge device
> and the access to the underlying devices. In particular, the
> functionality for the touch bar and light sensor is exposed via USB HID
> interfaces, and on devices with the T1 chip one of the HID devices is
> used for both functions. So this driver creates virtual HID devices, one
> per top-level report collection on each HID device (for a total of 3
> virtual HID devices). The sub-drivers then bind to these virtual HID
> devices.
> 
> This way the Touch Bar and ALS drivers can be kept in their own modules,
> while at the same time making them look very much like as if they were
> connected to the real HID devices. And those drivers then work (mostly)
> without further changes on MacBooks with the T2 chip that don't need
> this driver.
> 
> Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
> [Kerem Karabay: convert to a platform driver]
> [Kerem Karabay: fix appleib_forward_int_op]
> [Kerem Karabay: rely on HID core's parsing in appleib_add_device]
> Signed-off-by: Kerem Karabay <kekrby@gmail.com>
> Signed-off-by: Aditya Garg <gargaditya08@live.com>
> ---
>  drivers/hid/Kconfig         |  15 +
>  drivers/hid/Makefile        |   1 +
>  drivers/hid/apple-ibridge.c | 610 ++++++++++++++++++++++++++++++++++++
>  drivers/hid/apple-ibridge.h |  15 +
>  drivers/hid/hid-ids.h       |   1 +
>  drivers/hid/hid-quirks.c    |   3 +
>  6 files changed, 645 insertions(+)
>  create mode 100644 drivers/hid/apple-ibridge.c
>  create mode 100644 drivers/hid/apple-ibridge.h
> 
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index e2a5d30c8..e69afa5f4 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -130,6 +130,21 @@ config HID_APPLE
>  	Say Y here if you want support for keyboards of	Apple iBooks, PowerBooks,
>  	MacBooks, MacBook Pros and Apple Aluminum.
>  
> +config HID_APPLE_IBRIDGE
> +	tristate "Apple iBridge"
> +	depends on USB_HID
> +	depends on (X86 && ACPI) || COMPILE_TEST
> +	imply HID_SENSOR_HUB
> +	imply HID_SENSOR_ALS
> +	help
> +	This module provides the core support for the Apple T1 chip found
> +	on 2016 and 2017 MacBookPro's, also known as the iBridge. The drivers
> +	for the Touch Bar (apple-touchbar) and light sensor (hid-sensor-hub
> +	and hid-sensor-als) need to be enabled separately.
> +
> +	To compile this driver as a module, choose M here: the
> +	module will be called apple-ibridge.
> +
>  config HID_APPLEIR
>  	tristate "Apple infrared receiver"
>  	depends on (USB_HID)
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index e8014c1a2..b61373cd8 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_HID_ACCUTOUCH)	+= hid-accutouch.o
>  obj-$(CONFIG_HID_ALPS)		+= hid-alps.o
>  obj-$(CONFIG_HID_ACRUX)		+= hid-axff.o
>  obj-$(CONFIG_HID_APPLE)		+= hid-apple.o
> +obj-$(CONFIG_HID_APPLE_IBRIDGE)	+= apple-ibridge.o
>  obj-$(CONFIG_HID_APPLEIR)	+= hid-appleir.o
>  obj-$(CONFIG_HID_CREATIVE_SB0540)	+= hid-creative-sb0540.o
>  obj-$(CONFIG_HID_ASUS)		+= hid-asus.o
> diff --git a/drivers/hid/apple-ibridge.c b/drivers/hid/apple-ibridge.c
> new file mode 100644
> index 000000000..4d26f8d66
> --- /dev/null
> +++ b/drivers/hid/apple-ibridge.c
> @@ -0,0 +1,610 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Apple iBridge Driver
> + *
> + * Copyright (c) 2018 Ronald Tschalär
> + */
> +
> +/**
> + * DOC: Overview
> + *
> + * 2016 and 2017 MacBookPro models with a Touch Bar (MacBookPro13,[23] and
> + * MacBookPro14,[23]) have an Apple iBridge chip (also known as T1 chip) which
> + * exposes the touch bar, built-in webcam (iSight), ambient light sensor, and
> + * Secure Enclave Processor (SEP) for TouchID. It shows up in the system as a
> + * USB device with 3 configurations: 'Default iBridge Interfaces', 'Default
> + * iBridge Interfaces(OS X)', and 'Default iBridge Interfaces(Recovery)'.
> + *
> + * In the first (default after boot) configuration, 4 usb interfaces are
> + * exposed: 2 related to the webcam, and 2 USB HID interfaces representing
> + * the touch bar and the ambient light sensor. The webcam interfaces are
> + * already handled by the uvcvideo driver. However, there is a problem with
> + * the other two interfaces: one of them contains functionality (HID reports)
> + * used by both the touch bar and the ALS, which is an issue because the kernel
> + * allows only one driver to be attached to a given device. This driver exists
> + * to solve this issue.
> + *
> + * This driver is implemented as a HID driver that attaches to both HID
> + * interfaces and in turn creates several virtual child HID devices, one for
> + * each top-level collection found in each interfaces report descriptor. The
> + * touch bar and ALS drivers then attach to these virtual HID devices, and this
> + * driver forwards the operations between the real and virtual devices.
> + *
> + * One important aspect of this approach is that resulting (virtual) HID
> + * devices look much like the HID devices found on the later MacBookPro models
> + * which have a T2 chip, where there are separate USB interfaces for the touch
> + * bar and ALS functionality, which means that the touch bar and ALS drivers
> + * work (mostly) the same on both types of models.
> + *
> + * Lastly, this driver also takes care of the power-management for the
> + * iBridge when suspending and resuming.
> + */

Maybe add a pr_fmt definition here?

> +
> +#include <linux/platform_device.h>
> +#include <linux/acpi.h>
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/usb.h>
> +
> +#include "hid-ids.h"
> +#include "../hid/usbhid/usbhid.h"
> +#include "apple-ibridge.h"
> +
> +#define APPLEIB_BASIC_CONFIG	1
> +
> +static struct hid_device_id appleib_sub_hid_ids[] = {
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_LINUX_FOUNDATION,
> +			 USB_DEVICE_ID_IBRIDGE_TB) },
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_LINUX_FOUNDATION,
> +			 USB_DEVICE_ID_IBRIDGE_ALS) },
> +};

Structs like this should be "const".

> +
> +static struct {
> +	unsigned int usage;
> +	struct hid_device_id *dev_id;
> +} appleib_usage_map[] = {
> +	/* Default iBridge configuration, key inputs and mode settings */
> +	{ 0x00010006, &appleib_sub_hid_ids[0] },
> +	/* OS X iBridge configuration, digitizer inputs */
> +	{ 0x000D0005, &appleib_sub_hid_ids[0] },
> +	/* All iBridge configurations, display/DFR settings */
> +	{ 0xFF120001, &appleib_sub_hid_ids[0] },
> +	/* All iBridge configurations, ALS */
> +	{ 0x00200041, &appleib_sub_hid_ids[1] },
> +};

const

> +
> +struct appleib_device {
> +	acpi_handle asoc_socw;
> +};
> +
> +struct appleib_hid_dev_info {
> +	struct hid_device	*hdev;
> +	struct hid_device	*sub_hdevs[ARRAY_SIZE(appleib_sub_hid_ids)];
> +	bool			sub_open[ARRAY_SIZE(appleib_sub_hid_ids)];
> +};
> +
> +static int appleib_hid_raw_event(struct hid_device *hdev,
> +				 struct hid_report *report, u8 *data, int size)
> +{
> +	struct appleib_hid_dev_info *hdev_info = hid_get_drvdata(hdev);
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(hdev_info->sub_hdevs); i++) {
> +		if (READ_ONCE(hdev_info->sub_open[i]))
> +			hid_input_report(hdev_info->sub_hdevs[i], report->type,
> +					 data, size, 0);
> +	}
> +
> +	return 0;
> +}
> +
> +static __u8 *appleib_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> +				  unsigned int *rsize)
> +{
> +	/* Some fields have a size of 64 bits, which according to HID 1.11
> +	 * Section 8.4 is not valid ("An item field cannot span more than 4
> +	 * bytes in a report"). Furthermore, hid_field_extract() complains
> +	 * when encountering such a field. So turn them into two 32-bit fields
> +	 * instead.
> +	 */
> +
> +	if (*rsize == 634 &&
> +	    /* Usage Page 0xff12 (vendor defined) */
> +	    rdesc[212] == 0x06 && rdesc[213] == 0x12 && rdesc[214] == 0xff &&
> +	    /* Usage 0x51 */
> +	    rdesc[416] == 0x09 && rdesc[417] == 0x51 &&
> +	    /* report size 64 */
> +	    rdesc[432] == 0x75 && rdesc[433] == 64 &&
> +	    /* report count 1 */
> +	    rdesc[434] == 0x95 && rdesc[435] == 1) {
> +		rdesc[433] = 32;
> +		rdesc[435] = 2;
> +		hid_dbg(hdev, "Fixed up first 64-bit field\n");
> +	}
> +
> +	if (*rsize == 634 &&
> +	    /* Usage Page 0xff12 (vendor defined) */
> +	    rdesc[212] == 0x06 && rdesc[213] == 0x12 && rdesc[214] == 0xff &&
> +	    /* Usage 0x51 */
> +	    rdesc[611] == 0x09 && rdesc[612] == 0x51 &&
> +	    /* report size 64 */
> +	    rdesc[627] == 0x75 && rdesc[628] == 64 &&
> +	    /* report count 1 */
> +	    rdesc[629] == 0x95 && rdesc[630] == 1) {
> +		rdesc[628] = 32;
> +		rdesc[630] = 2;
> +		hid_dbg(hdev, "Fixed up second 64-bit field\n");
> +	}
> +
> +	return rdesc;
> +}
> +
> +#ifdef CONFIG_PM
> +/**
> + * appleib_forward_int_op() - Forward a hid-driver callback to all drivers on
> + * all virtual HID devices attached to the given real HID device.
> + * @hdev the real hid-device
> + * @forward a function that calls the callback on the given driver
> + * @args arguments for the forward function
> + *
> + * This is for callbacks that return a status as an int.
> + *
> + * Returns: 0 on success, or the first error returned by the @forward function.
> + */
> +static int appleib_forward_int_op(struct hid_device *hdev,
> +				  int (*forward)(struct hid_driver *,
> +						 struct hid_device *, void *),
> +				  void *args)
> +{
> +	struct appleib_hid_dev_info *hdev_info = hid_get_drvdata(hdev);
> +	struct hid_device *sub_hdev;
> +	int rc;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(hdev_info->sub_hdevs); i++) {
> +		sub_hdev = hdev_info->sub_hdevs[i];
> +		if (sub_hdev->driver) {
> +			rc = forward(sub_hdev->driver, sub_hdev, args);
> +			if (rc)
> +				return rc;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int appleib_hid_suspend_fwd(struct hid_driver *drv,
> +				   struct hid_device *hdev, void *args)
> +{
> +	int rc = 0;
> +
> +	if (drv->suspend)
> +		rc = drv->suspend(hdev, *(pm_message_t *)args);
> +
> +	return rc;
> +}
> +
> +static int appleib_hid_suspend(struct hid_device *hdev, pm_message_t message)
> +{
> +	return appleib_forward_int_op(hdev, appleib_hid_suspend_fwd, &message);
> +}
> +
> +static int appleib_hid_resume_fwd(struct hid_driver *drv,
> +				  struct hid_device *hdev, void *args)
> +{
> +	int rc = 0;
> +
> +	if (drv->resume)
> +		rc = drv->resume(hdev);
> +
> +	return rc;
> +}
> +
> +static int appleib_hid_resume(struct hid_device *hdev)
> +{
> +	return appleib_forward_int_op(hdev, appleib_hid_resume_fwd, NULL);
> +}
> +
> +static int appleib_hid_reset_resume_fwd(struct hid_driver *drv,
> +					struct hid_device *hdev, void *args)
> +{
> +	int rc = 0;
> +
> +	if (drv->reset_resume)
> +		rc = drv->reset_resume(hdev);
> +
> +	return rc;
> +}
> +
> +static int appleib_hid_reset_resume(struct hid_device *hdev)
> +{
> +	return appleib_forward_int_op(hdev, appleib_hid_reset_resume_fwd, NULL);
> +}
> +#endif /* CONFIG_PM */
> +
> +static int appleib_ll_start(struct hid_device *hdev)
> +{
> +	return 0;
> +}
> +
> +static void appleib_ll_stop(struct hid_device *hdev)
> +{
> +}
> +
> +static int appleib_set_open(struct hid_device *hdev, bool open)
> +{
> +	struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(hdev_info->sub_hdevs); i++) {
> +		/*
> +		 * hid_hw_open(), and hence appleib_ll_open(), is called
> +		 * from the driver's probe function, which in turn is called
> +		 * while adding the sub-hdev; but at this point we haven't yet
> +		 * added the sub-hdev to our list. So if we don't find the
> +		 * sub-hdev in our list assume it's in the process of being
> +		 * added and set the flag on the first unset sub-hdev.
> +		 */
> +		if (hdev_info->sub_hdevs[i] == hdev ||
> +		    !hdev_info->sub_hdevs[i]) {
> +			WRITE_ONCE(hdev_info->sub_open[i], open);
> +			return 0;
> +		}
> +	}
> +
> +	return -ENODEV;
> +}
> +
> +static int appleib_ll_open(struct hid_device *hdev)
> +{
> +	return appleib_set_open(hdev, true);
> +}
> +
> +static void appleib_ll_close(struct hid_device *hdev)
> +{
> +	appleib_set_open(hdev, false);
> +}
> +
> +static int appleib_ll_power(struct hid_device *hdev, int level)
> +{
> +	struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
> +
> +	return hid_hw_power(hdev_info->hdev, level);
> +}
> +
> +static int appleib_ll_parse(struct hid_device *hdev)
> +{
> +	/* we've already called hid_parse_report() */
> +	return 0;
> +}
> +
> +static void appleib_ll_request(struct hid_device *hdev,
> +			       struct hid_report *report, int reqtype)
> +{
> +	struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
> +
> +	hid_hw_request(hdev_info->hdev, report, reqtype);
> +}
> +
> +static int appleib_ll_wait(struct hid_device *hdev)
> +{
> +	struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
> +
> +	hid_hw_wait(hdev_info->hdev);
> +	return 0;
> +}
> +
> +static int appleib_ll_raw_request(struct hid_device *hdev,
> +				  unsigned char reportnum, __u8 *buf,
> +				  size_t len, unsigned char rtype, int reqtype)
> +{
> +	struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
> +
> +	return hid_hw_raw_request(hdev_info->hdev, reportnum, buf, len, rtype,
> +				  reqtype);
> +}
> +
> +static int appleib_ll_output_report(struct hid_device *hdev, __u8 *buf,
> +				    size_t len)
> +{
> +	struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
> +
> +	return hid_hw_output_report(hdev_info->hdev, buf, len);
> +}
> +
> +static struct hid_ll_driver appleib_ll_driver = {
> +	.start = appleib_ll_start,
> +	.stop = appleib_ll_stop,
> +	.open = appleib_ll_open,
> +	.close = appleib_ll_close,
> +	.power = appleib_ll_power,
> +	.parse = appleib_ll_parse,
> +	.request = appleib_ll_request,
> +	.wait = appleib_ll_wait,
> +	.raw_request = appleib_ll_raw_request,
> +	.output_report = appleib_ll_output_report,
> +};

const

> +
> +static struct hid_device_id *appleib_find_dev_id_for_usage(unsigned int usage)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(appleib_usage_map); i++) {
> +		if (appleib_usage_map[i].usage == usage)
> +			return appleib_usage_map[i].dev_id;
> +	}
> +
> +	return NULL;
> +}
> +
> +static struct hid_device *
> +appleib_add_sub_dev(struct appleib_hid_dev_info *hdev_info,
> +		    struct hid_device_id *dev_id)
> +{
> +	struct hid_device *sub_hdev;
> +	int rc;
> +
> +	sub_hdev = hid_allocate_device();
> +	if (IS_ERR(sub_hdev))
> +		return sub_hdev;
> +
> +	sub_hdev->dev.parent = &hdev_info->hdev->dev;
> +
> +	sub_hdev->bus = dev_id->bus;
> +	sub_hdev->group = dev_id->group;
> +	sub_hdev->vendor = dev_id->vendor;
> +	sub_hdev->product = dev_id->product;
> +
> +	sub_hdev->ll_driver = &appleib_ll_driver;
> +
> +	snprintf(sub_hdev->name, sizeof(sub_hdev->name),
> +		 "iBridge Virtual HID %s/%04x:%04x",
> +		 dev_name(sub_hdev->dev.parent), sub_hdev->vendor,
> +		 sub_hdev->product);
> +
> +	sub_hdev->driver_data = hdev_info;
> +
> +	rc = hid_add_device(sub_hdev);
> +	if (rc) {
> +		hid_destroy_device(sub_hdev);
> +		return ERR_PTR(rc);
> +	}
> +
> +	return sub_hdev;
> +}
> +
> +static struct appleib_hid_dev_info *appleib_add_device(struct hid_device *hdev)
> +{
> +	struct appleib_hid_dev_info *hdev_info;
> +	struct hid_device_id *dev_id;
> +	unsigned int usage;
> +	int i;
> +
> +	hdev_info = devm_kzalloc(&hdev->dev, sizeof(*hdev_info), GFP_KERNEL);
> +	if (!hdev_info)
> +		return ERR_PTR(-ENOMEM);
> +
> +	hdev_info->hdev = hdev;
> +
> +	for (i = 0; i < hdev->maxcollection; i++) {
> +		usage = hdev->collection[i].usage;
> +		dev_id = appleib_find_dev_id_for_usage(usage);
> +
> +		if (!dev_id) {
> +			hid_warn(hdev, "Unknown collection encountered with usage %x\n",
> +				 usage);
> +		} else {
> +			hdev_info->sub_hdevs[i] = appleib_add_sub_dev(hdev_info, dev_id);
> +
> +			if (IS_ERR(hdev_info->sub_hdevs[i])) {
> +				while (i-- > 0)
> +					hid_destroy_device(hdev_info->sub_hdevs[i]);
> +				return (void *)hdev_info->sub_hdevs[i];
> +			}
> +		}
> +	}
> +
> +	return hdev_info;
> +}
> +
> +static void appleib_remove_device(struct hid_device *hdev)
> +{
> +	struct appleib_hid_dev_info *hdev_info = hid_get_drvdata(hdev);
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(hdev_info->sub_hdevs); i++) {
> +		if (hdev_info->sub_hdevs[i])
> +			hid_destroy_device(hdev_info->sub_hdevs[i]);
> +	}
> +
> +	hid_set_drvdata(hdev, NULL);
> +}
> +
> +static int appleib_hid_probe(struct hid_device *hdev,
> +			     const struct hid_device_id *id)
> +{
> +	struct appleib_hid_dev_info *hdev_info;
> +	struct usb_device *udev;
> +	int rc;
> +
> +	/* check and set usb config first */
> +	udev = hid_to_usb_dev(hdev);

I don't think hid_to_usb_dev() does any check that hdev is connected via
USB and will produce undefined behavior when used on a non-USB device.

Use hid_is_usb() first.

> +
> +	if (udev->actconfig->desc.bConfigurationValue != APPLEIB_BASIC_CONFIG) {
> +		rc = usb_driver_set_configuration(udev, APPLEIB_BASIC_CONFIG);
> +		return rc ? rc : -ENODEV;
> +	}
> +
> +	rc = hid_parse(hdev);
> +	if (rc) {
> +		hid_err(hdev, "ib: hid parse failed (%d)\n", rc);
> +		goto error;
> +	}
> +
> +	rc = hid_hw_start(hdev, HID_CONNECT_DRIVER);
> +	if (rc) {
> +		hid_err(hdev, "ib: hw start failed (%d)\n", rc);
> +		goto error;
> +	}
> +
> +	hdev_info = appleib_add_device(hdev);
> +	if (IS_ERR(hdev_info)) {
> +		rc = PTR_ERR(hdev_info);
> +		goto stop_hw;
> +	}
> +
> +	hid_set_drvdata(hdev, hdev_info);
> +
> +	rc = hid_hw_open(hdev);
> +	if (rc) {
> +		hid_err(hdev, "ib: failed to open hid: %d\n", rc);
> +		goto remove_dev;
> +	}
> +
> +	return 0;
> +
> +remove_dev:
> +	appleib_remove_device(hdev);
> +stop_hw:
> +	hid_hw_stop(hdev);
> +error:
> +	return rc;
> +}
> +
> +static void appleib_hid_remove(struct hid_device *hdev)
> +{
> +	hid_hw_close(hdev);
> +	appleib_remove_device(hdev);
> +	hid_hw_stop(hdev);
> +}
> +
> +static const struct hid_device_id appleib_hid_ids[] = {
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IBRIDGE) },
> +	{ },
> +};
> +
> +static struct hid_driver appleib_hid_driver = {
> +	.name = "apple-ibridge-hid",
> +	.id_table = appleib_hid_ids,
> +	.probe = appleib_hid_probe,
> +	.remove = appleib_hid_remove,
> +	.raw_event = appleib_hid_raw_event,
> +	.report_fixup = appleib_report_fixup,
> +#ifdef CONFIG_PM
> +	.suspend = appleib_hid_suspend,
> +	.resume = appleib_hid_resume,
> +	.reset_resume = appleib_hid_reset_resume,
> +#endif
> +};

const

> +
> +static struct appleib_device *appleib_alloc_device(struct platform_device *pdev)
> +{
> +	struct appleib_device *ib_dev;
> +	acpi_status sts;
> +
> +	ib_dev = devm_kzalloc(&pdev->dev, sizeof(*ib_dev), GFP_KERNEL);
> +	if (!ib_dev)
> +		return ERR_PTR(-ENOMEM);
> +
> +	/* get iBridge acpi power control method for suspend/resume */
> +	sts = acpi_get_handle(ACPI_HANDLE(&pdev->dev), "SOCW", &ib_dev->asoc_socw);
> +	if (ACPI_FAILURE(sts)) {
> +		dev_err(&pdev->dev,
> +			"Error getting handle for ASOC.SOCW method: %s\n",
> +			acpi_format_exception(sts));
> +		return ERR_PTR(-ENXIO);
> +	}
> +
> +	/* ensure iBridge is powered on */
> +	sts = acpi_execute_simple_method(ib_dev->asoc_socw, NULL, 1);
> +	if (ACPI_FAILURE(sts))
> +		dev_warn(&pdev->dev, "SOCW(1) failed: %s\n",
> +			 acpi_format_exception(sts));
> +
> +	return ib_dev;
> +}
> +
> +static int appleib_probe(struct platform_device *pdev)
> +{
> +	struct appleib_device *ib_dev;
> +	int ret;
> +
> +	ib_dev = appleib_alloc_device(pdev);
> +	if (IS_ERR(ib_dev))
> +		return PTR_ERR(ib_dev);
> +
> +	ret = hid_register_driver(&appleib_hid_driver);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Error registering hid driver: %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, ib_dev);
> +
> +	return 0;
> +}
> +
> +static int appleib_remove(struct platform_device *pdev)
> +{
> +	hid_unregister_driver(&appleib_hid_driver);
> +
> +	return 0;
> +}
> +
> +static int appleib_suspend(struct platform_device *pdev, pm_message_t message)
> +{
> +	struct appleib_device *ib_dev;
> +	int rc;
> +
> +	ib_dev = platform_get_drvdata(pdev);
> +
> +	rc = acpi_execute_simple_method(ib_dev->asoc_socw, NULL, 0);
> +	if (ACPI_FAILURE(rc))
> +		dev_warn(&pdev->dev, "SOCW(0) failed: %s\n",
> +			 acpi_format_exception(rc));
> +
> +	return 0;
> +}
> +
> +static int appleib_resume(struct platform_device *pdev)
> +{
> +	struct appleib_device *ib_dev;
> +	int rc;
> +
> +	ib_dev = platform_get_drvdata(pdev);
> +
> +	rc = acpi_execute_simple_method(ib_dev->asoc_socw, NULL, 1);
> +	if (ACPI_FAILURE(rc))
> +		dev_warn(&pdev->dev, "SOCW(1) failed: %s\n",
> +			 acpi_format_exception(rc));
> +
> +	return 0;
> +}
> +
> +static const struct acpi_device_id appleib_acpi_match[] = {
> +	{ "APP7777", 0 },
> +	{ },

No trailing comma after end-of-array marker.

> +};
> +
> +MODULE_DEVICE_TABLE(acpi, appleib_acpi_match);
> +
> +static struct platform_driver appleib_driver = {
> +	.probe		= appleib_probe,
> +	.remove		= appleib_remove,
> +	.suspend	= appleib_suspend,
> +	.resume		= appleib_resume,
> +	.driver		= {
> +		.name		  = "apple-ibridge",
> +		.acpi_match_table = appleib_acpi_match,
> +	},
> +};
> +
> +module_platform_driver(appleib_driver);
> +
> +MODULE_AUTHOR("Ronald Tschalär");
> +MODULE_DESCRIPTION("Apple iBridge driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/hid/apple-ibridge.h b/drivers/hid/apple-ibridge.h
> new file mode 100644
> index 000000000..8aefcf615
> --- /dev/null
> +++ b/drivers/hid/apple-ibridge.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Apple iBridge Driver
> + *
> + * Copyright (c) 2018 Ronald Tschalär
> + */
> +
> +#ifndef __LINUX_APPLE_IBRDIGE_H
> +#define __LINUX_APPLE_IBRDIGE_H
> +
> +#define USB_VENDOR_ID_LINUX_FOUNDATION	0x1d6b
> +#define USB_DEVICE_ID_IBRIDGE_TB	0x0301
> +#define USB_DEVICE_ID_IBRIDGE_ALS	0x0302
> +
> +#endif
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 0f8c11842..0c62e6280 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -187,6 +187,7 @@
>  #define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_NUMPAD_2021   0x029f
>  #define USB_DEVICE_ID_APPLE_TOUCHBAR_BACKLIGHT 0x8102
>  #define USB_DEVICE_ID_APPLE_TOUCHBAR_DISPLAY 0x8302
> +#define USB_DEVICE_ID_APPLE_IBRIDGE	0x8600
>  
>  #define USB_VENDOR_ID_ASUS		0x0486
>  #define USB_DEVICE_ID_ASUS_T91MT	0x0185
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index be3ad0257..c03535c4b 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -319,6 +319,9 @@ static const struct hid_device_id hid_have_special_driver[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_TOUCHBAR_BACKLIGHT) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_TOUCHBAR_DISPLAY) },
>  #endif
> +#if IS_ENABLED(CONFIG_HID_APPLE_IBRIDGE)
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IBRIDGE) },
> +#endif
>  #if IS_ENABLED(CONFIG_HID_APPLEIR)
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL2) },
> -- 
> 2.37.2
> 

^ permalink raw reply

* [PATCH 3/3] HID: apple-magic-backlight: Add driver for keyboard backlight on internal Magic Keyboards
From: Aditya Garg @ 2023-02-10  3:45 UTC (permalink / raw)
  To: Jiri Kosina, jkosina@suse.cz, benjamin.tissoires@redhat.com,
	Andy Shevchenko, andy.shevchenko@gmail.com
  Cc: LKML, linux-input@vger.kernel.org, ronald@innovation.ch,
	kekrby@gmail.com, Orlando Chamberlain
In-Reply-To: <868AA58D-2399-4E4A-A6C6-73F88DB13992@live.com>

From: Orlando Chamberlain <orlandoch.dev@gmail.com>

This driver adds support for the keyboard backlight on Intel T2 Macs
with internal Magic Keyboards (MacBookPro16,x and MacBookAir9,1)

Signed-off-by: Orlando Chamberlain <orlandoch.dev@gmail.com>
Co-developed-by: Kerem Karabay <kekrby@gmail.com>
Signed-off-by: Kerem Karabay <kekrby@gmail.com>
Signed-off-by: Aditya Garg <gargaditya08@live.com>
---
 MAINTAINERS                         |   6 ++
 drivers/hid/Kconfig                 |  13 +++
 drivers/hid/Makefile                |   1 +
 drivers/hid/apple-magic-backlight.c | 143 ++++++++++++++++++++++++++++
 4 files changed, 163 insertions(+)
 create mode 100644 drivers/hid/apple-magic-backlight.c

diff --git a/MAINTAINERS b/MAINTAINERS
index fb1471cb5..3319f0c3e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9201,6 +9201,12 @@ F:	include/linux/pm.h
 F:	include/linux/suspend.h
 F:	kernel/power/
 
+HID APPLE MAGIC BACKLIGHT DRIVER
+M:	Orlando Chamberlain <orlandoch.dev@gmail.com>
+L:	linux-input@vger.kernel.org
+S:	Maintained
+F:	drivers/hid/apple-magic-backlight.c
+
 HID CORE LAYER
 M:	Jiri Kosina <jikos@kernel.org>
 M:	Benjamin Tissoires <benjamin.tissoires@redhat.com>
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 4ec669267..ad4612ec5 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -156,6 +156,19 @@ config HID_APPLE_TOUCHBAR
 	To compile this driver as a module, choose M here: the
 	module will be called apple-touchbar.
 
+config HID_APPLE_MAGIC_BACKLIGHT
+	tristate "Apple Magic Keyboard Backlight"
+	depends on USB_HID
+	depends on LEDS_CLASS
+	depends on NEW_LEDS
+	help
+	Say Y here if you want support for the keyboard backlight on Macs with
+	the magic keyboard (MacBookPro16,x and MacBookAir9,1). Note that this
+	driver is not for external magic keyboards.
+
+	To compile this driver as a module, choose M here: the
+	module will be called apple-magic-backlight.
+
 config HID_APPLEIR
 	tristate "Apple infrared receiver"
 	depends on (USB_HID)
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index c792e42fe..a961914ec 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_HID_ACRUX)		+= hid-axff.o
 obj-$(CONFIG_HID_APPLE)		+= hid-apple.o
 obj-$(CONFIG_HID_APPLE_IBRIDGE)	+= apple-ibridge.o
 obj-$(CONFIG_HID_APPLE_TOUCHBAR)	+= apple-touchbar.o
+obj-$(CONFIG_HID_APPLE_MAGIC_BACKLIGHT)	+= apple-magic-backlight.o
 obj-$(CONFIG_HID_APPLEIR)	+= hid-appleir.o
 obj-$(CONFIG_HID_CREATIVE_SB0540)	+= hid-creative-sb0540.o
 obj-$(CONFIG_HID_ASUS)		+= hid-asus.o
diff --git a/drivers/hid/apple-magic-backlight.c b/drivers/hid/apple-magic-backlight.c
new file mode 100644
index 000000000..9b128f6df
--- /dev/null
+++ b/drivers/hid/apple-magic-backlight.c
@@ -0,0 +1,143 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Apple Magic Keyboard Backlight Driver
+ *
+ * For Intel Macs with internal Magic Keyboard (MacBookPro16,1-4 and MacBookAir9,1)
+ *
+ * Copyright (c) 2022 Kerem Karabay <kekrby@gmail.com>
+ * Copyright (c) 2023 Orlando Chamberlain <orlandoch.dev@gmail.com>
+ */
+
+#include <linux/hid.h>
+#include <linux/usb.h>
+
+#include "hid-ids.h"
+
+#define USAGE_MAGIC_BL	0xff00000f
+
+#define APPLE_MAGIC_REPORT_ID_POWER 3
+#define APPLE_MAGIC_REPORT_ID_BRIGHTNESS 1
+
+struct apple_magic_backlight {
+	struct led_classdev cdev;
+	struct hid_device *hdev;
+	struct hid_report *brightness;
+	struct hid_report *power;
+};
+
+static void apple_magic_backlight_power_set(struct apple_magic_backlight *backlight,
+					   char power, char rate)
+{
+	struct hid_report *rep = backlight->power;
+
+	rep->field[0]->value[0] = power ? 1 : 0;
+	rep->field[1]->value[0] = 0x5e; /* Mimic Windows */
+	rep->field[1]->value[0] |= rate << 8;
+
+	hid_hw_request(backlight->hdev, backlight->power, HID_REQ_SET_REPORT);
+}
+
+static void apple_magic_backlight_brightness_set(struct apple_magic_backlight *backlight,
+						int brightness, char rate)
+{
+	struct hid_report *rep = backlight->brightness;
+
+	rep->field[0]->value[0] = brightness;
+	rep->field[1]->value[0] = 0x5e; /* Mimic Windows */
+	rep->field[1]->value[0] |= rate << 8;
+
+	hid_hw_request(backlight->hdev, backlight->brightness, HID_REQ_SET_REPORT);
+}
+
+static void apple_magic_backlight_set(struct apple_magic_backlight *backlight,
+				     int brightness, char rate)
+{
+	apple_magic_backlight_power_set(backlight, brightness, rate);
+	if (brightness)
+		apple_magic_backlight_brightness_set(backlight, brightness, rate);
+}
+
+static int apple_magic_backlight_led_set(struct led_classdev *led_cdev,
+					 enum led_brightness brightness)
+{
+	struct apple_magic_backlight *backlight = container_of(led_cdev,
+			struct apple_magic_backlight, cdev);
+
+	apple_magic_backlight_set(backlight, brightness, 1);
+	return 0;
+}
+
+static int apple_magic_backlight_probe(struct hid_device *hdev,
+				       const struct hid_device_id *id)
+{
+	struct apple_magic_backlight *backlight;
+	int rc;
+
+	rc = hid_parse(hdev);
+	if (rc)
+		return rc;
+
+	/* Ensure this usb endpoint is for the keyboard backlight, not touchbar
+	 * backlight.
+	 */
+	if (!(hdev->collection && hdev->collection[0].usage == USAGE_MAGIC_BL))
+		return -ENODEV;
+
+	backlight = devm_kzalloc(&hdev->dev, sizeof(*backlight), GFP_KERNEL);
+
+	if (!backlight)
+		return -ENOMEM;
+
+	hid_set_drvdata(hdev, backlight);
+
+	rc = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	if (rc)
+		return rc;
+
+	backlight->brightness = hid_register_report(hdev, HID_FEATURE_REPORT,
+			APPLE_MAGIC_REPORT_ID_BRIGHTNESS, 0);
+	backlight->power = hid_register_report(hdev, HID_FEATURE_REPORT,
+			APPLE_MAGIC_REPORT_ID_POWER, 0);
+
+	if (!backlight->brightness || !backlight->power) {
+		rc = -ENODEV;
+		goto hw_stop;
+	}
+
+	backlight->hdev = hdev;
+	backlight->cdev.name = "apple::kbd_backlight";
+	backlight->cdev.max_brightness = backlight->brightness->field[0]->logical_maximum;
+	backlight->cdev.brightness_set_blocking = apple_magic_backlight_led_set;
+
+	apple_magic_backlight_set(backlight, 0, 0);
+
+	return devm_led_classdev_register(&hdev->dev, &backlight->cdev);
+
+hw_stop:
+	hid_hw_stop(hdev);
+	return rc;
+}
+
+static void apple_magic_backlight_remove(struct hid_device *hdev)
+{
+	hid_hw_stop(hdev);
+}
+
+static const struct hid_device_id apple_magic_backlight_hid_ids[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_TOUCHBAR_BACKLIGHT) },
+	{ }
+};
+MODULE_DEVICE_TABLE(hid, apple_magic_backlight_hid_ids);
+
+static struct hid_driver apple_magic_backlight_hid_driver = {
+	.name = "apple-magic-backlight",
+	.id_table = apple_magic_backlight_hid_ids,
+	.probe = apple_magic_backlight_probe,
+	.remove = apple_magic_backlight_remove,
+};
+
+module_hid_driver(apple_magic_backlight_hid_driver);
+
+MODULE_DESCRIPTION("MacBook Magic Keyboard Backlight");
+MODULE_AUTHOR("Orlando Chamberlain <orlandoch.dev@gmail.com>");
+MODULE_LICENSE("GPL");
-- 
2.37.2


^ permalink raw reply related

* [PATCH 2/3] HID: apple-touchbar: Add driver for the Touch Bar on MacBook Pros
From: Aditya Garg @ 2023-02-10  3:44 UTC (permalink / raw)
  To: Jiri Kosina, jkosina@suse.cz, benjamin.tissoires@redhat.com,
	Andy Shevchenko, andy.shevchenko@gmail.com
  Cc: LKML, linux-input@vger.kernel.org, ronald@innovation.ch,
	kekrby@gmail.com, Orlando Chamberlain
In-Reply-To: <40274C3D-4F4F-479C-944C-EEBDC78F959C@live.com>

From: Ronald Tschalär <ronald@innovation.ch>

This driver enables basic touch bar functionality: enabling it, switching
between modes on FN key press, and dimming and turning the display
off/on when idle/active.

Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
[Kerem Karabay: use USB product IDs from hid-ids.h]
[Kerem Karabay: use hid_hw_raw_request except when setting the touchbar mode on T1 Macs]
[Kerem Karabay: update Kconfig description]
Signed-off-by: Kerem Karabay <kekrby@gmail.com>
[Orlando Chamberlain: add usage check to not bind to keyboard backlight interface]
Signed-off-by: Orlando Chamberlain <orlandoch.dev@gmail.com>
[Aditya Garg: check if apple-touchbar is enabled in the special driver list]
[Aditya Garg: fix suspend on T2 Macs]
Signed-off-by: Aditya Garg <gargaditya08@live.com>
---
 drivers/hid/Kconfig          |   11 +
 drivers/hid/Makefile         |    1 +
 drivers/hid/apple-touchbar.c | 1500 ++++++++++++++++++++++++++++++++++
 drivers/hid/hid-quirks.c     |    6 +-
 4 files changed, 1516 insertions(+), 2 deletions(-)
 create mode 100644 drivers/hid/apple-touchbar.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index e69afa5f4..4ec669267 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -134,6 +134,7 @@ config HID_APPLE_IBRIDGE
 	tristate "Apple iBridge"
 	depends on USB_HID
 	depends on (X86 && ACPI) || COMPILE_TEST
+	imply HID_APPLE_TOUCHBAR
 	imply HID_SENSOR_HUB
 	imply HID_SENSOR_ALS
 	help
@@ -145,6 +146,16 @@ config HID_APPLE_IBRIDGE
 	To compile this driver as a module, choose M here: the
 	module will be called apple-ibridge.
 
+config HID_APPLE_TOUCHBAR
+	tristate "Apple Touch Bar"
+	depends on USB_HID
+	help
+	Say Y here if you want support for the Touch Bar on x86
+	MacBook Pros.
+
+	To compile this driver as a module, choose M here: the
+	module will be called apple-touchbar.
+
 config HID_APPLEIR
 	tristate "Apple infrared receiver"
 	depends on (USB_HID)
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index b61373cd8..c792e42fe 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_HID_ALPS)		+= hid-alps.o
 obj-$(CONFIG_HID_ACRUX)		+= hid-axff.o
 obj-$(CONFIG_HID_APPLE)		+= hid-apple.o
 obj-$(CONFIG_HID_APPLE_IBRIDGE)	+= apple-ibridge.o
+obj-$(CONFIG_HID_APPLE_TOUCHBAR)	+= apple-touchbar.o
 obj-$(CONFIG_HID_APPLEIR)	+= hid-appleir.o
 obj-$(CONFIG_HID_CREATIVE_SB0540)	+= hid-creative-sb0540.o
 obj-$(CONFIG_HID_ASUS)		+= hid-asus.o
diff --git a/drivers/hid/apple-touchbar.c b/drivers/hid/apple-touchbar.c
new file mode 100644
index 000000000..ff6a8493b
--- /dev/null
+++ b/drivers/hid/apple-touchbar.c
@@ -0,0 +1,1500 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Apple Touch Bar Driver
+ *
+ * Copyright (c) 2017-2018 Ronald Tschalär
+ */
+
+/*
+ * Recent MacBookPro models (MacBookPro 13,[23] and later) have a touch bar,
+ * which is exposed via several USB interfaces. MacOS supports a fancy mode
+ * where arbitrary buttons can be defined; this driver currently only
+ * supports the simple mode that consists of 3 predefined layouts
+ * (escape-only, esc + special keys, and esc + function keys).
+ *
+ * The first USB HID interface supports two reports, an input report that
+ * is used to report the key presses, and an output report which can be
+ * used to set the touch bar "mode": touch bar off (in which case no touches
+ * are reported at all), escape key only, escape + 12 function keys, and
+ * escape + several special keys (including brightness, audio volume,
+ * etc). The second interface supports several, complex reports, most of
+ * which are unknown at this time, but one of which has been determined to
+ * allow for controlling of the touch bar's brightness: off (though touches
+ * are still reported), dimmed, and full brightness. This driver makes
+ * use of these two reports.
+ */
+
+#define dev_fmt(fmt) "tb: " fmt
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/input.h>
+#include <linux/jiffies.h>
+#include <linux/ktime.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/string.h>
+#include <linux/sysfs.h>
+#include <linux/usb/ch9.h>
+#include <linux/usb.h>
+#include <linux/workqueue.h>
+
+#include "hid-ids.h"
+#include "apple-ibridge.h"
+
+#define HID_UP_APPLE		0xff120000
+#define HID_USAGE_MODE		(HID_UP_CUSTOM | 0x0004)
+#define HID_USAGE_APPLE_APP	(HID_UP_APPLE  | 0x0001)
+#define HID_USAGE_DISP		(HID_UP_APPLE  | 0x0021)
+#define HID_USAGE_DISP_AUX1	(HID_UP_APPLE  | 0x0020)
+
+#define APPLETB_MAX_TB_KEYS	13	/* ESC, F1-F12 */
+
+#define APPLETB_CMD_MODE_ESC	0
+#define APPLETB_CMD_MODE_FN	1
+#define APPLETB_CMD_MODE_SPCL	2
+#define APPLETB_CMD_MODE_OFF	3
+#define APPLETB_CMD_MODE_UPD	254
+#define APPLETB_CMD_MODE_NONE	255
+
+#define APPLETB_CMD_DISP_ON	1
+#define APPLETB_CMD_DISP_DIM	2
+#define APPLETB_CMD_DISP_OFF	4
+#define APPLETB_CMD_DISP_UPD	254
+#define APPLETB_CMD_DISP_NONE	255
+
+#define APPLETB_FN_MODE_FKEYS	0
+#define APPLETB_FN_MODE_NORM	1
+#define APPLETB_FN_MODE_INV	2
+#define APPLETB_FN_MODE_SPCL	3
+#define APPLETB_FN_MODE_ESC	4
+#define APPLETB_FN_MODE_MAX	APPLETB_FN_MODE_ESC
+
+#define APPLETB_DEVID_KEYBOARD	1
+#define APPLETB_DEVID_TOUCHPAD	2
+
+#define APPLETB_MAX_DIM_TIME	30
+
+#define APPLETB_FEATURE_IS_T1	BIT(0)
+
+static int appletb_tb_def_idle_timeout = 5 * 60;
+module_param_named(idle_timeout, appletb_tb_def_idle_timeout, int, 0444);
+MODULE_PARM_DESC(idle_timeout, "Default touch bar idle timeout:\n"
+			       "    [>0] - turn touch bar display off after no keyboard, trackpad, or touch bar input has been received for this many seconds;\n"
+			       "         the display will be turned back on as soon as new input is received\n"
+			       "     0 - turn touch bar display off (input does not turn it on again)\n"
+			       "    -1 - turn touch bar display on (does not turn off automatically)\n"
+			       "    -2 - disable touch bar completely");
+
+static int appletb_tb_def_dim_timeout = -2;
+module_param_named(dim_timeout, appletb_tb_def_dim_timeout, int, 0444);
+MODULE_PARM_DESC(dim_timeout, "Default touch bar dim timeout:\n"
+			      "    >0 - dim touch bar display after no keyboard, trackpad, or touch bar input has been received for this many seconds\n"
+			      "         the display will be returned to full brightness as soon as new input is received\n"
+			      "     0 - dim touch bar display (input does not return it to full brightness)\n"
+			      "    -1 - disable timeout (touch bar never dimmed)\n"
+			      "    [-2] - calculate timeout based on idle-timeout");
+
+static int appletb_tb_def_fn_mode = APPLETB_FN_MODE_NORM;
+module_param_named(fnmode, appletb_tb_def_fn_mode, int, 0444);
+MODULE_PARM_DESC(fnmode, "Default Fn key mode:\n"
+			 "    0 - function-keys only\n"
+			 "    [1] - fn key switches from special to function-keys\n"
+			 "    2 - inverse of 1\n"
+			 "    3 - special keys only\n"
+			 "    4 - escape key only");
+
+static ssize_t idle_timeout_show(struct device *dev,
+				 struct device_attribute *attr, char *buf);
+static ssize_t idle_timeout_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t size);
+static DEVICE_ATTR_RW(idle_timeout);
+
+static ssize_t dim_timeout_show(struct device *dev,
+				struct device_attribute *attr, char *buf);
+static ssize_t dim_timeout_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t size);
+static DEVICE_ATTR_RW(dim_timeout);
+
+static ssize_t fnmode_show(struct device *dev, struct device_attribute *attr,
+			   char *buf);
+static ssize_t fnmode_store(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t size);
+static DEVICE_ATTR_RW(fnmode);
+
+static struct attribute *appletb_attrs[] = {
+	&dev_attr_idle_timeout.attr,
+	&dev_attr_dim_timeout.attr,
+	&dev_attr_fnmode.attr,
+	NULL,
+};
+
+static const struct attribute_group appletb_attr_group = {
+	.attrs = appletb_attrs,
+};
+
+struct appletb_device {
+	bool			active;
+	struct device		*log_dev;
+
+	struct hid_field	*mode_field;
+	struct hid_field	*disp_field;
+	struct hid_field	*disp_field_aux1;
+	struct appletb_iface_info {
+		struct hid_device	*hdev;
+		struct usb_interface	*usb_iface;
+		bool			suspended;
+	}			mode_iface, disp_iface;
+
+	struct input_handler	inp_handler;
+	struct input_handle	kbd_handle;
+	struct input_handle	tpd_handle;
+
+	bool			last_tb_keys_pressed[APPLETB_MAX_TB_KEYS];
+	bool			last_tb_keys_translated[APPLETB_MAX_TB_KEYS];
+	bool			last_fn_pressed;
+
+	ktime_t			last_event_time;
+
+	unsigned char		cur_tb_mode;
+	unsigned char		pnd_tb_mode;
+	unsigned char		cur_tb_disp;
+	unsigned char		pnd_tb_disp;
+	bool			tb_autopm_off;
+	bool			restore_autopm;
+	struct delayed_work	tb_work;
+	/* protects most of the above */
+	spinlock_t		tb_lock;
+
+	int			dim_timeout;
+	int			idle_timeout;
+	bool			dim_to_is_calc;
+	int			fn_mode;
+
+	bool			is_t1;
+};
+
+struct appletb_key_translation {
+	u16 from;
+	u16 to;
+};
+
+static const struct appletb_key_translation appletb_fn_codes[] = {
+	{ KEY_F1,  KEY_BRIGHTNESSDOWN },
+	{ KEY_F2,  KEY_BRIGHTNESSUP },
+	{ KEY_F3,  KEY_SCALE },		/* not used */
+	{ KEY_F4,  KEY_DASHBOARD },	/* not used */
+	{ KEY_F5,  KEY_KBDILLUMDOWN },
+	{ KEY_F6,  KEY_KBDILLUMUP },
+	{ KEY_F7,  KEY_PREVIOUSSONG },
+	{ KEY_F8,  KEY_PLAYPAUSE },
+	{ KEY_F9,  KEY_NEXTSONG },
+	{ KEY_F10, KEY_MUTE },
+	{ KEY_F11, KEY_VOLUMEDOWN },
+	{ KEY_F12, KEY_VOLUMEUP },
+};
+
+static struct appletb_device *appletb_dev;
+
+static bool appletb_disable_autopm(struct hid_device *hdev)
+{
+	int rc;
+
+	rc = hid_hw_power(hdev, PM_HINT_FULLON);
+
+	if (rc == 0)
+		return true;
+
+	hid_err(hdev,
+		"Failed to disable auto-pm on touch bar device (%d)\n", rc);
+	return false;
+}
+
+/*
+ * While the mode functionality is listed as a valid hid report in the usb
+ * interface descriptor, on a T1 it's not sent that way. Instead it's sent with
+ * different request-type and without a leading report-id in the data. Hence
+ * we need to send it as a custom usb control message rather via any of the
+ * standard hid_hw_*request() functions. The device might return EPIPE for a
+ * while after setting the display mode on T1 models, so retrying should be
+ * done on those models.
+ */
+static int appletb_set_tb_mode(struct appletb_device *tb_dev,
+			       unsigned char mode)
+{
+	struct hid_report *report;
+	void *buf;
+	bool autopm_off = false;
+	int rc;
+
+	if (!tb_dev->mode_iface.hdev)
+		return -ENOTCONN;
+
+	report = tb_dev->mode_field->report;
+
+	if (tb_dev->is_t1) {
+		buf = kmemdup(&mode, 1, GFP_KERNEL);
+	} else {
+		char data[] = { report->id, mode };
+
+		buf = kmemdup(data, sizeof(data), GFP_KERNEL);
+	}
+	if (!buf)
+		return -ENOMEM;
+
+	autopm_off = appletb_disable_autopm(tb_dev->mode_iface.hdev);
+
+	if (tb_dev->is_t1) {
+		int tries = 0;
+		struct usb_device *dev = interface_to_usbdev(tb_dev->mode_iface.usb_iface);
+		__u8 ifnum = tb_dev->mode_iface.usb_iface->cur_altsetting->desc.bInterfaceNumber;
+
+		do {
+			rc = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), HID_REQ_SET_REPORT,
+					     USB_DIR_OUT | USB_RECIP_INTERFACE | USB_TYPE_VENDOR,
+					     (report->type + 1) << 8 | report->id,
+					     ifnum, buf, 1, 2000);
+
+			if (rc != -EPIPE)
+				break;
+
+			usleep_range(1000 << tries, 3000 << tries);
+		} while (++tries < 5);
+	} else {
+		rc = hid_hw_raw_request(tb_dev->mode_iface.hdev, report->id,
+					(__u8 *) buf, 2, report->type,
+					HID_REQ_SET_REPORT);
+	}
+
+	if (rc < 0)
+		dev_err(tb_dev->log_dev,
+			"Failed to set touch bar mode to %u (%d)\n", mode, rc);
+
+	if (autopm_off)
+		hid_hw_power(tb_dev->mode_iface.hdev, PM_HINT_NORMAL);
+
+	kfree(buf);
+
+	return rc;
+}
+
+static int appletb_set_tb_disp(struct appletb_device *tb_dev,
+			       unsigned char disp)
+{
+	struct hid_report *report;
+	int rc;
+
+	if (!tb_dev->disp_iface.hdev)
+		return -ENOTCONN;
+
+	report = tb_dev->disp_field->report;
+
+	rc = hid_set_field(tb_dev->disp_field_aux1, 0, 1);
+	if (rc) {
+		dev_err(tb_dev->log_dev,
+			"Failed to set display report field (%d)\n", rc);
+		return rc;
+	}
+
+	rc = hid_set_field(tb_dev->disp_field, 0, disp);
+	if (rc) {
+		dev_err(tb_dev->log_dev,
+			"Failed to set display report field (%d)\n", rc);
+		return rc;
+	}
+
+	/*
+	 * Keep the USB interface powered on while the touch bar display is on
+	 * for better responsiveness.
+	 */
+	if (disp != APPLETB_CMD_DISP_OFF && !tb_dev->tb_autopm_off)
+		tb_dev->tb_autopm_off =
+			appletb_disable_autopm(report->device);
+
+	hid_hw_request(tb_dev->disp_iface.hdev, report, HID_REQ_SET_REPORT);
+
+	if (disp == APPLETB_CMD_DISP_OFF && tb_dev->tb_autopm_off) {
+		hid_hw_power(tb_dev->disp_iface.hdev, PM_HINT_NORMAL);
+		tb_dev->tb_autopm_off = false;
+	}
+
+	return rc;
+}
+
+static bool appletb_any_tb_key_pressed(struct appletb_device *tb_dev)
+{
+	return !!memchr_inv(tb_dev->last_tb_keys_pressed, 0,
+			    sizeof(tb_dev->last_tb_keys_pressed));
+}
+
+static void appletb_schedule_tb_update(struct appletb_device *tb_dev, s64 secs)
+{
+	schedule_delayed_work(&tb_dev->tb_work, msecs_to_jiffies(secs * 1000));
+}
+
+static void appletb_set_tb_worker(struct work_struct *work)
+{
+	struct appletb_device *tb_dev =
+		container_of(work, struct appletb_device, tb_work.work);
+	s64 time_left = 0, min_timeout, time_to_off;
+	unsigned char pending_mode;
+	unsigned char pending_disp;
+	unsigned char current_disp;
+	bool restore_autopm;
+	bool any_tb_key_pressed, need_reschedule;
+	int rc1 = 1, rc2 = 1;
+	unsigned long flags;
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	/* handle explicit mode-change request */
+	pending_mode = tb_dev->pnd_tb_mode;
+	pending_disp = tb_dev->pnd_tb_disp;
+	restore_autopm = tb_dev->restore_autopm;
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+
+	if (pending_mode != APPLETB_CMD_MODE_NONE)
+		rc1 = appletb_set_tb_mode(tb_dev, pending_mode);
+	if (pending_mode != APPLETB_CMD_MODE_NONE &&
+	    pending_disp != APPLETB_CMD_DISP_NONE)
+		msleep(25);
+	if (pending_disp != APPLETB_CMD_DISP_NONE)
+		rc2 = appletb_set_tb_disp(tb_dev, pending_disp);
+
+	if (restore_autopm && tb_dev->tb_autopm_off)
+		appletb_disable_autopm(tb_dev->disp_field->report->device);
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	need_reschedule = false;
+
+	if (rc1 == 0) {
+		tb_dev->cur_tb_mode = pending_mode;
+
+		if (tb_dev->pnd_tb_mode == pending_mode)
+			tb_dev->pnd_tb_mode = APPLETB_CMD_MODE_NONE;
+		else
+			need_reschedule = true;
+	}
+
+	if (rc2 == 0) {
+		tb_dev->cur_tb_disp = pending_disp;
+
+		if (tb_dev->pnd_tb_disp == pending_disp)
+			tb_dev->pnd_tb_disp = APPLETB_CMD_DISP_NONE;
+		else
+			need_reschedule = true;
+	}
+	current_disp = tb_dev->cur_tb_disp;
+
+	tb_dev->restore_autopm = false;
+
+	/* calculate time left to next timeout */
+	if (tb_dev->idle_timeout == -2 || tb_dev->idle_timeout == 0)
+		min_timeout = -1;
+	else if (tb_dev->idle_timeout == -1)
+		min_timeout = tb_dev->dim_timeout;
+	else if (tb_dev->dim_timeout <= 0)
+		min_timeout = tb_dev->idle_timeout;
+	else
+		min_timeout = min(tb_dev->dim_timeout, tb_dev->idle_timeout);
+
+	if (min_timeout > 0) {
+		s64 idle_time =
+			(ktime_ms_delta(ktime_get(), tb_dev->last_event_time) +
+			 500) / 1000;
+
+		time_left = max(min_timeout - idle_time, 0LL);
+		if (tb_dev->idle_timeout <= 0)
+			time_to_off = -1;
+		else if (idle_time >= tb_dev->idle_timeout)
+			time_to_off = 0;
+		else
+			time_to_off = tb_dev->idle_timeout - idle_time;
+	} else {
+		/* not used - just to appease the compiler */
+		time_to_off = 0;
+	}
+
+	any_tb_key_pressed = appletb_any_tb_key_pressed(tb_dev);
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+
+	dev_dbg(tb_dev->log_dev, "timeout calc: idle_timeout=%d dim_timeout=%d min_timeout=%lld time_left=%lld need_reschedule=%d any_tb_key_pressed=%d\n",
+		tb_dev->idle_timeout, tb_dev->dim_timeout, min_timeout,
+		time_left, need_reschedule, any_tb_key_pressed);
+
+	/* a new command arrived while we were busy - handle it */
+	if (need_reschedule) {
+		appletb_schedule_tb_update(tb_dev, 0);
+		return;
+	}
+
+	/* if no idle/dim timeout, we're done */
+	if (min_timeout <= 0)
+		return;
+
+	/* manage idle/dim timeout */
+	if (time_left > 0) {
+		/* we fired too soon or had a mode-change - re-schedule */
+		appletb_schedule_tb_update(tb_dev, time_left);
+	} else if (any_tb_key_pressed) {
+		/* keys are still pressed - re-schedule */
+		appletb_schedule_tb_update(tb_dev, min_timeout);
+	} else {
+		/* dim or idle timeout reached */
+		int next_disp = (time_to_off == 0) ? APPLETB_CMD_DISP_OFF :
+						     APPLETB_CMD_DISP_DIM;
+		if (next_disp != current_disp &&
+		    appletb_set_tb_disp(tb_dev, next_disp) == 0) {
+			spin_lock_irqsave(&tb_dev->tb_lock, flags);
+			tb_dev->cur_tb_disp = next_disp;
+			spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+		}
+
+		if (time_to_off > 0)
+			appletb_schedule_tb_update(tb_dev, time_to_off);
+	}
+}
+
+static u16 appletb_fn_to_special(u16 code)
+{
+	int idx;
+
+	for (idx = 0; idx < ARRAY_SIZE(appletb_fn_codes); idx++) {
+		if (appletb_fn_codes[idx].from == code)
+			return appletb_fn_codes[idx].to;
+	}
+
+	return 0;
+}
+
+static unsigned char appletb_get_cur_tb_mode(struct appletb_device *tb_dev)
+{
+	return tb_dev->pnd_tb_mode != APPLETB_CMD_MODE_NONE ?
+				tb_dev->pnd_tb_mode : tb_dev->cur_tb_mode;
+}
+
+static unsigned char appletb_get_cur_tb_disp(struct appletb_device *tb_dev)
+{
+	return tb_dev->pnd_tb_disp != APPLETB_CMD_DISP_NONE ?
+				tb_dev->pnd_tb_disp : tb_dev->cur_tb_disp;
+}
+
+static unsigned char appletb_get_fn_tb_mode(struct appletb_device *tb_dev)
+{
+	switch (tb_dev->fn_mode) {
+	case APPLETB_FN_MODE_ESC:
+		return APPLETB_CMD_MODE_ESC;
+
+	case APPLETB_FN_MODE_FKEYS:
+		return APPLETB_CMD_MODE_FN;
+
+	case APPLETB_FN_MODE_SPCL:
+		return APPLETB_CMD_MODE_SPCL;
+
+	case APPLETB_FN_MODE_INV:
+		return (tb_dev->last_fn_pressed) ? APPLETB_CMD_MODE_SPCL :
+						   APPLETB_CMD_MODE_FN;
+
+	case APPLETB_FN_MODE_NORM:
+	default:
+		return (tb_dev->last_fn_pressed) ? APPLETB_CMD_MODE_FN :
+						   APPLETB_CMD_MODE_SPCL;
+	}
+}
+
+/*
+ * Switch touch bar mode and display when mode or display not the desired ones.
+ */
+static void appletb_update_touchbar_no_lock(struct appletb_device *tb_dev,
+					    bool force)
+{
+	unsigned char want_mode;
+	unsigned char want_disp;
+	bool need_update = false;
+
+	/*
+	 * Calculate the new modes:
+	 *   idle_timeout:
+	 *     -2  mode/disp off
+	 *     -1  mode on, disp on/dim
+	 *      0  mode on, disp off
+	 *     >0  mode on, disp off after idle_timeout seconds
+	 *   dim_timeout (only valid if idle_timeout > 0 || idle_timeout == -1):
+	 *     -1  disp never dimmed
+	 *      0  disp always dimmed
+	 *     >0  disp dim after dim_timeout seconds
+	 */
+	if (tb_dev->idle_timeout == -2) {
+		want_mode = APPLETB_CMD_MODE_OFF;
+		want_disp = APPLETB_CMD_DISP_OFF;
+	} else {
+		want_mode = appletb_get_fn_tb_mode(tb_dev);
+		want_disp = tb_dev->idle_timeout ==  0 ? APPLETB_CMD_DISP_OFF :
+			    tb_dev->dim_timeout  ==  0 ? APPLETB_CMD_DISP_DIM :
+							 APPLETB_CMD_DISP_ON;
+	}
+
+	/*
+	 * See if we need to update the touch bar, taking into account that we
+	 * generally don't want to switch modes while a touch bar key is
+	 * pressed.
+	 */
+	if (appletb_get_cur_tb_mode(tb_dev) != want_mode &&
+	    !appletb_any_tb_key_pressed(tb_dev)) {
+		tb_dev->pnd_tb_mode = want_mode;
+		need_update = true;
+	}
+
+	if (appletb_get_cur_tb_disp(tb_dev) != want_disp &&
+	    (!appletb_any_tb_key_pressed(tb_dev) ||
+	     want_disp != APPLETB_CMD_DISP_OFF)) {
+		tb_dev->pnd_tb_disp = want_disp;
+		need_update = true;
+	}
+
+	if (force)
+		need_update = true;
+
+	/* schedule the update if desired */
+	dev_dbg_ratelimited(tb_dev->log_dev,
+			    "update: need_update=%d, want_mode=%d, cur-mode=%d, want_disp=%d, cur-disp=%d\n",
+			    need_update, want_mode, tb_dev->cur_tb_mode,
+			    want_disp, tb_dev->cur_tb_disp);
+
+	if (need_update) {
+		cancel_delayed_work(&tb_dev->tb_work);
+		appletb_schedule_tb_update(tb_dev, 0);
+	}
+}
+
+static void appletb_update_touchbar(struct appletb_device *tb_dev, bool force)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	if (tb_dev->active)
+		appletb_update_touchbar_no_lock(tb_dev, force);
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+}
+
+static void appletb_set_idle_timeout(struct appletb_device *tb_dev, int new)
+{
+	tb_dev->idle_timeout = new;
+
+	if (tb_dev->dim_to_is_calc && tb_dev->idle_timeout > 0)
+		tb_dev->dim_timeout = new - min(APPLETB_MAX_DIM_TIME, new / 3);
+	else if (tb_dev->dim_to_is_calc)
+		tb_dev->dim_timeout = -1;
+}
+
+static ssize_t idle_timeout_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct appletb_device *tb_dev = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", tb_dev->idle_timeout);
+}
+
+static ssize_t idle_timeout_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t size)
+{
+	struct appletb_device *tb_dev = dev_get_drvdata(dev);
+	long new;
+	int rc;
+
+	rc = kstrtol(buf, 0, &new);
+	if (rc || new > INT_MAX || new < -2)
+		return -EINVAL;
+
+	appletb_set_idle_timeout(tb_dev, new);
+	appletb_update_touchbar(tb_dev, true);
+
+	return size;
+}
+
+static void appletb_set_dim_timeout(struct appletb_device *tb_dev, int new)
+{
+	if (new == -2) {
+		tb_dev->dim_to_is_calc = true;
+		appletb_set_idle_timeout(tb_dev, tb_dev->idle_timeout);
+	} else {
+		tb_dev->dim_to_is_calc = false;
+		tb_dev->dim_timeout = new;
+	}
+}
+
+static ssize_t dim_timeout_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct appletb_device *tb_dev = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n",
+			tb_dev->dim_to_is_calc ? -2 : tb_dev->dim_timeout);
+}
+
+static ssize_t dim_timeout_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t size)
+{
+	struct appletb_device *tb_dev = dev_get_drvdata(dev);
+	long new;
+	int rc;
+
+	rc = kstrtol(buf, 0, &new);
+	if (rc || new > INT_MAX || new < -2)
+		return -EINVAL;
+
+	appletb_set_dim_timeout(tb_dev, new);
+	appletb_update_touchbar(tb_dev, true);
+
+	return size;
+}
+
+static ssize_t fnmode_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	struct appletb_device *tb_dev = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", tb_dev->fn_mode);
+}
+
+static ssize_t fnmode_store(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t size)
+{
+	struct appletb_device *tb_dev = dev_get_drvdata(dev);
+	long new;
+	int rc;
+
+	rc = kstrtol(buf, 0, &new);
+	if (rc || new > APPLETB_FN_MODE_MAX || new < 0)
+		return -EINVAL;
+
+	tb_dev->fn_mode = new;
+	appletb_update_touchbar(tb_dev, false);
+
+	return size;
+}
+
+static int appletb_tb_key_to_slot(unsigned int code)
+{
+	switch (code) {
+	case KEY_ESC:
+		return 0;
+	case KEY_F1:
+	case KEY_F2:
+	case KEY_F3:
+	case KEY_F4:
+	case KEY_F5:
+	case KEY_F6:
+	case KEY_F7:
+	case KEY_F8:
+	case KEY_F9:
+	case KEY_F10:
+		return code - KEY_F1 + 1;
+	case KEY_F11:
+	case KEY_F12:
+		return code - KEY_F11 + 11;
+	default:
+		return -1;
+	}
+}
+
+static int appletb_hid_event(struct hid_device *hdev, struct hid_field *field,
+			     struct hid_usage *usage, __s32 value)
+{
+	struct appletb_device *tb_dev = hid_get_drvdata(hdev);
+	unsigned int new_code = 0;
+	unsigned long flags;
+	bool send_dummy = false;
+	bool send_trnsl = false;
+	int slot;
+	int rc = 0;
+
+	if ((usage->hid & HID_USAGE_PAGE) != HID_UP_KEYBOARD ||
+	    usage->type != EV_KEY)
+		return 0;
+
+	/*
+	 * Skip non-touch-bar keys.
+	 *
+	 * Either the touch bar itself or usbhid generate a slew of key-down
+	 * events for all the meta keys. None of which we're at all interested
+	 * in.
+	 */
+	slot = appletb_tb_key_to_slot(usage->code);
+	if (slot < 0)
+		return 0;
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	if (!tb_dev->active) {
+		spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+		return 0;
+	}
+
+	new_code = appletb_fn_to_special(usage->code);
+
+	if (value != 2)
+		tb_dev->last_tb_keys_pressed[slot] = value;
+
+	tb_dev->last_event_time = ktime_get();
+
+	appletb_update_touchbar_no_lock(tb_dev, false);
+
+	/*
+	 * We want to suppress touch bar keys while the touch bar is off, but
+	 * we do want to wake up the screen if it's asleep, so generate a dummy
+	 * event in that case.
+	 */
+	if (tb_dev->cur_tb_mode == APPLETB_CMD_MODE_OFF ||
+	    tb_dev->cur_tb_disp == APPLETB_CMD_DISP_OFF) {
+		send_dummy = true;
+		rc = 1;
+	/* translate special keys */
+	} else if (new_code &&
+		   ((value > 0 &&
+		     appletb_get_cur_tb_mode(tb_dev) == APPLETB_CMD_MODE_SPCL)
+		    ||
+		    (value == 0 && tb_dev->last_tb_keys_translated[slot]))) {
+		tb_dev->last_tb_keys_translated[slot] = true;
+		send_trnsl = true;
+		rc = 1;
+	/* everything else handled normally */
+	} else {
+		tb_dev->last_tb_keys_translated[slot] = false;
+	}
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+
+	/*
+	 * Need to send these input events outside of the lock, as otherwise
+	 * we can run into the following deadlock:
+	 *            Task 1                         Task 2
+	 *     appletb_hid_event()            input_event()
+	 *       acquire tb_lock                acquire dev->event_lock
+	 *       input_event()                  appletb_inp_event()
+	 *         acquire dev->event_lock        acquire tb_lock
+	 */
+	if (send_dummy) {
+		input_event(field->hidinput->input, EV_KEY, KEY_UNKNOWN, 1);
+		input_event(field->hidinput->input, EV_KEY, KEY_UNKNOWN, 0);
+	} else if (send_trnsl) {
+		input_event(field->hidinput->input, usage->type, new_code,
+			    value);
+	}
+
+	return rc;
+}
+
+static void appletb_inp_event(struct input_handle *handle, unsigned int type,
+			      unsigned int code, int value)
+{
+	struct appletb_device *tb_dev = handle->private;
+	unsigned long flags;
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	if (!tb_dev->active) {
+		spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+		return;
+	}
+
+	if (type == EV_KEY && code == KEY_FN && value != 2)
+		tb_dev->last_fn_pressed = value;
+
+	tb_dev->last_event_time = ktime_get();
+
+	appletb_update_touchbar_no_lock(tb_dev, false);
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+}
+
+/* Find and save the usb-device associated with the touch bar input device */
+static struct usb_interface *appletb_get_usb_iface(struct hid_device *hdev)
+{
+	struct device *dev = &hdev->dev;
+
+	while (dev && !(dev->type && dev->type->name &&
+			!strcmp(dev->type->name, "usb_interface")))
+		dev = dev->parent;
+
+	return dev ? to_usb_interface(dev) : NULL;
+}
+
+static int appletb_inp_connect(struct input_handler *handler,
+			       struct input_dev *dev,
+			       const struct input_device_id *id)
+{
+	struct appletb_device *tb_dev = handler->private;
+	struct input_handle *handle;
+	int rc;
+
+	if (id->driver_info == APPLETB_DEVID_KEYBOARD) {
+		handle = &tb_dev->kbd_handle;
+		handle->name = "tbkbd";
+	} else if (id->driver_info == APPLETB_DEVID_TOUCHPAD) {
+		handle = &tb_dev->tpd_handle;
+		handle->name = "tbtpad";
+	} else {
+		dev_err(tb_dev->log_dev, "Unknown device id (%lu)\n",
+			id->driver_info);
+		return -ENOENT;
+	}
+
+	if (handle->dev) {
+		dev_err(tb_dev->log_dev,
+			"Duplicate connect to %s input device\n", handle->name);
+		return -EEXIST;
+	}
+
+	handle->open = 0;
+	handle->dev = input_get_device(dev);
+	handle->handler = handler;
+	handle->private = tb_dev;
+
+	rc = input_register_handle(handle);
+	if (rc)
+		goto err_free_dev;
+
+	rc = input_open_device(handle);
+	if (rc)
+		goto err_unregister_handle;
+
+	dev_dbg(tb_dev->log_dev, "Connected to %s input device\n",
+		handle == &tb_dev->kbd_handle ? "keyboard" : "touchpad");
+
+	return 0;
+
+ err_unregister_handle:
+	input_unregister_handle(handle);
+ err_free_dev:
+	input_put_device(handle->dev);
+	handle->dev = NULL;
+	return rc;
+}
+
+static void appletb_inp_disconnect(struct input_handle *handle)
+{
+	struct appletb_device *tb_dev = handle->private;
+
+	input_close_device(handle);
+	input_unregister_handle(handle);
+
+	dev_dbg(tb_dev->log_dev, "Disconnected from %s input device\n",
+		handle == &tb_dev->kbd_handle ? "keyboard" : "touchpad");
+
+	input_put_device(handle->dev);
+	handle->dev = NULL;
+}
+
+static int appletb_input_configured(struct hid_device *hdev,
+				    struct hid_input *hidinput)
+{
+	int idx;
+	struct input_dev *input = hidinput->input;
+
+	/*
+	 * Clear various input capabilities that are blindly set by the hid
+	 * driver (usbkbd.c)
+	 */
+	memset(input->evbit, 0, sizeof(input->evbit));
+	memset(input->keybit, 0, sizeof(input->keybit));
+	memset(input->ledbit, 0, sizeof(input->ledbit));
+
+	/* set our actual capabilities */
+	__set_bit(EV_KEY, input->evbit);
+	__set_bit(EV_REP, input->evbit);
+	__set_bit(EV_MSC, input->evbit);  /* hid-input generates MSC_SCAN */
+
+	for (idx = 0; idx < ARRAY_SIZE(appletb_fn_codes); idx++) {
+		input_set_capability(input, EV_KEY, appletb_fn_codes[idx].from);
+		input_set_capability(input, EV_KEY, appletb_fn_codes[idx].to);
+	}
+
+	input_set_capability(input, EV_KEY, KEY_ESC);
+	input_set_capability(input, EV_KEY, KEY_UNKNOWN);
+
+	return 0;
+}
+
+static struct appletb_iface_info *
+appletb_get_iface_info(struct appletb_device *tb_dev, struct hid_device *hdev)
+{
+	if (hdev == tb_dev->mode_iface.hdev)
+		return &tb_dev->mode_iface;
+	if (hdev == tb_dev->disp_iface.hdev)
+		return &tb_dev->disp_iface;
+	return NULL;
+}
+
+/**
+ * appletb_find_report_field() - Find the field in the report with the given
+ * usage.
+ * @report: the report to search
+ * @field_usage: the usage of the field to search for
+ *
+ * Returns: the hid field if found, or NULL if none found.
+ */
+static struct hid_field *appletb_find_report_field(struct hid_report *report,
+						   unsigned int field_usage)
+{
+	int f, u;
+
+	for (f = 0; f < report->maxfield; f++) {
+		struct hid_field *field = report->field[f];
+
+		if (field->logical == field_usage)
+			return field;
+
+		for (u = 0; u < field->maxusage; u++) {
+			if (field->usage[u].hid == field_usage)
+				return field;
+		}
+	}
+
+	return NULL;
+}
+
+/**
+ * appletb_find_hid_field() - Search all the reports of the device for the
+ * field with the given usage.
+ * @hdev: the device whose reports to search
+ * @application: the usage of application collection that the field must
+ *               belong to
+ * @field_usage: the usage of the field to search for
+ *
+ * Returns: the hid field if found, or NULL if none found.
+ */
+static struct hid_field *appletb_find_hid_field(struct hid_device *hdev,
+						unsigned int application,
+						unsigned int field_usage)
+{
+	static const int report_types[] = { HID_INPUT_REPORT, HID_OUTPUT_REPORT,
+					    HID_FEATURE_REPORT };
+	struct hid_report *report;
+	struct hid_field *field;
+	int t;
+
+	for (t = 0; t < ARRAY_SIZE(report_types); t++) {
+		struct list_head *report_list =
+			    &hdev->report_enum[report_types[t]].report_list;
+		list_for_each_entry(report, report_list, list) {
+			if (report->application != application)
+				continue;
+
+			field = appletb_find_report_field(report, field_usage);
+			if (field)
+				return field;
+		}
+	}
+
+	return NULL;
+}
+
+static int appletb_extract_report_and_iface_info(struct appletb_device *tb_dev,
+						 struct hid_device *hdev,
+						 const struct hid_device_id *id)
+{
+	struct appletb_iface_info *iface_info;
+	struct usb_interface *usb_iface;
+	struct hid_field *field;
+
+	field = appletb_find_hid_field(hdev, HID_GD_KEYBOARD, HID_USAGE_MODE);
+	if (field) {
+		iface_info = &tb_dev->mode_iface;
+		tb_dev->mode_field = field;
+		tb_dev->is_t1 = !!(id->driver_data & APPLETB_FEATURE_IS_T1);
+	} else {
+		field = appletb_find_hid_field(hdev, HID_USAGE_APPLE_APP,
+					       HID_USAGE_DISP);
+		if (!field)
+			return 0;
+
+		iface_info = &tb_dev->disp_iface;
+		tb_dev->disp_field = field;
+		tb_dev->disp_field_aux1 =
+			appletb_find_hid_field(hdev, HID_USAGE_APPLE_APP,
+					       HID_USAGE_DISP_AUX1);
+
+		if (!tb_dev->disp_field_aux1 ||
+		    tb_dev->disp_field_aux1->report !=
+						tb_dev->disp_field->report) {
+			dev_err(tb_dev->log_dev,
+				"Unexpected report structure for report %u in device %s\n",
+				tb_dev->disp_field->report->id,
+				dev_name(&hdev->dev));
+			return -ENODEV;
+		}
+	}
+
+	usb_iface = appletb_get_usb_iface(hdev);
+	if (!usb_iface) {
+		dev_err(tb_dev->log_dev,
+			"Failed to find usb interface for hid device %s\n",
+			dev_name(&hdev->dev));
+		return -ENODEV;
+	}
+
+	iface_info->hdev = hdev;
+	iface_info->usb_iface = usb_get_intf(usb_iface);
+	iface_info->suspended = false;
+
+	return 1;
+}
+
+static void appletb_clear_iface_info(struct appletb_device *tb_dev,
+				     struct hid_device *hdev)
+{
+	struct appletb_iface_info *iface_info;
+
+	iface_info = appletb_get_iface_info(tb_dev, hdev);
+	if (iface_info) {
+		usb_put_intf(iface_info->usb_iface);
+		iface_info->usb_iface = NULL;
+		iface_info->hdev = NULL;
+	}
+}
+
+static bool appletb_test_and_mark_active(struct appletb_device *tb_dev)
+{
+	unsigned long flags;
+	bool activated = false;
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	if (tb_dev->mode_iface.hdev && tb_dev->disp_iface.hdev &&
+	    !tb_dev->active) {
+		tb_dev->active = true;
+		activated = true;
+	}
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+
+	return activated;
+}
+
+static bool appletb_test_and_mark_inactive(struct appletb_device *tb_dev,
+					   struct hid_device *hdev)
+{
+	unsigned long flags;
+	bool deactivated = false;
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	if (tb_dev->mode_iface.hdev && tb_dev->disp_iface.hdev &&
+	    tb_dev->active &&
+	    (hdev == tb_dev->mode_iface.hdev ||
+	     hdev == tb_dev->disp_iface.hdev)) {
+		tb_dev->active = false;
+		deactivated = true;
+	}
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+
+	return deactivated;
+}
+
+static const struct input_device_id appletb_input_devices[] = {
+	{
+		.flags = INPUT_DEVICE_ID_MATCH_BUS |
+			INPUT_DEVICE_ID_MATCH_KEYBIT,
+		.bustype = BUS_SPI,
+		.keybit = { [BIT_WORD(KEY_FN)] = BIT_MASK(KEY_FN) },
+		.driver_info = APPLETB_DEVID_KEYBOARD,
+	},			/* Builtin SPI keyboard device */
+	{
+		.flags = INPUT_DEVICE_ID_MATCH_BUS |
+			INPUT_DEVICE_ID_MATCH_KEYBIT,
+		.bustype = BUS_SPI,
+		.keybit = { [BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH) },
+		.driver_info = APPLETB_DEVID_TOUCHPAD,
+	},			/* Builtin SPI touchpad device */
+	{
+		.flags = INPUT_DEVICE_ID_MATCH_BUS |
+			INPUT_DEVICE_ID_MATCH_VENDOR |
+			INPUT_DEVICE_ID_MATCH_KEYBIT,
+		.bustype = BUS_USB,
+		.vendor = 0x05ac /* USB_VENDOR_ID_APPLE */,
+		.keybit = { [BIT_WORD(KEY_FN)] = BIT_MASK(KEY_FN) },
+		.driver_info = APPLETB_DEVID_KEYBOARD,
+	},			/* Builtin USB keyboard device */
+	{
+		.flags = INPUT_DEVICE_ID_MATCH_BUS |
+			INPUT_DEVICE_ID_MATCH_VENDOR |
+			INPUT_DEVICE_ID_MATCH_KEYBIT,
+		.bustype = BUS_USB,
+		.vendor = 0x05ac /* USB_VENDOR_ID_APPLE */,
+		.keybit = { [BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH) },
+		.driver_info = APPLETB_DEVID_TOUCHPAD,
+	},			/* Builtin USB touchpad device */
+	{ },			/* Terminating zero entry */
+};
+
+static bool appletb_match_internal_device(struct input_handler *handler,
+					  struct input_dev *inp_dev)
+{
+	struct device *dev = &inp_dev->dev;
+
+	if (inp_dev->id.bustype == BUS_SPI)
+		return true;
+
+	/* in kernel: dev && !is_usb_device(dev) */
+	while (dev && !(dev->type && dev->type->name &&
+			!strcmp(dev->type->name, "usb_device")))
+		dev = dev->parent;
+
+	/*
+	 * Apple labels all their internal keyboards and trackpads as such,
+	 * instead of maintaining an ever expanding list of product-id's we
+	 * just look at the device's product name.
+	 */
+	if (dev)
+		return !!strstr(to_usb_device(dev)->product, "Internal Keyboard");
+
+	return false;
+}
+
+static int appletb_probe(struct hid_device *hdev,
+			 const struct hid_device_id *id)
+{
+	struct appletb_device *tb_dev = appletb_dev;
+	unsigned long flags;
+	int rc;
+
+	/* initialize the report info */
+	rc = hid_parse(hdev);
+	if (rc) {
+		dev_err(tb_dev->log_dev, "hid parse failed (%d)\n", rc);
+		goto error;
+	}
+
+	/* Ensure this usb endpoint is for the touchbar backlight, not keyboard
+	 * backlight.
+	 */
+	if ((hdev->product == USB_DEVICE_ID_APPLE_TOUCHBAR_BACKLIGHT) &&
+			!(hdev->collection && hdev->collection[0].usage ==
+				HID_USAGE_APPLE_APP)) {
+		return -ENODEV;
+	}
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	if (!tb_dev->log_dev)
+		tb_dev->log_dev = &hdev->dev;
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+
+	hid_set_drvdata(hdev, tb_dev);
+
+	rc = appletb_extract_report_and_iface_info(tb_dev, hdev, id);
+	if (rc < 0)
+		goto error;
+
+	rc = hid_hw_start(hdev, HID_CONNECT_DRIVER | HID_CONNECT_HIDINPUT);
+	if (rc) {
+		dev_err(tb_dev->log_dev, "hw start failed (%d)\n", rc);
+		goto clear_iface_info;
+	}
+
+	rc = hid_hw_open(hdev);
+	if (rc) {
+		dev_err(tb_dev->log_dev, "hw open failed (%d)\n", rc);
+		goto stop_hid;
+	}
+
+	/* do setup if we have both interfaces */
+	if (appletb_test_and_mark_active(tb_dev)) {
+		/* initialize the touch bar */
+		if (appletb_tb_def_fn_mode >= 0 &&
+		    appletb_tb_def_fn_mode <= APPLETB_FN_MODE_MAX)
+			tb_dev->fn_mode = appletb_tb_def_fn_mode;
+		else
+			tb_dev->fn_mode = APPLETB_FN_MODE_NORM;
+		appletb_set_idle_timeout(tb_dev, appletb_tb_def_idle_timeout);
+		appletb_set_dim_timeout(tb_dev, appletb_tb_def_dim_timeout);
+		tb_dev->last_event_time = ktime_get();
+
+		tb_dev->pnd_tb_mode = APPLETB_CMD_MODE_UPD;
+		tb_dev->pnd_tb_disp = APPLETB_CMD_DISP_UPD;
+
+		appletb_update_touchbar(tb_dev, false);
+
+		/* set up the input handler */
+		tb_dev->inp_handler.event = appletb_inp_event;
+		tb_dev->inp_handler.connect = appletb_inp_connect;
+		tb_dev->inp_handler.disconnect = appletb_inp_disconnect;
+		tb_dev->inp_handler.name = "appletb";
+		tb_dev->inp_handler.id_table = appletb_input_devices;
+		tb_dev->inp_handler.match = appletb_match_internal_device;
+		tb_dev->inp_handler.private = tb_dev;
+
+		rc = input_register_handler(&tb_dev->inp_handler);
+		if (rc) {
+			dev_err(tb_dev->log_dev,
+				"Unable to register keyboard handler (%d)\n",
+				rc);
+			goto mark_inactive;
+		}
+
+		/* initialize sysfs attributes */
+		rc = sysfs_create_group(&tb_dev->mode_iface.hdev->dev.kobj,
+					&appletb_attr_group);
+		if (rc) {
+			dev_err(tb_dev->log_dev,
+				"Failed to create sysfs attributes (%d)\n", rc);
+			goto unreg_handler;
+		}
+
+		dev_dbg(tb_dev->log_dev, "Touchbar activated\n");
+	}
+
+	return 0;
+
+unreg_handler:
+	input_unregister_handler(&tb_dev->inp_handler);
+mark_inactive:
+	appletb_test_and_mark_inactive(tb_dev, hdev);
+	cancel_delayed_work_sync(&tb_dev->tb_work);
+	hid_hw_close(hdev);
+stop_hid:
+	hid_hw_stop(hdev);
+clear_iface_info:
+	appletb_clear_iface_info(tb_dev, hdev);
+error:
+	return rc;
+}
+
+static void appletb_remove(struct hid_device *hdev)
+{
+	struct appletb_device *tb_dev = hid_get_drvdata(hdev);
+	unsigned long flags;
+
+	if (appletb_test_and_mark_inactive(tb_dev, hdev)) {
+		sysfs_remove_group(&tb_dev->mode_iface.hdev->dev.kobj,
+				   &appletb_attr_group);
+
+		input_unregister_handler(&tb_dev->inp_handler);
+
+		cancel_delayed_work_sync(&tb_dev->tb_work);
+		appletb_set_tb_mode(tb_dev, APPLETB_CMD_MODE_OFF);
+		appletb_set_tb_disp(tb_dev, APPLETB_CMD_DISP_ON);
+
+		if (tb_dev->tb_autopm_off)
+			hid_hw_power(tb_dev->disp_iface.hdev, PM_HINT_NORMAL);
+
+		dev_info(tb_dev->log_dev, "Touchbar deactivated\n");
+	}
+
+	hid_hw_close(hdev);
+	hid_hw_stop(hdev);
+	appletb_clear_iface_info(tb_dev, hdev);
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	if (tb_dev->log_dev == &hdev->dev) {
+		if (tb_dev->mode_iface.hdev)
+			tb_dev->log_dev = &tb_dev->mode_iface.hdev->dev;
+		else if (tb_dev->disp_iface.hdev)
+			tb_dev->log_dev = &tb_dev->disp_iface.hdev->dev;
+		else
+			tb_dev->log_dev = NULL;
+	}
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+}
+
+#ifdef CONFIG_PM
+static int appletb_suspend(struct hid_device *hdev, pm_message_t message)
+{
+	struct appletb_device *tb_dev = hid_get_drvdata(hdev);
+	struct appletb_iface_info *iface_info;
+	unsigned long flags;
+	bool all_suspended = false;
+
+	if (message.event != PM_EVENT_SUSPEND &&
+	    message.event != PM_EVENT_FREEZE)
+		return 0;
+
+	if (tb_dev->is_t1) {
+
+		/*
+		 * Wait for both interfaces to be suspended and no more async work
+		 * in progress.
+		 */
+
+		spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+		if (!tb_dev->mode_iface.suspended && !tb_dev->disp_iface.suspended) {
+			tb_dev->active = false;
+			cancel_delayed_work(&tb_dev->tb_work);
+		}
+
+		iface_info = appletb_get_iface_info(tb_dev, hdev);
+		if (iface_info)
+			iface_info->suspended = true;
+
+		if ((!tb_dev->mode_iface.hdev || tb_dev->mode_iface.suspended) &&
+		    (!tb_dev->disp_iface.hdev || tb_dev->disp_iface.suspended))
+			all_suspended = true;
+
+		spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+
+		flush_delayed_work(&tb_dev->tb_work);
+
+		if (!all_suspended)
+			return 0;
+
+		/*
+		 * The touch bar device itself remembers the last state when suspended
+		 * in some cases, but in others (e.g. when mode != off and disp == off)
+		 * it resumes with a different state; furthermore it may be only
+		 * partially responsive in that state. By turning both mode and disp
+		 * off we ensure it is in a good state when resuming (and this happens
+		 * to be the same state after booting/resuming-from-hibernate, so less
+		 * special casing between the two).
+		 */
+		if (message.event == PM_EVENT_SUSPEND) {
+			appletb_set_tb_mode(tb_dev, APPLETB_CMD_MODE_OFF);
+			appletb_set_tb_disp(tb_dev, APPLETB_CMD_DISP_OFF);
+		}
+
+		spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+		tb_dev->cur_tb_mode = APPLETB_CMD_MODE_OFF;
+		tb_dev->cur_tb_disp = APPLETB_CMD_DISP_OFF;
+
+		spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+
+		dev_info(tb_dev->log_dev, "Touchbar suspended.\n");
+	} else {
+		dev_info(tb_dev->log_dev, "T2 Mac detected, not handling suspend.\n");
+	}
+
+	return 0;
+}
+
+static int appletb_reset_resume(struct hid_device *hdev)
+{
+	struct appletb_device *tb_dev = hid_get_drvdata(hdev);
+	struct appletb_iface_info *iface_info;
+	unsigned long flags;
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	iface_info = appletb_get_iface_info(tb_dev, hdev);
+	if (iface_info)
+		iface_info->suspended = false;
+
+	if ((tb_dev->mode_iface.hdev && !tb_dev->mode_iface.suspended) &&
+	    (tb_dev->disp_iface.hdev && !tb_dev->disp_iface.suspended)) {
+		/*
+		 * Restore touch bar state. Note that autopm state is not
+		 * preserved, so need explicitly restore that here.
+		 */
+		tb_dev->active = true;
+		tb_dev->restore_autopm = true;
+		tb_dev->last_event_time = ktime_get();
+
+		appletb_update_touchbar_no_lock(tb_dev, true);
+
+		dev_info(tb_dev->log_dev, "Touchbar resumed.\n");
+	}
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+
+	return 0;
+}
+#endif
+
+static struct appletb_device *appletb_alloc_device(void)
+{
+	struct appletb_device *tb_dev;
+
+	tb_dev = kzalloc(sizeof(*tb_dev), GFP_KERNEL);
+	if (!tb_dev)
+		return NULL;
+
+	spin_lock_init(&tb_dev->tb_lock);
+	INIT_DELAYED_WORK(&tb_dev->tb_work, appletb_set_tb_worker);
+
+	return tb_dev;
+}
+
+static void appletb_free_device(struct appletb_device *tb_dev)
+{
+	cancel_delayed_work_sync(&tb_dev->tb_work);
+	kfree(tb_dev);
+}
+
+static const struct hid_device_id appletb_hid_ids[] = {
+	/* MacBook Pro's 2016, 2017, with T1 chip */
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LINUX_FOUNDATION,
+			 USB_DEVICE_ID_IBRIDGE_TB),
+	  .driver_data = APPLETB_FEATURE_IS_T1 },
+	/* MacBook Pro's 2018, 2019, with T2 chip: iBridge DFR brightness */
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE,
+			USB_DEVICE_ID_APPLE_TOUCHBAR_BACKLIGHT) },
+	/* MacBook Pro's 2018, 2019, with T2 chip: iBridge Display */
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE,
+			USB_DEVICE_ID_APPLE_TOUCHBAR_DISPLAY) },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(hid, appletb_hid_ids);
+
+static struct hid_driver appletb_hid_driver = {
+	.name = "apple-touchbar",
+	.id_table = appletb_hid_ids,
+	.probe = appletb_probe,
+	.remove = appletb_remove,
+	.event = appletb_hid_event,
+	.input_configured = appletb_input_configured,
+#ifdef CONFIG_PM
+	.suspend = appletb_suspend,
+	.reset_resume = appletb_reset_resume,
+#endif
+};
+
+static int __init appletb_init(void)
+{
+	struct appletb_device *tb_dev;
+	int rc;
+
+	tb_dev = appletb_alloc_device();
+	if (!tb_dev)
+		return -ENOMEM;
+
+	appletb_dev = tb_dev;
+
+	rc = hid_register_driver(&appletb_hid_driver);
+	if (rc)
+		goto error;
+
+	return 0;
+
+error:
+	appletb_free_device(tb_dev);
+	return rc;
+}
+
+static void __exit appletb_exit(void)
+{
+	hid_unregister_driver(&appletb_hid_driver);
+	appletb_free_device(appletb_dev);
+}
+
+module_init(appletb_init);
+module_exit(appletb_exit);
+
+MODULE_AUTHOR("Ronald Tschalär");
+MODULE_DESCRIPTION("MacBookPro Touch Bar driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index c03535c4b..e620190b5 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -316,12 +316,14 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_2021) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_FINGERPRINT_2021) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_TOUCHBAR_BACKLIGHT) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_TOUCHBAR_DISPLAY) },
 #endif
 #if IS_ENABLED(CONFIG_HID_APPLE_IBRIDGE)
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IBRIDGE) },
 #endif
+#if IS_ENABLED(CONFIG_HID_APPLE_TOUCHBAR)
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_TOUCHBAR_BACKLIGHT) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_TOUCHBAR_DISPLAY) },
+#endif
 #if IS_ENABLED(CONFIG_HID_APPLEIR)
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL2) },
-- 
2.37.2


^ permalink raw reply related

* [PATCH 1/3] HID: apple-ibridge: Add Apple iBridge HID driver for T1 chip.
From: Aditya Garg @ 2023-02-10  3:43 UTC (permalink / raw)
  To: Jiri Kosina, jkosina@suse.cz, benjamin.tissoires@redhat.com,
	Andy Shevchenko, andy.shevchenko@gmail.com
  Cc: LKML, linux-input@vger.kernel.org, ronald@innovation.ch,
	kekrby@gmail.com, Orlando Chamberlain
In-Reply-To: <E5D8BEBA-3C5B-460F-BD2C-39470A793CC3@live.com>

From: Ronald Tschalär <ronald@innovation.ch>

The iBridge device provides access to several devices, including:
- the Touch Bar
- the iSight webcam
- the light sensor
- the fingerprint sensor

This driver provides the core support for managing the iBridge device
and the access to the underlying devices. In particular, the
functionality for the touch bar and light sensor is exposed via USB HID
interfaces, and on devices with the T1 chip one of the HID devices is
used for both functions. So this driver creates virtual HID devices, one
per top-level report collection on each HID device (for a total of 3
virtual HID devices). The sub-drivers then bind to these virtual HID
devices.

This way the Touch Bar and ALS drivers can be kept in their own modules,
while at the same time making them look very much like as if they were
connected to the real HID devices. And those drivers then work (mostly)
without further changes on MacBooks with the T2 chip that don't need
this driver.

Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
[Kerem Karabay: convert to a platform driver]
[Kerem Karabay: fix appleib_forward_int_op]
[Kerem Karabay: rely on HID core's parsing in appleib_add_device]
Signed-off-by: Kerem Karabay <kekrby@gmail.com>
Signed-off-by: Aditya Garg <gargaditya08@live.com>
---
 drivers/hid/Kconfig         |  15 +
 drivers/hid/Makefile        |   1 +
 drivers/hid/apple-ibridge.c | 610 ++++++++++++++++++++++++++++++++++++
 drivers/hid/apple-ibridge.h |  15 +
 drivers/hid/hid-ids.h       |   1 +
 drivers/hid/hid-quirks.c    |   3 +
 6 files changed, 645 insertions(+)
 create mode 100644 drivers/hid/apple-ibridge.c
 create mode 100644 drivers/hid/apple-ibridge.h

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index e2a5d30c8..e69afa5f4 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -130,6 +130,21 @@ config HID_APPLE
 	Say Y here if you want support for keyboards of	Apple iBooks, PowerBooks,
 	MacBooks, MacBook Pros and Apple Aluminum.
 
+config HID_APPLE_IBRIDGE
+	tristate "Apple iBridge"
+	depends on USB_HID
+	depends on (X86 && ACPI) || COMPILE_TEST
+	imply HID_SENSOR_HUB
+	imply HID_SENSOR_ALS
+	help
+	This module provides the core support for the Apple T1 chip found
+	on 2016 and 2017 MacBookPro's, also known as the iBridge. The drivers
+	for the Touch Bar (apple-touchbar) and light sensor (hid-sensor-hub
+	and hid-sensor-als) need to be enabled separately.
+
+	To compile this driver as a module, choose M here: the
+	module will be called apple-ibridge.
+
 config HID_APPLEIR
 	tristate "Apple infrared receiver"
 	depends on (USB_HID)
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index e8014c1a2..b61373cd8 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_HID_ACCUTOUCH)	+= hid-accutouch.o
 obj-$(CONFIG_HID_ALPS)		+= hid-alps.o
 obj-$(CONFIG_HID_ACRUX)		+= hid-axff.o
 obj-$(CONFIG_HID_APPLE)		+= hid-apple.o
+obj-$(CONFIG_HID_APPLE_IBRIDGE)	+= apple-ibridge.o
 obj-$(CONFIG_HID_APPLEIR)	+= hid-appleir.o
 obj-$(CONFIG_HID_CREATIVE_SB0540)	+= hid-creative-sb0540.o
 obj-$(CONFIG_HID_ASUS)		+= hid-asus.o
diff --git a/drivers/hid/apple-ibridge.c b/drivers/hid/apple-ibridge.c
new file mode 100644
index 000000000..4d26f8d66
--- /dev/null
+++ b/drivers/hid/apple-ibridge.c
@@ -0,0 +1,610 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Apple iBridge Driver
+ *
+ * Copyright (c) 2018 Ronald Tschalär
+ */
+
+/**
+ * DOC: Overview
+ *
+ * 2016 and 2017 MacBookPro models with a Touch Bar (MacBookPro13,[23] and
+ * MacBookPro14,[23]) have an Apple iBridge chip (also known as T1 chip) which
+ * exposes the touch bar, built-in webcam (iSight), ambient light sensor, and
+ * Secure Enclave Processor (SEP) for TouchID. It shows up in the system as a
+ * USB device with 3 configurations: 'Default iBridge Interfaces', 'Default
+ * iBridge Interfaces(OS X)', and 'Default iBridge Interfaces(Recovery)'.
+ *
+ * In the first (default after boot) configuration, 4 usb interfaces are
+ * exposed: 2 related to the webcam, and 2 USB HID interfaces representing
+ * the touch bar and the ambient light sensor. The webcam interfaces are
+ * already handled by the uvcvideo driver. However, there is a problem with
+ * the other two interfaces: one of them contains functionality (HID reports)
+ * used by both the touch bar and the ALS, which is an issue because the kernel
+ * allows only one driver to be attached to a given device. This driver exists
+ * to solve this issue.
+ *
+ * This driver is implemented as a HID driver that attaches to both HID
+ * interfaces and in turn creates several virtual child HID devices, one for
+ * each top-level collection found in each interfaces report descriptor. The
+ * touch bar and ALS drivers then attach to these virtual HID devices, and this
+ * driver forwards the operations between the real and virtual devices.
+ *
+ * One important aspect of this approach is that resulting (virtual) HID
+ * devices look much like the HID devices found on the later MacBookPro models
+ * which have a T2 chip, where there are separate USB interfaces for the touch
+ * bar and ALS functionality, which means that the touch bar and ALS drivers
+ * work (mostly) the same on both types of models.
+ *
+ * Lastly, this driver also takes care of the power-management for the
+ * iBridge when suspending and resuming.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/usb.h>
+
+#include "hid-ids.h"
+#include "../hid/usbhid/usbhid.h"
+#include "apple-ibridge.h"
+
+#define APPLEIB_BASIC_CONFIG	1
+
+static struct hid_device_id appleib_sub_hid_ids[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LINUX_FOUNDATION,
+			 USB_DEVICE_ID_IBRIDGE_TB) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LINUX_FOUNDATION,
+			 USB_DEVICE_ID_IBRIDGE_ALS) },
+};
+
+static struct {
+	unsigned int usage;
+	struct hid_device_id *dev_id;
+} appleib_usage_map[] = {
+	/* Default iBridge configuration, key inputs and mode settings */
+	{ 0x00010006, &appleib_sub_hid_ids[0] },
+	/* OS X iBridge configuration, digitizer inputs */
+	{ 0x000D0005, &appleib_sub_hid_ids[0] },
+	/* All iBridge configurations, display/DFR settings */
+	{ 0xFF120001, &appleib_sub_hid_ids[0] },
+	/* All iBridge configurations, ALS */
+	{ 0x00200041, &appleib_sub_hid_ids[1] },
+};
+
+struct appleib_device {
+	acpi_handle asoc_socw;
+};
+
+struct appleib_hid_dev_info {
+	struct hid_device	*hdev;
+	struct hid_device	*sub_hdevs[ARRAY_SIZE(appleib_sub_hid_ids)];
+	bool			sub_open[ARRAY_SIZE(appleib_sub_hid_ids)];
+};
+
+static int appleib_hid_raw_event(struct hid_device *hdev,
+				 struct hid_report *report, u8 *data, int size)
+{
+	struct appleib_hid_dev_info *hdev_info = hid_get_drvdata(hdev);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(hdev_info->sub_hdevs); i++) {
+		if (READ_ONCE(hdev_info->sub_open[i]))
+			hid_input_report(hdev_info->sub_hdevs[i], report->type,
+					 data, size, 0);
+	}
+
+	return 0;
+}
+
+static __u8 *appleib_report_fixup(struct hid_device *hdev, __u8 *rdesc,
+				  unsigned int *rsize)
+{
+	/* Some fields have a size of 64 bits, which according to HID 1.11
+	 * Section 8.4 is not valid ("An item field cannot span more than 4
+	 * bytes in a report"). Furthermore, hid_field_extract() complains
+	 * when encountering such a field. So turn them into two 32-bit fields
+	 * instead.
+	 */
+
+	if (*rsize == 634 &&
+	    /* Usage Page 0xff12 (vendor defined) */
+	    rdesc[212] == 0x06 && rdesc[213] == 0x12 && rdesc[214] == 0xff &&
+	    /* Usage 0x51 */
+	    rdesc[416] == 0x09 && rdesc[417] == 0x51 &&
+	    /* report size 64 */
+	    rdesc[432] == 0x75 && rdesc[433] == 64 &&
+	    /* report count 1 */
+	    rdesc[434] == 0x95 && rdesc[435] == 1) {
+		rdesc[433] = 32;
+		rdesc[435] = 2;
+		hid_dbg(hdev, "Fixed up first 64-bit field\n");
+	}
+
+	if (*rsize == 634 &&
+	    /* Usage Page 0xff12 (vendor defined) */
+	    rdesc[212] == 0x06 && rdesc[213] == 0x12 && rdesc[214] == 0xff &&
+	    /* Usage 0x51 */
+	    rdesc[611] == 0x09 && rdesc[612] == 0x51 &&
+	    /* report size 64 */
+	    rdesc[627] == 0x75 && rdesc[628] == 64 &&
+	    /* report count 1 */
+	    rdesc[629] == 0x95 && rdesc[630] == 1) {
+		rdesc[628] = 32;
+		rdesc[630] = 2;
+		hid_dbg(hdev, "Fixed up second 64-bit field\n");
+	}
+
+	return rdesc;
+}
+
+#ifdef CONFIG_PM
+/**
+ * appleib_forward_int_op() - Forward a hid-driver callback to all drivers on
+ * all virtual HID devices attached to the given real HID device.
+ * @hdev the real hid-device
+ * @forward a function that calls the callback on the given driver
+ * @args arguments for the forward function
+ *
+ * This is for callbacks that return a status as an int.
+ *
+ * Returns: 0 on success, or the first error returned by the @forward function.
+ */
+static int appleib_forward_int_op(struct hid_device *hdev,
+				  int (*forward)(struct hid_driver *,
+						 struct hid_device *, void *),
+				  void *args)
+{
+	struct appleib_hid_dev_info *hdev_info = hid_get_drvdata(hdev);
+	struct hid_device *sub_hdev;
+	int rc;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(hdev_info->sub_hdevs); i++) {
+		sub_hdev = hdev_info->sub_hdevs[i];
+		if (sub_hdev->driver) {
+			rc = forward(sub_hdev->driver, sub_hdev, args);
+			if (rc)
+				return rc;
+		}
+	}
+
+	return 0;
+}
+
+static int appleib_hid_suspend_fwd(struct hid_driver *drv,
+				   struct hid_device *hdev, void *args)
+{
+	int rc = 0;
+
+	if (drv->suspend)
+		rc = drv->suspend(hdev, *(pm_message_t *)args);
+
+	return rc;
+}
+
+static int appleib_hid_suspend(struct hid_device *hdev, pm_message_t message)
+{
+	return appleib_forward_int_op(hdev, appleib_hid_suspend_fwd, &message);
+}
+
+static int appleib_hid_resume_fwd(struct hid_driver *drv,
+				  struct hid_device *hdev, void *args)
+{
+	int rc = 0;
+
+	if (drv->resume)
+		rc = drv->resume(hdev);
+
+	return rc;
+}
+
+static int appleib_hid_resume(struct hid_device *hdev)
+{
+	return appleib_forward_int_op(hdev, appleib_hid_resume_fwd, NULL);
+}
+
+static int appleib_hid_reset_resume_fwd(struct hid_driver *drv,
+					struct hid_device *hdev, void *args)
+{
+	int rc = 0;
+
+	if (drv->reset_resume)
+		rc = drv->reset_resume(hdev);
+
+	return rc;
+}
+
+static int appleib_hid_reset_resume(struct hid_device *hdev)
+{
+	return appleib_forward_int_op(hdev, appleib_hid_reset_resume_fwd, NULL);
+}
+#endif /* CONFIG_PM */
+
+static int appleib_ll_start(struct hid_device *hdev)
+{
+	return 0;
+}
+
+static void appleib_ll_stop(struct hid_device *hdev)
+{
+}
+
+static int appleib_set_open(struct hid_device *hdev, bool open)
+{
+	struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(hdev_info->sub_hdevs); i++) {
+		/*
+		 * hid_hw_open(), and hence appleib_ll_open(), is called
+		 * from the driver's probe function, which in turn is called
+		 * while adding the sub-hdev; but at this point we haven't yet
+		 * added the sub-hdev to our list. So if we don't find the
+		 * sub-hdev in our list assume it's in the process of being
+		 * added and set the flag on the first unset sub-hdev.
+		 */
+		if (hdev_info->sub_hdevs[i] == hdev ||
+		    !hdev_info->sub_hdevs[i]) {
+			WRITE_ONCE(hdev_info->sub_open[i], open);
+			return 0;
+		}
+	}
+
+	return -ENODEV;
+}
+
+static int appleib_ll_open(struct hid_device *hdev)
+{
+	return appleib_set_open(hdev, true);
+}
+
+static void appleib_ll_close(struct hid_device *hdev)
+{
+	appleib_set_open(hdev, false);
+}
+
+static int appleib_ll_power(struct hid_device *hdev, int level)
+{
+	struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
+
+	return hid_hw_power(hdev_info->hdev, level);
+}
+
+static int appleib_ll_parse(struct hid_device *hdev)
+{
+	/* we've already called hid_parse_report() */
+	return 0;
+}
+
+static void appleib_ll_request(struct hid_device *hdev,
+			       struct hid_report *report, int reqtype)
+{
+	struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
+
+	hid_hw_request(hdev_info->hdev, report, reqtype);
+}
+
+static int appleib_ll_wait(struct hid_device *hdev)
+{
+	struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
+
+	hid_hw_wait(hdev_info->hdev);
+	return 0;
+}
+
+static int appleib_ll_raw_request(struct hid_device *hdev,
+				  unsigned char reportnum, __u8 *buf,
+				  size_t len, unsigned char rtype, int reqtype)
+{
+	struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
+
+	return hid_hw_raw_request(hdev_info->hdev, reportnum, buf, len, rtype,
+				  reqtype);
+}
+
+static int appleib_ll_output_report(struct hid_device *hdev, __u8 *buf,
+				    size_t len)
+{
+	struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
+
+	return hid_hw_output_report(hdev_info->hdev, buf, len);
+}
+
+static struct hid_ll_driver appleib_ll_driver = {
+	.start = appleib_ll_start,
+	.stop = appleib_ll_stop,
+	.open = appleib_ll_open,
+	.close = appleib_ll_close,
+	.power = appleib_ll_power,
+	.parse = appleib_ll_parse,
+	.request = appleib_ll_request,
+	.wait = appleib_ll_wait,
+	.raw_request = appleib_ll_raw_request,
+	.output_report = appleib_ll_output_report,
+};
+
+static struct hid_device_id *appleib_find_dev_id_for_usage(unsigned int usage)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(appleib_usage_map); i++) {
+		if (appleib_usage_map[i].usage == usage)
+			return appleib_usage_map[i].dev_id;
+	}
+
+	return NULL;
+}
+
+static struct hid_device *
+appleib_add_sub_dev(struct appleib_hid_dev_info *hdev_info,
+		    struct hid_device_id *dev_id)
+{
+	struct hid_device *sub_hdev;
+	int rc;
+
+	sub_hdev = hid_allocate_device();
+	if (IS_ERR(sub_hdev))
+		return sub_hdev;
+
+	sub_hdev->dev.parent = &hdev_info->hdev->dev;
+
+	sub_hdev->bus = dev_id->bus;
+	sub_hdev->group = dev_id->group;
+	sub_hdev->vendor = dev_id->vendor;
+	sub_hdev->product = dev_id->product;
+
+	sub_hdev->ll_driver = &appleib_ll_driver;
+
+	snprintf(sub_hdev->name, sizeof(sub_hdev->name),
+		 "iBridge Virtual HID %s/%04x:%04x",
+		 dev_name(sub_hdev->dev.parent), sub_hdev->vendor,
+		 sub_hdev->product);
+
+	sub_hdev->driver_data = hdev_info;
+
+	rc = hid_add_device(sub_hdev);
+	if (rc) {
+		hid_destroy_device(sub_hdev);
+		return ERR_PTR(rc);
+	}
+
+	return sub_hdev;
+}
+
+static struct appleib_hid_dev_info *appleib_add_device(struct hid_device *hdev)
+{
+	struct appleib_hid_dev_info *hdev_info;
+	struct hid_device_id *dev_id;
+	unsigned int usage;
+	int i;
+
+	hdev_info = devm_kzalloc(&hdev->dev, sizeof(*hdev_info), GFP_KERNEL);
+	if (!hdev_info)
+		return ERR_PTR(-ENOMEM);
+
+	hdev_info->hdev = hdev;
+
+	for (i = 0; i < hdev->maxcollection; i++) {
+		usage = hdev->collection[i].usage;
+		dev_id = appleib_find_dev_id_for_usage(usage);
+
+		if (!dev_id) {
+			hid_warn(hdev, "Unknown collection encountered with usage %x\n",
+				 usage);
+		} else {
+			hdev_info->sub_hdevs[i] = appleib_add_sub_dev(hdev_info, dev_id);
+
+			if (IS_ERR(hdev_info->sub_hdevs[i])) {
+				while (i-- > 0)
+					hid_destroy_device(hdev_info->sub_hdevs[i]);
+				return (void *)hdev_info->sub_hdevs[i];
+			}
+		}
+	}
+
+	return hdev_info;
+}
+
+static void appleib_remove_device(struct hid_device *hdev)
+{
+	struct appleib_hid_dev_info *hdev_info = hid_get_drvdata(hdev);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(hdev_info->sub_hdevs); i++) {
+		if (hdev_info->sub_hdevs[i])
+			hid_destroy_device(hdev_info->sub_hdevs[i]);
+	}
+
+	hid_set_drvdata(hdev, NULL);
+}
+
+static int appleib_hid_probe(struct hid_device *hdev,
+			     const struct hid_device_id *id)
+{
+	struct appleib_hid_dev_info *hdev_info;
+	struct usb_device *udev;
+	int rc;
+
+	/* check and set usb config first */
+	udev = hid_to_usb_dev(hdev);
+
+	if (udev->actconfig->desc.bConfigurationValue != APPLEIB_BASIC_CONFIG) {
+		rc = usb_driver_set_configuration(udev, APPLEIB_BASIC_CONFIG);
+		return rc ? rc : -ENODEV;
+	}
+
+	rc = hid_parse(hdev);
+	if (rc) {
+		hid_err(hdev, "ib: hid parse failed (%d)\n", rc);
+		goto error;
+	}
+
+	rc = hid_hw_start(hdev, HID_CONNECT_DRIVER);
+	if (rc) {
+		hid_err(hdev, "ib: hw start failed (%d)\n", rc);
+		goto error;
+	}
+
+	hdev_info = appleib_add_device(hdev);
+	if (IS_ERR(hdev_info)) {
+		rc = PTR_ERR(hdev_info);
+		goto stop_hw;
+	}
+
+	hid_set_drvdata(hdev, hdev_info);
+
+	rc = hid_hw_open(hdev);
+	if (rc) {
+		hid_err(hdev, "ib: failed to open hid: %d\n", rc);
+		goto remove_dev;
+	}
+
+	return 0;
+
+remove_dev:
+	appleib_remove_device(hdev);
+stop_hw:
+	hid_hw_stop(hdev);
+error:
+	return rc;
+}
+
+static void appleib_hid_remove(struct hid_device *hdev)
+{
+	hid_hw_close(hdev);
+	appleib_remove_device(hdev);
+	hid_hw_stop(hdev);
+}
+
+static const struct hid_device_id appleib_hid_ids[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IBRIDGE) },
+	{ },
+};
+
+static struct hid_driver appleib_hid_driver = {
+	.name = "apple-ibridge-hid",
+	.id_table = appleib_hid_ids,
+	.probe = appleib_hid_probe,
+	.remove = appleib_hid_remove,
+	.raw_event = appleib_hid_raw_event,
+	.report_fixup = appleib_report_fixup,
+#ifdef CONFIG_PM
+	.suspend = appleib_hid_suspend,
+	.resume = appleib_hid_resume,
+	.reset_resume = appleib_hid_reset_resume,
+#endif
+};
+
+static struct appleib_device *appleib_alloc_device(struct platform_device *pdev)
+{
+	struct appleib_device *ib_dev;
+	acpi_status sts;
+
+	ib_dev = devm_kzalloc(&pdev->dev, sizeof(*ib_dev), GFP_KERNEL);
+	if (!ib_dev)
+		return ERR_PTR(-ENOMEM);
+
+	/* get iBridge acpi power control method for suspend/resume */
+	sts = acpi_get_handle(ACPI_HANDLE(&pdev->dev), "SOCW", &ib_dev->asoc_socw);
+	if (ACPI_FAILURE(sts)) {
+		dev_err(&pdev->dev,
+			"Error getting handle for ASOC.SOCW method: %s\n",
+			acpi_format_exception(sts));
+		return ERR_PTR(-ENXIO);
+	}
+
+	/* ensure iBridge is powered on */
+	sts = acpi_execute_simple_method(ib_dev->asoc_socw, NULL, 1);
+	if (ACPI_FAILURE(sts))
+		dev_warn(&pdev->dev, "SOCW(1) failed: %s\n",
+			 acpi_format_exception(sts));
+
+	return ib_dev;
+}
+
+static int appleib_probe(struct platform_device *pdev)
+{
+	struct appleib_device *ib_dev;
+	int ret;
+
+	ib_dev = appleib_alloc_device(pdev);
+	if (IS_ERR(ib_dev))
+		return PTR_ERR(ib_dev);
+
+	ret = hid_register_driver(&appleib_hid_driver);
+	if (ret) {
+		dev_err(&pdev->dev, "Error registering hid driver: %d\n",
+			ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, ib_dev);
+
+	return 0;
+}
+
+static int appleib_remove(struct platform_device *pdev)
+{
+	hid_unregister_driver(&appleib_hid_driver);
+
+	return 0;
+}
+
+static int appleib_suspend(struct platform_device *pdev, pm_message_t message)
+{
+	struct appleib_device *ib_dev;
+	int rc;
+
+	ib_dev = platform_get_drvdata(pdev);
+
+	rc = acpi_execute_simple_method(ib_dev->asoc_socw, NULL, 0);
+	if (ACPI_FAILURE(rc))
+		dev_warn(&pdev->dev, "SOCW(0) failed: %s\n",
+			 acpi_format_exception(rc));
+
+	return 0;
+}
+
+static int appleib_resume(struct platform_device *pdev)
+{
+	struct appleib_device *ib_dev;
+	int rc;
+
+	ib_dev = platform_get_drvdata(pdev);
+
+	rc = acpi_execute_simple_method(ib_dev->asoc_socw, NULL, 1);
+	if (ACPI_FAILURE(rc))
+		dev_warn(&pdev->dev, "SOCW(1) failed: %s\n",
+			 acpi_format_exception(rc));
+
+	return 0;
+}
+
+static const struct acpi_device_id appleib_acpi_match[] = {
+	{ "APP7777", 0 },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(acpi, appleib_acpi_match);
+
+static struct platform_driver appleib_driver = {
+	.probe		= appleib_probe,
+	.remove		= appleib_remove,
+	.suspend	= appleib_suspend,
+	.resume		= appleib_resume,
+	.driver		= {
+		.name		  = "apple-ibridge",
+		.acpi_match_table = appleib_acpi_match,
+	},
+};
+
+module_platform_driver(appleib_driver);
+
+MODULE_AUTHOR("Ronald Tschalär");
+MODULE_DESCRIPTION("Apple iBridge driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/apple-ibridge.h b/drivers/hid/apple-ibridge.h
new file mode 100644
index 000000000..8aefcf615
--- /dev/null
+++ b/drivers/hid/apple-ibridge.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Apple iBridge Driver
+ *
+ * Copyright (c) 2018 Ronald Tschalär
+ */
+
+#ifndef __LINUX_APPLE_IBRDIGE_H
+#define __LINUX_APPLE_IBRDIGE_H
+
+#define USB_VENDOR_ID_LINUX_FOUNDATION	0x1d6b
+#define USB_DEVICE_ID_IBRIDGE_TB	0x0301
+#define USB_DEVICE_ID_IBRIDGE_ALS	0x0302
+
+#endif
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 0f8c11842..0c62e6280 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -187,6 +187,7 @@
 #define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_NUMPAD_2021   0x029f
 #define USB_DEVICE_ID_APPLE_TOUCHBAR_BACKLIGHT 0x8102
 #define USB_DEVICE_ID_APPLE_TOUCHBAR_DISPLAY 0x8302
+#define USB_DEVICE_ID_APPLE_IBRIDGE	0x8600
 
 #define USB_VENDOR_ID_ASUS		0x0486
 #define USB_DEVICE_ID_ASUS_T91MT	0x0185
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index be3ad0257..c03535c4b 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -319,6 +319,9 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_TOUCHBAR_BACKLIGHT) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_TOUCHBAR_DISPLAY) },
 #endif
+#if IS_ENABLED(CONFIG_HID_APPLE_IBRIDGE)
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IBRIDGE) },
+#endif
 #if IS_ENABLED(CONFIG_HID_APPLEIR)
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL2) },
-- 
2.37.2


^ permalink raw reply related

* [PATCH 0/3] Touch Bar and Keyboard backlight driver for Intel Macs
From: Aditya Garg @ 2023-02-10  3:41 UTC (permalink / raw)
  To: Jiri Kosina, jkosina@suse.cz, benjamin.tissoires@redhat.com,
	Andy Shevchenko, andy.shevchenko@gmail.com
  Cc: LKML, linux-input@vger.kernel.org, ronald@innovation.ch,
	kekrby@gmail.com, Orlando Chamberlain

Greetings from t2linux!

2 years ago an attempt was made to send the driver for the Touch Bar on
MacBook Pros by Ronald Tschalär [1].

Due to some issues pointed out by the maintainers upstream, unfortunately
it didn't make it upstream. Now we at t2linux [2] have addressed those
issues in this patchset and also improved the touchbar driver for the T2
Macs. We also have added a new driver for keyboard backlight support on
T2 MacBooks with Magic Keyboard.

The first 2 patches of this patchset are the ones sent by Ronald before,
with the issues addressed as pointed out in [1].

The third patch introduces a new driver, apple-magic-backlight, which adds
support for keyboard backlight on T2 MacBooks with the Magic Keyboard.

Note: Apart from these drivers, for the T2 Macs, an additional driver
shall be required to communicate with the T2 Security Chip, as the Touch
Bar, the internal keyboard, trackpad, camera, ambient light sensor,
headphone controls, and NCM Ethernet are accessed through a USB VHCI
created with DMA channels to the "T2 Bridge Controller" 106b:1801 PCI
device. A work in progress linux driver called apple-bce [3], or USB
device passthrough to a Linux VM guest on a Windows host with Apple
Bootcamp drivers can be used to get Linux these USB devices working and
test these patches.

[1] https://lore.kernel.org/lkml/20210228012643.69944-1-ronald@innovation.ch/
[2] https://t2linux.org/
[3] https://github.com/t2linux/apple-bce-drv

Orlando Chamberlain (1):
  HID: apple-magic-backlight: Add driver for keyboard backlight on
    internal Magic Keyboards

Ronald Tschalär (2):
  HID: apple-ibridge: Add Apple iBridge HID driver for T1 chip
  HID: apple-touchbar: Add driver for the Touch Bar on MacBook Pros

 MAINTAINERS                         |    6 +
 drivers/hid/Kconfig                 |   39 +
 drivers/hid/Makefile                |    3 +
 drivers/hid/apple-ibridge.c         |  610 +++++++++++
 drivers/hid/apple-ibridge.h         |   15 +
 drivers/hid/apple-magic-backlight.c |  143 +++
 drivers/hid/apple-touchbar.c        | 1500 +++++++++++++++++++++++++++
 drivers/hid/hid-ids.h               |    1 +
 drivers/hid/hid-quirks.c            |    5 +
 9 files changed, 2322 insertions(+)
 create mode 100644 drivers/hid/apple-ibridge.c
 create mode 100644 drivers/hid/apple-ibridge.h
 create mode 100644 drivers/hid/apple-magic-backlight.c
 create mode 100644 drivers/hid/apple-touchbar.c

-- 
2.37.2


^ permalink raw reply

* Re: [PATCH v4 3/4] HID: cp2112: Fix driver not registering GPIO IRQ chip as threaded
From: Dmitry Torokhov @ 2023-02-10  0:32 UTC (permalink / raw)
  To: Daniel Kaehn
  Cc: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires,
	devicetree, linux-input, ethan.twardy
In-Reply-To: <CAP+ZCCfnbMpQX4qQee3QZgOft=vQ0BLBgqQUwzPNH7qYopufAQ@mail.gmail.com>

On Tue, Feb 07, 2023 at 06:34:32AM -0600, Daniel Kaehn wrote:
> On Mon, Feb 6, 2023 at 5:15 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > On Mon, Feb 06, 2023 at 07:50:15AM -0600, Danny Kaehn wrote:
> > > The CP2112 generates interrupts from a polling routine on a thread,
> > > and can only support threaded interrupts. This patch configures the
> > > gpiochip irq chip with this flag, disallowing consumers to request
> > > a hard IRQ from this driver, which resulted in a segfault previously.
> >
> > This looks like a bugfix not dependent on anything else in the series
> > and can be applied separately...
> 
> This is correct (though usage of this patchset to instantiate drivers
> which request interrupts will most of the time be broken without this
> patch). Does this mean I should submit this patch independently from
> the rest of the series? Or should I just include a message to the
> maintainer describing what you said (that this can be applied
> separately)?

I'd simply sent it separately and then you do not need to explain ;)

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v4 08/18] gpiolib: remove gpio_set_debounce()
From: Dmitry Torokhov @ 2023-02-10  0:30 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Arnd Bergmann, Linus Walleij, Bartosz Golaszewski, Tony Lindgren,
	Thomas Gleixner, Krzysztof Kozlowski, Geert Uytterhoeven,
	Michael Ellerman, Christophe Leroy, Vincenzo Palazzo,
	Devarsh Thakkar, Michael Walle, Dipen Patel, Thierry Reding,
	Stefan Schmidt, Miquel Raynal, Lee Jones, linux-gpio, linux-doc,
	linux-kernel, linux-doc-tw-discuss, linux-arm-kernel, linux-omap,
	linux-samsung-soc, linux-m68k, linuxppc-dev, linux-sh, linux-acpi,
	linux-tegra, linux-input, linux-media, linux-wpan, netdev,
	linux-wireless, brcm80211-dev-list.pdl, SHA-cyfmac-dev-list,
	linux-arch, devicetree, Bartosz Golaszewski, Jonathan Corbet,
	Alex Shi, Yanteng Si, Hu Haowen, Russell King, Aaro Koskinen,
	Janusz Krzysztofik, Andrew Lunn, Sebastian Hesselbarth,
	Gregory Clement, Alim Akhtar, Nicholas Piggin, Yoshinori Sato,
	Rich Felker, Mun Yew Tham, Keerthy, Mika Westerberg,
	Thierry Reding, Jonathan Hunter, Mauro Carvalho Chehab,
	Alexander Aring, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Arend van Spriel, Franky Lin, Hante Meuleman,
	Kalle Valo, Qiang Zhao, Li Yang, Rob Herring, Frank Rowand
In-Reply-To: <20230208173343.37582-9-andriy.shevchenko@linux.intel.com>

On Wed, Feb 08, 2023 at 07:33:33PM +0200, Andy Shevchenko wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> gpio_set_debounce() only has a single user, which is trivially
> converted to gpiod_set_debounce().
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks.

-- 
Dmitry

^ permalink raw reply

* [PATCH v3 2/2] HID: asus: use spinlock to safely schedule workers
From: Pietro Borrello @ 2023-02-09 23:58 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Hanno Zulla, Carlo Caione
  Cc: Cristiano Giuffrida, Bos, H.J., Jakob Koschel, Jiri Kosina,
	Roderick Colenbrander, linux-input, linux-kernel, Pietro Borrello
In-Reply-To: <20230125-hid-unregister-leds-v3-0-0a52ac225e00@diag.uniroma1.it>

Use spinlocks to deal with workers introducing a wrapper
asus_schedule_work(), and several spinlock checks.
Otherwise, asus_kbd_backlight_set() may schedule led->work after the
structure has been freed, causing a use-after-free.

Fixes: af22a610bc38 ("HID: asus: support backlight on USB keyboards")
Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
---
 drivers/hid/hid-asus.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index f99752b998f3..30e194803bd7 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -98,6 +98,7 @@ struct asus_kbd_leds {
 	struct hid_device *hdev;
 	struct work_struct work;
 	unsigned int brightness;
+	spinlock_t lock;
 	bool removed;
 };
 
@@ -490,13 +491,23 @@ static int rog_nkey_led_init(struct hid_device *hdev)
 	return ret;
 }
 
+static void asus_schedule_work(struct asus_kbd_leds *led)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&led->lock, flags);
+	if (!led->removed)
+		schedule_work(&led->work);
+	spin_unlock_irqrestore(&led->lock, flags);
+}
+
 static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
 				   enum led_brightness brightness)
 {
 	struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
 						 cdev);
 	led->brightness = brightness;
-	schedule_work(&led->work);
+	asus_schedule_work(led);
 }
 
 static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
@@ -512,15 +523,17 @@ static void asus_kbd_backlight_work(struct work_struct *work)
 	struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
 	u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 };
 	int ret;
+	unsigned long flags;
 
-	if (led->removed)
-		return;
+	spin_lock_irqsave(&led->lock, flags);
 
 	buf[4] = led->brightness;
 
 	ret = asus_kbd_set_report(led->hdev, buf, sizeof(buf));
 	if (ret < 0)
 		hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret);
+
+	spin_unlock_irqrestore(&led->lock, flags);
 }
 
 /* WMI-based keyboard backlight LED control (via asus-wmi driver) takes
@@ -584,6 +597,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
 	drvdata->kbd_backlight->cdev.brightness_set = asus_kbd_backlight_set;
 	drvdata->kbd_backlight->cdev.brightness_get = asus_kbd_backlight_get;
 	INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_backlight_work);
+	spin_lock_init(&drvdata->kbd_backlight->lock);
 
 	ret = devm_led_classdev_register(&hdev->dev, &drvdata->kbd_backlight->cdev);
 	if (ret < 0) {
@@ -1119,9 +1133,13 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 static void asus_remove(struct hid_device *hdev)
 {
 	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+	unsigned long flags;
 
 	if (drvdata->kbd_backlight) {
+		spin_lock_irqsave(&drvdata->kbd_backlight->lock, flags);
 		drvdata->kbd_backlight->removed = true;
+		spin_unlock_irqrestore(&drvdata->kbd_backlight->lock, flags);
+
 		cancel_work_sync(&drvdata->kbd_backlight->work);
 	}
 

-- 
2.25.1


^ permalink raw reply related

* [PATCH v3 1/2] HID: bigben: use spinlock to safely schedule workers
From: Pietro Borrello @ 2023-02-09 23:58 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Hanno Zulla, Carlo Caione
  Cc: Cristiano Giuffrida, Bos, H.J., Jakob Koschel, Jiri Kosina,
	Roderick Colenbrander, linux-input, linux-kernel, Pietro Borrello
In-Reply-To: <20230125-hid-unregister-leds-v3-0-0a52ac225e00@diag.uniroma1.it>

Use spinlocks to deal with workers introducing a wrapper
bigben_schedule_work(), and several spinlock checks.
Otherwise, bigben_set_led() may schedule bigben->worker after the
structure has been freed, causing a use-after-free.

Fixes: 4eb1b01de5b9 ("HID: hid-bigbenff: fix race condition for scheduled work during removal")
Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
---
 drivers/hid/hid-bigbenff.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-bigbenff.c b/drivers/hid/hid-bigbenff.c
index e8b16665860d..28769aa7fed6 100644
--- a/drivers/hid/hid-bigbenff.c
+++ b/drivers/hid/hid-bigbenff.c
@@ -174,6 +174,7 @@ static __u8 pid0902_rdesc_fixed[] = {
 struct bigben_device {
 	struct hid_device *hid;
 	struct hid_report *report;
+	spinlock_t lock;
 	bool removed;
 	u8 led_state;         /* LED1 = 1 .. LED4 = 8 */
 	u8 right_motor_on;    /* right motor off/on 0/1 */
@@ -184,15 +185,24 @@ struct bigben_device {
 	struct work_struct worker;
 };
 
+static inline void bigben_schedule_work(struct bigben_device *bigben)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&bigben->lock, flags);
+	if (!bigben->removed)
+		schedule_work(&bigben->worker);
+	spin_unlock_irqrestore(&bigben->lock, flags);
+}
 
 static void bigben_worker(struct work_struct *work)
 {
 	struct bigben_device *bigben = container_of(work,
 		struct bigben_device, worker);
 	struct hid_field *report_field = bigben->report->field[0];
+	unsigned long flags;
 
-	if (bigben->removed || !report_field)
-		return;
+	spin_lock_irqsave(&bigben->lock, flags);
 
 	if (bigben->work_led) {
 		bigben->work_led = false;
@@ -219,6 +229,8 @@ static void bigben_worker(struct work_struct *work)
 		report_field->value[7] = 0x00; /* padding */
 		hid_hw_request(bigben->hid, bigben->report, HID_REQ_SET_REPORT);
 	}
+
+	spin_unlock_irqrestore(&bigben->lock, flags);
 }
 
 static int hid_bigben_play_effect(struct input_dev *dev, void *data,
@@ -228,6 +240,7 @@ static int hid_bigben_play_effect(struct input_dev *dev, void *data,
 	struct bigben_device *bigben = hid_get_drvdata(hid);
 	u8 right_motor_on;
 	u8 left_motor_force;
+	unsigned long flags;
 
 	if (!bigben) {
 		hid_err(hid, "no device data\n");
@@ -242,10 +255,13 @@ static int hid_bigben_play_effect(struct input_dev *dev, void *data,
 
 	if (right_motor_on != bigben->right_motor_on ||
 			left_motor_force != bigben->left_motor_force) {
+		spin_lock_irqsave(&bigben->lock, flags);
 		bigben->right_motor_on   = right_motor_on;
 		bigben->left_motor_force = left_motor_force;
 		bigben->work_ff = true;
-		schedule_work(&bigben->worker);
+		spin_unlock_irqrestore(&bigben->lock, flags);
+
+		bigben_schedule_work(bigben);
 	}
 
 	return 0;
@@ -259,6 +275,7 @@ static void bigben_set_led(struct led_classdev *led,
 	struct bigben_device *bigben = hid_get_drvdata(hid);
 	int n;
 	bool work;
+	unsigned long flags;
 
 	if (!bigben) {
 		hid_err(hid, "no device data\n");
@@ -267,6 +284,7 @@ static void bigben_set_led(struct led_classdev *led,
 
 	for (n = 0; n < NUM_LEDS; n++) {
 		if (led == bigben->leds[n]) {
+			spin_lock_irqsave(&bigben->lock, flags);
 			if (value == LED_OFF) {
 				work = (bigben->led_state & BIT(n));
 				bigben->led_state &= ~BIT(n);
@@ -274,10 +292,11 @@ static void bigben_set_led(struct led_classdev *led,
 				work = !(bigben->led_state & BIT(n));
 				bigben->led_state |= BIT(n);
 			}
+			spin_unlock_irqrestore(&bigben->lock, flags);
 
 			if (work) {
 				bigben->work_led = true;
-				schedule_work(&bigben->worker);
+				bigben_schedule_work(bigben);
 			}
 			return;
 		}
@@ -307,8 +326,12 @@ static enum led_brightness bigben_get_led(struct led_classdev *led)
 static void bigben_remove(struct hid_device *hid)
 {
 	struct bigben_device *bigben = hid_get_drvdata(hid);
+	unsigned long flags;
 
+	spin_lock_irqsave(&bigben->lock, flags);
 	bigben->removed = true;
+	spin_unlock_irqrestore(&bigben->lock, flags);
+
 	cancel_work_sync(&bigben->worker);
 	hid_hw_stop(hid);
 }
@@ -362,6 +385,7 @@ static int bigben_probe(struct hid_device *hid,
 	set_bit(FF_RUMBLE, hidinput->input->ffbit);
 
 	INIT_WORK(&bigben->worker, bigben_worker);
+	spin_lock_init(&bigben->lock);
 
 	error = input_ff_create_memless(hidinput->input, NULL,
 		hid_bigben_play_effect);
@@ -402,7 +426,7 @@ static int bigben_probe(struct hid_device *hid,
 	bigben->left_motor_force = 0;
 	bigben->work_led = true;
 	bigben->work_ff = true;
-	schedule_work(&bigben->worker);
+	bigben_schedule_work(bigben);
 
 	hid_info(hid, "LED and force feedback support for BigBen gamepad\n");
 

-- 
2.25.1


^ permalink raw reply related

* [PATCH v3 0/2] HID: use spinlocks to safely schedule led workers
From: Pietro Borrello @ 2023-02-09 23:58 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Hanno Zulla, Carlo Caione
  Cc: Cristiano Giuffrida, Bos, H.J., Jakob Koschel, Jiri Kosina,
	Roderick Colenbrander, linux-input, linux-kernel, Pietro Borrello

I noticed a recurring pattern is present in multiple hid devices in the
Linux tree, where the LED controller of a device schedules a work_struct
to interact with the hardware.
The work_struct is embedded in the device structure and thus, is freed
at device removal.

The issue is that a LED worker may be scheduled by a timer concurrently
with device removal, causing the work_struct to be accessed after having
been freed.
I was able to trigger the issue in hid-bigbenff.c and hid-asus.c 
where the work_structs may be scheduled by the LED controller
while the device is disconnecting, triggering use-after-frees.
I can attach the reproducer, but it's very simple USB configuration, 
using the /dev/raw-gadget interface with some more USB interactions 
to manage LEDs configuration and pass checks in asus_kbd_init() 
and asus_kbd_get_functions() in case of hid-asus.c.
I triggered the issue by connecting a device and immediately 
disconnecting it, so that the remove function runs before the LED one
which remains pending.

I am attaching multiple patches for asus and bigben drivers. 
The proposed patches introduce safe wrappers to schedule the workers
safely with several spinlocks checks.

I attach the (partial for brevity) ODEBUG dumps:

```hid-bigbenff.c
[   37.803135][ T1170] usb 1-1: USB disconnect, device number 2
[   37.827979][ T1170] ODEBUG: free active (active state 0) object
type: work_struct hint: bigben_worker+0x0/0x860
[   37.829634][ T1170] WARNING: CPU: 0 PID: 1170 at
lib/debugobjects.c:505 debug_check_no_obj_freed+0x43a/0x630
[   37.830904][ T1170] Modules linked in:
[   37.831413][ T1170] CPU: 0 PID: 1170 Comm: kworker/0:3 Not tainted
6.1.0-rc4-dirty #43
[   37.832465][ T1170] Hardware name: QEMU Standard PC (i440FX + PIIX,
1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
[   37.833751][ T1170] Workqueue: usb_hub_wq hub_event
[   37.834409][ T1170] RIP: 0010:debug_check_no_obj_freed+0x43a/0x630
[   37.835218][ T1170] Code: 48 89 ef e8 28 82 58 ff 49 8b 14 24 4c 8b
45 00 48 c7 c7 40 5f 09 87 48 c7 c6 60 5b 09 87 89 d9 4d 89 f9 31 c0
e8 46 25 ef fe <0f> 0b 4c 8b 64 24 20 48 ba 00 00 00 00 00 fc ff df ff
05 4f 7c 17
[   37.837667][ T1170] RSP: 0018:ffffc900006fee60 EFLAGS: 00010246
[   37.838503][ T1170] RAX: 0d2d19ffcded3d00 RBX: 0000000000000000
RCX: ffff888117fc9b00
[   37.839519][ T1170] RDX: 0000000000000000 RSI: 0000000000000000
RDI: 0000000000000000
[   37.840570][ T1170] RBP: ffffffff86e88380 R08: ffffffff8130793b
R09: fffff520000dfd85
[   37.841618][ T1170] R10: fffff520000dfd85 R11: 0000000000000000
R12: ffffffff87095fb8
[   37.842649][ T1170] R13: ffff888117770ad8 R14: ffff888117770acc
R15: ffffffff852b7420
[   37.843728][ T1170] FS:  0000000000000000(0000)
GS:ffff8881f6600000(0000) knlGS:0000000000000000
[   37.844877][ T1170] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   37.845749][ T1170] CR2: 00007f992eaab380 CR3: 000000011834b000
CR4: 00000000001006f0
[   37.846794][ T1170] Call Trace:
[   37.847245][ T1170]  <TASK>
[   37.847643][ T1170]  slab_free_freelist_hook+0x89/0x160
[   37.848409][ T1170]  ? devres_release_all+0x262/0x350
[   37.849156][ T1170]  __kmem_cache_free+0x71/0x110
[   37.849829][ T1170]  devres_release_all+0x262/0x350
[   37.850478][ T1170]  ? devres_release+0x90/0x90
[   37.851118][ T1170]  device_release_driver_internal+0x5e5/0x8a0
[   37.851944][ T1170]  bus_remove_device+0x2ea/0x400
[   37.852611][ T1170]  device_del+0x64f/0xb40
[   37.853212][ T1170]  ? kill_device+0x150/0x150
[   37.853831][ T1170]  ? print_irqtrace_events+0x1f0/0x1f0
[   37.854564][ T1170]  hid_destroy_device+0x66/0x100
[   37.855226][ T1170]  usbhid_disconnect+0x9a/0xc0
[   37.855887][ T1170]  usb_unbind_interface+0x1e1/0x890
```

``` hid-asus.c
[   77.409878][ T1169] usb 1-1: USB disconnect, device number 2
[   77.423606][ T1169] ODEBUG: free active (active state 0) object
type: work_struct hint: asus_kbd_backlight_work+0x0/0x2c0
[   77.425222][ T1169] WARNING: CPU: 0 PID: 1169 at
lib/debugobjects.c:505 debug_check_no_obj_freed+0x43a/0x630
[   77.426599][ T1169] Modules linked in:
[   77.427322][ T1169] CPU: 0 PID: 1169 Comm: kworker/0:3 Not tainted
6.1.0-rc4-dirty #43
[   77.428404][ T1169] Hardware name: QEMU Standard PC (i440FX + PIIX,
1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
[   77.429644][ T1169] Workqueue: usb_hub_wq hub_event
[   77.430296][ T1169] RIP: 0010:debug_check_no_obj_freed+0x43a/0x630
[   77.431142][ T1169] Code: 48 89 ef e8 28 82 58 ff 49 8b 14 24 4c 8b
45 00 48 c7 c7 40 5f 09 87 48 c7 c6 60 5b 09 87 89 d9 4d 89 f9 31 c0
e8 46 25 ef fe <0f> 0b 4c 8b 64 24 20 48 ba 00 00 00 00 00 fc ff df ff
05 4f 7c 17
[   77.433691][ T1169] RSP: 0018:ffffc9000069ee60 EFLAGS: 00010246
[   77.434470][ T1169] RAX: b85d2b40c12d7600 RBX: 0000000000000000
RCX: ffff888117a78000
[   77.435507][ T1169] RDX: 0000000000000000 RSI: 0000000080000000
RDI: 0000000000000000
[   77.436521][ T1169] RBP: ffffffff86e88380 R08: ffffffff8130793b
R09: ffffed103ecc4ed6
[   77.437582][ T1169] R10: ffffed103ecc4ed6 R11: 0000000000000000
R12: ffffffff87095fb8
[   77.438593][ T1169] R13: ffff88810e348fe0 R14: ffff88810e348fd4
R15: ffffffff852b5780
[   77.439667][ T1169] FS:  0000000000000000(0000)
GS:ffff8881f6600000(0000) knlGS:0000000000000000
[   77.440842][ T1169] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   77.441688][ T1169] CR2: 00007ffc05495ff0 CR3: 000000010cdf0000
CR4: 00000000001006f0
[   77.442720][ T1169] Call Trace:
[   77.443167][ T1169]  <TASK>
[   77.443555][ T1169]  slab_free_freelist_hook+0x89/0x160
[   77.444302][ T1169]  ? devres_release_all+0x262/0x350
[   77.444990][ T1169]  __kmem_cache_free+0x71/0x110
[   77.445638][ T1169]  devres_release_all+0x262/0x350
[   77.446309][ T1169]  ? devres_release+0x90/0x90
[   77.446978][ T1169]  device_release_driver_internal+0x5e5/0x8a0
[   77.447748][ T1169]  bus_remove_device+0x2ea/0x400
[   77.448421][ T1169]  device_del+0x64f/0xb40
[   77.448976][ T1169]  ? kill_device+0x150/0x150
[   77.449577][ T1169]  ? print_irqtrace_events+0x1f0/0x1f0
[   77.450307][ T1169]  hid_destroy_device+0x66/0x100
[   77.450938][ T1169]  usbhid_disconnect+0x9a/0xc0
```

Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
---
Changes in v3:
- use spinlocks to prevent workers scheduling
- drop patches on sony & playstation hid drivers
- Link to v2: https://lore.kernel.org/r/20230125-hid-unregister-leds-v2-0-689cc62fc878@diag.uniroma1.it

Changes in v2:
- dualshock4: Clarify UAF
- dualsense:  Clarify UAF
- dualsense:  Unregister multicolor led controller
- Link to v1: https://lore.kernel.org/r/20230125-hid-unregister-leds-v1-0-9a5192dcef16@diag.uniroma1.it

---
Pietro Borrello (2):
      HID: bigben: use spinlock to safely schedule workers
      HID: asus: use spinlock to safely schedule workers

 drivers/hid/hid-asus.c     | 24 +++++++++++++++++++++---
 drivers/hid/hid-bigbenff.c | 34 +++++++++++++++++++++++++++++-----
 2 files changed, 50 insertions(+), 8 deletions(-)
---
base-commit: 2241ab53cbb5cdb08a6b2d4688feb13971058f65
change-id: 20230125-hid-unregister-leds-4cbf67099e1d

Best regards,
-- 
Pietro Borrello <borrello@diag.uniroma1.it>


^ permalink raw reply

* [PATCH v2 3/3] arm64: dts: qcom: sdm845-oneplus: add tri-state-key
From: Gergo Koteles @ 2023-02-09 23:25 UTC (permalink / raw)
  To: Dmitry Torokhov, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Rob Herring, Krzysztof Kozlowski, Jiri Kosina, Benjamin Tissoires
  Cc: linux-input, linux-kernel, linux-arm-msm, devicetree,
	Caleb Connolly, Gergo Koteles

The tri-state-key is a sound profile switch found on the OnePlus 6,
Android maps the states to "mute", "vibrate" and "ring". Expose them as
ABS_SND_PROFILE events.
The previous GPIO numbers were wrong. Update them to the correct
ones.

Co-developed-by: Caleb Connolly <caleb@connolly.tech>
Signed-off-by: Caleb Connolly <caleb@connolly.tech>
Signed-off-by: Gergo Koteles <soyer@irl.hu>
---
 .../boot/dts/qcom/sdm845-oneplus-common.dtsi  | 39 ++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
index 64638ea94db7..e45d4fdead82 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
@@ -52,6 +52,43 @@ key-vol-up {
 		};
 	};
 
+	tri-state-key {
+		compatible = "gpio-keys";
+		label = "Tri-state key";
+		pinctrl-names = "default";
+		pinctrl-0 = <&tri_state_key_default>;
+		state-top {
+			label = "Tri-state key top";
+			linux,input-type = <EV_ABS>;
+			linux,code = <ABS_SND_PROFILE>;
+			linux,input-value = <0>;
+			gpios = <&tlmm 126 GPIO_ACTIVE_LOW>;
+			debounce-interval = <50>;
+			linux,can-disable;
+		};
+
+		state-middle {
+			label = "Tri-state key middle";
+			linux,input-type = <EV_ABS>;
+			linux,code = <ABS_SND_PROFILE>;
+			linux,input-value = <1>;
+			gpios = <&tlmm 52 GPIO_ACTIVE_LOW>;
+			debounce-interval = <50>;
+			linux,can-disable;
+
+		};
+
+		state-bottom {
+			label = "Tri-state key bottom";
+			linux,input-type = <EV_ABS>;
+			linux,code = <ABS_SND_PROFILE>;
+			linux,input-value = <2>;
+			gpios = <&tlmm 24 GPIO_ACTIVE_LOW>;
+			debounce-interval = <50>;
+			linux,can-disable;
+		};
+	};
+
 	reserved-memory {
 		/*
 		 * The rmtfs_mem needs to be guarded due to "XPU limitations"
@@ -754,7 +791,7 @@ &tlmm {
 	gpio-reserved-ranges = <0 4>, <81 4>;
 
 	tri_state_key_default: tri-state-key-default-state {
-		pins = "gpio40", "gpio42", "gpio26";
+		pins = "gpio126", "gpio52", "gpio24";
 		function = "gpio";
 		drive-strength = <2>;
 		bias-disable;
-- 
2.39.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox