Messages in this thread Patch in this message | | | Subject | Re: [PATCH v1] clocksource: Avoid creating dead devices | From | Daniel Lezcano <> | Date | Mon, 16 Mar 2020 15:57:54 +0100 |
| |
On 08/03/2020 06:53, Saravana Kannan wrote: > On Wed, Mar 4, 2020 at 11:56 AM Daniel Lezcano > <daniel.lezcano@linaro.org> wrote: >> >> On 04/03/2020 20:30, Saravana Kannan wrote: >>> On Thu, Feb 27, 2020 at 1:22 PM Saravana Kannan <saravanak@google.com> wrote: >>>> >>>> On Thu, Feb 27, 2020 at 1:06 AM Daniel Lezcano >>>> <daniel.lezcano@linaro.org> wrote: >>>>> >>>>> On 11/01/2020 06:21, Saravana Kannan wrote: >>>>>> 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. >> >> 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.
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
| |