Re: [PATCH v2 05/20] drm/tilcdc: Convert legacy panel binding via DT overlay at boot time

Kory Maincent
2026-01-05T17:18:50Z

On Mon, 5 Jan 2026 17:22:20 +0100
Herve Codina <herve.codina@bootlin.com> wrote:

> Hi Luca, Kory,
>
> On Wed, 17 Dec 2025 15:23:26 +0100
> "Luca Ceresoli" <luca.ceresoli@bootlin.com> (by way of Kory Maincent
> <kory.maincent@bootlin.com>) wrote:
>
> > Hi,
> >
> > Cc: Hervé, can you review the DT overlay aspects?
>
> Yes sure.
>
> Here is my global review.
>
> Depending on the discussion on things I have spotted, I will go deeper in
> patch details.
>
> ...
>
> > > +
> > > +static void __init
> > > +tilcdc_panel_update_prop(struct device_node *node, char *name,
> > > + void *val, int length)
> > > +{
> > > + struct property *prop;
> > > +
> > > + prop = kzalloc(sizeof(*prop), GFP_KERNEL);
> > > + if (!prop)
> > > + return;
> > > +
> > > + prop->name = kstrdup(name, GFP_KERNEL);
> > > + prop->length = length;
> > > + prop->value = kmemdup(val, length, GFP_KERNEL);
> > > + of_update_property(node, prop);
>
> I would use OF changesets to perform the modification.
>
> OF changesets are kind of atomic. You first prepare all modifications in a
> changeset and then you apply the changeset.
> If something goes wrong, the changeset is removed.
>
> Also, if something goes wrong during the changeset preparation, you can abort
> without any modification on the live device-tree.

Ok, I will take a look at changeset to use it to copy all the properties to new
the panel-dpi node.

...

> > > +
> > > + ret = of_overlay_fdt_apply(dtbo_start, dtbo_size, &ovcs_id,
> > > NULL);
> > > + if (ret)
> > > + goto out;
>
> As soon as the overlay is applied, the driver handling the panel-dti node
> can be probed.
>
> Modifying some properties after applying the overlay could be not seen by the
> driver.

I disagree here. This driver is loaded at subsys_initcall (initcall 4) while
the panel is loaded at device_initcall (initcall 6) so the panel driver won't
probe in-between.

...

> > > +subsys_initcall(tilcdc_panel_legacy_init);
>
> IMHO, the call to tilcdc_panel_legacy_init() will be too late.
>
> subsys initcalls are called after arch initcalls.
>
> During arch initcalls, of_platform_populate_init() is called
> https://elixir.bootlin.com/linux/v6.19-rc3/source/drivers/of/platform.c#L599
>
> The root node is populated and handled by the platform bus.
>
> Later at subsys initcall, the tilcdc_panel_legacy_init() function is called.
> This function starts by applying the overlay and so a new node (panel-dpi)
> is added at the root node.
>
> This trigger an OF_RECONFIG_CHANGE_ADD event handled by the platform bus.
> https://elixir.bootlin.com/linux/v6.19-rc3/source/drivers/of/platform.c#L731
>
> If the "panel-dpi" compatible driver is available, its probe() is called but
> the panel-dpi DT node is not fully correct. Indeed, tilcdc_panel_copy_props()
> has not be called yet.

As said before, I don't think this will happen due to the initcall difference
value.

> Also, the legacy compatible string is removed after the
> of_platform_populate_init() call. The legacy driver could have been already
> probed.

Same here.

Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering