Hi,
On 1/1/24 16:42, Jonathan Cameron wrote:
> On Mon, 1 Jan 2024 14:32:34 +0100
> Hans de Goede <hdegoede@xxxxxxxxxx> 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@xxxxxxxxxx>
>
> 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.
That is a good idea, thanks.
I have prepared (and will send out shortly) a v2 implementing this.
Regards,
Hans
>> ---
>> 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";
>
