Re: [PATCH v5 02/47] hw/arm/xlnx-versal: prepare for FDT creation

["Edgar E. Iglesias" <edgar.iglesias@amd.com>]

On Thu, Sep 18, 2025 at 08:10:18AM +0200, Edgar E. Iglesias wrote:
> On Tue, Sep 16, 2025 at 09:30:46AM +0200, Luc Michel wrote:
> > Hi Edgar,
> >

[snip]

> > [snip]
> > 
> > > > +
> > > >  static void versal_realize(DeviceState *dev, Error **errp)
> > > >  {
> > > >      Versal *s = XLNX_VERSAL_BASE(dev);
> > > >      qemu_irq pic[XLNX_VERSAL_NR_IRQS];
> > > >  
> > > > +    if (s->cfg.fdt == NULL) {
> > > > +        int fdt_size;
> > > > +
> > > > +        s->cfg.fdt = create_device_tree(&fdt_size);
> > > > +    }
> > > > +
> > > > +    s->phandle.clk_25mhz = fdt_add_clk_node(s, "/clk25", 25 * 1000 * 1000);
> > > > +    s->phandle.clk_125mhz = fdt_add_clk_node(s, "/clk125", 125 * 1000 * 1000);
> > > > +
> > > 
> > > Should we be adding nodes if s->cfg.fdt wasn't created by us?
> > > If the user passes a dtb, I wonder if we should just assume the user
> > > knows what they are doing and use it as is...
> > > 
> > > Or do you have use-cases where it makes sense?
> > 
> > Note that the device tree created in the SoC code is just a dummy one to
> > avoid crashing when the SoC user does not provide one, as stated in the
> > commit message:
> > 
> > "If no FDT is passed, a dummy one is created internally as a stub to the
> > fdt function calls."
> 
> Aha, thanks!
> 
> But then is there really a case when the dummy one is needed? won't
> versal-virt always pass an fdt?
> 
> If that is the case then maybe we could just assert(s->cfg.fdt);


Luc, up to you if you want to add an assert rather than creating the
dummy stub. My preference would be to assert.

In any case, feel free to add my RB tag on the whole series:
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>