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
| ||
|
Message-ID: <87a6j5cait.fsf@dell.be.48ers.dk> Date: Tue, 19 Oct 2021 17:14:02 +0200 From: Peter Korsgaard <peter@...sgaard.com> To: Hans de Goede <hdegoede@...hat.com> Cc: Santosh Kumar Yadav <santoshkumar.yadav@...co.com>, santoshyadav30@...il.com, Mark Gross <mgross@...ux.intel.com>, Peter Korsgaard <peter.korsgaard@...co.com>, linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org Subject: Re: [PATCH] platform/x86: Support for EC-connected GPIOs for identify LED/button on Barco P50 board >>>>> "Hans" == Hans de Goede <hdegoede@...hat.com> writes: > Hi, > On 10/13/21 16:03, Santosh Kumar Yadav wrote: >> Add a driver providing access to the GPIOs for the identify button and led >> present on Barco P50 board, based on the pcengines-apuv2.c driver. >> >> There is unfortunately no suitable ACPI entry for the EC communication >> interface, so instead bind to boards with "P50" as their DMI product family >> and hard code the I/O port number (0x299). >> >> The driver also hooks up the leds-gpio and gpio-keys-polled drivers to the >> GPIOs, so they are finally exposed as: >> >> LED: >> /sys/class/leds/identify >> >> Button: (/proc/bus/input/devices) >> I: Bus=0019 Vendor=0001 Product=0001 Version=0100 >> N: Name="identify" >> P: Phys=gpio-keys-polled/input0 >> S: Sysfs=/devices/platform/barco-p50-gpio/gpio-keys-polled/input/input10 >> U: Uniq= >> H: Handlers=event10 >> B: PROP=0 >> B: EV=3 >> B: KEY=1000000 0 0 0 0 0 0 >> >> Signed-off-by: Santosh Kumar Yadav <santoshkumar.yadav@...co.com> >> Signed-off-by: Peter Korsgaard <peter@...sgaard.com> > Thanks, overall this looks pretty good. I've a couple of comments inline, > please send a v2 addresing this. .. >> +/* Board setup */ >> +static const struct dmi_system_id dmi_ids[] __initconst = { >> + { >> + .matches = { >> + DMI_EXACT_MATCH(DMI_PRODUCT_FAMILY, "P50") >> + }, >> + }, > But I'm a bit worried about the DMI match, it seems a bit too generic. > E.g. Lenovo also has a P50 laptop series. > For v2 please make the DMI match also on e.g. sys_vendor. Agreed, will add a match on vendor = Barco. > You should put a: > MODULE_DEVICE_TABLE(dmi, dmi_ids); > here, this will add a dmi based modalias to the module, so that it will > be automatically loaded at boot on systems which match the dmi_ids table. Ok. >> +MODULE_SOFTDEP("pre: platform:leds-gpio platform:gpio-keys-polled"); > Is this softdep really necessary ? I would expect things to work fine too if > the leds-gpio and gpio-keys-polled drivers are loaded automatically after > the platform_devices for them have been created . True. This was copied over from pcengines-apuv2.c, but we'll drop it for v2. -- Bye, Peter Korsgaard
Powered by blists - more mailing lists