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?
