[U-Boot] [PATCH 2/2] ARM: qemu-arm: define fdt_addr_r

[AKASHI Takahiro]

On Fri, Oct 19, 2018 at 09:46:51AM +0200, Alexander Graf wrote:
> 
> 
> On 19.10.18 08:33, AKASHI Takahiro wrote:
> > On Thu, Oct 18, 2018 at 09:25:04AM +0200, Alexander Graf wrote:
> >>
> >>
> >> On 18.10.18 00:25, Tuomas Tynkkynen wrote:
> >>> Hi Alexander,
> >>>
> >>> On Tue, 16 Oct 2018 15:04:26 +0200
> >>> Alexander Graf <agraf at suse.de> wrote:
> >>>
> >>> ...
> >>>>>   
> >>>>>> Glancing at cmd/pxe.c,
> >>>>>> there is a problem though, in that if ${fdt_addr_r} were defined,
> >>>>>> a PXE file using the FDTDIR directive would attempt loading a dtb
> >>>>>> file named "<NULL>-qemu-arm.dtb" instead of falling back to using
> >>>>>> ${fdt_addr}. That bug would need to be fixed first before applying
> >>>>>> this patch.  
> >>>>
> >>>> Well, and that load will fail and everyone's happy, no? 
> >>>
> >>> No, because currently if DTB loading from the filesystem/tftp is
> >>> attempted and it fails, it aborts the boot. It doesn't matter if it's
> >>> via a 'FDT' or 'FDTDIR' directive. In the case of typical hardware
> >>> that's probably the desired behaviour.
> >>>
> >>> I guess the qemu_arm + FDTDIR case can be fixed by not attempting
> >>> a .dtb load if the default fdtfile is not known due to $soc or $board
> >>> being unset. At least I doubt that some other board could be relying
> >>> on a filename containing literally "<NULL>" :)
> >>
> >> Well - IIRC $soc and $board should also always be defined ;). So yet
> >> another thing to fix in the QEMU port.
> > 
> > See my patch below. Are you happy with it?
> > (qemu-qemu-arm.dtb doesn't make sense to me, though :)
> > 
> >>>
> >>>> IMHO we should
> >>>> fall back to $fdtcontroladdr always
> >>>
> >>> FWIW, to me the idea of passing $fdtcontroladdr to the OS has always
> >>> filled me with dread. On top of the usual backwards- and
> >>> forwards-compatibility problems that happen when mixing and matching
> >>> kernels and DTBs from different releases, you now have to deal with
> >>
> >> That's something that we seriously as a community need to get sorted
> >> out. We're pushing hard for it in the EBBR context. The fact that people
> >> are afraid of putting *hardware desciption* into their firmware is just
> >> mind boggling.
> > 
> > I don't have a strong opinion, but it seems to me that 'fall-back' approach
> > is quite normal. If it doesn't work on a specific board, 'fdt_addr'
> > should be defined.
> > 
> > Thanks,
> > -Takahiro Akashi
> > 
> > 
> >>> issues like U-Boot specific .dts that are majorly diverged from Linux
> >>> ones, or where the .dts is otherwise from Linux but the U-Boot specific
> >>
> >> These case should really be the minority. And if you see those, please
> >> fix them.
> >>
> >>> additions break it for Linux, or where the .dts used is wrong for the
> >>
> >> I've never seen a case where a U-Boot addition broke the DT for Linux.
> >>
> >>> specific hardware revision but close enough for U-Boot's purposes,
> >>> and so on...
> >>
> >> Again, something that just needs fixing. Device trees belong to the
> >> entity that knows hardware, not to the OS. Otherwise you lose the
> >> abstraction layer that DT gives you and you lose the ability to run
> >> "generic" kernels. And of course you break the ecosystem, because now
> >> good luck running BSD, your own little toy OS, etc ;)
> >>
> >>
> >> Alex
> > 
> > ===8<===
> > From 47b26a86359a3b612e890f2ca2d5d20092f9f4e1 Mon Sep 17 00:00:00 2001
> > From: AKASHI Takahiro <takahiro.akashi at linaro.org>
> > Date: Fri, 19 Oct 2018 15:22:02 +0900
> > Subject: [PATCH] arm: qemu: rework Kconfig
> > 
> > Define SYS_SOC and move SYS_* to a canonical place (under board).
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> 
> Looks good enough to me, but it's up to Tuomas to double check and take
> as he's the qemu-arm maintainer :).
> 
> It also usually helps to post the patch as a new message, not buried
> inside a thread ;).

OK. Since I found a small bug, I will send out a new patch separately.

-Takahiro Akashi

> 
> Alex