difference between fdtdec and fdt_support ?

[Simon Glass]

Hi Marek,

On Thu, 6 Jan 2022 at 09:55, Marek Behún <marek.behun at nic.cz> wrote:
>
> 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.

Yes, indeed, but really these uses should be dropped and converted to
driver model.

>
> > >
> > > > >
> > > > > 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?

It isn't implemented, but we should add it. It is a simple piece of
code to flatten the tree.

Regards,
Simon