On Saturday 06 January 2024 17:09:29 Hans de Goede wrote:
> +/*
> + * Accelerometer's I2C address is not specified in DMI nor ACPI,
> + * so it is needed to define mapping table based on DMI product names.
> + */
> +static const struct {
> + const char *dmi_product_name;
> + unsigned short i2c_addr;
> +} dell_lis3lv02d_devices[] = {
> + /*
> + * Dell platform team told us that these Latitude devices have
> + * ST microelectronics accelerometer at I2C address 0x29.
> + */
> + { "Latitude E5250", 0x29 },
> + { "Latitude E5450", 0x29 },
> + { "Latitude E5550", 0x29 },
> + { "Latitude E6440", 0x29 },
> + { "Latitude E6440 ATG", 0x29 },
> + { "Latitude E6540", 0x29 },
> + /*
> + * Additional individual entries were added after verification.
> + */
> + { "Latitude 5480", 0x29 },
> + { "Vostro V131", 0x1d },
> + { "Vostro 5568", 0x29 },
> +};
> +
> +static void smo8800_instantiate_i2c_client(struct smo8800_device *smo8800)
> +{
> + struct i2c_board_info info = { };
> + struct i2c_adapter *adap = NULL;
> + const char *dmi_product_name;
> + u8 addr = 0;
> + int i;
> +
> + bus_for_each_dev(&i2c_bus_type, NULL, &adap, smo8800_find_i801);
> + if (!adap)
> + return;
> +
> + dmi_product_name = dmi_get_system_info(DMI_PRODUCT_NAME);
> + for (i = 0; i < ARRAY_SIZE(dell_lis3lv02d_devices); ++i) {
Before doing this array iteration it is needed to check for Dell vendor
like it was before:
if (!dmi_match(DMI_SYS_VENDOR, "Dell Inc."))
return false;
Or put vendor name into the devices list and check for it (in case you
want to extend list also for non-Dell machines).
> + if (strcmp(dmi_product_name,
> + dell_lis3lv02d_devices[i].dmi_product_name) == 0) {
> + addr = dell_lis3lv02d_devices[i].i2c_addr;
> + break;
> + }
> + }
> +
> + if (!addr) {
> + dev_warn(smo8800->dev,
> + "Accelerometer lis3lv02d is present on SMBus but its address is unknown, skipping registration\n");
> + goto put_adapter;
> + }
> +
> + info.addr = addr;
> + strscpy(info.type, "lis3lv02d", I2C_NAME_SIZE);
> +
> + smo8800->i2c_dev = i2c_new_client_device(adap, &info);
> + if (IS_ERR(smo8800->i2c_dev)) {
> + dev_err_probe(smo8800->dev, PTR_ERR(smo8800->i2c_dev),
> + "registering accel i2c_client\n");
> + smo8800->i2c_dev = NULL;
> + } else {
> + dev_info(smo8800->dev, "Registered %s accelerometer on address 0x%02x\n",
> + info.type, info.addr);
> + }
> +put_adapter:
> + i2c_put_adapter(adap);
> +}
> +
> static int smo8800_probe(struct platform_device *device)
> {
> int err;
> @@ -126,10 +237,12 @@ static int smo8800_probe(struct platform_device *device)
> return err;
> smo8800->irq = err;
>
> + smo8800_instantiate_i2c_client(smo8800);
Now after looking at this change again I see there a problem. If i2c-801
driver initialize i2c-801 device after this smo8800 is called then
accelerometer i2c device would not happen.
Also it has same problem if PCI i801 device is reloaded or reset.
With the current approach this was not an issue as during i801
initialization was smo i2c device automatically created and lis driver
was able to bind and initialize it at any time.
Before parent driver created its own direct children devices. After this
change, the child driver is trying to find who is the parent of its
device and injects its device to the parent in the device tree
hierarchy.
> +
> err = misc_register(&smo8800->miscdev);
> if (err) {
> dev_err(&device->dev, "failed to register misc dev: %d\n", err);
> - return err;
> + goto error_unregister_i2c_client;
> }
>
> err = request_threaded_irq(smo8800->irq, smo8800_interrupt_quick,
> @@ -150,6 +263,8 @@ static int smo8800_probe(struct platform_device *device)
>
> error:
> misc_deregister(&smo8800->miscdev);
> +error_unregister_i2c_client:
> + i2c_unregister_device(smo8800->i2c_dev);
> return err;
> }
>
> @@ -160,9 +275,9 @@ static void smo8800_remove(struct platform_device *device)
> free_irq(smo8800->irq, smo8800);
> misc_deregister(&smo8800->miscdev);
> dev_dbg(&device->dev, "device /dev/freefall unregistered\n");
> + i2c_unregister_device(smo8800->i2c_dev);
> }
>
> -/* NOTE: Keep this list in sync with drivers/i2c/busses/i2c-i801.c */
> static const struct acpi_device_id smo8800_ids[] = {
> { "SMO8800", 0 },
> { "SMO8801", 0 },
> @@ -189,3 +304,5 @@ module_platform_driver(smo8800_driver);
> MODULE_DESCRIPTION("Dell Latitude freefall driver (ACPI SMO88XX)");
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Sonal Santan, Pali Rohár");
> +/* Ensure the i2c-801 driver is loaded for i2c_client instantiation */
> +MODULE_SOFTDEP("pre: i2c-i801");
> --
> 2.43.0
>
