Re: [PATCH v1] clocksource: Avoid creating dead devices

[Saravana Kannan]

On Wed, Mar 4, 2020 at 11:56 AM Daniel Lezcano
<daniel.lezcano@xxxxxxxxxx> wrote:
>
> On 04/03/2020 20:30, Saravana Kannan wrote:
> > On Thu, Feb 27, 2020 at 1:22 PM Saravana Kannan <saravanak@xxxxxxxxxx> wrote:
> >>
> >> On Thu, Feb 27, 2020 at 1:06 AM Daniel Lezcano
> >> <daniel.lezcano@xxxxxxxxxx> 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.

Thanks,
Saravana