Re: [PATCH 0/2] arch: arm: gic-v3-its: stop abusing the device tree

Marc Zyngier
2021-10-28T09:03:48Z

On Wed, 27 Oct 2021 17:54:52 +0100,
Michael Walle <michael@walle.cc> wrote:
>
> Please stop throwing every ad-hoc information in the device tree. Use the
> official bindings (or maybe some bindings which will get approved soon).
>
> On the quest of syncing the device tree used in u-boot with the one used in
> linux, there is this nice piece:
>
> gic_lpi_base: syscon@0x80000000 {
> compatible = "gic-lpi-base";
> reg = <0x0 0x80000000 0x0 0x100000>;
> max-gic-redistributors = <2>;
> };
>
> There is no offical binding for it. Also, the chances that there will be
> one are virtually zero. We need to get rid of it. In fact, most information
> there are already known or can be deduced via the offical binding.

It is not "virtually zero". It is *exactly* zero. This node only shows
that the author didn't understand the nature of the problem, nor were
they aware of the existing solution which has been around since July
2018. This solution doesn't require any update to the binding, only to
reserve the memory.

I really wish people would stop piling crap in u-boot, and that the
u-boot maintainers would reach out to people familiar with the
architecture before merging this sort of changes.

>
> More than a month ago NXP [1] said they will look into it and try to get
> the bindings together. I don't think this will happen. Actually, I don't
> think the culprit is that commit, but rather the one which introduced that
> broken binding in the first place [2]. Therefore, revert it, too.
>
> The funny thing is, I don't even know why this is needed at all. Linux will
> happily setup the LPI for us. At least for the LS1028A (but I guess also
> for all other layerscape SoC) the u-boot LPI setup code is only called
> right before we jump to linux. So u-boot doesn't even seem to use the
> interrupts. Now I can't speak of the Broadcom NS3 SoC where this 'feature'
> was introduced in the first place [3]. Unfortunately, it's not mentioned in
> the commit log *why* it was introduced. But this also seem to have crept
> into the layerscape SoCs [4]. All layerscape boards have CONFIG_GIC_V3_ITS
> enabled, well except for one; mine, the kontron_sl28 (which has a LS1028A).
> I haven't noticed anything out of the ordinary, though. Why is
> CONFIG_GIC_V3_ITS needed?

Now, that's a very interesting question: u-boot doesn't know how to
drive an ITS (there is no support code for it, despite what
arch/arm/lib/gic-v3-its.c suggest). Only the LPI allocation code is
there. For what purpose? This is a pretty useless piece of code as far
as I can see, unless the author had some unspecified ulterior motives
(in which case a bit of documentation and a renaming of this file
wouldn't go amiss).

Thanks,

M.

--
Without deviation from the norm, progress is not possible.