Hi,
On 7/6/21 6:35 PM, Andy Shevchenko wrote:
> 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?
No, the drivers/acpi/pmic/intel_pmic_chtwc.c implementation accepts multiple
i2c addresses because the whiskey cove has multiple register banks split
over different i2c-addressses.
Regards,
Hans
