acpi: sbbr: relax a few test pass conditions

DBG2: Allow PL011 UART
SPCR: Allow IO-APIC interrupt type
_CRS: Relax failures to warnings for mif/maf tests

Signed-off-by: Sakar Arora <sakar.arora@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(-)

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@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;
>   			}
>