[PATCH] acpi: checksum: rename the test name from "checksum to "acpichecksum"

Alex Hung alex.hung at canonical.com
Mon Apr 29 08:05:38 UTC 2013


Hi,

I have some ideas to share on the naming of the tests.

As some names are general or the abbreviation are hard to understand, it 
may be a good idea to have a meaningful prefix. As firmware are all 
about hardware and software standards, it is good to use specification 
or document names such as acpi, cpu, pcie, uefi and so on.

With these prefixes, one knows which document to read when he/she has 
questions or needs to dig more info errors.

== examples ==

acpi (acpi specification):
mcfg -> acpimcfg (or acpi_mcfg)
fadt -> acpifadt (or acpi_fadt)
...

cpu (intel's software developer manuals)
mtrr -> cpumtrr (or cpu_mtrr)
msr -> cpumsr (or cpu_msr)
nx -> cpunx (or cpu_nx)
...

pcie (pci and pcie specifications)
aspm -> pcieaspm (or pcie_aspm)
maxreadfreq -> pciemaxreadfreq (or pcie_maxreadfreq)
...

and so on


We can also go a little further and implement a group-test feature:

  - "fwts acpi*" to test for acpi tests
  - "fwts cpu*" to test all cpu features
  - so on...

Any feedbacks are mostly appreciated.

Cheers,
Alex Hung


On 04/26/2013 03:22 AM, Colin Ian King wrote:
> On 25/04/13 03:43, Alex Hung wrote:
>> The name "checksum" is too generic. The rename can help a user to
>> understand
>> that the test targets ACPI tables and verifies the table checksum.
>>
>> Signed-off-by: Alex Hung <alex.hung at canonical.com>
>> ---
>>   src/acpi/checksum/checksum.c |   19 ++++++++++---------
>>   1 file changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/acpi/checksum/checksum.c b/src/acpi/checksum/checksum.c
>> index e56ca66..a8ec15c 100644
>> --- a/src/acpi/checksum/checksum.c
>> +++ b/src/acpi/checksum/checksum.c
>> @@ -26,7 +26,7 @@
>>
>>   #include "fwts.h"
>>
>> -static void checksum_rsdp(fwts_framework *fw, fwts_acpi_table_info
>> *table)
>> +static void acpi_checksum_rsdp(fwts_framework *fw,
>> fwts_acpi_table_info *table)
>>   {
>>       uint8_t checksum;
>>       fwts_acpi_table_rsdp *rsdp = (fwts_acpi_table_rsdp*)table->data;
>> @@ -89,7 +89,7 @@ static void checksum_rsdp(fwts_framework *fw,
>> fwts_acpi_table_info *table)
>>
>>   }
>>
>> -static int checksum_scan_tables(fwts_framework *fw)
>> +static int acpi_checksum_scan_tables(fwts_framework *fw)
>>   {
>>       int i;
>>
>> @@ -108,7 +108,7 @@ static int checksum_scan_tables(fwts_framework *fw)
>>           hdr = (fwts_acpi_table_header*)table->data;
>>
>>           if (strcmp("RSDP", table->name) == 0) {
>> -            checksum_rsdp(fw, table);
>> +            acpi_checksum_rsdp(fw, table);
>>               continue;
>>           }
>>
>> @@ -140,19 +140,20 @@ static int checksum_scan_tables(fwts_framework *fw)
>>       return FWTS_OK;
>>   }
>>
>> -static int checksum_test1(fwts_framework *fw)
>> +static int acpi_checksum_test1(fwts_framework *fw)
>>   {
>> -    return checksum_scan_tables(fw);
>> +    return acpi_checksum_scan_tables(fw);
>>   }
>>
>> -static fwts_framework_minor_test checksum_tests[] = {
>> -    { checksum_test1, "Check ACPI table checksums." },
>> +static fwts_framework_minor_test acpi_checksum_tests[] = {
>> +    { acpi_checksum_test1, "Check ACPI table checksums." },
>>       { NULL, NULL }
>>   };
>>
>>   static fwts_framework_ops checksum_ops = {
>>       .description = "Check ACPI table checksum.",
>> -    .minor_tests = checksum_tests
>> +    .minor_tests = acpi_checksum_tests
>>   };
>>
>> -FWTS_REGISTER("checksum", &checksum_ops, FWTS_TEST_ANYTIME,
>> FWTS_FLAG_BATCH);
>> +FWTS_REGISTER("acpichecksum", &checksum_ops,
>> +        FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH);
>>
>
> I've been thinking about this all day.  Indeed it is a good idea for us
> to name the tests in a meaningful way but we also need to consider
> ensuring we don't cause too much pain for users who are used to the
> current test naming scheme.
>
> I think that we should re-examine all the current tests and see if any
> more require renaming and *then* apply these changes in one go rather
> than making the changes in a piecemeal fashion.
>
> +1 for renaming this test, but also:
> 1)  We need the fwts-test renamed
> 2)  Can this wait until we have re-examined all the current test names.
>
> Colin
>




More information about the fwts-devel mailing list