lkml.org 
[lkml]   [2021]   [Nov]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.14 182/251] ACPI: PMIC: Fix intel_pmic_regs_handler() read accesses
    Date
    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 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) {
    --
    2.33.0


    \
     
     \ /
      Last update: 2021-11-24 13:45    [W:2.657 / U:0.056 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site