linux-kernel - Re: [PATCH v2] hwmon: corsair-cpro: Change to HID driver

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 for Android: free password hash cracker in your pocket

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

Message-ID: <7a45a7ac-ef4d-b9c8-fac7-6c226baf1329@roeck-us.net>
Date:   Wed, 15 Jul 2020 13:11:39 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Marius Zachmann <mail@...iuszachmann.de>
Cc:     Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] hwmon: corsair-cpro: Change to HID driver

On 7/15/20 12:55 PM, Marius Zachmann wrote:
> On 15.07.20 at 21:49:59 CEST, Guenter Roeck wrote
>> On 7/15/20 12:12 PM, Marius Zachmann wrote:
>>> On 15.07.20 at 17:35:08 CEST, Guenter Roeck wrote
>>>> On 7/15/20 8:14 AM, Marius Zachmann wrote:
>>>>> This changes corsair-cpro to a hid driver using hid reports.
>>>>>
>>>>> Signed-off-by: Marius Zachmann <mail@...iuszachmann.de>
>>>>>
>>>>> ---
>>>>
>>> ...
>>>>> -module_usb_driver(ccp_driver);
>>>>> +static int __init ccp_init(void)
>>>>> +{
>>>>> +	return hid_register_driver(&ccp_driver);
>>>>> +}
>>>>> +
>>>>> +static void __exit ccp_exit(void)
>>>>> +{
>>>>> +	hid_unregister_driver(&ccp_driver);
>>>>> +}
>>>>> +
>>>>> +/* make sure it is loaded after hid */
>>>>> +late_initcall(ccp_init);
>>>>> +module_exit(ccp_exit);
>>>>
>>>> Sorry for not noticing before, but can you use module_hid_driver() ?
>>>> That seems to work for other hid drivers.
>>>>
>>>> Thanks,
>>>> Guenter
>>>>
>>>
>>> When using module_hid_driver() and compiling the driver as built-in
>>> the driver init function will get called before hid and fail registering
>>> the driver. late_initcall forces it to be loaded after hid.
>>>
>>
>> That seems odd. Why does no other hid driver have that problem ?
>> Your argument is pretty much that module_hid_driver() would not work,
>> which seems unlikely.
>>
>> Thanks,
>> Guenter
>>
> 
> The order of initcalls is determined by the build order. Hwmon subsystem
> gets build before hid subsystem and therefore the initcalls are earlier
> than hid. But hid_register_driver needs the hid driver to be there.
> 
Hmm. I am not happy, but I guess we'll have to live with it.

Can you add the detailed explanation to the comment above ?
"make sure it is loaded after hid" doesn't tell the whole story,
and I want to avoid having to deal with "cleanup" patches later.

Alternatively, I wonder if it would make sense to force
this driver to be built as module. Have you thought about that ?

Thanks,
Guenter