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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 17 Feb 2021 20:47:52 +0800
From:   Perry Yuan <perry979106@...il.com>
To:     Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        Perry Yuan <Perry.Yuan@...l.com>, oder_chiou@...ltek.com,
        perex@...ex.cz, tiwai@...e.com, hdegoede@...hat.com,
        mgross@...ux.intel.com
Cc:     alsa-devel@...a-project.org, Mario.Limonciello@...l.com,
        linux-kernel@...r.kernel.org, lgirdwood@...il.com,
        platform-driver-x86@...r.kernel.org, broonie@...nel.org
Subject: Re: [PATCH v3 1/3] platform/x86: dell-privacy: Add support for Dell
 hardware privacy

Hi Pierre:
On 2021/2/16 22:56, Pierre-Louis Bossart wrote:
> 
>>>> +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?Because this privacy acpi driver file has dependency on the ec handle, 
so i want to determine if the driver can be loaded basing on the EC ID 
PNP0C09 matching.

So far,It works well for me to register the privacy driver with  the 
register sequence.
Dose it hurt to keep current registering process with 
platform_driver_probe used?

Perry