ACK: [PATCH 13/21] FADT: expand compliance tests for the SMI_CMD field

[Alex Hung]

On 2016-02-09 09:32 AM, Al Stone wrote:
> Minor tweaks to the tests of the SMI_CMD to make them only slightly
> more extensive, but also to fit within the resequencing of tests for
> reduced hardware mode..
>
> Signed-off-by: Al Stone <al.stone at linaro.org>
> ---
>   src/acpi/fadt/fadt.c | 68 ++++++++++++++++++++++++++--------------------------
>   1 file changed, 34 insertions(+), 34 deletions(-)
>
> diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c
> index 430bfd4..3da297e 100644
> --- a/src/acpi/fadt/fadt.c
> +++ b/src/acpi/fadt/fadt.c
> @@ -660,19 +660,8 @@ static void acpi_table_check_fadt_reduced_hardware(fwts_framework *fw)
>   			    "hardware mode.");
>   }
>
> -static void acpi_table_check_fadt_smi(
> -	fwts_framework *fw,
> -	const fwts_acpi_table_fadt *fadt,
> -	bool *passed)
> +static void acpi_table_check_fadt_smi_cmd(fwts_framework *fw)
>   {
> -	if (fwts_acpi_is_reduced_hardware(fadt)) {
> -		if (fadt->smi_cmd != 0) {
> -			fwts_warning(fw, "FADT SMI_CMD is not zero "
> -				"but should be in reduced hardware mode.");
> -		}
> -		return;
> -	}
> -
>   	/*
>   	 *  Section 5.2.9 (Fixed ACPI Description Table) of the ACPI 5.0
>   	 *  specification states that if SMI_CMD is zero then it is
> @@ -680,38 +669,49 @@ static void acpi_table_check_fadt_smi(
>   	 *  in that case, don't check SCI_INT being valid.
>   	 */
>   	if (fadt->smi_cmd != 0) {
> -		if (fadt->sci_int == 0) {
> -			*passed = false;
> +		if (fadt->sci_int == 0)
>   			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -				"FADTSCIIRQZero",
> -				"FADT SCI Interrupt is 0x00, should be defined.");
> -		}
> +				    "FADTSCIIRQZero",
> +				    "FADT SCI Interrupt is 0x00, but must "
> +				    "be defined since SMI command indicates "
> +				    "System Management Mode is supported.");
> +		else
> +			fwts_passed(fw,
> +				    "FADT SMI_CMD indicates System Management "
> +				    "Mode is supported, and the SCI Interrupt "
> +				    "is non-zero.");
>   	} else {
>   		if ((fadt->acpi_enable == 0) &&
>   		    (fadt->acpi_disable == 0) &&
>   		    (fadt->s4bios_req == 0) &&
>   		    (fadt->pstate_cnt == 0) &&
>   		    (fadt->cst_cnt == 0)) {
> -			/* Not an error, but intentional, but feedback this finding anyhow */
> -			fwts_log_info(fw, "The FADT SMI_CMD is zero, system "
> -				"does not support System Management Mode.");
> +			/*
> +			 * Not an error, but intentional, so feedback
> +			 * this finding.
> +			 */
> +			fwts_passed(fw, "The FADT SMI_CMD is zero, system "
> +				    "does not support System Management Mode.");
>   		}
>   		else {
> -			*passed = false;
>   			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -				"FADTSMICMDZero",
> -				"FADT SMI_CMD is 0x00, however, one or more of "
> -				"ACPI_ENABLE, ACPI_DISABLE, S4BIOS_REQ, PSTATE_CNT "
> -				"and CST_CNT are defined which means SMI_CMD should "
> -				"be defined otherwise SMI commands cannot be sent.");
> +				    "FADTSMICMDZero",
> +				    "FADT SMI_CMD is 0x00, however, one or "
> +				    "more of ACPI_ENABLE, ACPI_DISABLE, "
> +				    "S4BIOS_REQ, PSTATE_CNT and CST_CNT are "
> +				    "defined which means SMI_CMD should be "
> +				    "defined, otherwise SMI commands cannot "
> +				    "be sent.");
>   			fwts_advice(fw,
> -				"The configuration seems to suggest that SMI command "
> -				"should be defined to allow the kernel to trigger "
> -				"system management interrupts via the SMD_CMD port. "
> -				"The fact that SMD_CMD is zero which is invalid means "
> -				"that SMIs are not possible through the normal ACPI "
> -				"mechanisms. This means some firmware based machine "
> -				"specific functions will not work.");
> +				    "The configuration seems to suggest that "
> +				    "SMI command should be defined to allow "
> +				    "the kernel to trigger system management "
> +				    "interrupts via the SMD_CMD port.  The "
> +				    "fact that SMD_CMD is zero which is "
> +				    "invalid means that SMIs are not possible "
> +				    "through the normal ACPI mechanisms. This "
> +				    "means some firmware based machine "
> +				    "specific functions will not work.");
>   		}
>   	}
>   }
> @@ -858,7 +858,7 @@ static int fadt_test1(fwts_framework *fw)
>   	 */
>   	if (!fwts_acpi_is_reduced_hardware(fadt)) {
>   		fwts_log_info(fw, "FADT SCI_INT is %" PRIu8, fadt->sci_int);
> -		acpi_table_check_fadt_smi(fw, fadt, &passed);
> +		acpi_table_check_fadt_smi_cmd(fw);
>   		acpi_table_check_fadt_pm_tmr(fw, fadt, &passed);
>   		acpi_table_check_fadt_gpe(fw, fadt, &passed);
>   		acpi_table_check_fadt_pm_addr(fw, fadt, &passed);
>

Acked-by: Alex Hung <alex.hung at canonical.com>