On 2023/10/19 16:05, Yicong Yang wrote:
> On 2023/10/18 11:33, Shuai Xue wrote:
>>
...
>>
>>>
>>>> + return PTR_ERR(dwc_pcie_pmu_dev);
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void __exit dwc_pcie_pmu_exit(void)
>>>> +{
>>>> + platform_device_unregister(dwc_pcie_pmu_dev);
>>>> + platform_driver_unregister(&dwc_pcie_pmu_driver);
>>>> + cpuhp_remove_multi_state(dwc_pcie_pmu_hp_state);
>>>> +
>>>> + if (dwc_pcie_pmu_notify)
>>>
>>> If you have something unusual like this a driver module_exit() it definitely
>>> deserves a comment on why. I'm surprised by this as I'd expect the notifier
>>> to be unregistered in the driver remove so not sure why this is here.
>>> I've lost track of earlier discussions so if this was addressed then all
>>> we need is a comment here for the next person to run into it!
>>
>> All replied above, I will unregistered the notifier by devm_add_action_or_reset().
>>
>> I am curious about that what the difference between unregistered in module_exit()
>> and remove()?
>>
>
> From my understanding, if you register it in probe() then should undo it in remove().
> Otherwise you should register it in module_init(). Just make them coupled to make
> sure cleanup the resources correctly.
>
> This driver is a bit different since device and driver are created in module_init()
> so will works fine in most cases, because the device/driver removal will happens the
> same time when unloading the module. However if manually unbind the driver and device
> without unloading the module, we'll miss to unregister the notifier in the currently
> implementation.
>
I see, thank you for your patient explanation.
Thank you.
Best Regards,
Shuai
