Re: [PATCH v3 3/6] platform/x86: dell-smo8800: Move instantiation of lis3lv02d i2c_client from i2c-i801 to dell-smo8800

["Pali Rohár" <pali@kernel.org>]

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?