Hi,
On 6/24/24 8:14 PM, Pali Rohár wrote:
> On Monday 24 June 2024 13:15:16 Hans de Goede wrote:
>> +static int match_acpi_device_ids(struct device *dev, const void *data)
>> +{
>
> You can mark this function as __init as it is called only from
> dell_lis3lv02d_init to free space.
>
>> + const struct acpi_device_id *ids = data;
>> +
>> + return acpi_match_device(ids, dev) ? 1 : 0;
>> +}
>> +
>> +static int __init dell_lis3lv02d_init(void)
>> +{
>> + struct device *dev;
>> + int err;
>> +
>> + /*
>> + * First check for a matching platform_device. This protects against
>> + * SMO88xx ACPI fwnodes which actually do have an I2C resource, which
>> + * will already have an i2c_client instantiated (not a platform_device).
>> + */
>> + 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;
>
> Is zero return value expected? Should not be it something like -ENODEV?
This is a module_init() function returning non 0 leads to an error getting
logged and the modprobe command to return a non 0 value which is not what
we want.
>> + }
>> + put_device(dev);
>> +
>> + lis3lv02d_dmi_id = dmi_first_match(lis3lv02d_devices);
>> + if (!lis3lv02d_dmi_id) {
>
> You can cache the value lis3lv02d_dmi_id->driver_data instead of caching
> lis3lv02d_dmi_id pointer and then you can mark lis3lv02d_devices array
> as __initconst to free additional space not needed at runtime on x86
> machines without accelerometer where CONFIG_DELL_SMO8800=y.
This is a good idea, done for v5.
Regards,
Hans
>
>> + pr_warn("accelerometer is present on SMBus but its address is unknown, skipping registration\n");
>> + return 0;
>> + }
>> +
>> + /*
>> + * 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;
>> +}
>> +module_init(dell_lis3lv02d_init);
>> +
>> +static void __exit dell_lis3lv02d_module_exit(void)
>> +{
>> + if (!notifier_registered)
>> + return;
>> +
>> + bus_unregister_notifier(&i2c_bus_type, &i2c_nb);
>> + cancel_work_sync(&i2c_work);
>> + i2c_unregister_device(i2c_dev);
>> +}
>> +module_exit(dell_lis3lv02d_module_exit);
>> +
>> +MODULE_DESCRIPTION("lis3lv02d i2c-client instantiation for ACPI SMO88xx devices");
>> +MODULE_AUTHOR("Hans de Goede <hansg@xxxxxxxxxx>");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.45.1
>>
>
