Daniel Lezcano
2020-03-16 14:57:54 UTC
On Wed, Mar 4, 2020 at 11:56 AM Daniel Lezcano
TBH, I'm missing the rational with the explanation and the code. Can you
elaborate or rephrase it?
Ok, let me start from the top.
When the kernel boots, timer_probe() is called (via time_init()) way
before any of the initcalls are called in do_initcalls().
In systems with CONFIG_OF, of_platform_default_populate_init() gets
called at arch_initcall_sync() level.
of_platform_default_populate_init() is what kicks off creating
platform devices from device nodes in DT. However, if the struct
device_node that corresponds to a device node in DT has OF_POPULATED
flag set, a platform device is NOT created for it (because it's
considered already "populated"/taken care of).
When a timer driver registers using TIMER_OF_DECLARE(), the driver's
init code is called from timer_probe() on the struct device_node that
corresponds to the timer device node. At this point the timer is
already "probed". If you don't mark this device node with
OF_POPULATED, at arch_initcall_sync() it's going to have a pointless
struct platform_device created that's just using up memory and
pointless.
So my patch sets the OF_POPULATED flag for all timer device_node's
that are successfully probed from timer_probe().
If a timer driver doesn't use TIMER_OF_DECLARE() and just registers as
a platform device, the driver init function won't be called from
timer_probe() and it's corresponding devices won't have OF_POPULATED
set in their device_node. So platform_devices will be created for them
and they'll probe as normal platform devices. This is why my change
doesn't break drivers/clocksource/ingenic-timer.c.
Btw, this is no different from what irqchip does with IRQCHIP_DECLARE.
Hope that clears it up.
Yes, thanks for the explanation.On Thu, Feb 27, 2020 at 1:06 AM Daniel Lezcano
Timer initialization is done during early boot way before the driver
core starts processing devices and drivers. Timers initialized during
this early boot period don't really need or use a struct device.
However, for timers represented as device tree nodes, the struct devices
are still created and sit around unused and wasting memory. This change
avoid this by marking the device tree nodes as "populated" if the
corresponding timer is successfully initialized.
core starts processing devices and drivers. Timers initialized during
this early boot period don't really need or use a struct device.
However, for timers represented as device tree nodes, the struct devices
are still created and sit around unused and wasting memory. This change
avoid this by marking the device tree nodes as "populated" if the
corresponding timer is successfully initialized.
elaborate or rephrase it?
When the kernel boots, timer_probe() is called (via time_init()) way
before any of the initcalls are called in do_initcalls().
In systems with CONFIG_OF, of_platform_default_populate_init() gets
called at arch_initcall_sync() level.
of_platform_default_populate_init() is what kicks off creating
platform devices from device nodes in DT. However, if the struct
device_node that corresponds to a device node in DT has OF_POPULATED
flag set, a platform device is NOT created for it (because it's
considered already "populated"/taken care of).
When a timer driver registers using TIMER_OF_DECLARE(), the driver's
init code is called from timer_probe() on the struct device_node that
corresponds to the timer device node. At this point the timer is
already "probed". If you don't mark this device node with
OF_POPULATED, at arch_initcall_sync() it's going to have a pointless
struct platform_device created that's just using up memory and
pointless.
So my patch sets the OF_POPULATED flag for all timer device_node's
that are successfully probed from timer_probe().
If a timer driver doesn't use TIMER_OF_DECLARE() and just registers as
a platform device, the driver init function won't be called from
timer_probe() and it's corresponding devices won't have OF_POPULATED
set in their device_node. So platform_devices will be created for them
and they'll probe as normal platform devices. This is why my change
doesn't break drivers/clocksource/ingenic-timer.c.
Btw, this is no different from what irqchip does with IRQCHIP_DECLARE.
Hope that clears it up.
Why not just set the OF_POPULATED if the probe succeeds?
Like:
diff --git a/drivers/clocksource/timer-probe.c
b/drivers/clocksource/timer-probe.c
index ee9574da53c0..f290639ff824 100644
--- a/drivers/clocksource/timer-probe.c
+++ b/drivers/clocksource/timer-probe.c
@@ -35,6 +35,7 @@ void __init timer_probe(void)
continue;
}
+ of_node_set_flag(np, OF_POPULATED);
timers++;
}
instead of setting the flag and clearing it in case of failure?
--
<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
<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