Hi,
On 3/23/21 7:57 PM, Pierre-Louis Bossart wrote:
> Minor comments below.
<snip<
>> +int __init dell_privacy_acpi_init(void)
>
> is the __init necessary? You call this routine from another which already has this qualifier.
Yes this is necessary, all functions which are only used during module_load / from the
module's init function must be marked as __init so that the kernel can unload them
after module loading is done.
I do wonder if this one should not be static though (I've not looked at this patch in
detail yet).
>
>> +{
>> + int err;
>> + struct platform_device *pdev;
>> +
>> + if (!wmi_has_guid(DELL_PRIVACY_GUID))
>> + return -ENODEV;
>> +
>> + privacy_acpi = kzalloc(sizeof(*privacy_acpi), GFP_KERNEL);
>> + if (!privacy_acpi)
>> + return -ENOMEM;
>> +
>> + err = platform_driver_register(&dell_privacy_platform_drv);
>> + if (err)
>> + goto pdrv_err;
>> +
>> + pdev = platform_device_register_simple(
>> + PRIVACY_PLATFORM_NAME, PLATFORM_DEVID_NONE, NULL, 0);
>> + if (IS_ERR(pdev)) {
>> + err = PTR_ERR(pdev);
>> + goto pdev_err;
>> + }
>> +
>> + return 0;
>> +
>> +pdev_err:
>> + platform_device_unregister(pdev);
>> +pdrv_err:
>> + kfree(privacy_acpi);
>> + return err;
>> +}
>> +
>> +void __exit dell_privacy_acpi_exit(void)
>
> is the __exit required here?
Idem. Also static ?
Regards,
Hans
>
>> +{
>> + struct platform_device *pdev = to_platform_device(privacy_acpi->dev);
>> +
>> + platform_device_unregister(pdev);
>> + platform_driver_unregister(&dell_privacy_platform_drv);
>> + kfree(privacy_acpi);
>> +}
>> +
>> +MODULE_AUTHOR("Perry Yuan <perry_yuan@xxxxxxxx>");
>> +MODULE_DESCRIPTION("DELL Privacy ACPI Driver");
>> +MODULE_LICENSE("GPL");
>
>
