difference between fdtdec and fdt_support ?

[Marek Behún]

On Thu, 6 Jan 2022 09:15:17 -0700
Simon Glass <sjg at chromium.org> wrote:

> Hi Marek,
> 
> On Thu, 6 Jan 2022 at 09:10, Marek Behún <marek.behun at nic.cz> wrote:
> >
> > On Thu, 6 Jan 2022 08:48:48 -0700
> > Simon Glass <sjg at chromium.org> wrote:
> >  
> > > Hi Marek,
> > >
> > > On Thu, 6 Jan 2022 at 05:21, Marek Behún <marek.behun at nic.cz> wrote:  
> > > >
> > > > Hi Simon,
> > > >
> > > > I am a little confused.
> > > >
> > > > We have
> > > >   common/fdt_support.c
> > > > and
> > > >   lib/fdtdec.c
> > > >
> > > > The second one implements for example fdtdec_get_is_enabled(), which I
> > > > would rather expect in fdt_support by name fdt_node_is_available(), or
> > > > something like that.  
> > >
> > > Should be moved to ofnode  
> >
> > ?? But this function is needed for example when fixing device tree for
> > Linux.
> >  
> > > >
> > > > Also fdtdec does a strange thing with compatible strings: it declares
> > > > an enum for compatible strings and then a map which maps this enum
> > > > values to compatible strings... Why not just use the compatible strings?  
> > >
> > > Did you see the comment?
> > >
> > >  * NOTE: This list is basically a TODO list for things that need to be
> > >  * converted to driver model. So don't add new things here unless there is a
> > >  * good reason why driver-model conversion is infeasible. Examples include
> > >  * things which are used before driver model is available.
> > >
> > > This is effectively a list of things that should be converted to
> > > driver model. The list should then go away.  
> >
> > Hmm. But can't that be simply made into a list in a comment? Because
> > currently this is compiled in for every board that uses any such
> > function from fdtdec, even if they don't use any string from those
> > compatible strings.  
> 
> Well another option would be to delete the boards that need it, since
> no one has seen fit to resolve this all these years. Or create some
> drivers for them.

I still don't quite understand why the compatible strings have to be in
this fdtdec.c file.

For example in
  arch/arm/mach-socfpga/clock_manager_arria10.c
we call
  node = fdtdec_next_compatible(blob, 0,
                       COMPAT_ALTERA_SOCFPGA_CLK_INIT);
This constant, COMPAT_ALTERA_SOCFPGA_CLK_INIT, is an enum constant,
which is then in fdtdec.c translated to string compatible with
  compat_names[id]
for which fdt_node_offset_by_compatible() is used.

So why not simply put this string constant into
  arch/arm/mach-socfpga/clock_manager_arria10.c
by calling
  node = fdt_node_offset_by_compatible(blob, node,
                        "altr,socfpga-a10-clk-init");
??

That way at least the string literals won't be compiled in into every
u-boot binary, even those that don't need those literals at all.

> >  
> > > >
> > > > What is the purpose of having two files implementing fdt stuff?  
> > >
> > > fdtdec - for reading from the DT. Should go away and be replaced with
> > > the ofnode API, and fdtaddr.c
> > > fdt_support - for updating the DT, e.g. for fixups before booting an OS  
> >
> > Thanks! Okay that makes sense. This means that we should also have
> > fdt_node_is_available() in fdt_support.c which does the same thing.
> > (For now this can be made a static inline function that calls
> >  fdtdec_get_is_enabled().)  
> 
> OK.
> 
> BTW fdt_support.c should really move to use ofnode. There are few
> ofnode 'write' functions at present, but we should fill those out so
> we don't need to use flattree for anything, with OF_LIVE is enabled.

Can OF_LIVE then generate dtb for kernel?

Marek