Re: [PATCH v3 1/3] platform/x86: dell-privacy: Add support for Dell hardware privacy

From: Pierre-Louis Bossart
Date: Tue Feb 16 2021 - 09:58:53 EST



+static const struct acpi_device_id privacy_acpi_device_ids[] = {
+    {"PNP0C09", 0},
+    { },
+};
+MODULE_DEVICE_TABLE(acpi, privacy_acpi_device_ids);
+
+static struct platform_driver dell_privacy_platform_drv = {
+    .driver = {
+        .name = PRIVACY_PLATFORM_NAME,
+        .acpi_match_table = ACPI_PTR(privacy_acpi_device_ids),
+    },

no .probe?
Originally i added the probe here, but it cause the driver  .probe called twice. after i use platform_driver_probe to register the driver loading process, the duplicated probe issue resolved.

I

+    .remove = dell_privacy_acpi_remove,
+};
+
+int __init dell_privacy_acpi_init(void)
+{
+    int err;
+    struct platform_device *pdev;
+    int privacy_capable = wmi_has_guid(DELL_PRIVACY_GUID);
+
+    if (!wmi_has_guid(DELL_PRIVACY_GUID))
+        return -ENODEV;
+
+    privacy_acpi = kzalloc(sizeof(*privacy_acpi), GFP_KERNEL);
+    if (!privacy_acpi)
+        return -ENOMEM;
+
+    pdev = platform_device_register_simple(
+            PRIVACY_PLATFORM_NAME, PLATFORM_DEVID_NONE, NULL, 0);
+    if (IS_ERR(pdev)) {
+        err = PTR_ERR(pdev);
+        goto pdev_err;
+    }
+    err = platform_driver_probe(&dell_privacy_platform_drv,
+            dell_privacy_acpi_probe);
+    if (err)
+        goto pdrv_err;

why is the probe done here? Put differently, what prevents you from using a 'normal' platform driver, and do the leds_setup in the .probe()?
At first ,I used the normal platform driver framework, however tt cause the driver  .probe called twice. after i use platform_driver_probe to register the driver loading process, the duplicated probe issue resolved.

This sounds very odd...

this looks like a conflict with the ACPI subsystem finding a device and probing the driver that's associated with the PNP0C09 HID, and then this module __init creating a device manually which leads to a second probe

Neither the platform_device_register_simple() nor platform_driver_probe() seem necessary?