[PATCH] acpi: sbbr: relax a few test pass conditions
Alex Hung
alex.hung at canonical.com
Mon Dec 11 13:03:44 UTC 2017
On 2017-12-11 06:17 PM, Sakar Arora wrote:
> DBG2: Allow PL011 UART
> SPCR: Allow IO-APIC interrupt type
> _CRS: Relax failures to warnings for mif/maf tests
I think this patch should split into two or three patches, as each
serves different purpose.
It is also better to provide some references, ex. where PL011_UART can
be found and so on.
>
> Signed-off-by: Sakar Arora <sakar.arora at arm.com>
> ---
> src/acpi/dbg2/dbg2.c | 8 +-
> src/acpi/spcr/spcr.c | 6 +-
> src/lib/src/fwts_acpi_object_eval.c | 149 ++++++++++++++++++++++++------------
> 3 files changed, 111 insertions(+), 52 deletions(-)
>
> diff --git a/src/acpi/dbg2/dbg2.c b/src/acpi/dbg2/dbg2.c
> index b826c27..f1301de 100644
> --- a/src/acpi/dbg2/dbg2.c
> +++ b/src/acpi/dbg2/dbg2.c
> @@ -28,8 +28,9 @@
>
> #include "fwts_acpi_object_eval.h"
>
> -#define SBBR_DBG2_PORT_SERIAL 0x8000
> -#define SBBR_DBG2_ARM_SBSA_UART 0x000E
> +#define SBBR_DBG2_PORT_SERIAL 0x8000
> +#define SBBR_DBG2_ARM_SBSA_UART 0x000E
> +#define SBBR_DBG2_ARM_PL011_UART 0x0003
>
> static fwts_acpi_table_info *table;
>
> @@ -72,7 +73,8 @@ static int dbg2_test2(fwts_framework *fw)
> if (((uint8_t*)info + info->length) >= ((uint8_t*)table + table->length))
> break;
> if ((info->port_type == SBBR_DBG2_PORT_SERIAL) &&
> - (info->port_subtype == SBBR_DBG2_ARM_SBSA_UART)) {
> + (info->port_subtype == SBBR_DBG2_ARM_SBSA_UART ||
> + info->port_subtype == SBBR_DBG2_ARM_PL011_UART)) {
The original indentation is not perfect, but could you please align the
condition? The indentation may differ from one editor to another, but it
should look aligned in vi(m) or emacs.
> fwts_passed(fw,
> "DBG2 provides a standard serial debug "
> "port and describes ARM SBSA Generic UART");
> diff --git a/src/acpi/spcr/spcr.c b/src/acpi/spcr/spcr.c
> index 2f37e3f..6a9ac3a 100644
> --- a/src/acpi/spcr/spcr.c
> +++ b/src/acpi/spcr/spcr.c
> @@ -74,9 +74,11 @@ static int spcr_sbbr_gsiv_test(fwts_framework *fw)
> {
> if (fw->flags & FWTS_FLAG_TEST_SBBR) {
> const uint8_t ARMH_GIC_INTR_MASK = 0x08;
> + const uint8_t IO_APIC_INTR_MASK = 0x02;
>
> - if ( (spcr->interrupt_type == ARMH_GIC_INTR_MASK) &&
> - (spcr->gsi != 0x0000000000000000) )
> + if ((spcr->interrupt_type == ARMH_GIC_INTR_MASK ||
> + spcr->interrupt_type == IO_APIC_INTR_MASK) &&
> + spcr->gsi != 0x0000000000000000)
The indentation may differ from one editor to another, but it should
look aligned in vi(m) or emacs.
> fwts_passed(fw, "SPCR appears to be populated with correct GSIV interrupt"
> "routing information for ARM PL011 UART Device");
> else
> diff --git a/src/lib/src/fwts_acpi_object_eval.c b/src/lib/src/fwts_acpi_object_eval.c
> index b356441..b3537b0 100644
> --- a/src/lib/src/fwts_acpi_object_eval.c
> +++ b/src/lib/src/fwts_acpi_object_eval.c
> @@ -1270,32 +1270,51 @@ static void method_test_CRS_mif_maf(
> if (len == 0) {
> if ((mif == 1) && (maf == 1)) {
> snprintf(tmp, sizeof(tmp), "Method%s%sMifMafBothOne", objname, tag);
> - fwts_failed(fw, LOG_LEVEL_MEDIUM,
> - tmp,
> - "%s %s _MIF and _MAF flags are both "
> - "set to one which is invalid when "
> - "the length field is 0.",
> - name, type);
> + if (fw->flags & FWTS_FLAG_TEST_SBBR)
> + fwts_warning(fw, tmp,
> + "%s %s _MIF and _MAF flags are both "
> + "set to one which is invalid when "
> + "the length field is 0.",
> + name, type);
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + tmp,
> + "%s %s _MIF and _MAF flags are both "
> + "set to one which is invalid when "
> + "the length field is 0.",
> + name, type);
Please explain more: 1. why should this be a warning instead of a
failure for SBBR? 2. why does this apply for SBBR only?
The same questions apply to the below changes too.
> fwts_advice(fw, "%s", mif_maf_advice);
> *passed = false;
> }
> if ((mif == 1) && (min % (granularity + 1) != 0)) {
> snprintf(tmp, sizeof(tmp), "Method%s%sMinNotMultipleOfGran", objname, tag);
> - fwts_failed(fw, LOG_LEVEL_MEDIUM,
> - tmp,
> - "%s %s _MIN address is not a multiple "
> - "of the granularity when _MIF is 1.",
> - name, type);
> + if (fw->flags & FWTS_FLAG_TEST_SBBR)
> + fwts_warning(fw, tmp,
> + "%s %s _MIN address is not a multiple "
> + "of the granularity when _MIF is 1.",
> + name, type);
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + tmp,
> + "%s %s _MIN address is not a multiple "
> + "of the granularity when _MIF is 1.",
> + name, type);
> fwts_advice(fw, "%s", mif_maf_advice);
> *passed = false;
> }
> if ((maf == 1) && (max % (granularity - 1) != 0)) {
> snprintf(tmp, sizeof(tmp), "Method%s%sMaxNotMultipleOfGran", objname, tag);
> - fwts_failed(fw, LOG_LEVEL_MEDIUM,
> - tmp,
> - "%s %s _MAX address is not a multiple "
> - "of the granularity when _MAF is 1.",
> - name, type);
> + if (fw->flags & FWTS_FLAG_TEST_SBBR)
> + fwts_warning(fw, tmp,
> + "%s %s _MAX address is not a multiple "
> + "of the granularity when _MAF is 1.",
> + name, type);
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + tmp,
> + "%s %s _MAX address is not a multiple "
> + "of the granularity when _MAF is 1.",
> + name, type);
> fwts_advice(fw, "%s", mif_maf_advice);
> *passed = false;
> }
> @@ -1303,58 +1322,94 @@ static void method_test_CRS_mif_maf(
> if ((mif == 0) && (maf == 0) &&
> (len % (granularity + 1) != 0)) {
> snprintf(tmp, sizeof(tmp), "Method%s%sLenNotMultipleOfGran", objname, tag);
> - fwts_failed(fw, LOG_LEVEL_MEDIUM,
> - tmp,
> - "%s %s length is not a multiple "
> - "of the granularity when _MIF "
> - "and _MIF are 0.",
> - name, type);
> + if (fw->flags & FWTS_FLAG_TEST_SBBR)
> + fwts_warning(fw, tmp,
> + "%s %s length is not a multiple "
> + "of the granularity when _MIF "
> + "and _MIF are 0.",
> + name, type);
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + tmp,
> + "%s %s length is not a multiple "
> + "of the granularity when _MIF "
> + "and _MIF are 0.",
> + name, type);
> fwts_advice(fw, "%s", mif_maf_advice);
> *passed = false;
> }
> if (((mif == 0) && (maf == 1)) || ((mif == 1) && (maf == 0))) {
> snprintf(tmp, sizeof(tmp), "Method%s%sMifMafInvalid", objname, tag);
> - fwts_failed(fw, LOG_LEVEL_MEDIUM,
> - tmp,
> - "%s %s _MIF and _MAF flags are either "
> - "0 and 1 or 1 and 0 which is invalid when "
> - "the length field is non-zero.",
> - name, type);
> + if (fw->flags & FWTS_FLAG_TEST_SBBR)
> + fwts_warning(fw, tmp,
> + "%s %s _MIF and _MAF flags are either "
> + "0 and 1 or 1 and 0 which is invalid when "
> + "the length field is non-zero.",
> + name, type);
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + tmp,
> + "%s %s _MIF and _MAF flags are either "
> + "0 and 1 or 1 and 0 which is invalid when "
> + "the length field is non-zero.",
> + name, type);
> fwts_advice(fw, "%s", mif_maf_advice);
> *passed = false;
> }
> if ((mif == 1) && (maf == 1)) {
> if (granularity != 0) {
> snprintf(tmp, sizeof(tmp), "Method%s%sGranularityNotZero", objname, tag);
> - fwts_failed(fw, LOG_LEVEL_MEDIUM,
> - tmp,
> - "%s %s granularity 0x%" PRIx64
> - " is not zero as expected when "
> - "_MIF and _MAF are both 1.",
> - name, type, granularity);
> + if (fw->flags & FWTS_FLAG_TEST_SBBR)
> + fwts_warning(fw, tmp,
> + "%s %s granularity 0x%" PRIx64
> + " is not zero as expected when "
> + "_MIF and _MAF are both 1.",
> + name, type, granularity);
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + tmp,
> + "%s %s granularity 0x%" PRIx64
> + " is not zero as expected when "
> + "_MIF and _MAF are both 1.",
> + name, type, granularity);
> fwts_advice(fw, "%s", mif_maf_advice);
> *passed = false;
> }
> if (min > max) {
> snprintf(tmp, sizeof(tmp), "Method%s%sMaxLessThanMin", objname, tag);
> - fwts_failed(fw, LOG_LEVEL_MEDIUM,
> - tmp,
> - "%s %s minimum address range 0x%" PRIx64
> - " is greater than the maximum address "
> - "range 0x%" PRIx64 ".",
> - name, type, min, max);
> + if (fw->flags & FWTS_FLAG_TEST_SBBR)
> + fwts_warning(fw, tmp,
> + "%s %s minimum address range 0x%" PRIx64
> + " is greater than the maximum address "
> + "range 0x%" PRIx64 ".",
> + name, type, min, max);
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + tmp,
> + "%s %s minimum address range 0x%" PRIx64
> + " is greater than the maximum address "
> + "range 0x%" PRIx64 ".",
> + name, type, min, max);
> fwts_advice(fw, "%s", mif_maf_advice);
> *passed = false;
> }
> if (max - min + 1 != len) {
> snprintf(tmp, sizeof(tmp), "Method%s%sLengthInvalid", objname, tag);
> - fwts_failed(fw, LOG_LEVEL_MEDIUM,
> - tmp,
> - "%s %s length 0x%" PRIx64
> - " does not match the difference between "
> - "the minimum and maximum address ranges "
> - "0x%" PRIx64 "-0x%" PRIx64 ".",
> - name, type, len, min, max);
> + if (fw->flags & FWTS_FLAG_TEST_SBBR)
> + fwts_warning(fw, tmp,
> + "%s %s length 0x%" PRIx64
> + " does not match the difference between "
> + "the minimum and maximum address ranges "
> + "0x%" PRIx64 "-0x%" PRIx64 ".",
> + name, type, len, min, max);
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + tmp,
> + "%s %s length 0x%" PRIx64
> + " does not match the difference between "
> + "the minimum and maximum address ranges "
> + "0x%" PRIx64 "-0x%" PRIx64 ".",
> + name, type, len, min, max);
> fwts_advice(fw, "%s", mif_maf_advice);
> *passed = false;
> }
>
--
Cheers,
Alex Hung
More information about the fwts-devel
mailing list