Re: [PATCH 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 Sunday 24 December 2023 22:36:19 Hans de Goede wrote:
> +static int smo8800_find_i801(struct device *dev, void *data)
> +{
> +	static const u16 i801_idf_pci_device_ids[] = {
> +		0x1d70, /* Patsburg (PCH) */
> +		0x1d71, /* Patsburg (PCH) */
> +		0x1d72, /* Patsburg (PCH) */
> +		0x8d7d, /* Wellsburg (PCH) */
> +		0x8d7e, /* Wellsburg (PCH) */
> +		0x8d7f, /* Wellsburg (PCH) */
> +	};

I'm not happy with seeing another hardcoded list of device ids in the
driver. Are not we able to find compatible i801 adapter without need to
hardcode this list there in smo driver? And if really not, what about
sharing (or exporting) list from the i801 driver?

I'm just worried that this PCI id list will increase in the future and
it would be required to add a new and new id into it...

> +	struct i2c_adapter *adap, **adap_ret = data;
> +	struct pci_dev *pdev;
> +	int i;
> +
> +	adap = i2c_verify_adapter(dev);
> +	if (!adap)
> +		return 0;
> +
> +	if (!strstarts(adap->name, "SMBus I801 adapter"))
> +		return 0;
> +
> +	/* The parent of an I801 adapter is always a PCI device */
> +	pdev = to_pci_dev(adap->dev.parent);
> +	for (i = 0; i < ARRAY_SIZE(i801_idf_pci_device_ids); i++) {
> +		if (pdev->device == i801_idf_pci_device_ids[i])
> +			return 0; /* Only register client on main SMBus channel */
> +	}
> +
> +	*adap_ret = i2c_get_adapter(adap->nr);
> +	return 1;
> +}