| Message ID | 1271152487-4339-2-git-send-email-apw@canonical.com |
|---|---|
| State | Superseded |
| Delegated to: | Andy Whitcroft |
| Headers | show |
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@canonical.com> Acked-by: Stefan Bader <stefan.bader@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.
On Tue, 2010-04-13 at 10:54 +0100, 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@canonical.com> Indeed this appears to be a modified version of the original patch and initial testing seems positive. Acked-by: Leann Ogasawara <leann.ogasawara@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); > + } > + > 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) { > -- > 1.7.0 > >
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); + } + 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) {
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@canonical.com> --- drivers/acpi/acpica/exprep.c | 12 ++++++++++++ drivers/acpi/ec.c | 7 ++----- 2 files changed, 14 insertions(+), 5 deletions(-)