[PATCH v4 1/2] arch: arm: use dt and UCLASS_IRQ to get gic details

Rayagonda Kokatanur rayagonda.kokatanur at broadcom.com
Mon Sep 7 18:32:52 CEST 2020


Hi Stefan,


On Thu, Aug 27, 2020 at 5:01 PM Stefan Roese <sr at denx.de> wrote:
>
> Hi Rayagonda,
>
> On 26.07.20 19:07, Rayagonda Kokatanur wrote:
> > Use device tree and UCLASS_IRQ driver to get following
> > Generic Interrupt Controller (GIC) details,
> >
> > -GIC Distributor interface (GICD) base address and
> > -GIC Redistributors (GICR) base address.
> >
> > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur at broadcom.com>
> > Reviewed-by: Simon Glass <sjg at chromium.org>
>
> How is this supposed to work on an ARM platform? I'm currently testing
> TOT on an NXP LX2160 platform and it fails with this bootup message:
>
> Error binding driver 'gic-v3': -96
> Some drivers failed to bind
>
> Debugging revealed that UCLASS_IRQ is missing for this platform. The
> driver "irq-uclass.c" is only selectable for X86 & SANDBOX:
>
> config IRQ
>         bool "Intel Interrupt controller"
>         depends on X86 || SANDBOX
>         help
>           This enables support for Intel interrupt controllers, including ITSS.
>           Some devices have extra features, such as Apollo Lake. The
>           device has its own uclass since there are several operations
>           involved.
>
> This Kconfig help is also not correct any more, as the UCLASS_IRQ
> driver is not Intel specific (any more).
>
> FWICT, it would be best to make CONFIG_IRQ selectable for all platforms
> and select it in the "config GIC_V3_ITS" definition, similar to how it
> done with REGMAP & SYSCON. And change the "config IRQ" help text above.
>
> What do you think?

Sorry I missed your mail.

I am okay with your approach.

Also adding the following code at the end of
"arch/arm/lib/gic-v3-its.c" file, will solve the issue.
I found that this piece of code is not required for the latest uboot
hence I didn't add this as part of my patch.

UCLASS_DRIVER(irq) = {
.id = UCLASS_IRQ,
.name = "irq",
};

Best regards,
Rayagonda

>
> Thanks,
> Stefan
>
> > ---
> > Changes from v3:
> >   -Address review comments from Simon,
> >    Correct the data type of variables
> >
> > Changes from v1:
> >   -Address review comments from Tom Rini,
> >    Fix build warning messages.
> >
> >   arch/arm/lib/gic-v3-its.c | 73 +++++++++++++++++++++++++++++++++++----
> >   1 file changed, 66 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/arm/lib/gic-v3-its.c b/arch/arm/lib/gic-v3-its.c
> > index 90f37a123c..5f88643245 100644
> > --- a/arch/arm/lib/gic-v3-its.c
> > +++ b/arch/arm/lib/gic-v3-its.c
> > @@ -3,6 +3,7 @@
> >    * Copyright 2019 Broadcom.
> >    */
> >   #include <common.h>
> > +#include <dm.h>
> >   #include <asm/gic.h>
> >   #include <asm/gic-v3.h>
> >   #include <asm/io.h>
> > @@ -15,6 +16,48 @@ static u32 lpi_id_bits;
> >   #define LPI_PROPBASE_SZ             ALIGN(BIT(LPI_NRBITS), SZ_64K)
> >   #define LPI_PENDBASE_SZ             ALIGN(BIT(LPI_NRBITS) / 8, SZ_64K)
> >
> > +/*
> > + * gic_v3_its_priv - gic details
> > + *
> > + * @gicd_base: gicd base address
> > + * @gicr_base: gicr base address
> > + */
> > +struct gic_v3_its_priv {
> > +     ulong gicd_base;
> > +     ulong gicr_base;
> > +};
> > +
> > +static int gic_v3_its_get_gic_addr(struct gic_v3_its_priv *priv)
> > +{
> > +     struct udevice *dev;
> > +     fdt_addr_t addr;
> > +     int ret;
> > +
> > +     ret = uclass_get_device_by_driver(UCLASS_IRQ,
> > +                                       DM_GET_DRIVER(arm_gic_v3_its), &dev);
> > +     if (ret) {
> > +             pr_err("%s: failed to get %s irq device\n", __func__,
> > +                    DM_GET_DRIVER(arm_gic_v3_its)->name);
> > +             return ret;
> > +     }
> > +
> > +     addr = dev_read_addr_index(dev, 0);
> > +     if (addr == FDT_ADDR_T_NONE) {
> > +             pr_err("%s: failed to get GICD address\n", __func__);
> > +             return -EINVAL;
> > +     }
> > +     priv->gicd_base = addr;
> > +
> > +     addr = dev_read_addr_index(dev, 1);
> > +     if (addr == FDT_ADDR_T_NONE) {
> > +             pr_err("%s: failed to get GICR address\n", __func__);
> > +             return -EINVAL;
> > +     }
> > +     priv->gicr_base = addr;
> > +
> > +     return 0;
> > +}
> > +
> >   /*
> >    * Program the GIC LPI configuration tables for all
> >    * the re-distributors and enable the LPI table
> > @@ -23,15 +66,18 @@ static u32 lpi_id_bits;
> >    */
> >   int gic_lpi_tables_init(u64 base, u32 num_redist)
> >   {
> > +     struct gic_v3_its_priv priv;
> >       u32 gicd_typer;
> >       u64 val;
> >       u64 tmp;
> >       int i;
> >       u64 redist_lpi_base;
> > -     u64 pend_base = GICR_BASE + GICR_PENDBASER;
> > +     u64 pend_base;
> >
> > -     gicd_typer = readl(GICD_BASE + GICD_TYPER);
> > +     if (gic_v3_its_get_gic_addr(&priv))
> > +             return -EINVAL;
> >
> > +     gicd_typer = readl((uintptr_t)(priv.gicd_base + GICD_TYPER));
> >       /* GIC support for Locality specific peripheral interrupts (LPI's) */
> >       if (!(gicd_typer & GICD_TYPER_LPIS)) {
> >               pr_err("GIC implementation does not support LPI's\n");
> > @@ -46,7 +92,7 @@ int gic_lpi_tables_init(u64 base, u32 num_redist)
> >       for (i = 0; i < num_redist; i++) {
> >               u32 offset = i * GIC_REDISTRIBUTOR_OFFSET;
> >
> > -             if ((readl((uintptr_t)(GICR_BASE + offset))) &
> > +             if ((readl((uintptr_t)(priv.gicr_base + offset))) &
> >                   GICR_CTLR_ENABLE_LPIS) {
> >                       pr_err("Re-Distributor %d LPI is already enabled\n",
> >                              i);
> > @@ -64,19 +110,21 @@ int gic_lpi_tables_init(u64 base, u32 num_redist)
> >              GICR_PROPBASER_RAWAWB |
> >              ((LPI_NRBITS - 1) & GICR_PROPBASER_IDBITS_MASK));
> >
> > -     writeq(val, (GICR_BASE + GICR_PROPBASER));
> > -     tmp = readl(GICR_BASE + GICR_PROPBASER);
> > +     writeq(val, (uintptr_t)(priv.gicr_base + GICR_PROPBASER));
> > +     tmp = readl((uintptr_t)(priv.gicr_base + GICR_PROPBASER));
> >       if ((tmp ^ val) & GICR_PROPBASER_SHAREABILITY_MASK) {
> >               if (!(tmp & GICR_PROPBASER_SHAREABILITY_MASK)) {
> >                       val &= ~(GICR_PROPBASER_SHAREABILITY_MASK |
> >                               GICR_PROPBASER_CACHEABILITY_MASK);
> >                       val |= GICR_PROPBASER_NC;
> > -                     writeq(val, (GICR_BASE + GICR_PROPBASER));
> > +                     writeq(val,
> > +                            (uintptr_t)(priv.gicr_base + GICR_PROPBASER));
> >               }
> >       }
> >
> >       redist_lpi_base = base + LPI_PROPBASE_SZ;
> >
> > +     pend_base = priv.gicr_base + GICR_PENDBASER;
> >       for (i = 0; i < num_redist; i++) {
> >               u32 offset = i * GIC_REDISTRIBUTOR_OFFSET;
> >
> > @@ -94,9 +142,20 @@ int gic_lpi_tables_init(u64 base, u32 num_redist)
> >               }
> >
> >               /* Enable LPI for the redistributor */
> > -             writel(GICR_CTLR_ENABLE_LPIS, (uintptr_t)(GICR_BASE + offset));
> > +             writel(GICR_CTLR_ENABLE_LPIS,
> > +                    (uintptr_t)(priv.gicr_base + offset));
> >       }
> >
> >       return 0;
> >   }
> >
> > +static const struct udevice_id gic_v3_its_ids[] = {
> > +     { .compatible = "arm,gic-v3" },
> > +     {}
> > +};
> > +
> > +U_BOOT_DRIVER(arm_gic_v3_its) = {
> > +     .name           = "gic-v3",
> > +     .id             = UCLASS_IRQ,
> > +     .of_match       = gic_v3_its_ids,
> > +};
> >
>
>
> Viele Grüße,
> Stefan
>
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de


More information about the U-Boot mailing list