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: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
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: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
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: Andrejus Basovas <cpp@gcc.lt>
Signed-off-by: Andrejus Basovas <cpp@gcc.lt>
Co-developed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
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: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
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: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
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: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
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: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Drop the IRQ mapping messages, because they are really not
interesting at all.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
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: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
The values of various defines used in the driver are not aligned
properly when tabsize is set to 8 (I guess they were created with
a different tabsize).
Properly align the defines to make the code easier to read.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
The continue statement at the end of a for-loop has no effect,
invert the if expression and remove the continue.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
It turns out that the "T3 MRD" DMI_BOARD_NAME value is used in a lot of
different Cherry Trail x5-z8300 / x5-z8350 based Mini-PC / HDMI-stick
models from Ace PC / Meegopad / MinisForum / Wintel (and likely also
other vendors).
Most of the other DMI strings on these boxes unfortunately contain various
generic values like "Default string" or "$(DEFAULT_STRING)", so we cannot
match on them. These devices do have their chassis-type correctly set to a
value of "3" (desktop) which is a pleasant surprise, so also match on that.
This should avoid the quirk accidentally also getting applied to laptops /
tablets (which do actually have a battery). Although in my quite large
database of Bay and Cherry Trail based devices DMIdecode dumps I don't
have any laptops / tables with a board-name of "T3 MRD", so this should
not be an issue.
BugLink: https://askubuntu.com/questions/1206714/how-can-a-mini-pc-be-stopped-from-being-detected-as-a-laptop-with-a-battery/
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
As documented in the updated "Naming" chapter of
Documentation/process/coding-style.rst, usage of the work blacklist
should be avoided where possible.
Rename the list of devices which have no battery to the
axp288_no_battery_list, which also more accurately describes the
contents of the list.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
The Mele PCG03 is another mini PC using the AXP288 PMIC where the EFI
code does not disable the charger part of the PMIC causing us to report
a discharging battery with a random battery charge to userspace.
Add it to the deny-list to avoid the bogus battery status reporting.
Cc: Rasmus Porsager <rasmus@beat.dk>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
The Meegopad T02 is a PC in stick format and doesn't have a battery,
it is reported with a random and constant battery charge but as
discharging to userspace.
Add it to the blacklist to avoid the bogus battery status reporting.
Signed-off-by: Rafael Gandolfi <rafirafi.at@gmail.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
The Intel Compute Stick `STK1A32SC` can have a system vendor of
"Intel(R) Client Systems".
Broaden the Intel Compute Stick DMI checks so that they match "Intel
Corporation" as well as "Intel(R) Client Systems".
This fixes an issue where the STK1A32SC compute sticks were still
exposing a battery with the existing blacklist entry.
Signed-off-by: Jeffery Miller <jmiller@neverware.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
The Minix Neo Z83-4 is another mini PC using the AXP288 PMIC where the
EFI code does not disable the charger part of the PMIC causing us to report
battery readings (of always 100%) to userspace even though there is no
battery in this wall-outlet powered device.
Add it to the blacklist to avoid the bogus battery status reporting.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
The blacklist is getting big enough that it is good to have some sort
of fixed order for it, sort it alphabetically.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Based on 1 normalized pattern(s):
this program is free software you can redistribute it and or modify
it under the terms of the gnu general public license as published by
the free software foundation version 2 of the license this program
is distributed in the hope that it will be useful but without any
warranty without even the implied warranty of merchantability or
fitness for a particular purpose see the gnu general public license
for more details
extracted by the scancode license scanner the SPDX license identifier
GPL-2.0-only
has been chosen to replace the boilerplate/reference in 100 file(s).
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Alexios Zavras <alexios.zavras@intel.com>
Reviewed-by: Allison Randal <allison@lohutok.net>
Cc: linux-spdx@vger.kernel.org
Link: https://lkml.kernel.org/r/20190529141900.918357685@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The ACEPC T8 and T11 Cherry Trail Z8350 mini PCs use an AXP288 and as PCs,
rather then portables, they does not have a battery. Still for some
reason the AXP288 not only thinks there is a battery, it actually
thinks it is discharging while the PC is running, slowly going to
0% full, causing userspace to shutdown the system due to the battery
being critically low after a while.
This commit adds the ACEPC T8 and T11 to the axp288 fuel-gauge driver
blacklist, so that we stop reporting bogus battery readings on this device.
BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1690852
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Use DEFINE_SHOW_ATTRIBUTE macro to simplify the code.
Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Userspace class/power_supply consumers such as upower, already know some
supplies need to be polled to get up2date info. Doing this in the kernel
and then waking up userspace just causes unnecessary wakeups and i2c
transfers.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
Some devices with an AXP288 PMIC do not have a battery at all, or use
external charger and fuelgauge ICs instead of the AXP288 builtin
functionality.
On such devices we should not bind to the fuelgauge function to avoid
exporting a non working power_supply class device.
This also avoids the following errors repeating over and over again in
dmesg:
axp288_fuel_gauge axp288_fuel_gauge: capacity measurement not valid
axp288_fuel_gauge axp288_fuel_gauge: Error 0xe2 contents not valid
power_supply axp288_fuel_gauge: driver failed to report 'charge_now'
property: -6
Cc: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
Commit 2b5a4b4bf2 ("power: supply: axp288_fuel_gauge: Rework
get_status()"), switched from 0A current detection to using the capacity
register for full detection.
It turns out this fixes full reporting on some devices which keep trickle
charging long after the capacity register reach 100%, but breaks it on
some other devices where the charger stops charging before the capacity
register reaches 100%. This commit fixes this by also checking for
0A current when the reported capacity is above 90%.
Fixes: 2b5a4b4bf2 ("psy: axp288_fuel_gauge: Rework get_status()")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
The ECS EF20EA laptop ships an AXP288 but it is actually using a
different, separate FG chip for AC and battery monitoring. On this
laptop we need to keep using the regular ACPI driver and disable the
AXP288 FG to avoid reporting two batteries to userspace.
Signed-off-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
The Intel Compute Stick (Cherry Trail version) and the Meegopad T08 HDMI
stick, both use an axp288 PMIC. They also both have this wired up in such
a way that the detection logic in the PMIC claims that a valid battery is
present, resuling in GNOME and KDE showing a full-battery in their status
bar and power-settings, while these devices do not have a battery.
For lack of a better fix add a DMI blacklist and do not register the
axp288_fuel_gauge psy on devices on the blacklist.
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
First check the discharge current, and when that is non 0 use that without
also checking the charge current (which will be 0 then). This makes
get_current() do only 1 i2c read instead of 2 when on battery.
This is esp. important given the pmic i2c bus mutex stuff used on boards
with an axp288 because the SoC's own punit also may access the axp288,
which makes i2c accesses more expensive then normal.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
Relying on the (dis)charge current reporting for reporting FULL back to
userspace does not work really well and often leads to the reported status
getting stuck at e.g. 98/99% (the fuelgauge is not perfect) for hours.
What happens is that when the battery is full the axp288 keeps charging it
with a very low current. Until it is really really full and once really
really full, some inaccuracies in the adc lead to it then sometimes
reporting a small discharging rate, even though an external pwr source is
used. So we end up with a status of "charging" for hours after the battery
is actually already full and sometimes this then flip-flops to discharging.
This commit fixes this by first checking if a valid Vbus is present and if
it is present using the fuel-gauge's reported percentage to check for a
full battery.
This commit also changes how get_status() determines if the battery is
charging or discharging when not reporting it as full. We still use the
current direction for this, but instead of reading 4 extra registers for
this (2 16 bit regs), simplify things by using the current-direction bit
in the power-status register, which already gets read anyways.
This also reduces the amount of i2c reads to 1 when on battery and 2
when a valid Vbus is present.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
Get iio-channels once during boot, delaying the probe if the axp288_adc
drivers has not loaded yet, instead of getting them on demand each time
we need them.
This fixes the following errors in dmesg:
axp288_fuel_gauge axp288_fuel_gauge: ADC charge current read failed:-19
Which were caused by the ondemand iio-channel read code not finding the
channel when the axp288_adc driver had not loaded yet.
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
Setting the irq_enable bits is taken care of by the irq chip when we
request the irqs and the driver should not be meddling with the
irq?_en registers itself.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sre@kernel.org>
In order for the MSB -> LSB latching to work correctly we must read the
2 8 bit registers of a 12 bit value in one consecutive read.
This fixes voltage_ocv reporting inconsistent values on my tablet.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sre@kernel.org>
In order for the MSB -> LSB latching to work correctly we must read the
2 8 bit registers of a 15 bit value in one consecutive read.
This fixes charge_full reporting 3498768 on some reads and 3354624 one
other reads on my tablet (for the 3354624 value the raw LSB is 0x00).
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sre@kernel.org>
If reading the register fails, return the actual error code, instead
of the uninitialized val variable;
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sre@kernel.org>
When the axp288_faul_gauge driver was originally merged, it was
merged with a dependency on some other driver providing platform
data for it.
However the battery-data-framework which should provide that data
never got merged, resulting in x86 tablets / laptops with an axp288
having no working battery monitor, as before this commit the driver
would simply return -ENODEV if there is no platform data.
This commit removes the dependency on the platform_data instead
checking that the firmware has initialized the fuel-gauge and
reading the info back from the pmic.
What is missing from the read-back info is the table to map raw adc
values to temperature, so this commit drops the temperature and
temperature limits properties. The min voltage, charge design and
model name info is also missing. Note that none of these are really
important for userspace to have.
All other functionality is preserved and actually made available
by this commit.
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=88471
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sre@kernel.org>
If the driver is built as a module, autoload won't work because the module
alias information is not filled. So user-space can't match the registered
device with the corresponding module.
Export the module alias information using the MODULE_DEVICE_TABLE() macro.
Before this patch:
$ modinfo drivers/power/supply/axp288_fuel_gauge.ko | grep alias
$
After this patch:
$ modinfo drivers/power/supply/axp288_fuel_gauge.ko | grep alias
alias: platform:axp288_fuel_gauge
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Signed-off-by: Sebastian Reichel <sre@kernel.org>
This moves all power supply drivers from drivers/power/
to drivers/power/supply/. The intention is a cleaner
source tree, since drivers/power/ also contains frameworks
unrelated to power supply, like adaptive voltage scaling.
Signed-off-by: Sebastian Reichel <sre@kernel.org>
Hans de Goede