From 88dcb04a813265e1a5a1bc74af2db7efa7d62ee6 Mon Sep 17 00:00:00 2001 From: Bob Moore Date: Wed, 18 Feb 2009 16:01:04 +0800 Subject: [PATCH] ACPICA: Restructure bit register access functions Update code for acpi_read_bit_register and acpi_write_bit_register. Simplified code path, condensed duplicate code. Signed-off-by: Bob Moore Signed-off-by: Lin Ming Signed-off-by: Len Brown --- drivers/acpi/acpica/hwxface.c | 181 ++++++++++++---------------------- 1 file changed, 64 insertions(+), 117 deletions(-) diff --git a/drivers/acpi/acpica/hwxface.c b/drivers/acpi/acpica/hwxface.c index 529251c7f911..caad51680bd6 100644 --- a/drivers/acpi/acpica/hwxface.c +++ b/drivers/acpi/acpica/hwxface.c @@ -133,8 +133,8 @@ acpi_status acpi_read(u32 *value, struct acpi_generic_address *reg) *value = 0; /* - * Two address spaces supported: Memory or IO. - * PCI_Config is not supported here because the GAS struct is insufficient + * Two address spaces supported: Memory or IO. PCI_Config is + * not supported here because the GAS structure is insufficient */ switch (reg->space_id) { case ACPI_ADR_SPACE_SYSTEM_MEMORY: @@ -266,11 +266,12 @@ ACPI_EXPORT_SYMBOL(acpi_write) ******************************************************************************/ acpi_status acpi_read_bit_register(u32 register_id, u32 *return_value) { - u32 register_value = 0; struct acpi_bit_register_info *bit_reg_info; + u32 register_value; + u32 value; acpi_status status; - ACPI_FUNCTION_TRACE(acpi_read_bit_register); + ACPI_FUNCTION_TRACE_U32(acpi_read_bit_register, register_id); /* Get the info structure corresponding to the requested ACPI Register */ @@ -283,23 +284,22 @@ acpi_status acpi_read_bit_register(u32 register_id, u32 *return_value) status = acpi_hw_register_read(bit_reg_info->parent_register, ®ister_value); - - if (ACPI_SUCCESS(status)) { - - /* Normalize the value that was read */ - - register_value = - ((register_value & bit_reg_info->access_bit_mask) - >> bit_reg_info->bit_position); - - *return_value = register_value; - - ACPI_DEBUG_PRINT((ACPI_DB_IO, "Read value %8.8X register %X\n", - register_value, - bit_reg_info->parent_register)); + if (ACPI_FAILURE(status)) { + return_ACPI_STATUS(status); } - return_ACPI_STATUS(status); + /* Normalize the value that was read, mask off other bits */ + + value = ((register_value & bit_reg_info->access_bit_mask) + >> bit_reg_info->bit_position); + + ACPI_DEBUG_PRINT((ACPI_DB_IO, + "BitReg %X, ParentReg %X, Actual %8.8X, ReturnValue %8.8X\n", + register_id, bit_reg_info->parent_register, + register_value, value)); + + *return_value = value; + return_ACPI_STATUS(AE_OK); } ACPI_EXPORT_SYMBOL(acpi_read_bit_register) @@ -321,13 +321,16 @@ ACPI_EXPORT_SYMBOL(acpi_read_bit_register) * SUPPORTS: Bit fields in PM1 Status, PM1 Enable, PM1 Control, and * PM2 Control. * + * Note that at this level, the fact that there may be actually two + * hardware registers (A and B - and B may not exist) is abstracted. + * ******************************************************************************/ acpi_status acpi_write_bit_register(u32 register_id, u32 value) { - u32 register_value = 0; struct acpi_bit_register_info *bit_reg_info; - acpi_status status; acpi_cpu_flags lock_flags; + u32 register_value; + acpi_status status = AE_OK; ACPI_FUNCTION_TRACE_U32(acpi_write_bit_register, register_id); @@ -335,127 +338,71 @@ acpi_status acpi_write_bit_register(u32 register_id, u32 value) bit_reg_info = acpi_hw_get_bit_register_info(register_id); if (!bit_reg_info) { - ACPI_ERROR((AE_INFO, "Bad ACPI HW RegisterId: %X", - register_id)); return_ACPI_STATUS(AE_BAD_PARAMETER); } lock_flags = acpi_os_acquire_lock(acpi_gbl_hardware_lock); - /* Always do a register read first so we can insert the new bits */ - - status = acpi_hw_register_read(bit_reg_info->parent_register, - ®ister_value); - if (ACPI_FAILURE(status)) { - goto unlock_and_exit; - } - /* - * Decode the Register ID - * Register ID = [Register block ID] | [bit ID] - * - * Check bit ID to fine locate Register offset. - * Check Mask to determine Register offset, and then read-write. + * At this point, we know that the parent register is one of the + * following: PM1 Status, PM1 Enable, PM1 Control, or PM2 Control */ - switch (bit_reg_info->parent_register) { - case ACPI_REGISTER_PM1_STATUS: - + if (bit_reg_info->parent_register != ACPI_REGISTER_PM1_STATUS) { /* - * Status Registers are different from the rest. Clear by - * writing 1, and writing 0 has no effect. So, the only relevant - * information is the single bit we're interested in, all others should - * be written as 0 so they will be left unchanged. + * 1) Case for PM1 Enable, PM1 Control, and PM2 Control + * + * Perform a register read to preserve the bits that we are not + * interested in */ - value = ACPI_REGISTER_PREPARE_BITS(value, - bit_reg_info->bit_position, - bit_reg_info-> - access_bit_mask); - if (value) { - status = - acpi_hw_register_write(ACPI_REGISTER_PM1_STATUS, - (u16) value); - register_value = 0; - } - break; - - case ACPI_REGISTER_PM1_ENABLE: - - ACPI_REGISTER_INSERT_VALUE(register_value, - bit_reg_info->bit_position, - bit_reg_info->access_bit_mask, - value); - - status = acpi_hw_register_write(ACPI_REGISTER_PM1_ENABLE, - (u16) register_value); - break; - - case ACPI_REGISTER_PM1_CONTROL: - - /* - * Write the PM1 Control register. - * Note that at this level, the fact that there are actually TWO - * registers (A and B - and B may not exist) is abstracted. - */ - ACPI_DEBUG_PRINT((ACPI_DB_IO, "PM1 control: Read %X\n", - register_value)); - - ACPI_REGISTER_INSERT_VALUE(register_value, - bit_reg_info->bit_position, - bit_reg_info->access_bit_mask, - value); - - status = acpi_hw_register_write(ACPI_REGISTER_PM1_CONTROL, - (u16) register_value); - break; - - case ACPI_REGISTER_PM2_CONTROL: - - status = acpi_hw_register_read(ACPI_REGISTER_PM2_CONTROL, + status = acpi_hw_register_read(bit_reg_info->parent_register, ®ister_value); if (ACPI_FAILURE(status)) { goto unlock_and_exit; } - ACPI_DEBUG_PRINT((ACPI_DB_IO, - "PM2 control: Read %X from %8.8X%8.8X\n", - register_value, - ACPI_FORMAT_UINT64(acpi_gbl_FADT. - xpm2_control_block. - address))); - + /* + * Insert the input bit into the value that was just read + * and write the register + */ ACPI_REGISTER_INSERT_VALUE(register_value, bit_reg_info->bit_position, bit_reg_info->access_bit_mask, value); - ACPI_DEBUG_PRINT((ACPI_DB_IO, - "About to write %4.4X to %8.8X%8.8X\n", - register_value, - ACPI_FORMAT_UINT64(acpi_gbl_FADT. - xpm2_control_block. - address))); + status = acpi_hw_register_write(bit_reg_info->parent_register, + register_value); + } else { + /* + * 2) Case for PM1 Status + * + * The Status register is different from the rest. Clear an event + * by writing 1, writing 0 has no effect. So, the only relevant + * information is the single bit we're interested in, all others + * should be written as 0 so they will be left unchanged. + */ + register_value = ACPI_REGISTER_PREPARE_BITS(value, + bit_reg_info-> + bit_position, + bit_reg_info-> + access_bit_mask); - status = acpi_hw_register_write(ACPI_REGISTER_PM2_CONTROL, - (u8) (register_value)); - break; + /* No need to write the register if value is all zeros */ - default: - break; + if (register_value) { + status = + acpi_hw_register_write(ACPI_REGISTER_PM1_STATUS, + register_value); + } } - unlock_and_exit: + ACPI_DEBUG_PRINT((ACPI_DB_IO, + "BitReg %X, ParentReg %X, Value %8.8X, Actual %8.8X\n", + register_id, bit_reg_info->parent_register, value, + register_value)); + +unlock_and_exit: acpi_os_release_lock(acpi_gbl_hardware_lock, lock_flags); - - /* Normalize the value that was read */ - - ACPI_DEBUG_EXEC(register_value = - ((register_value & bit_reg_info->access_bit_mask) >> - bit_reg_info->bit_position)); - - ACPI_DEBUG_PRINT((ACPI_DB_IO, - "Set bits: %8.8X actual %8.8X register %X\n", value, - register_value, bit_reg_info->parent_register)); return_ACPI_STATUS(status); }