[PATCH 1/2] ACPI / PMIC: XPower: optimize I2C-bus accesses

[PATCH 1/2] ACPI / PMIC: XPower: optimize I2C-bus accesses

[Hans de Goede]

The I2C-bus to the XPower AXP288 is shared between the Linux kernel and
the SoCs PUNIT. The PUNIT has a semaphore which the kernel must "lock"
before it may use the bus and while the kernel holds the semaphore the CPU
and GPU power-states must not be changed otherwise the system will freeze.

This is a complex process, which is quite expensive. This is all done by
iosf_mbi_block_punit_i2c_access(). To ensure that no unguarded I2C-bus
accesses happen, iosf_mbi_block_punit_i2c_access() gets called by the
I2C-bus-driver for every I2C transfer. Because this is so expensive it
is allowed to call iosf_mbi_block_punit_i2c_access() in a nested
fashion, so that higher-level code which does multiple I2C-transfers can
call it once for a group of transfers, turning the calls done by the
I2C-bus-driver into no-ops.

Move / add iosf_mbi_block_punit_i2c_access() calls in / to the XPower
OpRegion code so that the PUNIT semaphore only needs to be taken once
for each OpRegion access.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/pmic/intel_pmic_xpower.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/pmic/intel_pmic_xpower.c b/drivers/acpi/pmic/intel_pmic_xpower.c
index a091d5a8392c..644a495a4f13 100644
--- a/drivers/acpi/pmic/intel_pmic_xpower.c
+++ b/drivers/acpi/pmic/intel_pmic_xpower.c
@@ -178,15 +178,17 @@ static int intel_xpower_pmic_update_power(struct regmap *regmap, int reg,
 {
 	int data, ret;
 
-	/* GPIO1 LDO regulator needs special handling */
-	if (reg == XPOWER_GPI1_CTRL)
-		return regmap_update_bits(regmap, reg, GPI1_LDO_MASK,
-					  on ? GPI1_LDO_ON : GPI1_LDO_OFF);
-
 	ret = iosf_mbi_block_punit_i2c_access();
 	if (ret)
 		return ret;
 
+	/* GPIO1 LDO regulator needs special handling */
+	if (reg == XPOWER_GPI1_CTRL) {
+		ret = regmap_update_bits(regmap, reg, GPI1_LDO_MASK,
+					 on ? GPI1_LDO_ON : GPI1_LDO_OFF);
+		goto out;
+	}
+
 	if (regmap_read(regmap, reg, &data)) {
 		ret = -EIO;
 		goto out;
@@ -218,6 +220,10 @@ static int intel_xpower_pmic_get_raw_temp(struct regmap *regmap, int reg)
 	int ret, adc_ts_pin_ctrl;
 	u8 buf[2];
 
+	ret = iosf_mbi_block_punit_i2c_access();
+	if (ret)
+		return ret;
+
 	/*
 	 * The current-source used for the battery temp-sensor (TS) is shared
 	 * with the GPADC. For proper fuel-gauge and charger operation the TS
@@ -231,14 +237,14 @@ static int intel_xpower_pmic_get_raw_temp(struct regmap *regmap, int reg)
 	 */
 	ret = regmap_read(regmap, AXP288_ADC_TS_PIN_CTRL, &adc_ts_pin_ctrl);
 	if (ret)
-		return ret;
+		goto out;
 
 	if (adc_ts_pin_ctrl & AXP288_ADC_TS_CURRENT_ON_OFF_MASK) {
 		ret = regmap_update_bits(regmap, AXP288_ADC_TS_PIN_CTRL,
 					 AXP288_ADC_TS_CURRENT_ON_OFF_MASK,
 					 AXP288_ADC_TS_CURRENT_ON_ONDEMAND);
 		if (ret)
-			return ret;
+			goto out;
 
 		/* Wait a bit after switching the current-source */
 		usleep_range(6000, 10000);
@@ -254,6 +260,9 @@ static int intel_xpower_pmic_get_raw_temp(struct regmap *regmap, int reg)
 				   AXP288_ADC_TS_CURRENT_ON);
 	}
 
+out:
+	iosf_mbi_unblock_punit_i2c_access();
+
 	return ret;
 }
 
-- 
2.31.1

[PATCH 2/2] ACPI / PMIC: XPower: optimize MIPI PMIQ sequence I2C-bus accesses

[Hans de Goede]

The I2C-bus to the XPower AXP288 is shared between the Linux kernel and
the SoCs PUNIT. The PUNIT has a semaphore which the kernel must "lock"
before it may use the bus and while the kernel holds the semaphore the CPU
and GPU power-states must not be changed otherwise the system will freeze.

This is a complex process, which is quite expensive. This is all done by
iosf_mbi_block_punit_i2c_access(). To ensure that no unguarded I2C-bus
accesses happen, iosf_mbi_block_punit_i2c_access() gets called by the
I2C-bus-driver for every I2C transfer. Because this is so expensive it
is allowed to call iosf_mbi_block_punit_i2c_access() in a nested
fashion, so that higher-level code which does multiple I2C-transfers can
call it once for a group of transfers, turning the calls done by the
I2C-bus-driver into no-ops.

The default exec_mipi_pmic_seq_element implementation from
drivers/acpi/pmic/intel_pmic.c does a regmap_update_bits() call and
the involved registers are typically marked as volatile in the regmap,
so this leads to 2 I2C-bus accesses.

Add a XPower AXP288 specific implementation of exec_mipi_pmic_seq_element
which calls iosf_mbi_block_punit_i2c_access() calls before the
regmap_update_bits() call to avoid having to do the whole expensive
acquire PUNIT semaphore dance twice.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/pmic/intel_pmic_xpower.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/acpi/pmic/intel_pmic_xpower.c b/drivers/acpi/pmic/intel_pmic_xpower.c
index 644a495a4f13..93c516ad361e 100644
--- a/drivers/acpi/pmic/intel_pmic_xpower.c
+++ b/drivers/acpi/pmic/intel_pmic_xpower.c
@@ -266,10 +266,34 @@ static int intel_xpower_pmic_get_raw_temp(struct regmap *regmap, int reg)
 	return ret;
 }
 
+static int intel_xpower_exec_mipi_pmic_seq_element(struct regmap *regmap,
+						   u16 i2c_address, u32 reg_address,
+						   u32 value, u32 mask)
+{
+	int ret;
+
+	if (i2c_address != 0x34) {
+		pr_err("%s: Unexpected i2c-addr: 0x%02x (reg-addr 0x%x value 0x%x mask 0x%x)\n",
+		       __func__, i2c_address, reg_address, value, mask);
+		return -ENXIO;
+	}
+
+	ret = iosf_mbi_block_punit_i2c_access();
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(regmap, reg_address, mask, value);
+
+	iosf_mbi_unblock_punit_i2c_access();
+
+	return ret;
+}
+
 static struct intel_pmic_opregion_data intel_xpower_pmic_opregion_data = {
 	.get_power = intel_xpower_pmic_get_power,
 	.update_power = intel_xpower_pmic_update_power,
 	.get_raw_temp = intel_xpower_pmic_get_raw_temp,
+	.exec_mipi_pmic_seq_element = intel_xpower_exec_mipi_pmic_seq_element,
 	.power_table = power_table,
 	.power_table_count = ARRAY_SIZE(power_table),
 	.thermal_table = thermal_table,
-- 
2.31.1

Re: [PATCH 1/2] ACPI / PMIC: XPower: optimize I2C-bus accesses

[Andy Shevchenko]

On Fri, Jul 2, 2021 at 7:50 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> The I2C-bus to the XPower AXP288 is shared between the Linux kernel and
> the SoCs PUNIT. The PUNIT has a semaphore which the kernel must "lock"
> before it may use the bus and while the kernel holds the semaphore the CPU
> and GPU power-states must not be changed otherwise the system will freeze.
>
> This is a complex process, which is quite expensive. This is all done by
> iosf_mbi_block_punit_i2c_access(). To ensure that no unguarded I2C-bus
> accesses happen, iosf_mbi_block_punit_i2c_access() gets called by the
> I2C-bus-driver for every I2C transfer. Because this is so expensive it
> is allowed to call iosf_mbi_block_punit_i2c_access() in a nested
> fashion, so that higher-level code which does multiple I2C-transfers can
> call it once for a group of transfers, turning the calls done by the
> I2C-bus-driver into no-ops.
>
> Move / add iosf_mbi_block_punit_i2c_access() calls in / to the XPower
> OpRegion code so that the PUNIT semaphore only needs to be taken once
> for each OpRegion access.

We usually spell "P-Unit" instead of "PUNIT".
Otherwise it looks good to me, thanks!
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/acpi/pmic/intel_pmic_xpower.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/acpi/pmic/intel_pmic_xpower.c b/drivers/acpi/pmic/intel_pmic_xpower.c
> index a091d5a8392c..644a495a4f13 100644
> --- a/drivers/acpi/pmic/intel_pmic_xpower.c
> +++ b/drivers/acpi/pmic/intel_pmic_xpower.c
> @@ -178,15 +178,17 @@ static int intel_xpower_pmic_update_power(struct regmap *regmap, int reg,
>  {
>         int data, ret;
>
> -       /* GPIO1 LDO regulator needs special handling */
> -       if (reg == XPOWER_GPI1_CTRL)
> -               return regmap_update_bits(regmap, reg, GPI1_LDO_MASK,
> -                                         on ? GPI1_LDO_ON : GPI1_LDO_OFF);
> -
>         ret = iosf_mbi_block_punit_i2c_access();
>         if (ret)
>                 return ret;
>
> +       /* GPIO1 LDO regulator needs special handling */
> +       if (reg == XPOWER_GPI1_CTRL) {
> +               ret = regmap_update_bits(regmap, reg, GPI1_LDO_MASK,
> +                                        on ? GPI1_LDO_ON : GPI1_LDO_OFF);
> +               goto out;
> +       }
> +
>         if (regmap_read(regmap, reg, &data)) {
>                 ret = -EIO;
>                 goto out;
> @@ -218,6 +220,10 @@ static int intel_xpower_pmic_get_raw_temp(struct regmap *regmap, int reg)
>         int ret, adc_ts_pin_ctrl;
>         u8 buf[2];
>
> +       ret = iosf_mbi_block_punit_i2c_access();
> +       if (ret)
> +               return ret;
> +
>         /*
>          * The current-source used for the battery temp-sensor (TS) is shared
>          * with the GPADC. For proper fuel-gauge and charger operation the TS
> @@ -231,14 +237,14 @@ static int intel_xpower_pmic_get_raw_temp(struct regmap *regmap, int reg)
>          */
>         ret = regmap_read(regmap, AXP288_ADC_TS_PIN_CTRL, &adc_ts_pin_ctrl);
>         if (ret)
> -               return ret;
> +               goto out;
>
>         if (adc_ts_pin_ctrl & AXP288_ADC_TS_CURRENT_ON_OFF_MASK) {
>                 ret = regmap_update_bits(regmap, AXP288_ADC_TS_PIN_CTRL,
>                                          AXP288_ADC_TS_CURRENT_ON_OFF_MASK,
>                                          AXP288_ADC_TS_CURRENT_ON_ONDEMAND);
>                 if (ret)
> -                       return ret;
> +                       goto out;
>
>                 /* Wait a bit after switching the current-source */
>                 usleep_range(6000, 10000);
> @@ -254,6 +260,9 @@ static int intel_xpower_pmic_get_raw_temp(struct regmap *regmap, int reg)
>                                    AXP288_ADC_TS_CURRENT_ON);
>         }
>
> +out:
> +       iosf_mbi_unblock_punit_i2c_access();
> +
>         return ret;
>  }
>
> --
> 2.31.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] ACPI / PMIC: XPower: optimize MIPI PMIQ sequence I2C-bus accesses

[Andy Shevchenko]

On Fri, Jul 2, 2021 at 7:51 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> The I2C-bus to the XPower AXP288 is shared between the Linux kernel and
> the SoCs PUNIT. The PUNIT has a semaphore which the kernel must "lock"
> before it may use the bus and while the kernel holds the semaphore the CPU
> and GPU power-states must not be changed otherwise the system will freeze.
>
> This is a complex process, which is quite expensive. This is all done by
> iosf_mbi_block_punit_i2c_access(). To ensure that no unguarded I2C-bus
> accesses happen, iosf_mbi_block_punit_i2c_access() gets called by the
> I2C-bus-driver for every I2C transfer. Because this is so expensive it
> is allowed to call iosf_mbi_block_punit_i2c_access() in a nested
> fashion, so that higher-level code which does multiple I2C-transfers can
> call it once for a group of transfers, turning the calls done by the
> I2C-bus-driver into no-ops.
>
> The default exec_mipi_pmic_seq_element implementation from
> drivers/acpi/pmic/intel_pmic.c does a regmap_update_bits() call and
> the involved registers are typically marked as volatile in the regmap,
> so this leads to 2 I2C-bus accesses.
>
> Add a XPower AXP288 specific implementation of exec_mipi_pmic_seq_element
> which calls iosf_mbi_block_punit_i2c_access() calls before the
> regmap_update_bits() call to avoid having to do the whole expensive
> acquire PUNIT semaphore dance twice.

Same as per patch 1.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/acpi/pmic/intel_pmic_xpower.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/drivers/acpi/pmic/intel_pmic_xpower.c b/drivers/acpi/pmic/intel_pmic_xpower.c
> index 644a495a4f13..93c516ad361e 100644
> --- a/drivers/acpi/pmic/intel_pmic_xpower.c
> +++ b/drivers/acpi/pmic/intel_pmic_xpower.c
> @@ -266,10 +266,34 @@ static int intel_xpower_pmic_get_raw_temp(struct regmap *regmap, int reg)
>         return ret;
>  }
>
> +static int intel_xpower_exec_mipi_pmic_seq_element(struct regmap *regmap,
> +                                                  u16 i2c_address, u32 reg_address,
> +                                                  u32 value, u32 mask)
> +{
> +       int ret;
> +
> +       if (i2c_address != 0x34) {
> +               pr_err("%s: Unexpected i2c-addr: 0x%02x (reg-addr 0x%x value 0x%x mask 0x%x)\n",
> +                      __func__, i2c_address, reg_address, value, mask);
> +               return -ENXIO;
> +       }
> +
> +       ret = iosf_mbi_block_punit_i2c_access();
> +       if (ret)
> +               return ret;
> +
> +       ret = regmap_update_bits(regmap, reg_address, mask, value);
> +
> +       iosf_mbi_unblock_punit_i2c_access();
> +
> +       return ret;
> +}
> +
>  static struct intel_pmic_opregion_data intel_xpower_pmic_opregion_data = {
>         .get_power = intel_xpower_pmic_get_power,
>         .update_power = intel_xpower_pmic_update_power,
>         .get_raw_temp = intel_xpower_pmic_get_raw_temp,
> +       .exec_mipi_pmic_seq_element = intel_xpower_exec_mipi_pmic_seq_element,
>         .power_table = power_table,
>         .power_table_count = ARRAY_SIZE(power_table),
>         .thermal_table = thermal_table,
> --
> 2.31.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] ACPI / PMIC: XPower: optimize I2C-bus accesses

[Hans de Goede]

Hi,

On 7/2/21 6:54 PM, Andy Shevchenko wrote:
> On Fri, Jul 2, 2021 at 7:50 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> The I2C-bus to the XPower AXP288 is shared between the Linux kernel and
>> the SoCs PUNIT. The PUNIT has a semaphore which the kernel must "lock"
>> before it may use the bus and while the kernel holds the semaphore the CPU
>> and GPU power-states must not be changed otherwise the system will freeze.
>>
>> This is a complex process, which is quite expensive. This is all done by
>> iosf_mbi_block_punit_i2c_access(). To ensure that no unguarded I2C-bus
>> accesses happen, iosf_mbi_block_punit_i2c_access() gets called by the
>> I2C-bus-driver for every I2C transfer. Because this is so expensive it
>> is allowed to call iosf_mbi_block_punit_i2c_access() in a nested
>> fashion, so that higher-level code which does multiple I2C-transfers can
>> call it once for a group of transfers, turning the calls done by the
>> I2C-bus-driver into no-ops.
>>
>> Move / add iosf_mbi_block_punit_i2c_access() calls in / to the XPower
>> OpRegion code so that the PUNIT semaphore only needs to be taken once
>> for each OpRegion access.
> 
> We usually spell "P-Unit" instead of "PUNIT".
> Otherwise it looks good to me, thanks!
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Thank you for the review.

Is your Reviewed-by for just this patch, or for the series ?

Regards,

Hans



> 
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/acpi/pmic/intel_pmic_xpower.c | 23 ++++++++++++++++-------
>>  1 file changed, 16 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/acpi/pmic/intel_pmic_xpower.c b/drivers/acpi/pmic/intel_pmic_xpower.c
>> index a091d5a8392c..644a495a4f13 100644
>> --- a/drivers/acpi/pmic/intel_pmic_xpower.c
>> +++ b/drivers/acpi/pmic/intel_pmic_xpower.c
>> @@ -178,15 +178,17 @@ static int intel_xpower_pmic_update_power(struct regmap *regmap, int reg,
>>  {
>>         int data, ret;
>>
>> -       /* GPIO1 LDO regulator needs special handling */
>> -       if (reg == XPOWER_GPI1_CTRL)
>> -               return regmap_update_bits(regmap, reg, GPI1_LDO_MASK,
>> -                                         on ? GPI1_LDO_ON : GPI1_LDO_OFF);
>> -
>>         ret = iosf_mbi_block_punit_i2c_access();
>>         if (ret)
>>                 return ret;
>>
>> +       /* GPIO1 LDO regulator needs special handling */
>> +       if (reg == XPOWER_GPI1_CTRL) {
>> +               ret = regmap_update_bits(regmap, reg, GPI1_LDO_MASK,
>> +                                        on ? GPI1_LDO_ON : GPI1_LDO_OFF);
>> +               goto out;
>> +       }
>> +
>>         if (regmap_read(regmap, reg, &data)) {
>>                 ret = -EIO;
>>                 goto out;
>> @@ -218,6 +220,10 @@ static int intel_xpower_pmic_get_raw_temp(struct regmap *regmap, int reg)
>>         int ret, adc_ts_pin_ctrl;
>>         u8 buf[2];
>>
>> +       ret = iosf_mbi_block_punit_i2c_access();
>> +       if (ret)
>> +               return ret;
>> +
>>         /*
>>          * The current-source used for the battery temp-sensor (TS) is shared
>>          * with the GPADC. For proper fuel-gauge and charger operation the TS
>> @@ -231,14 +237,14 @@ static int intel_xpower_pmic_get_raw_temp(struct regmap *regmap, int reg)
>>          */
>>         ret = regmap_read(regmap, AXP288_ADC_TS_PIN_CTRL, &adc_ts_pin_ctrl);
>>         if (ret)
>> -               return ret;
>> +               goto out;
>>
>>         if (adc_ts_pin_ctrl & AXP288_ADC_TS_CURRENT_ON_OFF_MASK) {
>>                 ret = regmap_update_bits(regmap, AXP288_ADC_TS_PIN_CTRL,
>>                                          AXP288_ADC_TS_CURRENT_ON_OFF_MASK,
>>                                          AXP288_ADC_TS_CURRENT_ON_ONDEMAND);
>>                 if (ret)
>> -                       return ret;
>> +                       goto out;
>>
>>                 /* Wait a bit after switching the current-source */
>>                 usleep_range(6000, 10000);
>> @@ -254,6 +260,9 @@ static int intel_xpower_pmic_get_raw_temp(struct regmap *regmap, int reg)
>>                                    AXP288_ADC_TS_CURRENT_ON);
>>         }
>>
>> +out:
>> +       iosf_mbi_unblock_punit_i2c_access();
>> +
>>         return ret;
>>  }
>>
>> --
>> 2.31.1
>>
> 
> 

Re: [PATCH 1/2] ACPI / PMIC: XPower: optimize I2C-bus accesses

[Andy Shevchenko]

On Fri, Jul 2, 2021 at 7:58 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 7/2/21 6:54 PM, Andy Shevchenko wrote:
> > On Fri, Jul 2, 2021 at 7:50 PM Hans de Goede <hdegoede@redhat.com> wrote:

...

> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> Thank you for the review.
>
> Is your Reviewed-by for just this patch, or for the series ?

I have not seen a cover letter (is there one?), so by default (i.e. if
not told otherwise) it applies only to this patch.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] ACPI / PMIC: XPower: optimize I2C-bus accesses

[Hans de Goede]

Hi,

On 7/2/21 7:00 PM, Andy Shevchenko wrote:
> On Fri, Jul 2, 2021 at 7:58 PM Hans de Goede <hdegoede@redhat.com> wrote:
>> On 7/2/21 6:54 PM, Andy Shevchenko wrote:
>>> On Fri, Jul 2, 2021 at 7:50 PM Hans de Goede <hdegoede@redhat.com> wrote:
> 
> ...
> 
>>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>>
>> Thank you for the review.
>>
>> Is your Reviewed-by for just this patch, or for the series ?
> 
> I have not seen a cover letter (is there one?), so by default (i.e. if
> not told otherwise) it applies only to this patch.

Right, that is why I asked, I had not seen your 2/2 reply yet,
I guess I should have just waited a bit longer, sorry about the noise.

Regards,

Hans

Re: [PATCH 1/2] ACPI / PMIC: XPower: optimize I2C-bus accesses

[Hans de Goede]

Hi,

On 7/2/21 6:50 PM, Hans de Goede wrote:
> The I2C-bus to the XPower AXP288 is shared between the Linux kernel and
> the SoCs PUNIT. The PUNIT has a semaphore which the kernel must "lock"
> before it may use the bus and while the kernel holds the semaphore the CPU
> and GPU power-states must not be changed otherwise the system will freeze.
> 
> This is a complex process, which is quite expensive. This is all done by
> iosf_mbi_block_punit_i2c_access(). To ensure that no unguarded I2C-bus
> accesses happen, iosf_mbi_block_punit_i2c_access() gets called by the
> I2C-bus-driver for every I2C transfer. Because this is so expensive it
> is allowed to call iosf_mbi_block_punit_i2c_access() in a nested
> fashion, so that higher-level code which does multiple I2C-transfers can
> call it once for a group of transfers, turning the calls done by the
> I2C-bus-driver into no-ops.
> 
> Move / add iosf_mbi_block_punit_i2c_access() calls in / to the XPower
> OpRegion code so that the PUNIT semaphore only needs to be taken once
> for each OpRegion access.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

I just noticed that one of the blocks has a usleep_range(6000, 10000)
in there which means that we now hold the P-Unit semaphore over the
sleep, which is not good.

Self nack. I'll send out an updated series fixing this.

Regards,

Hans




> ---
>  drivers/acpi/pmic/intel_pmic_xpower.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/acpi/pmic/intel_pmic_xpower.c b/drivers/acpi/pmic/intel_pmic_xpower.c
> index a091d5a8392c..644a495a4f13 100644
> --- a/drivers/acpi/pmic/intel_pmic_xpower.c
> +++ b/drivers/acpi/pmic/intel_pmic_xpower.c
> @@ -178,15 +178,17 @@ static int intel_xpower_pmic_update_power(struct regmap *regmap, int reg,
>  {
>  	int data, ret;
>  
> -	/* GPIO1 LDO regulator needs special handling */
> -	if (reg == XPOWER_GPI1_CTRL)
> -		return regmap_update_bits(regmap, reg, GPI1_LDO_MASK,
> -					  on ? GPI1_LDO_ON : GPI1_LDO_OFF);
> -
>  	ret = iosf_mbi_block_punit_i2c_access();
>  	if (ret)
>  		return ret;
>  
> +	/* GPIO1 LDO regulator needs special handling */
> +	if (reg == XPOWER_GPI1_CTRL) {
> +		ret = regmap_update_bits(regmap, reg, GPI1_LDO_MASK,
> +					 on ? GPI1_LDO_ON : GPI1_LDO_OFF);
> +		goto out;
> +	}
> +
>  	if (regmap_read(regmap, reg, &data)) {
>  		ret = -EIO;
>  		goto out;
> @@ -218,6 +220,10 @@ static int intel_xpower_pmic_get_raw_temp(struct regmap *regmap, int reg)
>  	int ret, adc_ts_pin_ctrl;
>  	u8 buf[2];
>  
> +	ret = iosf_mbi_block_punit_i2c_access();
> +	if (ret)
> +		return ret;
> +
>  	/*
>  	 * The current-source used for the battery temp-sensor (TS) is shared
>  	 * with the GPADC. For proper fuel-gauge and charger operation the TS
> @@ -231,14 +237,14 @@ static int intel_xpower_pmic_get_raw_temp(struct regmap *regmap, int reg)
>  	 */
>  	ret = regmap_read(regmap, AXP288_ADC_TS_PIN_CTRL, &adc_ts_pin_ctrl);
>  	if (ret)
> -		return ret;
> +		goto out;
>  
>  	if (adc_ts_pin_ctrl & AXP288_ADC_TS_CURRENT_ON_OFF_MASK) {
>  		ret = regmap_update_bits(regmap, AXP288_ADC_TS_PIN_CTRL,
>  					 AXP288_ADC_TS_CURRENT_ON_OFF_MASK,
>  					 AXP288_ADC_TS_CURRENT_ON_ONDEMAND);
>  		if (ret)
> -			return ret;
> +			goto out;
>  
>  		/* Wait a bit after switching the current-source */
>  		usleep_range(6000, 10000);
> @@ -254,6 +260,9 @@ static int intel_xpower_pmic_get_raw_temp(struct regmap *regmap, int reg)
>  				   AXP288_ADC_TS_CURRENT_ON);
>  	}
>  
> +out:
> +	iosf_mbi_unblock_punit_i2c_access();
> +
>  	return ret;
>  }
>  
>