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

[Kory Maincent <kory.maincent@bootlin.com>]

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
https://bootlin.com