On Sun, 12 Feb 2023 13:18:33 +0200
Andy Shevchenko <andy@xxxxxxxxxxxxx> wrote:
> On Fri, Feb 10, 2023 at 03:45:15AM +0000, Aditya Garg wrote:
> > From: Orlando Chamberlain <orlandoch.dev@xxxxxxxxx>
> >
> > This driver adds support for the keyboard backlight on Intel T2 Macs
> > with internal Magic Keyboards (MacBookPro16,x and MacBookAir9,1)
>
> ...
>
> > +#include <linux/hid.h>
> > +#include <linux/usb.h>
>
> Seems lack of some header inclusions, e.g. where struct led_classdev
> is defined or -ERRNO codes.
>
> > +#include "hid-ids.h"
>
> ...
>
> > +static void apple_magic_backlight_power_set(struct
> > apple_magic_backlight *backlight,
> > + char power, char rate)
>
> char is a beast, can we use u8 here and in similar cases?
>
> ...
>
> > + /* Ensure this usb endpoint is for the keyboard backlight,
> > not touchbar
> > + * backlight.
> > + */
>
> /*
> * Multi-line comment style
> * goes like this.
> */
>
> ...
>
> > + backlight = devm_kzalloc(&hdev->dev, sizeof(*backlight),
> > GFP_KERNEL);
>
> > +
>
> Redundant blank line.
>
> > + if (!backlight)
> > + return -ENOMEM;
>
> ...
>
> > +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,
> > +};
>
> > +
>
> Redundant blank line.
>
> > +module_hid_driver(apple_magic_backlight_hid_driver);
>
Thanks for pointing out all these, I'll make those changes in v2.
