Hi Pali,
On 6/22/24 6:43 PM, Pali Rohár wrote:
> On Saturday 22 June 2024 16:20:15 Pali Rohár wrote:
>> On Saturday 22 June 2024 16:06:01 Hans de Goede wrote:
>>>> Also does the whole device table has to be such verbose with lot of
>>>> duplicated information (which probably also increase size of every linux
>>>> image which includes this driver into it)?
>>>
>>> struct dmi_system_id is the default way to specify DMI matches in
>>> the kernel. This avoids code duplication in the form of writing
>>> a DYI function to do the matching.
>>>
>>> In v2 of the patch-set I only matched on product-name, but you asked
>>> in the review of v2 to also match on sys-vendor and you mentioned
>>> we may want to support other sys-vendors too, since some other brands
>>> have SMO88xx ACPI devices too. This more or less automatically leads
>>> to using the kernel's standard, existing, DMI matching mechanism.
>>>
>>> We really want to avoid coming up with something "new" ourselves here
>>> leading to unnecessary code duplication.
>>>
>>> Regards,
>>>
>>> Hans
>>
>> Ok, then let that table as you have it now.
>
> Definition of the table can be simplified by defining a macro which
> expand to those verbose parts which are being repeating, without need to
> introduce something "new". E.g.:
>
> #define DELL_LIS3LV02D_DMI_ENTRY(product_name, i2c_addr) \
> { \
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), \
> DMI_MATCH(DMI_PRODUCT_NAME, product_name), \
> }, \
> .driver_data = (void *)(i2c_addr), \
> }
>
> static const struct dmi_system_id smo8800_lis3lv02d_devices[] = {
> DELL_LIS3LV02D_DMI_ENTRY("Latitude E5250", 0x29),
> DELL_LIS3LV02D_DMI_ENTRY("Latitude E5450", 0x29),
> ...
> { }
> };
>
> Any opinion about this?
Thank you that is a good idea. I'll do as you suggest for v4 with
the addition of Andy's suggestion to use const in the cast.
Regards,
Hans
