On Fri, Jul 2, 2021 at 7:51 PM Hans de Goede <hdegoede@xxxxxxxxxx> 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@xxxxxxxxx>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> 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
