Linux-Watchdog Archive on lore.kernel.org
 help / color / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Christophe Leroy <christophe.leroy@c-s.fr>,
	Paul Cercueil <paul@crapouillou.net>
Cc: Wim Van Sebroeck <wim@linux-watchdog.org>,
	od@zcrc.me, linux-watchdog@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 4/4] watchdog: jz4740: Make probe function __init_or_module
Date: Tue, 11 Jun 2019 04:02:02 -0700
Message-ID: <f64db403-f8be-4e39-81e7-515b1c1c0c17@roeck-us.net> (raw)
In-Reply-To: <78ef0273-5ec6-f522-428a-8429f40684ca@c-s.fr>

On 6/10/19 10:16 PM, Christophe Leroy wrote:
> Hi Paul,
> 
> Le 08/06/2019 à 11:57, Paul Cercueil a écrit :
>> 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?
> 
> No it doesn't. This flag is for built-in functions that are needed by init and modules init. If the kernel doesn't support modules, it can drop the function after init. If the kernel supports modules, it has to keep the function. That's what this flag is made for.
> 
> If your need is to mark a function so that it gets discarded after init or module init, just mark it __init. If it is built in, it will be dropped after init. If it is in a module, it will be dropped after the module is initialised.
> 
>>
>>> 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.
> 
> I guess it would make sense for watchdog drivers, we don't expect this kind of driver to be unbinded, do we ?
> 

Except for debugging, where it can be extremely useful to be able to do it.

Guenter

> Christophe
> 
>>
>>> 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;
>>>>
>>
> 


      reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-07 16:24 [PATCH v2 0/4] JZ4740 watchdog cleanups Paul Cercueil
2019-06-07 16:24 ` [PATCH v2 1/4] watchdog: jz4740: Use register names from <linux/mfd/ingenic-tcu.h> Paul Cercueil
2019-06-07 16:24 ` [PATCH v2 2/4] watchdog: jz4740: Avoid starting watchdog in set_timeout Paul Cercueil
2019-06-07 16:24 ` [PATCH v2 3/4] watchdog: jz4740: Drop dependency on MACH_JZ47xx Paul Cercueil
2019-06-07 16:24 ` [PATCH v2 4/4] watchdog: jz4740: Make probe function __init_or_module Paul Cercueil
2019-06-07 17:29   ` Guenter Roeck
2019-06-08  7:51   ` Christophe Leroy
2019-06-08  9:57     ` Paul Cercueil
2019-06-11  5:16       ` Christophe Leroy
2019-06-11 11:02         ` Guenter Roeck [this message]

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f64db403-f8be-4e39-81e7-515b1c1c0c17@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=christophe.leroy@c-s.fr \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=od@zcrc.me \
    --cc=paul@crapouillou.net \
    --cc=wim@linux-watchdog.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Watchdog Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-watchdog/0 linux-watchdog/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-watchdog linux-watchdog/ https://lore.kernel.org/linux-watchdog \
		linux-watchdog@vger.kernel.org linux-watchdog@archiver.kernel.org
	public-inbox-index linux-watchdog


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-watchdog


AGPL code for this site: git clone https://public-inbox.org/ public-inbox