On Fri, Jun 19, 2015 at 09:46:06AM +0100, Hanjun Guo wrote:

[...]

> +
> +static int __init
> +match_gic_redist(struct acpi_subtable_header *header, const unsigned long 
> end)
> +{
> +     return 0;
> +}
> +static bool __init acpi_gic_redist_is_present(void)
> +{
> +     int count;
> +
> +     /* scan MADT table to find if we have redistributor entries */
> +     count  =  acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
> +                                     match_gic_redist, 0);
> +
> +     /* has at least one GIC redistributor entry */
> +     if (count > 0)
> +             return true;
> +     else
> +             return false;
> +}

return count > 0;

What about systems where the redistributor data is in the GICC subtable ? Do
you treat them as GIC V2 :) ?

On a side note, having to define an empty function like match_gic_redist is
horrible.

I wonder whether it is not better to refactor map_madt_entry(), create
a MADT subtable iterator out of it and make that code generic, instead
of being forced to add these useless MADT handlers just to count
entries, it is not the first I noticed.

> +static int __init acpi_gic_version_init(void)
> +{
> +     int count;
> +     void __iomem *dist_base;
> +     u32 reg;
> +
> +     count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
> +                                     acpi_gic_parse_distributor, 0);
> +
> +     if (count <= 0) {
> +             pr_err("No valid GIC distributor entry exists\n");
> +             return -ENODEV;
> +     }
> +
> +     if (gic_version >= ACPI_MADT_GIC_VERSION_RESERVED) {
> +             pr_err("Invalid GIC version %d in MADT\n", gic_version);
> +             return -EINVAL;
> +     }
> +
> +     /*
> +      * when the GIC version is 0, we fallback to hardware discovery.
> +      * this is also needed to keep compatiable with ACPI 5.1,
> +      * which has no gic_version field in distributor structure and
> +      * reserved as 0.
> +      *
> +      * For hardware discovery, the offset for GICv1/2 and GICv3/4 to
> +      * get the GIC version is different (0xFE8 for GICv1/2 and 0xFFE8
> +      * for GICv3/4), so we need to handle it separately.

> +      */
> +     if (gic_version == ACPI_MADT_GIC_VERSION_NONE) {
> +             /* it's GICv3/v4 if redistributor is present */
> +             if (acpi_gic_redist_is_present()) {

See above, IIUC you might have systems with GIC v3 where this call
would fail, and we do not want to fall back to GIC v2.

Lorenzo

> +                     dist_base = ioremap(dist_phy_base,
> +                                         ACPI_GICV3_DIST_MEM_SIZE);
> +                     if (!dist_base)
> +                             return -ENOMEM;
> +
> +                     reg = readl_relaxed(dist_base + GICD_PIDR2) &
> +                                         GIC_PIDR2_ARCH_MASK;
> +                     if (reg == GIC_PIDR2_ARCH_GICv3)
> +                             gic_version = ACPI_MADT_GIC_VERSION_V3;
> +                     else
> +                             gic_version = ACPI_MADT_GIC_VERSION_V4;
> +
> +                     iounmap(dist_base);
> +             } else {
> +                     gic_version = ACPI_MADT_GIC_VERSION_V2;
> +             }

> +     }
> +
> +     return 0;
> +}
> diff --git a/include/linux/irqchip/arm-gic-acpi.h 
> b/include/linux/irqchip/arm-gic-acpi.h
> index de3419e..13bc676 100644
> --- a/include/linux/irqchip/arm-gic-acpi.h
> +++ b/include/linux/irqchip/arm-gic-acpi.h
> @@ -19,6 +19,7 @@
>   */
>  #define ACPI_GICV2_DIST_MEM_SIZE     (SZ_4K)
>  #define ACPI_GIC_CPU_IF_MEM_SIZE     (SZ_8K)
> +#define ACPI_GICV3_DIST_MEM_SIZE     (SZ_64K)
>  
>  struct acpi_table_header;
>  
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to