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

[Guenter Roeck <linux@roeck-us.net>]

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@mariuszachmann.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