Skip to content

Commit 009a789

Browse files
jwrdegoederafaeljw
authored andcommitted
ACPI: PMIC: Fix intel_pmic_regs_handler() read accesses
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: 0afa877 ("ACPI / PMIC: intel: add REGS operation region support") Signed-off-by: Hans de Goede <[email protected]> Signed-off-by: Rafael J. Wysocki <[email protected]>
1 parent c0d6586 commit 009a789

File tree

1 file changed

+28
-23
lines changed

1 file changed

+28
-23
lines changed

drivers/acpi/pmic/intel_pmic.c

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -211,31 +211,36 @@ static acpi_status intel_pmic_regs_handler(u32 function,
211211
void *handler_context, void *region_context)
212212
{
213213
struct intel_pmic_opregion *opregion = region_context;
214-
int result = 0;
214+
int result = -EINVAL;
215+
216+
if (function == ACPI_WRITE) {
217+
switch (address) {
218+
case 0:
219+
return AE_OK;
220+
case 1:
221+
opregion->ctx.addr |= (*value64 & 0xff) << 8;
222+
return AE_OK;
223+
case 2:
224+
opregion->ctx.addr |= *value64 & 0xff;
225+
return AE_OK;
226+
case 3:
227+
opregion->ctx.val = *value64 & 0xff;
228+
return AE_OK;
229+
case 4:
230+
if (*value64) {
231+
result = regmap_write(opregion->regmap, opregion->ctx.addr,
232+
opregion->ctx.val);
233+
} else {
234+
result = regmap_read(opregion->regmap, opregion->ctx.addr,
235+
&opregion->ctx.val);
236+
}
237+
opregion->ctx.addr = 0;
238+
}
239+
}
215240

216-
switch (address) {
217-
case 0:
218-
return AE_OK;
219-
case 1:
220-
opregion->ctx.addr |= (*value64 & 0xff) << 8;
221-
return AE_OK;
222-
case 2:
223-
opregion->ctx.addr |= *value64 & 0xff;
241+
if (function == ACPI_READ && address == 3) {
242+
*value64 = opregion->ctx.val;
224243
return AE_OK;
225-
case 3:
226-
opregion->ctx.val = *value64 & 0xff;
227-
return AE_OK;
228-
case 4:
229-
if (*value64) {
230-
result = regmap_write(opregion->regmap, opregion->ctx.addr,
231-
opregion->ctx.val);
232-
} else {
233-
result = regmap_read(opregion->regmap, opregion->ctx.addr,
234-
&opregion->ctx.val);
235-
if (result == 0)
236-
*value64 = opregion->ctx.val;
237-
}
238-
memset(&opregion->ctx, 0x00, sizeof(opregion->ctx));
239244
}
240245

241246
if (result < 0) {

0 commit comments

Comments
 (0)