Re: [PATCH v4 1/2] arch: arm: use dt and UCLASS_IRQ to get gic details
Hi Stefan,
On Thu, Aug 27, 2020 at 5:01 PM Stefan Roese 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
> > Reviewed-by: Simon Glass
>
> 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
> > +#include
> > #include
> > #include
> > #include
> > @@ -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),
> > );
> > + 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())
> > + 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 =
Re: [PATCH v4 1/2] arch: arm: use dt and UCLASS_IRQ to get gic details
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
Reviewed-by: Simon Glass
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?
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
+#include
#include
#include
#include
@@ -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), );
+ 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())
+ 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) &
Re: [PATCH v4 1/2] arch: arm: use dt and UCLASS_IRQ to get gic details
On Sun, Jul 26, 2020 at 10:37:32PM +0530, 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 > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
[PATCH v4 1/2] arch: arm: use dt and UCLASS_IRQ to get gic details
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
Reviewed-by: Simon Glass
---
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
+#include
#include
#include
#include
@@ -15,6 +16,48 @@ static u32 lpi_id_bits;
#define LPI_PROPBASE_SZALIGN(BIT(LPI_NRBITS), SZ_64K)
#define LPI_PENDBASE_SZALIGN(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), );
+ 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())
+ 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
