Hi,
On 3/25/21 8:52 AM, Yuan, Perry wrote:
Hi Hans.
-----Original Message----- From: Hans de Goede hdegoede@redhat.com Sent: Wednesday, March 24, 2021 3:40 AM To: Pierre-Louis Bossart; Yuan, Perry; pobrn@protonmail.com; oder_chiou@realtek.com; perex@perex.cz; tiwai@suse.com; mgross@linux.intel.com; Limonciello, Mario Cc: alsa-devel@alsa-project.org; linux-kernel@vger.kernel.org; lgirdwood@gmail.com; platform-driver-x86@vger.kernel.org; broonie@kernel.org Subject: Re: [PATCH v5 1/2] platform/x86: dell-privacy: Add support for Dell hardware privacy
[EXTERNAL EMAIL]
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
If adding static to the function, I will be worried about that the init and exit cannot be called by another file .
That is right, which is why I added the "?".
But this is no longer relevant after my detailed review of the patch, so lets discuss things further in the detailed review email-thread.
Regards,
Hans