[2/2] HID: apple-magic-backlight: Add driver for keyboard backlight on internal Magic Keyboards
Commit Message
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>
---
MAINTAINERS | 6 ++
drivers/hid/Kconfig | 13 +++
drivers/hid/Makefile | 1 +
drivers/hid/apple-magic-backlight.c | 125 ++++++++++++++++++++++++++++
4 files changed, 145 insertions(+)
create mode 100644 drivers/hid/apple-magic-backlight.c
Comments
On Thu, Feb 16, 2023 at 03:12:28PM +1100, Orlando Chamberlain wrote:
> 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>
The last Signed-off-by should be yours.
See Documentation/process/submitting-patches.rst.
> ---
> MAINTAINERS | 6 ++
> drivers/hid/Kconfig | 13 +++
> drivers/hid/Makefile | 1 +
> drivers/hid/apple-magic-backlight.c | 125 ++++++++++++++++++++++++++++
> 4 files changed, 145 insertions(+)
> create mode 100644 drivers/hid/apple-magic-backlight.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fb1471cb5ed3..3319f0c3ed1e 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 e2a5d30c8895..f4702d32ce2f 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -130,6 +130,19 @@ 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_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 e8014c1a2f8b..5cbfe85dd31b 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_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 000000000000..ed5bcf5bb599
> --- /dev/null
> +++ b/drivers/hid/apple-magic-backlight.c
> @@ -0,0 +1,125 @@
> +// 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>
No need for linux/usb.h.
> +#include <linux/leds.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <dt-bindings/leds/common.h>
> +
> +#include "hid-ids.h"
> +
> +#define HID_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_report_set(struct hid_report *rep, u16 value, u8 rate)
> +{
Could "value" be a s32? "logical_maximum" from the field is a s32,
so there is no risk of truncation.
> + rep->field[0]->value[0] = value;
> + rep->field[1]->value[0] = 0x5e; /* Mimic Windows */
> + rep->field[1]->value[0] |= rate << 8;
> +
> + hid_hw_request(rep->device, rep, HID_REQ_SET_REPORT);
> +}
> +
> +static void apple_magic_backlight_set(struct apple_magic_backlight *backlight,
> + int brightness, char rate)
> +{
> + apple_magic_backlight_report_set(backlight->power, brightness ? 1 : 0, rate);
> + if (brightness)
> + apple_magic_backlight_report_set(backlight->brightness, 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 == HID_USAGE_MAGIC_BL))
> + return -ENODEV;
I don't think hdev->collection can ever be NULL.
Also personally I would prefer this:
!hdev->collection || hdev->collection[0].usage != HID_USAGE_MAGIC_BL
Like it is done with the reports below.
> +
> + backlight = devm_kzalloc(&hdev->dev, sizeof(*backlight), GFP_KERNEL);
> + if (!backlight)
> + return -ENOMEM;
> +
> + hid_set_drvdata(hdev, backlight);
Is this needed?
> +
> + 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 = ":white:" LED_FUNCTION_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 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,
> +};
> +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.39.1
>
Note: Only your cover letter has the "v2" prefix.
Normally git format-patch should apply this properly to all patches when
using --reroll-count.
On Thu, 16 Feb 2023 05:41:05 +0000
Thomas Weißschuh <thomas@t-8ch.de> wrote:
> On Thu, Feb 16, 2023 at 03:12:28PM +1100, Orlando Chamberlain wrote:
> > 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>
>
> The last Signed-off-by should be yours.
> See Documentation/process/submitting-patches.rst.
I'll reorder that in v3, thanks for pointing it out.
>
> > ---
> > MAINTAINERS | 6 ++
> > drivers/hid/Kconfig | 13 +++
> > drivers/hid/Makefile | 1 +
> > drivers/hid/apple-magic-backlight.c | 125
> > ++++++++++++++++++++++++++++ 4 files changed, 145 insertions(+)
> > create mode 100644 drivers/hid/apple-magic-backlight.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index fb1471cb5ed3..3319f0c3ed1e 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 e2a5d30c8895..f4702d32ce2f 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -130,6 +130,19 @@ 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_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 e8014c1a2f8b..5cbfe85dd31b 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_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 000000000000..ed5bcf5bb599
> > --- /dev/null
> > +++ b/drivers/hid/apple-magic-backlight.c
> > @@ -0,0 +1,125 @@
> > +// 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>
>
> No need for linux/usb.h.
I'll fix that in v3.
>
> > +#include <linux/leds.h>
> > +#include <linux/device.h>
> > +#include <linux/errno.h>
> > +#include <dt-bindings/leds/common.h>
> > +
> > +#include "hid-ids.h"
> > +
> > +#define HID_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_report_set(struct hid_report
> > *rep, u16 value, u8 rate) +{
>
> Could "value" be a s32? "logical_maximum" from the field is a s32,
> so there is no risk of truncation.
That makes sense, I'll put it in v3.
>
> > + rep->field[0]->value[0] = value;
> > + rep->field[1]->value[0] = 0x5e; /* Mimic Windows */
> > + rep->field[1]->value[0] |= rate << 8;
> > +
> > + hid_hw_request(rep->device, rep, HID_REQ_SET_REPORT);
> > +}
> > +
> > +static void apple_magic_backlight_set(struct apple_magic_backlight
> > *backlight,
> > + int brightness, char rate)
> > +{
> > + apple_magic_backlight_report_set(backlight->power,
> > brightness ? 1 : 0, rate);
> > + if (brightness)
> > +
> > apple_magic_backlight_report_set(backlight->brightness, 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 ==
> > HID_USAGE_MAGIC_BL))
> > + return -ENODEV;
>
> I don't think hdev->collection can ever be NULL.
I'll remove that check, I don't think it ever was null for me.
>
> Also personally I would prefer this:
>
> !hdev->collection || hdev->collection[0].usage != HID_USAGE_MAGIC_BL
That is neater, I'll use it in v3.
>
> Like it is done with the reports below.
>
> > +
> > + backlight = devm_kzalloc(&hdev->dev, sizeof(*backlight),
> > GFP_KERNEL);
> > + if (!backlight)
> > + return -ENOMEM;
> > +
> > + hid_set_drvdata(hdev, backlight);
>
> Is this needed?
Nope, I'll take that out in v3.
>
> > +
> > + 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 = ":white:"
> > LED_FUNCTION_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 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,
> > +};
> > +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.39.1
> >
>
> Note: Only your cover letter has the "v2" prefix.
> Normally git format-patch should apply this properly to all patches
> when using --reroll-count.
I realised that after I sent the patches, I had manually added v2 to
the cover letter but I forgot about the patches themselves. I didn't
know about --reroll-count, thanks!
On Thu, 16 Feb 2023 15:12:28 +1100
Orlando Chamberlain <orlandoch.dev@gmail.com> wrote:
> 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>
> ---
> MAINTAINERS | 6 ++
> drivers/hid/Kconfig | 13 +++
> drivers/hid/Makefile | 1 +
> drivers/hid/apple-magic-backlight.c | 125
A general question to the hid/input folks:
Is it alright that this doesn't start with "hid-"? All the other
drivers start with that so I'm not sure if its an issue. If it is, then
I can rename it to "hid-apple-magic-backlight".
> ++++++++++++++++++++++++++++ 4 files changed, 145 insertions(+)
> create mode 100644 drivers/hid/apple-magic-backlight.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fb1471cb5ed3..3319f0c3ed1e 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 e2a5d30c8895..f4702d32ce2f 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -130,6 +130,19 @@ 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_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 e8014c1a2f8b..5cbfe85dd31b 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_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 000000000000..ed5bcf5bb599
> --- /dev/null
> +++ b/drivers/hid/apple-magic-backlight.c
> @@ -0,0 +1,125 @@
> +// 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 <linux/leds.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <dt-bindings/leds/common.h>
> +
> +#include "hid-ids.h"
> +
> +#define HID_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_report_set(struct hid_report *rep,
> u16 value, u8 rate) +{
> + rep->field[0]->value[0] = value;
> + rep->field[1]->value[0] = 0x5e; /* Mimic Windows */
> + rep->field[1]->value[0] |= rate << 8;
> +
> + hid_hw_request(rep->device, rep, HID_REQ_SET_REPORT);
> +}
> +
> +static void apple_magic_backlight_set(struct apple_magic_backlight
> *backlight,
> + int brightness, char rate)
> +{
> + apple_magic_backlight_report_set(backlight->power,
> brightness ? 1 : 0, rate);
> + if (brightness)
> +
> apple_magic_backlight_report_set(backlight->brightness, 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 ==
> HID_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 = ":white:" LED_FUNCTION_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 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,
> +};
> +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");
On Thu, Feb 16, 2023 at 10:15 AM Orlando Chamberlain
<orlandoch.dev@gmail.com> wrote:
> On Thu, 16 Feb 2023 15:12:28 +1100
> Orlando Chamberlain <orlandoch.dev@gmail.com> wrote:
...
> > drivers/hid/apple-magic-backlight.c | 125
>
> A general question to the hid/input folks:
>
> Is it alright that this doesn't start with "hid-"? All the other
> drivers start with that so I'm not sure if its an issue.
What makes apple so special?
> If it is, then
> I can rename it to "hid-apple-magic-backlight".
If there is a good justification of not doing that, put it into the
commit message.
On Thu, Feb 16, 2023 at 7:41 AM Thomas Weißschuh <thomas@t-8ch.de> wrote:
> On Thu, Feb 16, 2023 at 03:12:28PM +1100, Orlando Chamberlain wrote:
...
> Note: Only your cover letter has the "v2" prefix.
> Normally git format-patch should apply this properly to all patches when
> using --reroll-count.
There is an option -v<X>, where <X> is a version applied to all
patches in the lot.
@@ -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>
@@ -130,6 +130,19 @@ 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_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)
@@ -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_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
new file mode 100644
@@ -0,0 +1,125 @@
+// 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 <linux/leds.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <dt-bindings/leds/common.h>
+
+#include "hid-ids.h"
+
+#define HID_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_report_set(struct hid_report *rep, u16 value, u8 rate)
+{
+ rep->field[0]->value[0] = value;
+ rep->field[1]->value[0] = 0x5e; /* Mimic Windows */
+ rep->field[1]->value[0] |= rate << 8;
+
+ hid_hw_request(rep->device, rep, HID_REQ_SET_REPORT);
+}
+
+static void apple_magic_backlight_set(struct apple_magic_backlight *backlight,
+ int brightness, char rate)
+{
+ apple_magic_backlight_report_set(backlight->power, brightness ? 1 : 0, rate);
+ if (brightness)
+ apple_magic_backlight_report_set(backlight->brightness, 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 == HID_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 = ":white:" LED_FUNCTION_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 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,
+};
+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");