[Linaro-uefi,2/2] Platforms/ARM/Juno: ameliorate misleading GTDT name.

Message ID 20160706202243.10792-3-evan.lloyd@arm.com
State Accepted
Commit a2f30df028247d22ecc4c2c9659a99db6fb00fc4
Headers show

Commit Message

Evan Lloyd July 6, 2016, 8:22 p.m.
From: Evan Lloyd <evan.lloyd@arm.com>

This commit modifies the name of a structure that is entirely internal
to  Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
The name EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES is changed to
GENERIC_TIMER_DESCRIPTION_TABLE.

The name EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES was misleading in
that it appears to be from the standard headers, and caused a bug
where the very similar EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE was
accidentally used instead.

This is only a source name change with no functional modification.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
---
 Platforms/ARM/Juno/AcpiTables/Gtdt.aslc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Graeme Gregory July 7, 2016, 10:05 a.m. | #1
On Wed, Jul 06, 2016 at 09:22:43PM +0100, evan.lloyd@arm.com wrote:
> From: Evan Lloyd <evan.lloyd@arm.com>
> 
> This commit modifies the name of a structure that is entirely internal
> to  Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
> The name EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES is changed to
> GENERIC_TIMER_DESCRIPTION_TABLE.
> 
> The name EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES was misleading in
> that it appears to be from the standard headers, and caused a bug
> where the very similar EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE was
> accidentally used instead.
> 
> This is only a source name change with no functional modification.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
> ---
>  Platforms/ARM/Juno/AcpiTables/Gtdt.aslc | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc b/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
> index 3995059114b27586ddacc60c6a9c1346e5051c57..5e2daf9abb8344ff121dace4fa827838b10ac287 100644
> --- a/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
> +++ b/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
> @@ -64,15 +64,15 @@
>  #if (JUNO_WATCHDOG_COUNT != 0)
>      EFI_ACPI_5_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE     Watchdogs[JUNO_WATCHDOG_COUNT];
>  #endif
> -  } EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES;
> +  } GENERIC_TIMER_DESCRIPTION_TABLE;
>  

I did a similar thing in FVP code after typoing one too many times.

Reviewed-by: Graeme Gregory <graeme.gregory@linaro.org>

>    #pragma pack ()
>  
> -  EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
> +  GENERIC_TIMER_DESCRIPTION_TABLE Gtdt = {
>      {
>        ARM_ACPI_HEADER(
>          EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
> -        EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES,
> +        GENERIC_TIMER_DESCRIPTION_TABLE,
>          EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE_REVISION
>        ),
>        SYSTEM_TIMER_BASE_ADDRESS,                    // UINT64  PhysicalAddress
> -- 
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> 
> _______________________________________________
> Linaro-uefi mailing list
> Linaro-uefi@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/linaro-uefi
Ryan Harkin July 7, 2016, 10:29 a.m. | #2
ameliorate - that's the first time I've had to use a dictionary to
read a patch :-)

On 7 July 2016 at 11:05, Graeme Gregory <graeme.gregory@linaro.org> wrote:
> On Wed, Jul 06, 2016 at 09:22:43PM +0100, evan.lloyd@arm.com wrote:
>> From: Evan Lloyd <evan.lloyd@arm.com>
>>
>> This commit modifies the name of a structure that is entirely internal
>> to  Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
>> The name EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES is changed to
>> GENERIC_TIMER_DESCRIPTION_TABLE.
>>
>> The name EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES was misleading in
>> that it appears to be from the standard headers, and caused a bug
>> where the very similar EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE was
>> accidentally used instead.
>>
>> This is only a source name change with no functional modification.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
>> ---
>>  Platforms/ARM/Juno/AcpiTables/Gtdt.aslc | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc b/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
>> index 3995059114b27586ddacc60c6a9c1346e5051c57..5e2daf9abb8344ff121dace4fa827838b10ac287 100644
>> --- a/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
>> +++ b/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
>> @@ -64,15 +64,15 @@
>>  #if (JUNO_WATCHDOG_COUNT != 0)
>>      EFI_ACPI_5_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE     Watchdogs[JUNO_WATCHDOG_COUNT];
>>  #endif
>> -  } EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES;
>> +  } GENERIC_TIMER_DESCRIPTION_TABLE;
>>
>
> I did a similar thing in FVP code after typoing one too many times.
>
> Reviewed-by: Graeme Gregory <graeme.gregory@linaro.org>
>
>>    #pragma pack ()
>>
>> -  EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
>> +  GENERIC_TIMER_DESCRIPTION_TABLE Gtdt = {
>>      {
>>        ARM_ACPI_HEADER(
>>          EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
>> -        EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES,
>> +        GENERIC_TIMER_DESCRIPTION_TABLE,
>>          EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE_REVISION
>>        ),
>>        SYSTEM_TIMER_BASE_ADDRESS,                    // UINT64  PhysicalAddress
>> --
>> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>>
>> _______________________________________________
>> Linaro-uefi mailing list
>> Linaro-uefi@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/linaro-uefi
> _______________________________________________
> Linaro-uefi mailing list
> Linaro-uefi@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/linaro-uefi
Ard Biesheuvel July 7, 2016, 12:14 p.m. | #3
On 6 July 2016 at 22:22,  <evan.lloyd@arm.com> wrote:
> From: Evan Lloyd <evan.lloyd@arm.com>
>
> This commit modifies the name of a structure that is entirely internal
> to  Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
> The name EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES is changed to
> GENERIC_TIMER_DESCRIPTION_TABLE.
>
> The name EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES was misleading in
> that it appears to be from the standard headers, and caused a bug
> where the very similar EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE was
> accidentally used instead.
>
> This is only a source name change with no functional modification.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>

Adjudicated-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  Platforms/ARM/Juno/AcpiTables/Gtdt.aslc | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc b/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
> index 3995059114b27586ddacc60c6a9c1346e5051c57..5e2daf9abb8344ff121dace4fa827838b10ac287 100644
> --- a/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
> +++ b/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
> @@ -64,15 +64,15 @@
>  #if (JUNO_WATCHDOG_COUNT != 0)
>      EFI_ACPI_5_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE     Watchdogs[JUNO_WATCHDOG_COUNT];
>  #endif
> -  } EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES;
> +  } GENERIC_TIMER_DESCRIPTION_TABLE;
>
>    #pragma pack ()
>
> -  EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
> +  GENERIC_TIMER_DESCRIPTION_TABLE Gtdt = {
>      {
>        ARM_ACPI_HEADER(
>          EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
> -        EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES,
> +        GENERIC_TIMER_DESCRIPTION_TABLE,
>          EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE_REVISION
>        ),
>        SYSTEM_TIMER_BASE_ADDRESS,                    // UINT64  PhysicalAddress
> --
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>
Leif Lindholm July 7, 2016, 1:34 p.m. | #4
On Thu, Jul 07, 2016 at 02:14:13PM +0200, Ard Biesheuvel wrote:
> On 6 July 2016 at 22:22,  <evan.lloyd@arm.com> wrote:
> > From: Evan Lloyd <evan.lloyd@arm.com>
> >
> > This commit modifies the name of a structure that is entirely internal
> > to  Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
> > The name EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES is changed to
> > GENERIC_TIMER_DESCRIPTION_TABLE.
> >
> > The name EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES was misleading in
> > that it appears to be from the standard headers, and caused a bug
> > where the very similar EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE was
> > accidentally used instead.
> >
> > This is only a source name change with no functional modification.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
> 
> Adjudicated-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Thanks all - series pushed :)

> > ---
> >  Platforms/ARM/Juno/AcpiTables/Gtdt.aslc | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc b/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
> > index 3995059114b27586ddacc60c6a9c1346e5051c57..5e2daf9abb8344ff121dace4fa827838b10ac287 100644
> > --- a/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
> > +++ b/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
> > @@ -64,15 +64,15 @@
> >  #if (JUNO_WATCHDOG_COUNT != 0)
> >      EFI_ACPI_5_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE     Watchdogs[JUNO_WATCHDOG_COUNT];
> >  #endif
> > -  } EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES;
> > +  } GENERIC_TIMER_DESCRIPTION_TABLE;
> >
> >    #pragma pack ()
> >
> > -  EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
> > +  GENERIC_TIMER_DESCRIPTION_TABLE Gtdt = {
> >      {
> >        ARM_ACPI_HEADER(
> >          EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
> > -        EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES,
> > +        GENERIC_TIMER_DESCRIPTION_TABLE,
> >          EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE_REVISION
> >        ),
> >        SYSTEM_TIMER_BASE_ADDRESS,                    // UINT64  PhysicalAddress
> > --
> > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> >

Patch

diff --git a/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc b/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
index 3995059114b27586ddacc60c6a9c1346e5051c57..5e2daf9abb8344ff121dace4fa827838b10ac287 100644
--- a/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
+++ b/Platforms/ARM/Juno/AcpiTables/Gtdt.aslc
@@ -64,15 +64,15 @@ 
 #if (JUNO_WATCHDOG_COUNT != 0)
     EFI_ACPI_5_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE     Watchdogs[JUNO_WATCHDOG_COUNT];
 #endif
-  } EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES;
+  } GENERIC_TIMER_DESCRIPTION_TABLE;
 
   #pragma pack ()
 
-  EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
+  GENERIC_TIMER_DESCRIPTION_TABLE Gtdt = {
     {
       ARM_ACPI_HEADER(
         EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
-        EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES,
+        GENERIC_TIMER_DESCRIPTION_TABLE,
         EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE_REVISION
       ),
       SYSTEM_TIMER_BASE_ADDRESS,                    // UINT64  PhysicalAddress