linux-kernel - Re: [PATCH 3/3] HID: apple-magic-backlight: Add driver for keyboard backlight on internal Magic Keyboards

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y+jLCebet+aekGRv@smile.fi.intel.com>
Date:   Sun, 12 Feb 2023 13:18:33 +0200
From:   Andy Shevchenko <andy@...radead.org>
To:     Aditya Garg <gargaditya08@...e.com>
Cc:     Jiri Kosina <jikos@...nel.org>,
        "jkosina@...e.cz" <jkosina@...e.cz>,
        "benjamin.tissoires@...hat.com" <benjamin.tissoires@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        "linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
        "ronald@...ovation.ch" <ronald@...ovation.ch>,
        "kekrby@...il.com" <kekrby@...il.com>,
        Orlando Chamberlain <orlandoch.dev@...il.com>
Subject: Re: [PATCH 3/3] HID: apple-magic-backlight: Add driver for keyboard
 backlight on internal Magic Keyboards

On Fri, Feb 10, 2023 at 03:45:15AM +0000, Aditya Garg wrote:
> From: Orlando Chamberlain <orlandoch.dev@...il.com>
> 
> 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);

-- 
With Best Regards,
Andy Shevchenko