Hi Matthias

On 04/04/24 13:34, Matthias Schiffer wrote:
On Wed, 2024-04-03 at 17:51 +0200, Michael Walle wrote:
Hi,

And on top of that, it will just be a base board and there will
likely be some carrier device trees (overlay? I'm not sure yet).

As far as I can tell, you've put the memory configuration into the
device tree, so I'll probably need to switch between them somehow.

The "k3-<soc>-ddr.dtsi" file will be present in your k3-<board>r5.dts
which makes sense, the memory configuration depends on the board.


k3-<board>-ddr.dtsi* (e.g J721E EVM vs. SK boards consume different memory
configurations.

Right.

And one board might have multiple configuration depending on the
variant of the board. Typically, one board is available with
different memory options. i.e. 1GiB, 4GiB and so on. The actual RAM
chips can come from different manufacturers. So all all, I presume
there will be different RAM settings, i.e. different
k3-<soc>-ddr.dtsi. But I have to switch between the setting during
runtime because there will be only one boot image for that board.

This is a runtime dynamic DDR configuration support you are describing
correct? This means you would be including all the supported memory option
DTSIs in your k3-<board>-r5.dts correct and probably do some board magic
code in the SPL DDR driver to choose the DTB. How is this affecting the
packing of the final bootloader which will anyways pack the whole R5 DTB?

Correct, the DDR configuration should be chosen at runtime after
reading some board strappings. Unless, it will work with with the
same configuration which seems unlikely to me. But it is not an
unusual configuration I'd say.

I haven't looked into this in detail, but to me it seems not that
obvious how to do that in a generic/upstreamable way. Multiple
device nodes sounds wrong. Thus, I'd likely need different device
trees for the different memory configurations for the R5 SPL. Not
sure that is yet possible with u-boot, though. If you have any
better idea, I'm all ears.

Also, regarding the board variants, I'll probably need to choose
between multiple device trees. That is invisible to the user,
because u-boot will choose the correct DTB according a board
strapping, which btw. works really fine, see for example
(boards/kontron/sl28/spl.c:board_fit_config_name_match).

Again, this is assuming that there is some HW blown register available
for the board to use (or in our earlier K3 case, the EEPROM) but that is
not necessarily true every time.

No, that is of course board dependent. It is just an example that
there are boards with more than one DTB.

Let's step back a bit. Right now, there is
    k3-<soc>-<board>-binman.dtsi
which is fine. But it seems, that TI is heading towards a common
    k3-<soc>-binman.dtsi
which is intended to be used by all the boards that are using that
particular SoC, correct me if I'm wrong here. Now the problem with
that is that you hardcode the FIT configuations which are really
board dependent and assume that there will be exactly one DTB per
board, i.e. your "#define SPL_BOARD_DTB" etc.


Correct, but as I mentioned in the earlier message, if your board supports
more than 1 FIT configuration, you can easily extend the image and add more
configurations.

Thus, what I was trying to say is that you should split all the
board independent configuration (dt fragments) from the board
specific configuration.

And again, of course I could just ignore the k3-<soc>-binman.dtsi
and just use a suitable copy "k3-<soc>-<myboard>-binman.dtsi" for my
board. But as I said, I'm not sure, this is the way to go and I have
a slight feeling I will be asked to reuse the "k3-<soc>-binman.dtsi"
when I submit my board support.


I don't think it makes much sense to hardcode your generic
*-binman.dtsi to just one FIT configuration. I'd rather see a split
between generic things which are shared across all boards and board
specifics, like the FIT configuration. I mean I could just copy all

Correct me if I'm wrong, but my understanding is that you would want to
add more FDT blobs in the *-binman.dtsi correct? That is still possible,
adding another "fdt-1" and "conf-1" in the

Something like this in your <board>-u-boot.dtsi,

tispl {
        insert-template = <&ti_spl>;
        fit {
                images {
                        fdt-1 {
                                ...
                        };
                };
                configurations {
                        conf-1 {
                                ...
                        };
                };
        };
};

Then you have the information at two places. One being the "#define
SPL_BOARD_DTB" stuff and the other one being in this additional DT
fragment. That is really confusing.


Hm... maybe. I personally don't see it as confusing. Even when picking
between multiple DTBs, you will have a default DTB in any case, marking that
as a macro wouldn't be confusing right? We'll need to get a third opinion on
here then, I had seen your ping on IRC [1], putting it here for the others
as well.


As I see it, it's not like we are making the fdt-0 non overridable, you
can still override it in your configs to make it cleaner if you want for
your board template, I don't think that -

Though it is not overriding but rather merging, correct? So one
would need to first erase the node just to create it again. Which
looks more like a workaround.

tispl {
        insert-template = <&ti_spl>;
        fit {
                images {
                        fdt-0 {
                                ...
                        };
                        fdt-1 {
                                ...
                        };
                };
                configurations {
                        conf-0 {
                                ...
                        };
                        conf-1 {
                                ...
                        };
                };
        };
};


is not doable. It might be a bit duplicate but if I think about it but
we are not losing out on extending the templates for multiple DTBs even
with this design. I know it might not be what you want but I feel that
for single DTB it's really convenient with the macro stuff and we don't
have to override any of the other binman nodes.

I've raised my concern about stuffing board dependent stuff into the
now generic "k3-<soc>-binman.dtsi". I get it that it will work for
90% of the boards and that it is very convenient. I'd have rather
seen a split of lets say
   k3-<soc>-binman.dtsi
and
   k3-one-dtb-template-binman.dtsi

All the generic stuff goes into k3-soc-binman.dtsi whereas 90% of
the boards might still use the second dtsi with some define magic.
But it seems you've already made your mind up on that :)

-michael


My hope would be that we can get rid of board-specific binman configuration in 
the common case
altogether for the K3 SoC families, using @fdt-SEQ etc. generator sections that 
will just include
all FDTs configured in Kconfig.

A while ago [1], this was blocked on ti-secure signing not working with 
generator sections, and
according to my experiments, this is still the case in U-Boot 2024.01 (and 
looking at the Git log,
also 2024.04/master/next). Are there still plans to make this work? Are there 
any other blockers?

Best regards,
Matthias


[1] https://lists.denx.de/pipermail/u-boot/2023-July/525095.html



Yeah! This was a series that I pushed away to the side when runtime multi-DTB selection was removed. You're right, this seems like the only way we can come to a common ground.

Let me see if I can bring it back up again, the last blocker was signing not working with the generator as you mentioned.

But again in the interest of time... this would mean this cleaning up effort be kept on hold. If we can agree to move to using the generator later as the final solution, can we pick up this series for now?




--
Thanking You
Neha Malcom Francis

Reply via email to