Hi,

Am 2021-10-31 17:45, schrieb Z.Q. Hou:
-----Original Message-----
From: Marc Zyngier [mailto:m...@kernel.org]
Sent: 2021年10月29日 5:09
To: Michael Walle <mich...@walle.cc>
Cc: u-boot@lists.denx.de; Vladimir Oltean <vladimir.olt...@nxp.com>; Z.Q. Hou
<zhiqiang....@nxp.com>; Bharat Gooty <bharat.go...@broadcom.com>;
Rayagonda Kokatanur <rayagonda.kokata...@broadcom.com>; Simon Glass
<s...@chromium.org>; Priyanka Jain <priyanka.j...@nxp.com>; Tom Rini
<tr...@konsulko.com>
Subject: Re: [PATCH 2/2] Revert "arch: arm: use dt and UCLASS_SYSCON to get gic
lpi details"

On Wed, 27 Oct 2021 17:54:54 +0100,
Michael Walle <mich...@walle.cc> wrote:
>
> Stop using the device tree as a source for ad-hoc information.
>
> This reverts commit 2ae7adc659f7fca9ea65df4318e5bca2b8274310.
>
> Signed-off-by: Michael Walle <mich...@walle.cc>
> ---
>  arch/arm/Kconfig                        |  2 -
>  arch/arm/cpu/armv8/fsl-layerscape/soc.c | 27 +++++++++-
>  arch/arm/include/asm/gic-v3.h           |  4 +-
>  arch/arm/lib/gic-v3-its.c               | 66 +++----------------------
>  4 files changed, 36 insertions(+), 63 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index
> 02f8306f15..86c1ebde05 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -82,8 +82,6 @@ config GICV3
>
>  config GIC_V3_ITS
>    bool "ARM GICV3 ITS"
> -  select REGMAP
> -  select SYSCON
>    select IRQ
>    help
>      ARM GICV3 Interrupt translation service (ITS).
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> index c0e100d21c..a08ed3f544 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c

Why is this FSL specific?

> @@ -41,11 +41,36 @@ DECLARE_GLOBAL_DATA_PTR;  #endif
>
>  #ifdef CONFIG_GIC_V3_ITS
> +#define PENDTABLE_MAX_SZ  ALIGN(BIT(ITS_MAX_LPI_NRBITS), SZ_64K)
> +#define PROPTABLE_MAX_SZ  ALIGN(BIT(ITS_MAX_LPI_NRBITS) / 8,
SZ_64K)

This looks completely wrong.

The pending table needs one bit per LPI, and the property table one byte per LPI.
Here, you have it the other way around.

It's a typo, will fix after the revert patch applied.

Please keep me on CC.

..
Finally, ITS_MAX_LPI_NRBITS is hardcoded to 16, while it can actually vary from 14
to 32 (and even further limited by some hypervisors), depending on the
implementation. Granted, this was broken before this patch, and in most cases,
64k is more than enough.


This is only for Layerscape platforms, so hardcoded to 16 bit works.

But why is this layerscape only? Can't we make it so it works on any
platform. If I understand Marc correctly, it should be possible.

..
> +  ret = gic_lpi_tables_init(gic_lpi_base, cpu_numcores());

This really should fetch the number of CPUs from the DT rather then some SoC
specific black magic...

Currently in most Layerscape platforms' DTS file there isn't cpu
nodes. On Layerscape platforms the implemented core number can be get
from GUT register.

So this would be the perfect time to actually sync the device
trees with linux.

-michael

Reply via email to