[PATCH 1/1] ACPI: EC: Allow multibyte access to EC (v2)

Stefan Bader stefan.bader at canonical.com
Tue Apr 13 10:28:01 UTC 2010


All in all this seems to be the better approach

Andy Whitcroft wrote:
> Allow more than a single byte to be read or written to the EC.  This
> is needed by some Dell BIOSs.
> 
> Based on the original version by Alexey Starikovskiy.
> 
> BugLink: http://bugs.launchpad.net/bugs/526354
> Signed-off-by: Andy Whitcroft <apw at canonical.com>
Acked-by: Stefan Bader <stefan.bader at canonical.com>
> ---
>  drivers/acpi/acpica/exprep.c |   12 ++++++++++++
>  drivers/acpi/ec.c            |    7 ++-----
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/acpi/acpica/exprep.c b/drivers/acpi/acpica/exprep.c
> index 52fec07..c02d543 100644
> --- a/drivers/acpi/acpica/exprep.c
> +++ b/drivers/acpi/acpica/exprep.c
> @@ -468,6 +468,18 @@ acpi_status acpi_ex_prep_field_value(struct acpi_create_field_info *info)
>  
>  		acpi_ut_add_reference(obj_desc->field.region_obj);
>  
> +		/* allow full data read from EC address space */
> +		if (obj_desc->field.region_obj->region.space_id ==
> +			ACPI_ADR_SPACE_EC) {
> +			if (obj_desc->common_field.bit_length > 8)
> +				obj_desc->common_field.access_bit_width =
> +				ACPI_ROUND_UP(obj_desc->common_field.
> +							bit_length, 8);
> +				obj_desc->common_field.access_byte_width =
> +				ACPI_DIV_8(obj_desc->common_field.
> +							access_bit_width);
> +		}

It would be interesting to find out whether it would not be better to have the
bit length _always_ roounded up to a multiple of 8 and not only when multiple
bytes are needed.

> +
>  		ACPI_DEBUG_PRINT((ACPI_DB_BFIELD,
>  				  "RegionField: BitOff %X, Off %X, Gran %X, Region %p\n",
>  				  obj_desc->field.start_field_bit_offset,
> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> index f1670e0..b654a96 100644
> --- a/drivers/acpi/ec.c
> +++ b/drivers/acpi/ec.c
> @@ -601,10 +601,7 @@ acpi_ec_space_handler(u32 function, acpi_physical_address address,
>  	if (function != ACPI_READ && function != ACPI_WRITE)
>  		return AE_BAD_PARAMETER;
>  
> -	if (bits != 8 && acpi_strict)
> -		return AE_BAD_PARAMETER;
> -
> -	if (EC_FLAGS_MSI)
> +	if (EC_FLAGS_MSI || bits > 8)
>  		acpi_ec_burst_enable(ec);
>  
>  	if (function == ACPI_READ) {
> @@ -626,7 +623,7 @@ acpi_ec_space_handler(u32 function, acpi_physical_address address,
>  		}
>  	}
>  
> -	if (EC_FLAGS_MSI)
> +	if (EC_FLAGS_MSI || bits > 8)
>  		acpi_ec_burst_disable(ec);
>  
>  	switch (result) {


As the new loop had issues with not reading anything when the bit width was
below 8 bits and also assumes little endianess (can we assume acpi is only
running on x86?) it sounds generally to be better to leave it as is.




More information about the kernel-team mailing list