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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Aug 5, 2024 at 3:38 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
> Various Dell laptops have an lis3lv02d freefall/accelerometer sensor.
> The lis3lv02d chip has an interrupt line as well as an I2C connection to
> the system's main SMBus.
>
> The lis3lv02d is described in the ACPI tables by an SMO88xx ACPI device,
> but the SMO88xx ACPI fwnodes are incomplete and only list an IRQ resource.
>
> So far this has been worked around with some SMO88xx specific quirk code
> in the generic i2c-i801 driver, but it is not necessary to handle the Dell
> specific instantiation of i2c_client-s for SMO88xx ACPI devices there.
>
> The kernel already instantiates platform_device-s for these with an
> acpi:SMO88xx modalias. The drivers/platform/x86/dell/dell-smo8800.c
> driver binds to this platform device but this only deals with
> the interrupt resource. Add a drivers/platform/x86/dell/dell-lis3lv02d.c
> which will matches on the same acpi:SMO88xx modaliases and move
> the i2c_client instantiation from the generic i2c-i801 driver there.
>
> Moving the i2c_client instantiation has the following advantages:
>
> 1. This moves the SMO88xx ACPI device quirk handling away from the generic
> i2c-i801 module which is loaded on all Intel x86 machines to a module
> which will only be loaded when there is an ACPI SMO88xx device.
>
> 2. This removes the duplication of the SMO88xx ACPI Hardware ID (HID) table
> between the i2c-i801 and dell-smo8800 drivers.
>
> 3. This allows extending the quirk handling by adding new code and related
> module parameters to the dell-lis3lv02d driver, without needing to modify
> the i2c-i801 code.

...

> +static void instantiate_i2c_client(struct work_struct *work)
> +{
> +       struct i2c_board_info info = { };
> +       struct i2c_adapter *adap = NULL;
> +
> +       if (i2c_dev)
> +               return;
> +
> +       bus_for_each_dev(&i2c_bus_type, NULL, &adap, find_i801);
> +       if (!adap)
> +               return;

May i2c_for_each_dev() be used here?

> +       info.addr = i2c_addr;
> +       strscpy(info.type, "lis3lv02d", I2C_NAME_SIZE);
> +
> +       i2c_dev = i2c_new_client_device(adap, &info);
> +       if (IS_ERR(i2c_dev)) {
> +               pr_err("error %ld registering i2c_client\n", PTR_ERR(i2c_dev));
> +               i2c_dev = NULL;
> +       } else {
> +               pr_debug("registered lis3lv02d on address 0x%02x\n", info.addr);
> +       }
> +
> +       i2c_put_adapter(adap);
> +}

...

> +static int __init match_acpi_device_ids(struct device *dev, const void *data)
> +{
> +       const struct acpi_device_id *ids = data;

Wondering if this is needed. Can the compiler implicitly cast const
void * to the const something * ?

> +       return acpi_match_device(ids, dev) ? 1 : 0;
> +}

> +        */
> +       dev = bus_find_device(&platform_bus_type, NULL, smo8800_ids, match_acpi_device_ids);
> +       if (!dev) {
> +               pr_debug("No SMO88xx platform-device found\n");
> +               return 0;
> +       }
> +       put_device(dev);
> +
> +       lis3lv02d_dmi_id = dmi_first_match(lis3lv02d_devices);
> +       if (!lis3lv02d_dmi_id) {
> +               pr_warn("accelerometer is present on SMBus but its address is unknown, skipping registration\n");
> +               return 0;
> +       }
> +
> +       i2c_addr = (long)lis3lv02d_dmi_id->driver_data;
> +
> +       /*
> +        * Register i2c-bus notifier + queue initial scan for lis3lv02d
> +        * i2c_client instantiation.
> +        */
> +       err = bus_register_notifier(&i2c_bus_type, &i2c_nb);
> +       if (err)
> +               return err;
> +
> +       notifier_registered = true;
> +
> +       queue_work(system_long_wq, &i2c_work);
> +       return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko





[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux