Re: [PATCH] iio: accel: da280: Simplify id-matching

[Jonathan Cameron <jic23@kernel.org>]

On Mon,  1 Jan 2024 14:32:34 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> da280_match_acpi_device() is a DIY version of acpi_device_get_match_data(),
> so it can be dropped.
> 
> And things can be simplified further by using i2c_get_match_data() which
> will also check i2c_client_id style ids.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Hi Hans and happy new year,

This runs into a slightly nasty corner case (which can't actually happen because
we know it will match) of a NULL return on failure to match which ends up
being the first enum entry whereas we should probably return an error.

My preferred cleanup would be to make both id tables point to a set of structs
that encode the device differences as data rather than ids.

struct da280_chip_info {
	const char *name;
	int num_channels;
}

or something along those lines.  Then we can rely on the generic lookup function
without taking care about the 0 value.

Jonathan


> ---
>  drivers/iio/accel/da280.c | 18 +-----------------
>  1 file changed, 1 insertion(+), 17 deletions(-)
> 
> diff --git a/drivers/iio/accel/da280.c b/drivers/iio/accel/da280.c
> index 572bfe9694b0..e4cd4b3a28ab 100644
> --- a/drivers/iio/accel/da280.c
> +++ b/drivers/iio/accel/da280.c
> @@ -89,17 +89,6 @@ static const struct iio_info da280_info = {
>  	.read_raw	= da280_read_raw,
>  };
>  
> -static enum da280_chipset da280_match_acpi_device(struct device *dev)
> -{
> -	const struct acpi_device_id *id;
> -
> -	id = acpi_match_device(dev->driver->acpi_match_table, dev);
> -	if (!id)
> -		return -EINVAL;
> -
> -	return (enum da280_chipset) id->driver_data;
> -}
> -
>  static void da280_disable(void *client)
>  {
>  	da280_enable(client, false);
> @@ -107,7 +96,6 @@ static void da280_disable(void *client)
>  
>  static int da280_probe(struct i2c_client *client)
>  {
> -	const struct i2c_device_id *id = i2c_client_get_device_id(client);
>  	int ret;
>  	struct iio_dev *indio_dev;
>  	struct da280_data *data;
> @@ -128,11 +116,7 @@ static int da280_probe(struct i2c_client *client)
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->channels = da280_channels;
>  
> -	if (ACPI_HANDLE(&client->dev)) {
> -		chip = da280_match_acpi_device(&client->dev);
> -	} else {
> -		chip = id->driver_data;
> -	}
> +	chip = (uintptr_t)i2c_get_match_data(client);
>  
>  	if (chip == da217) {
>  		indio_dev->name = "da217";