Re: [PATCH v2 4/4] watchdog: jz4740: Make probe function __init_or_module

[Paul Cercueil <paul@crapouillou.net>]

Hi Christophe,

Le sam. 8 juin 2019 à 9:51, Christophe Leroy <christophe.leroy@c-s.fr> 
a écrit :
> Hi Paul,
> 
> Le 07/06/2019 à 18:24, Paul Cercueil a écrit :
>> This allows the probe function to be dropped after the kernel 
>> finished
>> its initialization, in the case where the driver was not compiled as 
>> a
>> module.
> 
> I'm not sure that's what  __init_or_module flag does.
> 
> As far as I understand, this flag makes the function being dropped 
> only when the kernel is built without modules support, ie without 
> CONFIG_MODULES. See 
> https://elixir.bootlin.com/linux/latest/source/include/linux/module.h#L145

So it doesn't depend on the driver being built-in or compiled as a 
module?

> In addition, I'm not sure you can simply define a probe function as 
> __init. What if someone tries to unbind and rebind the device through 
> sysfs for instance ?

Ouch. I feel stupid now.

> It seems there is a special function called __platform_driver_probe() 
> for registering devices when the probe function is to be in __init, 
> see 
> https://elixir.bootlin.com/linux/latest/source/drivers/base/platform.c#L684

Yes, but only usable by drivers that won't defer probe, and it removes 
the bind/unbind attributes from sysfs,
so it shouldn't be used for non-critical drivers, I think.

> Christophe
> 
>> 
>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>> ---
>> 
>> Notes:
>>      v2: New patch
>> 
>>   drivers/watchdog/jz4740_wdt.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/watchdog/jz4740_wdt.c 
>> b/drivers/watchdog/jz4740_wdt.c
>> index 7519d80c5d05..2061788c1939 100644
>> --- a/drivers/watchdog/jz4740_wdt.c
>> +++ b/drivers/watchdog/jz4740_wdt.c
>> @@ -157,7 +157,7 @@ static const struct of_device_id 
>> jz4740_wdt_of_matches[] = {
>>   MODULE_DEVICE_TABLE(of, jz4740_wdt_of_matches);
>>   #endif
>>   \x7f-static int jz4740_wdt_probe(struct platform_device *pdev)
>> +static int __init_or_module jz4740_wdt_probe(struct platform_device 
>> *pdev)
>>   {
>>   	struct device *dev = &pdev->dev;
>>   	struct jz4740_wdt_drvdata *drvdata;
>>