| From 9b7eb342922690c5ee2ad511057bf713f731ce29 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Sun, 31 Oct 2021 16:31:35 +0100 |
| Subject: ACPI: PMIC: Fix intel_pmic_regs_handler() read accesses |
| |
| From: Hans de Goede <hdegoede@redhat.com> |
| |
| [ Upstream commit 009a789443fe4c8e6b1ecb7c16b4865c026184cd ] |
| |
| The handling of PMIC register reads through writing 0 to address 4 |
| of the OpRegion is wrong. Instead of returning the read value |
| through the value64, which is a no-op for function == ACPI_WRITE calls, |
| store the value and then on a subsequent function == ACPI_READ with |
| address == 3 (the address for the value field of the OpRegion) |
| return the stored value. |
| |
| This has been tested on a Xiaomi Mi Pad 2 and makes the ACPI battery dev |
| there mostly functional (unfortunately there are still other issues). |
| |
| Here are the SET() / GET() functions of the PMIC ACPI device, |
| which use this OpRegion, which clearly show the new behavior to |
| be correct: |
| |
| OperationRegion (REGS, 0x8F, Zero, 0x50) |
| Field (REGS, ByteAcc, NoLock, Preserve) |
| { |
| CLNT, 8, |
| SA, 8, |
| OFF, 8, |
| VAL, 8, |
| RWM, 8 |
| } |
| |
| Method (GET, 3, Serialized) |
| { |
| If ((AVBE == One)) |
| { |
| CLNT = Arg0 |
| SA = Arg1 |
| OFF = Arg2 |
| RWM = Zero |
| If ((AVBG == One)) |
| { |
| GPRW = Zero |
| } |
| } |
| |
| Return (VAL) /* \_SB_.PCI0.I2C7.PMI5.VAL_ */ |
| } |
| |
| Method (SET, 4, Serialized) |
| { |
| If ((AVBE == One)) |
| { |
| CLNT = Arg0 |
| SA = Arg1 |
| OFF = Arg2 |
| VAL = Arg3 |
| RWM = One |
| If ((AVBG == One)) |
| { |
| GPRW = One |
| } |
| } |
| } |
| |
| Fixes: 0afa877a5650 ("ACPI / PMIC: intel: add REGS operation region support") |
| Signed-off-by: Hans de Goede <hdegoede@redhat.com> |
| Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| drivers/acpi/pmic/intel_pmic.c | 51 +++++++++++++++++++--------------- |
| 1 file changed, 28 insertions(+), 23 deletions(-) |
| |
| diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c |
| index 452041398b347..36d5a5d50b2ff 100644 |
| --- a/drivers/acpi/pmic/intel_pmic.c |
| +++ b/drivers/acpi/pmic/intel_pmic.c |
| @@ -211,31 +211,36 @@ static acpi_status intel_pmic_regs_handler(u32 function, |
| void *handler_context, void *region_context) |
| { |
| struct intel_pmic_opregion *opregion = region_context; |
| - int result = 0; |
| + int result = -EINVAL; |
| + |
| + if (function == ACPI_WRITE) { |
| + switch (address) { |
| + case 0: |
| + return AE_OK; |
| + case 1: |
| + opregion->ctx.addr |= (*value64 & 0xff) << 8; |
| + return AE_OK; |
| + case 2: |
| + opregion->ctx.addr |= *value64 & 0xff; |
| + return AE_OK; |
| + case 3: |
| + opregion->ctx.val = *value64 & 0xff; |
| + return AE_OK; |
| + case 4: |
| + if (*value64) { |
| + result = regmap_write(opregion->regmap, opregion->ctx.addr, |
| + opregion->ctx.val); |
| + } else { |
| + result = regmap_read(opregion->regmap, opregion->ctx.addr, |
| + &opregion->ctx.val); |
| + } |
| + opregion->ctx.addr = 0; |
| + } |
| + } |
| |
| - switch (address) { |
| - case 0: |
| - return AE_OK; |
| - case 1: |
| - opregion->ctx.addr |= (*value64 & 0xff) << 8; |
| - return AE_OK; |
| - case 2: |
| - opregion->ctx.addr |= *value64 & 0xff; |
| + if (function == ACPI_READ && address == 3) { |
| + *value64 = opregion->ctx.val; |
| return AE_OK; |
| - case 3: |
| - opregion->ctx.val = *value64 & 0xff; |
| - return AE_OK; |
| - case 4: |
| - if (*value64) { |
| - result = regmap_write(opregion->regmap, opregion->ctx.addr, |
| - opregion->ctx.val); |
| - } else { |
| - result = regmap_read(opregion->regmap, opregion->ctx.addr, |
| - &opregion->ctx.val); |
| - if (result == 0) |
| - *value64 = opregion->ctx.val; |
| - } |
| - memset(&opregion->ctx, 0x00, sizeof(opregion->ctx)); |
| } |
| |
| if (result < 0) { |
| -- |
| 2.33.0 |
| |