On 20:45 Thu 25 Sep     , Edgar E. Iglesias wrote:
> 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 <[email protected]>

OK, let me reroll with an assert here. I guess we can always come back
to this if needed (I think I had the user supplied DTB use-case in mind
initially when I did this. I then realized later that we unconditionally
build the fdt anyway).

Thanks

-- 
Luc

Reply via email to