Skip to content

L
linux
Switch branch/tag
  1. 17 Aug, 2021 2 commits
  3. 16 Aug, 2021 9 commits
  5. 14 Aug, 2021 2 commits
  7. 13 Aug, 2021 11 commits
  9. 06 Aug, 2021 7 commits
  11. 05 Aug, 2021 9 commits
    • Hans de Goede's avatar
      power: supply: axp288_fuel_gauge: Take the P-Unit semaphore only once during probe() · 213e19d6
      Hans de Goede authored 
      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. If not explicitly taken by the I2C-driver,
then this semaphore is automatically taken by the I2C-bus-driver for
each I2C-transfer and this is a quite expensive operation.

Explicitly take the semaphore in probe() around the register-accesses
done during probe, so that this only needs to be done once, rather then
once per register-access.
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarSebastian Reichel <sebastian.reichel@collabora.com>
      213e19d6
    • Hans de Goede's avatar
      power: supply: axp288_fuel_gauge: Move the AXP20X_CC_CTRL check together with the other checks · 964b3e9b
      Hans de Goede authored 
      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. If not explicitly taken by the I2C-driver,
then this semaphore is automatically taken by the I2C-bus-driver for
each I2C-transfer.

Move the AXP20X_CC_CTRL check done in probe() together with the other
register-accesses done in probe, so that we can take the semaphore once
for the entire set of register-accesses.
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarSebastian Reichel <sebastian.reichel@collabora.com>
      964b3e9b
    • Andrejus Basovas's avatar
      power: supply: axp288_fuel_gauge: Refresh all registers in one go · 394088f0
      Andrejus Basovas authored 
      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.

Userspace power-supply API users typically will read all provided
properties in one go, refreshing the last read values when
power_supply_changed() is called by the driver and/or periodically
(e.g. every 2 minutes).

The reading of all properties in one go causes the P-Unit semaphore
to quickly be taken and released multiple times in a row. Certain
PMIC registers like AXP20X_FG_RES are even used in multiple properties
so they get read multiple times, leading to a P-Unit take + release
each time the register is read.

As already mentioned the taking of the P-Unit semaphore is a quite
expensive operation and it has also been reported that the
"hammering" of the P-Unit semaphore done by the axp288_fuel_gauge
driver can even cause stability issues with the system as a whole.

Switch over to a scheme where the axp288_fuel_gauge driver keeps
a local copy of all the registers which it uses for properties
and make it only refresh its copy of the registers if the values
are older then 1 minute; or when a fuel-gauge interrupt has
triggered since the last read.

This not only reduces the amount of reads, it also makes the code
do all the reads in one go, rather then reading specific registers
based on which property is being queried. This allows calling
iosf_mbi_block_punit_i2c_access() once before doing all the reads,
so that we now only take the P-Unit semaphore once per update.
Tested-by: default avatarAndrejus Basovas <cpp@gcc.lt>
Signed-off-by: default avatarAndrejus Basovas <cpp@gcc.lt>
Co-developed-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarSebastian Reichel <sebastian.reichel@collabora.com>
      394088f0
    • Hans de Goede's avatar
      power: supply: axp288_fuel_gauge: Only read PWR_OP_MODE, FG_LOW_CAP_REG regs once · c371d449
      Hans de Goede authored 
      Accessing registers on the AXP288 is quite expensive, so we should avoid
doing unnecessary accesses.

The FG_LOW_CAP_REG never changes underneath us, so we only need to read
it once. Devices with an AXP288 do not have user-replace (let alone
hot-swappable) batteries and the only bit we care about in the
PWR_OP_MODE register is the CHRG_STAT_BAT_PRESENT bit, so we can get
away with only reading the PWR_OP_MODE register once too.

Note that the FG_LOW_CAP_REG is not marked volatile in the regmap, so we
were effectively already reading it once. This change makes this explicit,
this is done as preparation of a further patch which moves all remaining
register accesses in fuel_gauge_get_property() out of that function.
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarSebastian Reichel <sebastian.reichel@collabora.com>
      c371d449
    • Hans de Goede's avatar
      power: supply: axp288_fuel_gauge: Store struct device pointer in axp288_fg_info · 7eef3e66
      Hans de Goede authored 
      Directly store the struct device pointer in axp288_fg_info, rather then
storing a pointer to the struct platform_device there and then using
"&info->pdev->dev" everywhere.
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarSebastian Reichel <sebastian.reichel@collabora.com>
      7eef3e66
    • Hans de Goede's avatar
      power: supply: axp288_fuel_gauge: Drop retry logic from fuel_gauge_reg_readb() · f17bda7f
      Hans de Goede authored 
      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. This semaphore is automatically taken by the
I2C-bus-driver.

The retry on -EBUSY logic in fuel_gauge_reg_readb() likely was added to
deal with the I2C-bus-drive returning -EBUSY when it failed to take the
semaphore, but this really should never happen. The semaphore code even
has a WARN_ON(ret) to log a kernel backtrace if this does somehow happen,
when this happens something is seriously wrong and the system typically
freezes soon afterwards.

TL;DR: the regmap_read() should never fail with -EBUSY so the retries
are unnecessary.
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarSebastian Reichel <sebastian.reichel@collabora.com>
      f17bda7f
    • Hans de Goede's avatar
      power: supply: axp288_fuel_gauge: Report register-address on readb / writeb errors · caa534c3
      Hans de Goede authored 
      When fuel_gauge_reg_readb()/_writeb() fails, report which register we
were trying to read / write when the error happened.

Also reword the message a bit:
- Drop the axp288 prefix, dev_err() already prints this
- Switch from telegram / abbreviated style to a normal sentence, aligning
  the message with those from fuel_gauge_read_*bit_word()
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarSebastian Reichel <sebastian.reichel@collabora.com>
      caa534c3
    • Hans de Goede's avatar
      power: supply: axp288_fuel_gauge: Silence the chatty IRQ mapping code · 8f6cc48e
      Hans de Goede authored 
      Drop the IRQ mapping messages, because they are really not
interesting at all.
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarSebastian Reichel <sebastian.reichel@collabora.com>
      8f6cc48e
    • Hans de Goede's avatar
      power: supply: axp288_fuel_gauge: Remove debugfs support · fc0db655
      Hans de Goede authored 
      The debugfs code is simply just dumping a bunch of registers, the same
information can also easily be gotten through the regmap debugfs
interface or through the i2cdump utility.

I've not used the debugfs interface once in all these years that I've
been working on the axp288_fuel_gauge driver, so lets just remove it.

Note this also removes the temperature-channels from the list of
IIO ADC channels used by the driver, since these were only used in the
debugfs interface.
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarSebastian Reichel <sebastian.reichel@collabora.com>
      fc0db655
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7