This is a note to let you know that I've just added the patch titled
ACPI: PMIC: Fix intel_pmic_regs_handler() read accesses
to the 4.14-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
acpi-pmic-fix-intel_pmic_regs_handler-read-accesses.patch
and it can be found in the queue-4.14 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
commit 08cfabe628fe4551f6d088d2c88d183b68394a07
Author: Hans de Goede <hdegoede@xxxxxxxxxx>
Date: Sun Oct 31 16:31:35 2021 +0100
ACPI: PMIC: Fix intel_pmic_regs_handler() read accesses
[ 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@xxxxxxxxxx>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
index ca18e0d23df97..db63d3463617a 100644
--- a/drivers/acpi/pmic/intel_pmic.c
+++ b/drivers/acpi/pmic/intel_pmic.c
@@ -216,31 +216,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;
+ if (function == ACPI_READ && address == 3) {
+ *value64 = opregion->ctx.val;
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);
- if (result == 0)
- *value64 = opregion->ctx.val;
- }
- memset(&opregion->ctx, 0x00, sizeof(opregion->ctx));
}
if (result < 0) {
