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

[Andy Shevchenko <andy.shevchenko@gmail.com>]

On Tue, Jul 06, 2021 at 06:09:23PM +0200, Hans de Goede wrote:
> The I2C-bus to the XPower AXP288 is shared between the Linux kernel and
> the SoCs P-Unit. The P-Unit 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 P-Unit semaphore dance twice.

...

The idea for the further improvement

> +	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;
> +	}

We have this in intel_pmic.c. Can we reorganize the code the way we will have
this check solely in the intel_pmic.c?

-- 
With Best Regards,
Andy Shevchenko