Re: [Xen-devel] [PATCH RFC 15/35] ARM64 / ACPI: Define ACPI_IRQ_MODEL_GIC needed for arm

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 4 Feb 2015, parth.dixit@xxxxxxxxxx wrote:
> From: Naresh Bhat <naresh.bhat@xxxxxxxxxx>
> 
> Needed because ARM64 uses GIC which is defined in ACPI 5.0 spec.
> 
> Signed-off-by: Al Stone <al.stone@xxxxxxxxxx>
> Signed-off-by: Hanjun Guo <hanjun.guo@xxxxxxxxxx>
> Signed-off-by: Naresh Bhat <naresh.bhat@xxxxxxxxxx>
> ---
>  xen/arch/arm/arm64/acpi/arm-core.c |  6 ++-
>  xen/drivers/acpi/tables.c          | 95 
> ++++++++++++++++++++------------------
>  2 files changed, 55 insertions(+), 46 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/acpi/arm-core.c 
> b/xen/arch/arm/arm64/acpi/arm-core.c
> index 84b0032..cc11fce 100644
> --- a/xen/arch/arm/arm64/acpi/arm-core.c
> +++ b/xen/arch/arm/arm64/acpi/arm-core.c
> @@ -55,7 +55,11 @@ int acpi_psci_use_hvc;
>  /* available_cpus means enabled cpu in MADT */
>  static int available_cpus;
>  
> -enum acpi_irq_model_id acpi_irq_model = ACPI_IRQ_MODEL_PLATFORM;
> +/*
> + * Since we're on ARM, the default interrupt routing model
> + * clearly has to be GIC.
> + */
> +enum acpi_irq_model_id acpi_irq_model = ACPI_IRQ_MODEL_GIC;
>  
>  struct acpi_arm_root acpi_arm_rsdp_info;     /* info about RSDP from FDT */
>  
> diff --git a/xen/drivers/acpi/tables.c b/xen/drivers/acpi/tables.c
> index 5314f0b..ad680f8 100644
> --- a/xen/drivers/acpi/tables.c
> +++ b/xen/drivers/acpi/tables.c
> @@ -244,60 +244,65 @@ acpi_parse_entries(unsigned long table_size,
>                  struct acpi_table_header *table_header,
>                  int entry_id, unsigned int max_entries)
>  {
> -        struct acpi_subtable_header *entry;
> -        int count = 0;
> -        unsigned long table_end;
> +    struct acpi_subtable_header *entry;
> +    int count = 0;
> +    unsigned long table_end;
>  
> -        if (acpi_disabled)
> -                return -ENODEV;
> +    if ( acpi_disabled )
> +        return -ENODEV;
>  
> -        if (!handler)
> -                return -EINVAL;
> +    if ( !handler )
> +        return -EINVAL;
>  
> -        if (!table_size)
> -                return -EINVAL;
> +    if ( !table_size )
> +        return -EINVAL;
>  
> -        if (!table_header) {
> -                printk("Table header not present\n");
> -                return -ENODEV;
> -        }
> +    if ( !table_header )
> +    {
> +        printk("Table header not present\n");
> +        return -ENODEV;
> +    }
>  
> -        table_end = (unsigned long)table_header + table_header->length;
> +    table_end = (unsigned long)table_header + table_header->length;
>  
> -        /* Parse all entries looking for a match. */
> +    /* Parse all entries looking for a match. */

All these changes shouldn't be here.
As you are introducing this function in one of the previous patches, you
should have the indentation right in the first place.


> -        entry = (struct acpi_subtable_header *)
> -            ((unsigned long)table_header + table_size);
> -
> -        while (((unsigned long)entry) + sizeof(struct acpi_subtable_header) <
> -               table_end) {
> -                if (entry->type == entry_id
> -                    && (!max_entries || count++ < max_entries))
> -                        if (handler(entry, table_end)) {
> -                                count = -EINVAL;
> -                                goto err;
> -                }
> -
> -                /*
> -                 * If entry->length is 0, break from this loop to avoid
> -                 * infinite loop.
> -                 */
> -                if (entry->length == 0) {
> -                        printk("[0x%02x] Invalid zero length\n", entry_id);
> -                        count = -EINVAL;
> -                        goto err;
> -                }
> -
> -                entry = (struct acpi_subtable_header *)
> -                    ((unsigned long)entry + entry->length);
> -        }
> +    entry = (struct acpi_subtable_header *)
> +        ((unsigned long)table_header + table_size);
> +
> +    while ( ( (unsigned long)entry ) + sizeof(struct acpi_subtable_header) <
> +            table_end)
> +    {
> +        if ( entry->type == entry_id
> +            && ( !max_entries || count++ < max_entries ) )
> +            if ( handler(entry, table_end) )
> +            {
> +                count = -EINVAL;
> +                goto err;
> +            }
>  
> -        if (max_entries && count > max_entries) {
> -                printk("[0x%02x] ignored %i entries of %i found\n",
> -                        entry_id, count - max_entries, count);
> +        /*
> +         * If entry->length is 0, break from this loop to avoid
> +         * infinite loop.
> +         */
> +        if ( entry->length == 0 )
> +        {
> +            printk("[0x%02x] Invalid zero length\n", entry_id);
> +            count = -EINVAL;
> +            goto err;
>          }
>  
> -err:
> +        entry = (struct acpi_subtable_header *)
> +            ((unsigned long)entry + entry->length);
> +    }
> +
> +    if ( max_entries && count > max_entries )
> +    {
> +        printk("[0x%02x] ignored %i entries of %i found\n",
> +            entry_id, count - max_entries, count);
> +    }
> +
> +    err:
>          return count;
>  }

With all these indentation changes I cannot tell if you made any other
changes in this patch. Please remove all the indentation changes.


> @@ -394,7 +399,7 @@ int __init acpi_table_parse(char *id, acpi_table_handler 
> handler)
>               return 1;
>  }
>  
> -/* 
> +/*
>   * The BIOS is supposed to supply a single APIC/MADT,
>   * but some report two.  Provide a knob to use either.
>   * (don't you wish instance 0 and 1 were not the same?)
> -- 
> 1.9.1
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel