>>>>> "Hans" == Hans de Goede <hdegoede@xxxxxxxxxx> 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@xxxxxxxxx>
>> Signed-off-by: Peter Korsgaard <peter@xxxxxxxxxxxxx>
> 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
