[U-Boot] libfdt: make fdt_increase_size() available to everyone

[Wolfgang Denk]

Dear Timur Tabi,

In message <4BFD4E83.2080202 at freescale.com> you wrote:
> 
> > Well, in a way that may be image-type dependent, but that is not
> > board-specific.
> 
> Technically, that's true, but in most cases the function that returns the
> address/size of the firmware would exist in board code.
> 
> For example, the MPC8323 has a bug in its QE UART support, so if you want to
> use the QE as a UART, you need to download a special QE firmware to
> work-around the bug.  So even though all MPC8323 boards have a QE that can
> run in UART mode, only those boards that have a QE wired to an RS232 port
> need to enable the UART work-around, so only those boards would embed the
> firmware in the device tree.

Right. Then these boards can enable the feature in their board config
files, and get what they want. But it's still generic code, and not
spoecific to any special board.

> >> But either way, the firmware needs to be wrapped inside an image object.  I
> >> think Kumar was implying that he didn't want to make *any* image type (FIT
> >> or legacy) mandatory.
> > 
> > And where would you then get type and size information from?
> 
> For example, QE firmware comes with its own header that includes size
> information, as well as a magic number and a CRC.  See struct qe_firmware in

But we want a generic solution, and this includes firmware blobs that
may not contain such information. Ergo we use what we use elsewhere in
U-Boot for the very purpose: U-Boot image files.

> For those firmware types that don't have such a header built-in, they would
> need to be wrapped in a legacy/FIT image.

And for the sake of using common code we will do this always.

> > Maybe we can abstrct off most of this, and/or leave it to image type
> > specific handlers?
> 
> I'm not sure that's a good idea.  We can't predict what kind of firmware
> we'll need to support in the future, and a board-specific fdt_board_size()
> function is, in my opinion, a more elegant way to solve the problem.

You completely fail to understand what "board-specific" means. After
so many rounds of discussion and so many attempts to explain this, you
still don't get it. I'm on the verge of giving up.

This code is in no way board-specific, so board-specific functions
make no sense.

Just swallow and accept this if you cannot understand it.


> >> #include "qe.h"
> > 
> > No. There would be no "qe.h" needed in that generic code.
> > 
> >> if (strcmp(image->ih_name, "QE Firmware") == 0) {
> >> 	/*
> >> 	 * Embed the firmware in the device tree using the binding
> >> 	 * definition in Documentation/powerpc/dts-bindings/fsl/cpm_qe
> >> 	 * /qe.txt
> >>          */
> >> }
> > 
> > No. More like
> > 
> > 	if (strcmp(image->ih_name, "QE Firmware") == 0)
> > 		handle_fdt_fw_qe(image);
> 
> Where is the function prototype for handle_fdt_fw_qe()?  Wouldn't it be in qe.h?

Grrrrrghhhh!!!!!

Please open your eyes and start reading!!!

Just 15 lines above I wrote: "There would be no "qe.h" needed in that
generic code." Then what good couldit do if we add a prototype to that
header file that we need in common code? Nothing!

Obviously we need that prototype in a header file included by that
common code, and this has NOTHING to do with any *qe*.h files.

> Also, I really think the strcmp is a bad idea, because then we have to put a
> specific string in the ih_name field, not something unique to the actual
> firmware in the image.

Well, I said that this might need discussion - I offered image
subtypes as an alternative, maybe there are other/better ideas. Input
welcome.

> > or similar. Probably #ifdef'ed for machines that have enabled QE
> > support in their board config, or with a weak handle_fdt_fw_qe() that
> > gets filled in for QE aware boards only so the compiler optimizes away
> > that call everywhere else.
> 
> So it would look like board_init_r()?

Not really. board_init_r() is still architecture specific - ARM and
PowerPC and MIPS and ... use different implementations.

Here this is feature dependent, not architecture or CPU or board
dependent code.

> >> So you agree with Kumar's idea of having a weak function that embeds the
> >> firmware into the device tree, but the firmware must always be wrapped in an
> >> image format?
> > 
> > Yes. Note that there is NOT any board-specific code.
> 
> Kumar's function would be defined in a board-specific source file.  Using

I don;t think so. It would not be accepted for mainline then.

> the QE firmware example for the 8323 above, I would add this code to
> board/freescale/mpc832xemds/mpc832xemds.c:

Do you see my NAK?  NAK!!!

In which way is this specific to the mpc832xemds board?

Would my custom 8323 board not need the same thing? Do you expect us
to duplicate this code to all boards which need this QE feature?


> > In which way would that be worse compared to
> > 
> > 	if (strcmp(image->ih_name, "QE Firmware") == 0)
> > 		handle_fdt_fw_qe(image);
> > 	else if (strcmp(image->ih_name, "AMD FPGA Firmware") == 0)
> > 		handle_fdt_fw_amd(image);
> > 	...
> > ?
> 
> It wouldn't be.  I don't like either method.

I'll happily accept more elegant / powerful approaches.

> I believe we should have a board-specific function that figures out how much
> extra space is needed, and just returns a single integer that the
> boot_relocate_fdt() uses to pad the FDT when it relocates it.
> 
> That board-specific function can do whatever it needs to do to figure out
> which firmware binaries need to be embedded, where they are, how big they
> are, and more importantly, what format they're in.
> 
> Later on, function ft_board_setup() will be the function that actually
> embeds the firmware into the device tree.

Seems you really cannot look beyond your own nose, it seems. 

Can't you ask somebody else to implement this for you?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The philosophy exam was a piece of cake  -  which  was  a  bit  of  a
surprise, actually, because I was expecting some questions on a sheet
of paper.