Re: [PATCH v1 1/2] kernel: time: export sched_clock_register function

From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: Freddy Hsin <freddy.hsin@mediatek.com>,
	linux-mediatek@lists.infradead.or,
	linux-arm-kernel@lists.infradead.org,
	"Ahmed S. Darwish" <a.darwish@linutronix.de>,
	Paul Cercueil <paul@crapouillou.net>,
	"Ben Dooks (Codethink)" <ben.dooks@codethink.co.uk>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>
Cc: Saravana Kannan <saravanak@google.com>,
	linux-kernel@vger.kernel.org, chang-an.chen@mediatek.com,
	Baolin Wang <baolin.wang7@gmail.com>,
	wsd_upstream@mediatek.com, kuohong.wang@mediatek.com,
	stanley.chu@mediatek.com
Subject: Re: [PATCH v1 1/2] kernel: time: export sched_clock_register function
Date: Wed, 29 Jul 2020 16:09:44 +0200	[thread overview]
Message-ID: <6535cd1b-327c-db4b-e6d8-9a29a5847e17@linaro.org> (raw)
In-Reply-To: <1595931377-21627-2-git-send-email-freddy.hsin@mediatek.com>

Hi Freddy,

On 28/07/2020 12:16, Freddy Hsin wrote:
> export sched_clock_register function, because the Mediatek timer
> loadable module depends on this function

That is not a sufficient reason to ask for exporting a symbol from the
core framework.

It is the second patch asking for exporting the symbols because of GKI.

If we want to do this move, it is important to understand what that will
imply by doing the changes as a whole:

 - Identify the routines which will need EXPORT_SYMBOL_GPL

 - Show it is safe to use them with what that implies as module. eg.
What about cpuidle drivers using the broadcast timers?

 - As pointed by Thomas, TIMER_OF_DECLARE must be adapted to handle the
module cases

The conversion to all modules is not as straightforward as just
exporting a symbol and the changes must be provided with the proof the
impact has been studied and anticipated.

> Signed-off-by: Freddy Hsin <freddy.hsin@mediatek.com>
> ---
>  kernel/time/sched_clock.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
> index fa3f800..e1d5afe 100644
> --- a/kernel/time/sched_clock.c
> +++ b/kernel/time/sched_clock.c
> @@ -239,6 +239,7 @@ static enum hrtimer_restart sched_clock_poll(struct hrtimer *hrt)
>  
>  	pr_debug("Registered %pS as sched_clock source\n", read);
>  }
> +EXPORT_SYMBOL(sched_clock_register);

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog