On Wed, Nov 25, 2020 at 10:37 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
> From: Jeremy Cline <jeremy@xxxxxxxxxx>
>
> Some BOSC0200 acpi_device-s describe two accelerometers in a single ACPI
> device. Normally we would handle this by letting the special
> drivers/platform/x86/i2c-multi-instantiate.c driver handle the BOSC0200
> ACPI id and let it instantiate 2 bmc150_accel type i2c_client-s for us.
>
> But doing so changes the modalias for the first accelerometer
> (which is already supported and used on many devices) from
> acpi:BOSC0200 to i2c:bmc150_accel. The modalias is not only used
> to load the driver, but is also used by hwdb matches in
> /lib/udev/hwdb.d/60-sensor.hwdb which provide a mountmatrix to
> userspace by setting the ACCEL_MOUNT_MATRIX udev property.
>
> Switching the handling of the BOSC0200 over to i2c-multi-instantiate.c
> will break the hwdb matches causing the ACCEL_MOUNT_MATRIX udev prop
> to no longer be set. So switching over to i2c-multi-instantiate.c is
> not an option.
I'm wondering if we can meanwhile update hwdb to support
i2c-multi-instantiate cases in the future and in a few years switch to
it.
> Changes by Hans de Goede:
> -Add explanation to the commit message why i2c-multi-instantiate.c
> cannot be used
> -Also set the dev_name, fwnode and irq i2c_board_info struct members
> for the 2nd client
...
> + ret = bmc150_accel_core_probe(&client->dev, regmap, client->irq, name, block_supported);
> + if (ret)
> + return ret;
> +
> + /*
> + * Some BOSC0200 acpi_devices describe 2 accelerometers in a single ACPI
> + * device, try instantiating a second i2c_client for an I2cSerialBusV2
> + * ACPI resource with index 1. The !id check avoids recursion when
> + * bmc150_accel_probe() gets called for the second client.
> + */
> + if (!id && adev && strcmp(acpi_device_hid(adev), "BOSC0200") == 0) {
> + struct i2c_board_info board_info = {
> + .type = "bmc150_accel",
> + /* The 2nd accel sits in the base of 2-in-1s */
> + .dev_name = "BOSC0200:base",
Hmm... Can we use '.' (dot) rather than ':' (colon) to avoid confusion
with ACPI device naming schema? (Or was it on purpose?)
And this seems to be the only device in the system, second as this is
not allowed as far as I understand. Right? But theoretically I can
create an ACPI SSDT with quite similar excerpt and sensor and
enumerate it via ConfigFS (I understand that is quite unlikely).
> + .fwnode = client->dev.fwnode,
> + .irq = -ENOENT,
> + };
> + struct i2c_client *second_dev;
> +
> + second_dev = i2c_acpi_new_device(&client->dev, 1, &board_info);
> + if (!IS_ERR(second_dev))
> + bmc150_set_second_device(second_dev);
> + }
...
> static int bmc150_accel_remove(struct i2c_client *client)
> {
> + struct i2c_client *second_dev = bmc150_get_second_device(client);
> + if (second_dev)
Redundant.
> + i2c_unregister_device(second_dev);
> +
> return bmc150_accel_core_remove(&client->dev);
> }
--
With Best Regards,
Andy Shevchenko
