linux-kernel - Re: [PATCH RFC] timer: of: Create a platform_device before the framework is initialized

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aGRhIrZq1tMR8yGO@google.com>
Date: Tue, 1 Jul 2025 15:28:50 -0700
From: William McVicker <willmcvicker@...gle.com>
To: Arnd Bergmann <arnd@...db.de>
Cc: Daniel Lezcano <daniel.lezcano@...aro.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org,
	Lorenzo Pieralisi <lorenzo.pieralisi@...aro.org>,
	Hans de Goede <hansg@...nel.org>,
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
	Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
	Rob Herring <robh@...nel.org>, Thomas Gleixner <tglx@...utronix.de>,
	John Stultz <jstultz@...gle.com>, Stephen Boyd <sboyd@...nel.org>,
	Saravana Kannan <saravanak@...gle.com>,
	Linux-Arch <linux-arch@...r.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE" <devicetree@...r.kernel.org>
Subject: Re: [PATCH RFC] timer: of: Create a platform_device before the
 framework is initialized

On 07/01/2025, Arnd Bergmann wrote:
> On Tue, Jul 1, 2025, at 20:21, William McVicker wrote:
> > On 07/01/2025, Arnd Bergmann wrote:
> >> On Tue, Jul 1, 2025, at 01:53, William McVicker wrote:
> >> >> @@ -1550,6 +1553,8 @@ typedef void (*of_init_fn_1)(struct device_node *);
> >> >>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_1_ret)
> >> >>  #define OF_DECLARE_2(table, name, compat, fn) \
> >> >>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_2)
> >> >> +#define OF_DECLARE_PDEV(table, name, compat, fn) \
> >> >> +		_OF_DECLARE(table, name, compat, fn, of_init_fn_pdev)
> >> >
> >> > To support auto-module loading you'll need to also define the
> >> > MODULE_DEVICE_TABLE() as part of TIMER_OF_DECLARE_PDEV().
> >> >
> >> > I haven't tested the patch yet, but aside from my comment above it LGTM.
> >> 
> >> The patch doesn't actually have a module_platform_driver_probe()
> >> yet either, so loading the module wouldn't actually do anything.
> >
> > Probing with TIMER_OF_DECLARE() just consists of running the match table's data
> > function pointer. So that is covered by Daniel's patch AFAICT. However, it's
> > not clear if this implementation allows you to load the kernel module after the
> > device boots? For example, will the Exynos MCT timer probe if I load the
> > exynos_mct driver after the device boots? My guess is you'd need to register
> > the device as a platform device with a dedicated probe function to handle that.
> 
> Yes, that's what I meant: the loadable module needs a module_init()
> function that registers the actual platform driver in order for the
> probe function to be called. module_platform_driver_probe()
> is the way I would suggest to arrange it, though that is just a
> convenience helper around the registration.
> 
> The platform device at that point is created by the normal DT scan,
> so there is no need to create an extra one. On the contrary, in case
> we successfully call the probe function from timer_init(), we end
> up with two separate 'struct platform_device' instances 
> 
>      Arnd

So then does it even make sense to have `timer_pdev_of_probe()` if it's very
unlikely that the module will even be loaded by the time `timer_probe()` runs?
Would it make sense for TIMER_OF_DECLARE_PDEV() to just have a special else case
with the module boiler plate stuff for when the driver is built as a module?
Something like:

--->o---

#if !defined(MODULE)
#define TIMER_OF_DECLARE_PDEV(...) TIMER_OF_DECLARE(...)
#else
static int timer_pdev_probe(struct platform_device *pdev)
{
	struct device *dev = &pdev->dev;
	int (*timer_init)(struct device_node *np);

	timer_init = of_device_get_match_data(dev);
	if (!timer_init)
		return -EINVAL;

	return timer_init(dev->of_node);
}

#define TIMER_OF_DECLARE_PDEV(...) \
	OF_DECLARE_PDEV(timer_pdev, name, compat, fn) \ # make this define MODULE_DEVICE_TABLE() as well
	<create struct platform_driver instance> \
	<call module_platform_driver_probe(..., timer_pdev_probe)
#endif

--->o---

--Will