All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/10] power: supply: axp288_fuel_gauge: Reduce number of register accesses + cleanups
@ 2021-07-30  9:55 Hans de Goede
  2021-07-30  9:55 ` [PATCH v2 01/10] power: supply: axp288_fuel_gauge: Fix define alignment Hans de Goede
                   ` (9 more replies)
  0 siblings, 10 replies; 17+ messages in thread
From: Hans de Goede @ 2021-07-30  9:55 UTC (permalink / raw)
  To: Sebastian Reichel; +Cc: Hans de Goede, Andrejus Basovas, linux-pm

Changes in v2:
- Add a "depends on X86" to Kconfig since the iosf_mbi functions are X86 only
  (the AXP288 PMIC is only used on X86 devices)

And here is the v1 cover-letter again:

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.

To ensure that no unguarded I2C-bus accesses happen, the semaphore is
taken by the I2C-bus-driver for every I2C transfer. When upower refreshes
its battery stats it reads all the power-supply properties at once,
leading to the semaphore getting hammered which sometimes causes the
system to hang.

Andrejus maintains a large "fleet" of affected Cherry Trail tablets
and was seeing these hangs semi regularly. After discussing this with
me Andrejus wrote the caching patch in this series which greatly reduces
the number of semaphore accesses and since then there have been no
reports of hangs in the fleet of devices which he maintains.

I've cleaned up Andrejus work a bit before submitting it upstream and
while working on this I found a slew of other issues in this driver
which bugged me enough to write a bunch of cleanup patches. I've also
added some extra patches to also reduce the semaphore use during driver
probe.

Regards,

Hans


Andrejus Basovas (1):
  power: supply: axp288_fuel_gauge: Refresh all registers in one go

Hans de Goede (9):
  power: supply: axp288_fuel_gauge: Fix define alignment
  power: supply: axp288_fuel_gauge: Remove debugfs support
  power: supply: axp288_fuel_gauge: Silence the chatty IRQ mapping code
  power: supply: axp288_fuel_gauge: Report register-address on readb /
    writeb errors
  power: supply: axp288_fuel_gauge: Drop retry logic from
    fuel_gauge_reg_readb()
  power: supply: axp288_fuel_gauge: Store struct device pointer in
    axp288_fg_info
  power: supply: axp288_fuel_gauge: Only read PWR_OP_MODE,
    FG_LOW_CAP_REG regs once
  power: supply: axp288_fuel_gauge: Move the AXP20X_CC_CTRL check
    together with the other checks
  power: supply: axp288_fuel_gauge: Take the P-Unit semaphore only once
    during probe()

 drivers/power/supply/Kconfig             |   2 +-
 drivers/power/supply/axp288_fuel_gauge.c | 489 +++++++++--------------
 2 files changed, 187 insertions(+), 304 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 01/10] power: supply: axp288_fuel_gauge: Fix define alignment
  2021-07-30  9:55 [PATCH v2 00/10] power: supply: axp288_fuel_gauge: Reduce number of register accesses + cleanups Hans de Goede
@ 2021-07-30  9:55 ` Hans de Goede
  2021-07-30  9:55 ` [PATCH v2 02/10] power: supply: axp288_fuel_gauge: Remove debugfs support Hans de Goede
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Hans de Goede @ 2021-07-30  9:55 UTC (permalink / raw)
  To: Sebastian Reichel; +Cc: Hans de Goede, Andrejus Basovas, linux-pm

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>
---
 drivers/power/supply/axp288_fuel_gauge.c | 38 ++++++++++++------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/power/supply/axp288_fuel_gauge.c b/drivers/power/supply/axp288_fuel_gauge.c
index 2ba2d8d6b8e6..99928789040d 100644
--- a/drivers/power/supply/axp288_fuel_gauge.c
+++ b/drivers/power/supply/axp288_fuel_gauge.c
@@ -23,34 +23,34 @@
 #include <linux/seq_file.h>
 #include <asm/unaligned.h>
 
-#define PS_STAT_VBUS_TRIGGER		(1 << 0)
-#define PS_STAT_BAT_CHRG_DIR		(1 << 2)
-#define PS_STAT_VBAT_ABOVE_VHOLD	(1 << 3)
-#define PS_STAT_VBUS_VALID		(1 << 4)
-#define PS_STAT_VBUS_PRESENT		(1 << 5)
+#define PS_STAT_VBUS_TRIGGER			(1 << 0)
+#define PS_STAT_BAT_CHRG_DIR			(1 << 2)
+#define PS_STAT_VBAT_ABOVE_VHOLD		(1 << 3)
+#define PS_STAT_VBUS_VALID			(1 << 4)
+#define PS_STAT_VBUS_PRESENT			(1 << 5)
 
-#define CHRG_STAT_BAT_SAFE_MODE		(1 << 3)
+#define CHRG_STAT_BAT_SAFE_MODE			(1 << 3)
 #define CHRG_STAT_BAT_VALID			(1 << 4)
-#define CHRG_STAT_BAT_PRESENT		(1 << 5)
+#define CHRG_STAT_BAT_PRESENT			(1 << 5)
 #define CHRG_STAT_CHARGING			(1 << 6)
 #define CHRG_STAT_PMIC_OTP			(1 << 7)
 
 #define CHRG_CCCV_CC_MASK			0xf     /* 4 bits */
-#define CHRG_CCCV_CC_BIT_POS		0
+#define CHRG_CCCV_CC_BIT_POS			0
 #define CHRG_CCCV_CC_OFFSET			200     /* 200mA */
-#define CHRG_CCCV_CC_LSB_RES		200     /* 200mA */
+#define CHRG_CCCV_CC_LSB_RES			200     /* 200mA */
 #define CHRG_CCCV_ITERM_20P			(1 << 4)    /* 20% of CC */
 #define CHRG_CCCV_CV_MASK			0x60        /* 2 bits */
-#define CHRG_CCCV_CV_BIT_POS		5
+#define CHRG_CCCV_CV_BIT_POS			5
 #define CHRG_CCCV_CV_4100MV			0x0     /* 4.10V */
 #define CHRG_CCCV_CV_4150MV			0x1     /* 4.15V */
 #define CHRG_CCCV_CV_4200MV			0x2     /* 4.20V */
 #define CHRG_CCCV_CV_4350MV			0x3     /* 4.35V */
 #define CHRG_CCCV_CHG_EN			(1 << 7)
 
-#define FG_CNTL_OCV_ADJ_STAT		(1 << 2)
+#define FG_CNTL_OCV_ADJ_STAT			(1 << 2)
 #define FG_CNTL_OCV_ADJ_EN			(1 << 3)
-#define FG_CNTL_CAP_ADJ_STAT		(1 << 4)
+#define FG_CNTL_CAP_ADJ_STAT			(1 << 4)
 #define FG_CNTL_CAP_ADJ_EN			(1 << 5)
 #define FG_CNTL_CC_EN				(1 << 6)
 #define FG_CNTL_GAUGE_EN			(1 << 7)
@@ -71,23 +71,23 @@
 #define FG_CC_CAP_VALID				(1 << 7)
 #define FG_CC_CAP_VAL_MASK			0x7F
 
-#define FG_LOW_CAP_THR1_MASK		0xf0    /* 5% tp 20% */
+#define FG_LOW_CAP_THR1_MASK			0xf0    /* 5% tp 20% */
 #define FG_LOW_CAP_THR1_VAL			0xa0    /* 15 perc */
-#define FG_LOW_CAP_THR2_MASK		0x0f    /* 0% to 15% */
+#define FG_LOW_CAP_THR2_MASK			0x0f    /* 0% to 15% */
 #define FG_LOW_CAP_WARN_THR			14  /* 14 perc */
 #define FG_LOW_CAP_CRIT_THR			4   /* 4 perc */
 #define FG_LOW_CAP_SHDN_THR			0   /* 0 perc */
 
-#define NR_RETRY_CNT    3
-#define DEV_NAME	"axp288_fuel_gauge"
+#define NR_RETRY_CNT				3
+#define DEV_NAME				"axp288_fuel_gauge"
 
 /* 1.1mV per LSB expressed in uV */
 #define VOLTAGE_FROM_ADC(a)			((a * 11) / 10)
 /* properties converted to uV, uA */
-#define PROP_VOLT(a)		((a) * 1000)
-#define PROP_CURR(a)		((a) * 1000)
+#define PROP_VOLT(a)				((a) * 1000)
+#define PROP_CURR(a)				((a) * 1000)
 
-#define AXP288_FG_INTR_NUM	6
+#define AXP288_FG_INTR_NUM			6
 enum {
 	QWBTU_IRQ = 0,
 	WBTU_IRQ,
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 02/10] power: supply: axp288_fuel_gauge: Remove debugfs support
  2021-07-30  9:55 [PATCH v2 00/10] power: supply: axp288_fuel_gauge: Reduce number of register accesses + cleanups Hans de Goede
  2021-07-30  9:55 ` [PATCH v2 01/10] power: supply: axp288_fuel_gauge: Fix define alignment Hans de Goede
@ 2021-07-30  9:55 ` Hans de Goede
  2021-07-30  9:56 ` [PATCH v2 03/10] power: supply: axp288_fuel_gauge: Silence the chatty IRQ mapping code Hans de Goede
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Hans de Goede @ 2021-07-30  9:55 UTC (permalink / raw)
  To: Sebastian Reichel; +Cc: Hans de Goede, Andrejus Basovas, linux-pm

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>
---
 drivers/power/supply/axp288_fuel_gauge.c | 123 -----------------------
 1 file changed, 123 deletions(-)

diff --git a/drivers/power/supply/axp288_fuel_gauge.c b/drivers/power/supply/axp288_fuel_gauge.c
index 99928789040d..d189849564db 100644
--- a/drivers/power/supply/axp288_fuel_gauge.c
+++ b/drivers/power/supply/axp288_fuel_gauge.c
@@ -19,8 +19,6 @@
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
 #include <linux/iio/consumer.h>
-#include <linux/debugfs.h>
-#include <linux/seq_file.h>
 #include <asm/unaligned.h>
 
 #define PS_STAT_VBUS_TRIGGER			(1 << 0)
@@ -98,9 +96,6 @@ enum {
 };
 
 enum {
-	BAT_TEMP = 0,
-	PMIC_TEMP,
-	SYSTEM_TEMP,
 	BAT_CHRG_CURR,
 	BAT_D_CURR,
 	BAT_VOLT,
@@ -204,119 +199,6 @@ static int fuel_gauge_read_12bit_word(struct axp288_fg_info *info, int reg)
 	return (buf[0] << 4) | ((buf[1] >> 4) & 0x0f);
 }
 
-#ifdef CONFIG_DEBUG_FS
-static int fuel_gauge_debug_show(struct seq_file *s, void *data)
-{
-	struct axp288_fg_info *info = s->private;
-	int raw_val, ret;
-
-	seq_printf(s, " PWR_STATUS[%02x] : %02x\n",
-		AXP20X_PWR_INPUT_STATUS,
-		fuel_gauge_reg_readb(info, AXP20X_PWR_INPUT_STATUS));
-	seq_printf(s, "PWR_OP_MODE[%02x] : %02x\n",
-		AXP20X_PWR_OP_MODE,
-		fuel_gauge_reg_readb(info, AXP20X_PWR_OP_MODE));
-	seq_printf(s, " CHRG_CTRL1[%02x] : %02x\n",
-		AXP20X_CHRG_CTRL1,
-		fuel_gauge_reg_readb(info, AXP20X_CHRG_CTRL1));
-	seq_printf(s, "       VLTF[%02x] : %02x\n",
-		AXP20X_V_LTF_DISCHRG,
-		fuel_gauge_reg_readb(info, AXP20X_V_LTF_DISCHRG));
-	seq_printf(s, "       VHTF[%02x] : %02x\n",
-		AXP20X_V_HTF_DISCHRG,
-		fuel_gauge_reg_readb(info, AXP20X_V_HTF_DISCHRG));
-	seq_printf(s, "    CC_CTRL[%02x] : %02x\n",
-		AXP20X_CC_CTRL,
-		fuel_gauge_reg_readb(info, AXP20X_CC_CTRL));
-	seq_printf(s, "BATTERY CAP[%02x] : %02x\n",
-		AXP20X_FG_RES,
-		fuel_gauge_reg_readb(info, AXP20X_FG_RES));
-	seq_printf(s, "    FG_RDC1[%02x] : %02x\n",
-		AXP288_FG_RDC1_REG,
-		fuel_gauge_reg_readb(info, AXP288_FG_RDC1_REG));
-	seq_printf(s, "    FG_RDC0[%02x] : %02x\n",
-		AXP288_FG_RDC0_REG,
-		fuel_gauge_reg_readb(info, AXP288_FG_RDC0_REG));
-	seq_printf(s, "     FG_OCV[%02x] : %04x\n",
-		AXP288_FG_OCVH_REG,
-		fuel_gauge_read_12bit_word(info, AXP288_FG_OCVH_REG));
-	seq_printf(s, " FG_DES_CAP[%02x] : %04x\n",
-		AXP288_FG_DES_CAP1_REG,
-		fuel_gauge_read_15bit_word(info, AXP288_FG_DES_CAP1_REG));
-	seq_printf(s, "  FG_CC_MTR[%02x] : %04x\n",
-		AXP288_FG_CC_MTR1_REG,
-		fuel_gauge_read_15bit_word(info, AXP288_FG_CC_MTR1_REG));
-	seq_printf(s, " FG_OCV_CAP[%02x] : %02x\n",
-		AXP288_FG_OCV_CAP_REG,
-		fuel_gauge_reg_readb(info, AXP288_FG_OCV_CAP_REG));
-	seq_printf(s, "  FG_CC_CAP[%02x] : %02x\n",
-		AXP288_FG_CC_CAP_REG,
-		fuel_gauge_reg_readb(info, AXP288_FG_CC_CAP_REG));
-	seq_printf(s, " FG_LOW_CAP[%02x] : %02x\n",
-		AXP288_FG_LOW_CAP_REG,
-		fuel_gauge_reg_readb(info, AXP288_FG_LOW_CAP_REG));
-	seq_printf(s, "TUNING_CTL0[%02x] : %02x\n",
-		AXP288_FG_TUNE0,
-		fuel_gauge_reg_readb(info, AXP288_FG_TUNE0));
-	seq_printf(s, "TUNING_CTL1[%02x] : %02x\n",
-		AXP288_FG_TUNE1,
-		fuel_gauge_reg_readb(info, AXP288_FG_TUNE1));
-	seq_printf(s, "TUNING_CTL2[%02x] : %02x\n",
-		AXP288_FG_TUNE2,
-		fuel_gauge_reg_readb(info, AXP288_FG_TUNE2));
-	seq_printf(s, "TUNING_CTL3[%02x] : %02x\n",
-		AXP288_FG_TUNE3,
-		fuel_gauge_reg_readb(info, AXP288_FG_TUNE3));
-	seq_printf(s, "TUNING_CTL4[%02x] : %02x\n",
-		AXP288_FG_TUNE4,
-		fuel_gauge_reg_readb(info, AXP288_FG_TUNE4));
-	seq_printf(s, "TUNING_CTL5[%02x] : %02x\n",
-		AXP288_FG_TUNE5,
-		fuel_gauge_reg_readb(info, AXP288_FG_TUNE5));
-
-	ret = iio_read_channel_raw(info->iio_channel[BAT_TEMP], &raw_val);
-	if (ret >= 0)
-		seq_printf(s, "axp288-batttemp : %d\n", raw_val);
-	ret = iio_read_channel_raw(info->iio_channel[PMIC_TEMP], &raw_val);
-	if (ret >= 0)
-		seq_printf(s, "axp288-pmictemp : %d\n", raw_val);
-	ret = iio_read_channel_raw(info->iio_channel[SYSTEM_TEMP], &raw_val);
-	if (ret >= 0)
-		seq_printf(s, "axp288-systtemp : %d\n", raw_val);
-	ret = iio_read_channel_raw(info->iio_channel[BAT_CHRG_CURR], &raw_val);
-	if (ret >= 0)
-		seq_printf(s, "axp288-chrgcurr : %d\n", raw_val);
-	ret = iio_read_channel_raw(info->iio_channel[BAT_D_CURR], &raw_val);
-	if (ret >= 0)
-		seq_printf(s, "axp288-dchrgcur : %d\n", raw_val);
-	ret = iio_read_channel_raw(info->iio_channel[BAT_VOLT], &raw_val);
-	if (ret >= 0)
-		seq_printf(s, "axp288-battvolt : %d\n", raw_val);
-
-	return 0;
-}
-
-DEFINE_SHOW_ATTRIBUTE(fuel_gauge_debug);
-
-static void fuel_gauge_create_debugfs(struct axp288_fg_info *info)
-{
-	info->debug_file = debugfs_create_file("fuelgauge", 0666, NULL,
-		info, &fuel_gauge_debug_fops);
-}
-
-static void fuel_gauge_remove_debugfs(struct axp288_fg_info *info)
-{
-	debugfs_remove(info->debug_file);
-}
-#else
-static inline void fuel_gauge_create_debugfs(struct axp288_fg_info *info)
-{
-}
-static inline void fuel_gauge_remove_debugfs(struct axp288_fg_info *info)
-{
-}
-#endif
-
 static void fuel_gauge_get_status(struct axp288_fg_info *info)
 {
 	int pwr_stat, fg_res, curr, ret;
@@ -753,9 +635,6 @@ static int axp288_fuel_gauge_probe(struct platform_device *pdev)
 	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
 	struct power_supply_config psy_cfg = {};
 	static const char * const iio_chan_name[] = {
-		[BAT_TEMP] = "axp288-batt-temp",
-		[PMIC_TEMP] = "axp288-pmic-temp",
-		[SYSTEM_TEMP] = "axp288-system-temp",
 		[BAT_CHRG_CURR] = "axp288-chrg-curr",
 		[BAT_D_CURR] = "axp288-chrg-d-curr",
 		[BAT_VOLT] = "axp288-batt-volt",
@@ -844,7 +723,6 @@ static int axp288_fuel_gauge_probe(struct platform_device *pdev)
 		goto out_free_iio_chan;
 	}
 
-	fuel_gauge_create_debugfs(info);
 	fuel_gauge_init_irq(info);
 
 	return 0;
@@ -869,7 +747,6 @@ static int axp288_fuel_gauge_remove(struct platform_device *pdev)
 	int i;
 
 	power_supply_unregister(info->bat);
-	fuel_gauge_remove_debugfs(info);
 
 	for (i = 0; i < AXP288_FG_INTR_NUM; i++)
 		if (info->irq[i] >= 0)
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 03/10] power: supply: axp288_fuel_gauge: Silence the chatty IRQ mapping code
  2021-07-30  9:55 [PATCH v2 00/10] power: supply: axp288_fuel_gauge: Reduce number of register accesses + cleanups Hans de Goede
  2021-07-30  9:55 ` [PATCH v2 01/10] power: supply: axp288_fuel_gauge: Fix define alignment Hans de Goede
  2021-07-30  9:55 ` [PATCH v2 02/10] power: supply: axp288_fuel_gauge: Remove debugfs support Hans de Goede
@ 2021-07-30  9:56 ` Hans de Goede
  2021-07-30  9:56 ` [PATCH v2 04/10] power: supply: axp288_fuel_gauge: Report register-address on readb / writeb errors Hans de Goede
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Hans de Goede @ 2021-07-30  9:56 UTC (permalink / raw)
  To: Sebastian Reichel; +Cc: Hans de Goede, Andrejus Basovas, linux-pm

Drop the IRQ mapping messages, because they are really not
interesting at all.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/axp288_fuel_gauge.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/power/supply/axp288_fuel_gauge.c b/drivers/power/supply/axp288_fuel_gauge.c
index d189849564db..43cc171101f1 100644
--- a/drivers/power/supply/axp288_fuel_gauge.c
+++ b/drivers/power/supply/axp288_fuel_gauge.c
@@ -537,9 +537,6 @@ static void fuel_gauge_init_irq(struct axp288_fg_info *info)
 				pirq, info->irq[i]);
 			info->irq[i] = -1;
 			goto intr_failed;
-		} else {
-			dev_info(&info->pdev->dev, "HW IRQ %d -> VIRQ %d\n",
-				pirq, info->irq[i]);
 		}
 	}
 	return;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 04/10] power: supply: axp288_fuel_gauge: Report register-address on readb / writeb errors
  2021-07-30  9:55 [PATCH v2 00/10] power: supply: axp288_fuel_gauge: Reduce number of register accesses + cleanups Hans de Goede
                   ` (2 preceding siblings ...)
  2021-07-30  9:56 ` [PATCH v2 03/10] power: supply: axp288_fuel_gauge: Silence the chatty IRQ mapping code Hans de Goede
@ 2021-07-30  9:56 ` Hans de Goede
  2021-07-30  9:56 ` [PATCH v2 05/10] power: supply: axp288_fuel_gauge: Drop retry logic from fuel_gauge_reg_readb() Hans de Goede
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Hans de Goede @ 2021-07-30  9:56 UTC (permalink / raw)
  To: Sebastian Reichel; +Cc: Hans de Goede, Andrejus Basovas, linux-pm

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>
---
 drivers/power/supply/axp288_fuel_gauge.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/axp288_fuel_gauge.c b/drivers/power/supply/axp288_fuel_gauge.c
index 43cc171101f1..796153caf5e0 100644
--- a/drivers/power/supply/axp288_fuel_gauge.c
+++ b/drivers/power/supply/axp288_fuel_gauge.c
@@ -142,7 +142,7 @@ static int fuel_gauge_reg_readb(struct axp288_fg_info *info, int reg)
 	}
 
 	if (ret < 0) {
-		dev_err(&info->pdev->dev, "axp288 reg read err:%d\n", ret);
+		dev_err(&info->pdev->dev, "Error reading reg 0x%02x err: %d\n", reg, ret);
 		return ret;
 	}
 
@@ -156,7 +156,7 @@ static int fuel_gauge_reg_writeb(struct axp288_fg_info *info, int reg, u8 val)
 	ret = regmap_write(info->regmap, reg, (unsigned int)val);
 
 	if (ret < 0)
-		dev_err(&info->pdev->dev, "axp288 reg write err:%d\n", ret);
+		dev_err(&info->pdev->dev, "Error writing reg 0x%02x err: %d\n", reg, ret);
 
 	return ret;
 }
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 05/10] power: supply: axp288_fuel_gauge: Drop retry logic from fuel_gauge_reg_readb()
  2021-07-30  9:55 [PATCH v2 00/10] power: supply: axp288_fuel_gauge: Reduce number of register accesses + cleanups Hans de Goede
                   ` (3 preceding siblings ...)
  2021-07-30  9:56 ` [PATCH v2 04/10] power: supply: axp288_fuel_gauge: Report register-address on readb / writeb errors Hans de Goede
@ 2021-07-30  9:56 ` Hans de Goede
  2021-07-30  9:56 ` [PATCH v2 06/10] power: supply: axp288_fuel_gauge: Store struct device pointer in axp288_fg_info Hans de Goede
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Hans de Goede @ 2021-07-30  9:56 UTC (permalink / raw)
  To: Sebastian Reichel; +Cc: Hans de Goede, Andrejus Basovas, linux-pm

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>
---
 drivers/power/supply/axp288_fuel_gauge.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/power/supply/axp288_fuel_gauge.c b/drivers/power/supply/axp288_fuel_gauge.c
index 796153caf5e0..d58a1f81fcea 100644
--- a/drivers/power/supply/axp288_fuel_gauge.c
+++ b/drivers/power/supply/axp288_fuel_gauge.c
@@ -76,7 +76,6 @@
 #define FG_LOW_CAP_CRIT_THR			4   /* 4 perc */
 #define FG_LOW_CAP_SHDN_THR			0   /* 0 perc */
 
-#define NR_RETRY_CNT				3
 #define DEV_NAME				"axp288_fuel_gauge"
 
 /* 1.1mV per LSB expressed in uV */
@@ -132,15 +131,10 @@ static enum power_supply_property fuel_gauge_props[] = {
 
 static int fuel_gauge_reg_readb(struct axp288_fg_info *info, int reg)
 {
-	int ret, i;
 	unsigned int val;
+	int ret;
 
-	for (i = 0; i < NR_RETRY_CNT; i++) {
-		ret = regmap_read(info->regmap, reg, &val);
-		if (ret != -EBUSY)
-			break;
-	}
-
+	ret = regmap_read(info->regmap, reg, &val);
 	if (ret < 0) {
 		dev_err(&info->pdev->dev, "Error reading reg 0x%02x err: %d\n", reg, ret);
 		return ret;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 06/10] power: supply: axp288_fuel_gauge: Store struct device pointer in axp288_fg_info
  2021-07-30  9:55 [PATCH v2 00/10] power: supply: axp288_fuel_gauge: Reduce number of register accesses + cleanups Hans de Goede
                   ` (4 preceding siblings ...)
  2021-07-30  9:56 ` [PATCH v2 05/10] power: supply: axp288_fuel_gauge: Drop retry logic from fuel_gauge_reg_readb() Hans de Goede
@ 2021-07-30  9:56 ` Hans de Goede
  2021-07-30  9:56 ` [PATCH v2 07/10] power: supply: axp288_fuel_gauge: Only read PWR_OP_MODE, FG_LOW_CAP_REG regs once Hans de Goede
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Hans de Goede @ 2021-07-30  9:56 UTC (permalink / raw)
  To: Sebastian Reichel; +Cc: Hans de Goede, Andrejus Basovas, linux-pm

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>
---
 drivers/power/supply/axp288_fuel_gauge.c | 59 ++++++++++--------------
 1 file changed, 24 insertions(+), 35 deletions(-)

diff --git a/drivers/power/supply/axp288_fuel_gauge.c b/drivers/power/supply/axp288_fuel_gauge.c
index d58a1f81fcea..1366027edf49 100644
--- a/drivers/power/supply/axp288_fuel_gauge.c
+++ b/drivers/power/supply/axp288_fuel_gauge.c
@@ -102,7 +102,7 @@ enum {
 };
 
 struct axp288_fg_info {
-	struct platform_device *pdev;
+	struct device *dev;
 	struct regmap *regmap;
 	struct regmap_irq_chip_data *regmap_irqc;
 	int irq[AXP288_FG_INTR_NUM];
@@ -136,7 +136,7 @@ static int fuel_gauge_reg_readb(struct axp288_fg_info *info, int reg)
 
 	ret = regmap_read(info->regmap, reg, &val);
 	if (ret < 0) {
-		dev_err(&info->pdev->dev, "Error reading reg 0x%02x err: %d\n", reg, ret);
+		dev_err(info->dev, "Error reading reg 0x%02x err: %d\n", reg, ret);
 		return ret;
 	}
 
@@ -150,7 +150,7 @@ static int fuel_gauge_reg_writeb(struct axp288_fg_info *info, int reg, u8 val)
 	ret = regmap_write(info->regmap, reg, (unsigned int)val);
 
 	if (ret < 0)
-		dev_err(&info->pdev->dev, "Error writing reg 0x%02x err: %d\n", reg, ret);
+		dev_err(info->dev, "Error writing reg 0x%02x err: %d\n", reg, ret);
 
 	return ret;
 }
@@ -162,15 +162,13 @@ static int fuel_gauge_read_15bit_word(struct axp288_fg_info *info, int reg)
 
 	ret = regmap_bulk_read(info->regmap, reg, buf, 2);
 	if (ret < 0) {
-		dev_err(&info->pdev->dev, "Error reading reg 0x%02x err: %d\n",
-			reg, ret);
+		dev_err(info->dev, "Error reading reg 0x%02x err: %d\n", reg, ret);
 		return ret;
 	}
 
 	ret = get_unaligned_be16(buf);
 	if (!(ret & FG_15BIT_WORD_VALID)) {
-		dev_err(&info->pdev->dev, "Error reg 0x%02x contents not valid\n",
-			reg);
+		dev_err(info->dev, "Error reg 0x%02x contents not valid\n", reg);
 		return -ENXIO;
 	}
 
@@ -184,8 +182,7 @@ static int fuel_gauge_read_12bit_word(struct axp288_fg_info *info, int reg)
 
 	ret = regmap_bulk_read(info->regmap, reg, buf, 2);
 	if (ret < 0) {
-		dev_err(&info->pdev->dev, "Error reading reg 0x%02x err: %d\n",
-			reg, ret);
+		dev_err(info->dev, "Error reading reg 0x%02x err: %d\n", reg, ret);
 		return ret;
 	}
 
@@ -199,8 +196,7 @@ static void fuel_gauge_get_status(struct axp288_fg_info *info)
 
 	pwr_stat = fuel_gauge_reg_readb(info, AXP20X_PWR_INPUT_STATUS);
 	if (pwr_stat < 0) {
-		dev_err(&info->pdev->dev,
-			"PWR STAT read failed:%d\n", pwr_stat);
+		dev_err(info->dev, "PWR STAT read failed: %d\n", pwr_stat);
 		return;
 	}
 
@@ -210,7 +206,7 @@ static void fuel_gauge_get_status(struct axp288_fg_info *info)
 
 	fg_res = fuel_gauge_reg_readb(info, AXP20X_FG_RES);
 	if (fg_res < 0) {
-		dev_err(&info->pdev->dev, "FG RES read failed: %d\n", fg_res);
+		dev_err(info->dev, "FG RES read failed: %d\n", fg_res);
 		return;
 	}
 	if (!(fg_res & FG_REP_CAP_VALID))
@@ -232,7 +228,7 @@ static void fuel_gauge_get_status(struct axp288_fg_info *info)
 
 	ret = iio_read_channel_raw(info->iio_channel[BAT_D_CURR], &curr);
 	if (ret < 0) {
-		dev_err(&info->pdev->dev, "FG get current failed: %d\n", ret);
+		dev_err(info->dev, "FG get current failed: %d\n", ret);
 		return;
 	}
 	if (curr == 0) {
@@ -355,8 +351,7 @@ static int fuel_gauge_get_property(struct power_supply *ps,
 			goto fuel_gauge_read_err;
 
 		if (!(ret & FG_REP_CAP_VALID))
-			dev_err(&info->pdev->dev,
-				"capacity measurement not valid\n");
+			dev_err(info->dev, "capacity measurement not valid\n");
 		val->intval = (ret & FG_REP_CAP_VAL_MASK);
 		break;
 	case POWER_SUPPLY_PROP_CAPACITY_ALERT_MIN:
@@ -455,35 +450,31 @@ static irqreturn_t fuel_gauge_thread_handler(int irq, void *dev)
 	}
 
 	if (i >= AXP288_FG_INTR_NUM) {
-		dev_warn(&info->pdev->dev, "spurious interrupt!!\n");
+		dev_warn(info->dev, "spurious interrupt!!\n");
 		return IRQ_NONE;
 	}
 
 	switch (i) {
 	case QWBTU_IRQ:
-		dev_info(&info->pdev->dev,
-			"Quit Battery under temperature in work mode IRQ (QWBTU)\n");
+		dev_info(info->dev, "Quit Battery under temperature in work mode IRQ (QWBTU)\n");
 		break;
 	case WBTU_IRQ:
-		dev_info(&info->pdev->dev,
-			"Battery under temperature in work mode IRQ (WBTU)\n");
+		dev_info(info->dev, "Battery under temperature in work mode IRQ (WBTU)\n");
 		break;
 	case QWBTO_IRQ:
-		dev_info(&info->pdev->dev,
-			"Quit Battery over temperature in work mode IRQ (QWBTO)\n");
+		dev_info(info->dev, "Quit Battery over temperature in work mode IRQ (QWBTO)\n");
 		break;
 	case WBTO_IRQ:
-		dev_info(&info->pdev->dev,
-			"Battery over temperature in work mode IRQ (WBTO)\n");
+		dev_info(info->dev, "Battery over temperature in work mode IRQ (WBTO)\n");
 		break;
 	case WL2_IRQ:
-		dev_info(&info->pdev->dev, "Low Batt Warning(2) INTR\n");
+		dev_info(info->dev, "Low Batt Warning(2) INTR\n");
 		break;
 	case WL1_IRQ:
-		dev_info(&info->pdev->dev, "Low Batt Warning(1) INTR\n");
+		dev_info(info->dev, "Low Batt Warning(1) INTR\n");
 		break;
 	default:
-		dev_warn(&info->pdev->dev, "Spurious Interrupt!!!\n");
+		dev_warn(info->dev, "Spurious Interrupt!!!\n");
 	}
 
 	power_supply_changed(info->bat);
@@ -508,16 +499,15 @@ static const struct power_supply_desc fuel_gauge_desc = {
 	.external_power_changed	= fuel_gauge_external_power_changed,
 };
 
-static void fuel_gauge_init_irq(struct axp288_fg_info *info)
+static void fuel_gauge_init_irq(struct axp288_fg_info *info, struct platform_device *pdev)
 {
 	int ret, i, pirq;
 
 	for (i = 0; i < AXP288_FG_INTR_NUM; i++) {
-		pirq = platform_get_irq(info->pdev, i);
+		pirq = platform_get_irq(pdev, i);
 		info->irq[i] = regmap_irq_get_virq(info->regmap_irqc, pirq);
 		if (info->irq[i] < 0) {
-			dev_warn(&info->pdev->dev,
-				"regmap_irq get virq failed for IRQ %d: %d\n",
+			dev_warn(info->dev, "regmap_irq get virq failed for IRQ %d: %d\n",
 				pirq, info->irq[i]);
 			info->irq[i] = -1;
 			goto intr_failed;
@@ -526,8 +516,7 @@ static void fuel_gauge_init_irq(struct axp288_fg_info *info)
 				NULL, fuel_gauge_thread_handler,
 				IRQF_ONESHOT, DEV_NAME, info);
 		if (ret) {
-			dev_warn(&info->pdev->dev,
-				"request irq failed for IRQ %d: %d\n",
+			dev_warn(info->dev, "request irq failed for IRQ %d: %d\n",
 				pirq, info->irq[i]);
 			info->irq[i] = -1;
 			goto intr_failed;
@@ -649,7 +638,7 @@ static int axp288_fuel_gauge_probe(struct platform_device *pdev)
 	if (!info)
 		return -ENOMEM;
 
-	info->pdev = pdev;
+	info->dev = &pdev->dev;
 	info->regmap = axp20x->regmap;
 	info->regmap_irqc = axp20x->regmap_irqc;
 	info->status = POWER_SUPPLY_STATUS_UNKNOWN;
@@ -714,7 +703,7 @@ static int axp288_fuel_gauge_probe(struct platform_device *pdev)
 		goto out_free_iio_chan;
 	}
 
-	fuel_gauge_init_irq(info);
+	fuel_gauge_init_irq(info, pdev);
 
 	return 0;
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 07/10] power: supply: axp288_fuel_gauge: Only read PWR_OP_MODE, FG_LOW_CAP_REG regs once
  2021-07-30  9:55 [PATCH v2 00/10] power: supply: axp288_fuel_gauge: Reduce number of register accesses + cleanups Hans de Goede
                   ` (5 preceding siblings ...)
  2021-07-30  9:56 ` [PATCH v2 06/10] power: supply: axp288_fuel_gauge: Store struct device pointer in axp288_fg_info Hans de Goede
@ 2021-07-30  9:56 ` Hans de Goede
  2021-07-30  9:56 ` [PATCH v2 08/10] power: supply: axp288_fuel_gauge: Refresh all registers in one go Hans de Goede
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Hans de Goede @ 2021-07-30  9:56 UTC (permalink / raw)
  To: Sebastian Reichel; +Cc: Hans de Goede, Andrejus Basovas, linux-pm

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>
---
 drivers/power/supply/axp288_fuel_gauge.c | 37 ++++++++++++++----------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/power/supply/axp288_fuel_gauge.c b/drivers/power/supply/axp288_fuel_gauge.c
index 1366027edf49..8011628d3704 100644
--- a/drivers/power/supply/axp288_fuel_gauge.c
+++ b/drivers/power/supply/axp288_fuel_gauge.c
@@ -111,6 +111,8 @@ struct axp288_fg_info {
 	struct mutex lock;
 	int status;
 	int max_volt;
+	int pwr_op;
+	int low_cap;
 	struct dentry *debug_file;
 };
 
@@ -336,11 +338,7 @@ static int fuel_gauge_get_property(struct power_supply *ps,
 		val->intval = PROP_CURR(value);
 		break;
 	case POWER_SUPPLY_PROP_PRESENT:
-		ret = fuel_gauge_reg_readb(info, AXP20X_PWR_OP_MODE);
-		if (ret < 0)
-			goto fuel_gauge_read_err;
-
-		if (ret & CHRG_STAT_BAT_PRESENT)
+		if (info->pwr_op & CHRG_STAT_BAT_PRESENT)
 			val->intval = 1;
 		else
 			val->intval = 0;
@@ -355,10 +353,7 @@ static int fuel_gauge_get_property(struct power_supply *ps,
 		val->intval = (ret & FG_REP_CAP_VAL_MASK);
 		break;
 	case POWER_SUPPLY_PROP_CAPACITY_ALERT_MIN:
-		ret = fuel_gauge_reg_readb(info, AXP288_FG_LOW_CAP_REG);
-		if (ret < 0)
-			goto fuel_gauge_read_err;
-		val->intval = (ret & 0x0f);
+		val->intval = (info->low_cap & 0x0f);
 		break;
 	case POWER_SUPPLY_PROP_TECHNOLOGY:
 		val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
@@ -398,7 +393,7 @@ static int fuel_gauge_set_property(struct power_supply *ps,
 		const union power_supply_propval *val)
 {
 	struct axp288_fg_info *info = power_supply_get_drvdata(ps);
-	int ret = 0;
+	int new_low_cap, ret = 0;
 
 	mutex_lock(&info->lock);
 	switch (prop) {
@@ -407,12 +402,12 @@ static int fuel_gauge_set_property(struct power_supply *ps,
 			ret = -EINVAL;
 			break;
 		}
-		ret = fuel_gauge_reg_readb(info, AXP288_FG_LOW_CAP_REG);
-		if (ret < 0)
-			break;
-		ret &= 0xf0;
-		ret |= (val->intval & 0xf);
-		ret = fuel_gauge_reg_writeb(info, AXP288_FG_LOW_CAP_REG, ret);
+		new_low_cap = info->low_cap;
+		new_low_cap &= 0xf0;
+		new_low_cap |= (val->intval & 0xf);
+		ret = fuel_gauge_reg_writeb(info, AXP288_FG_LOW_CAP_REG, new_low_cap);
+		if (ret == 0)
+			info->low_cap = new_low_cap;
 		break;
 	default:
 		ret = -EINVAL;
@@ -695,6 +690,16 @@ static int axp288_fuel_gauge_probe(struct platform_device *pdev)
 		break;
 	}
 
+	ret = fuel_gauge_reg_readb(info, AXP20X_PWR_OP_MODE);
+	if (ret < 0)
+		goto out_free_iio_chan;
+	info->pwr_op = ret;
+
+	ret = fuel_gauge_reg_readb(info, AXP288_FG_LOW_CAP_REG);
+	if (ret < 0)
+		goto out_free_iio_chan;
+	info->low_cap = ret;
+
 	psy_cfg.drv_data = info;
 	info->bat = power_supply_register(&pdev->dev, &fuel_gauge_desc, &psy_cfg);
 	if (IS_ERR(info->bat)) {
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 08/10] power: supply: axp288_fuel_gauge: Refresh all registers in one go
  2021-07-30  9:55 [PATCH v2 00/10] power: supply: axp288_fuel_gauge: Reduce number of register accesses + cleanups Hans de Goede
                   ` (6 preceding siblings ...)
  2021-07-30  9:56 ` [PATCH v2 07/10] power: supply: axp288_fuel_gauge: Only read PWR_OP_MODE, FG_LOW_CAP_REG regs once Hans de Goede
@ 2021-07-30  9:56 ` Hans de Goede
  2021-07-30 13:39     ` kernel test robot
  2021-07-30 16:14     ` kernel test robot
  2021-07-30  9:56 ` [PATCH v2 09/10] power: supply: axp288_fuel_gauge: Move the AXP20X_CC_CTRL check together with the other checks Hans de Goede
  2021-07-30  9:56 ` [PATCH v2 10/10] power: supply: axp288_fuel_gauge: Take the P-Unit semaphore only once during probe() Hans de Goede
  9 siblings, 2 replies; 17+ messages in thread
From: Hans de Goede @ 2021-07-30  9:56 UTC (permalink / raw)
  To: Sebastian Reichel; +Cc: Hans de Goede, Andrejus Basovas, linux-pm

From: Andrejus Basovas <cpp@gcc.lt>

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>
---
Changes in v2:
- Add a "depends on X86" to Kconfig since the iosf_mbi functions are X86 only
  (the AXP288 PMIC is only used on X86 devices)
---
 drivers/power/supply/Kconfig             |   2 +-
 drivers/power/supply/axp288_fuel_gauge.c | 201 ++++++++++++-----------
 2 files changed, 106 insertions(+), 97 deletions(-)

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 11f5368e810e..56e29d4247c5 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -358,7 +358,7 @@ config AXP288_CHARGER
 
 config AXP288_FUEL_GAUGE
 	tristate "X-Powers AXP288 Fuel Gauge"
-	depends on MFD_AXP20X && IIO
+	depends on MFD_AXP20X && IIO && X86
 	help
 	  Say yes here to have support for X-Power power management IC (PMIC)
 	  Fuel Gauge. The device provides battery statistics and status
diff --git a/drivers/power/supply/axp288_fuel_gauge.c b/drivers/power/supply/axp288_fuel_gauge.c
index 8011628d3704..8db8ab0827e4 100644
--- a/drivers/power/supply/axp288_fuel_gauge.c
+++ b/drivers/power/supply/axp288_fuel_gauge.c
@@ -2,7 +2,8 @@
 /*
  * axp288_fuel_gauge.c - Xpower AXP288 PMIC Fuel Gauge Driver
  *
- * Copyright (C) 2016-2017 Hans de Goede <hdegoede@redhat.com>
+ * Copyright (C) 2020-2021 Andrejus Basovas <xxx@yyy.tld>
+ * Copyright (C) 2016-2021 Hans de Goede <hdegoede@redhat.com>
  * Copyright (C) 2014 Intel Corporation
  *
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -20,6 +21,7 @@
 #include <linux/power_supply.h>
 #include <linux/iio/consumer.h>
 #include <asm/unaligned.h>
+#include <asm/iosf_mbi.h>
 
 #define PS_STAT_VBUS_TRIGGER			(1 << 0)
 #define PS_STAT_BAT_CHRG_DIR			(1 << 2)
@@ -84,6 +86,7 @@
 #define PROP_VOLT(a)				((a) * 1000)
 #define PROP_CURR(a)				((a) * 1000)
 
+#define AXP288_REG_UPDATE_INTERVAL		(60 * HZ)
 #define AXP288_FG_INTR_NUM			6
 enum {
 	QWBTU_IRQ = 0,
@@ -114,6 +117,18 @@ struct axp288_fg_info {
 	int pwr_op;
 	int low_cap;
 	struct dentry *debug_file;
+
+	char valid;                 /* zero until following fields are valid */
+	unsigned long last_updated; /* in jiffies */
+
+	int pwr_stat;
+	int fg_res;
+	int bat_volt;
+	int d_curr;
+	int c_curr;
+	int ocv;
+	int fg_cc_mtr1;
+	int fg_des_cap1;
 };
 
 static enum power_supply_property fuel_gauge_props[] = {
@@ -192,25 +207,78 @@ static int fuel_gauge_read_12bit_word(struct axp288_fg_info *info, int reg)
 	return (buf[0] << 4) | ((buf[1] >> 4) & 0x0f);
 }
 
-static void fuel_gauge_get_status(struct axp288_fg_info *info)
+static int fuel_gauge_update_registers(struct axp288_fg_info *info)
 {
-	int pwr_stat, fg_res, curr, ret;
+	int ret;
 
-	pwr_stat = fuel_gauge_reg_readb(info, AXP20X_PWR_INPUT_STATUS);
-	if (pwr_stat < 0) {
-		dev_err(info->dev, "PWR STAT read failed: %d\n", pwr_stat);
-		return;
+	if (info->valid && time_before(jiffies, info->last_updated + AXP288_REG_UPDATE_INTERVAL))
+		return 0;
+
+	dev_dbg(info->dev, "Fuel Gauge updating register values...\n");
+
+	ret = iosf_mbi_block_punit_i2c_access();
+	if (ret < 0)
+		return ret;
+
+	ret = fuel_gauge_reg_readb(info, AXP20X_PWR_INPUT_STATUS);
+	if (ret < 0)
+		goto out;
+	info->pwr_stat = ret;
+
+	ret = fuel_gauge_reg_readb(info, AXP20X_FG_RES);
+	if (ret < 0)
+		goto out;
+	info->fg_res = ret;
+
+	ret = iio_read_channel_raw(info->iio_channel[BAT_VOLT], &info->bat_volt);
+	if (ret < 0)
+		goto out;
+
+	if (info->pwr_stat & PS_STAT_BAT_CHRG_DIR) {
+		info->d_curr = 0;
+		ret = iio_read_channel_raw(info->iio_channel[BAT_CHRG_CURR], &info->c_curr);
+		if (ret < 0)
+			goto out;
+	} else {
+		info->c_curr = 0;
+		ret = iio_read_channel_raw(info->iio_channel[BAT_D_CURR], &info->d_curr);
+		if (ret < 0)
+			goto out;
 	}
 
+	ret = fuel_gauge_read_12bit_word(info, AXP288_FG_OCVH_REG);
+	if (ret < 0)
+		goto out;
+	info->ocv = ret;
+
+	ret = fuel_gauge_read_15bit_word(info, AXP288_FG_CC_MTR1_REG);
+	if (ret < 0)
+		goto out;
+	info->fg_cc_mtr1 = ret;
+
+	ret = fuel_gauge_read_15bit_word(info, AXP288_FG_DES_CAP1_REG);
+	if (ret < 0)
+		goto out;
+	info->fg_des_cap1 = ret;
+
+	info->last_updated = jiffies;
+	info->valid = 1;
+	ret = 0;
+out:
+	iosf_mbi_unblock_punit_i2c_access();
+	return ret;
+}
+
+static void fuel_gauge_get_status(struct axp288_fg_info *info)
+{
+	int pwr_stat = info->pwr_stat;
+	int fg_res = info->fg_res;
+	int curr = info->d_curr;
+
 	/* Report full if Vbus is valid and the reported capacity is 100% */
 	if (!(pwr_stat & PS_STAT_VBUS_VALID))
 		goto not_full;
 
-	fg_res = fuel_gauge_reg_readb(info, AXP20X_FG_RES);
-	if (fg_res < 0) {
-		dev_err(info->dev, "FG RES read failed: %d\n", fg_res);
-		return;
-	}
 	if (!(fg_res & FG_REP_CAP_VALID))
 		goto not_full;
 
@@ -228,11 +296,6 @@ static void fuel_gauge_get_status(struct axp288_fg_info *info)
 	if (fg_res < 90 || (pwr_stat & PS_STAT_BAT_CHRG_DIR))
 		goto not_full;
 
-	ret = iio_read_channel_raw(info->iio_channel[BAT_D_CURR], &curr);
-	if (ret < 0) {
-		dev_err(info->dev, "FG get current failed: %d\n", ret);
-		return;
-	}
 	if (curr == 0) {
 		info->status = POWER_SUPPLY_STATUS_FULL;
 		return;
@@ -245,61 +308,16 @@ static void fuel_gauge_get_status(struct axp288_fg_info *info)
 		info->status = POWER_SUPPLY_STATUS_DISCHARGING;
 }
 
-static int fuel_gauge_get_vbatt(struct axp288_fg_info *info, int *vbatt)
-{
-	int ret = 0, raw_val;
-
-	ret = iio_read_channel_raw(info->iio_channel[BAT_VOLT], &raw_val);
-	if (ret < 0)
-		goto vbatt_read_fail;
-
-	*vbatt = VOLTAGE_FROM_ADC(raw_val);
-vbatt_read_fail:
-	return ret;
-}
-
-static int fuel_gauge_get_current(struct axp288_fg_info *info, int *cur)
-{
-	int ret, discharge;
-
-	/* First check discharge current, so that we do only 1 read on bat. */
-	ret = iio_read_channel_raw(info->iio_channel[BAT_D_CURR], &discharge);
-	if (ret < 0)
-		return ret;
-
-	if (discharge > 0) {
-		*cur = -1 * discharge;
-		return 0;
-	}
-
-	return iio_read_channel_raw(info->iio_channel[BAT_CHRG_CURR], cur);
-}
-
-static int fuel_gauge_get_vocv(struct axp288_fg_info *info, int *vocv)
-{
-	int ret;
-
-	ret = fuel_gauge_read_12bit_word(info, AXP288_FG_OCVH_REG);
-	if (ret >= 0)
-		*vocv = VOLTAGE_FROM_ADC(ret);
-
-	return ret;
-}
-
 static int fuel_gauge_battery_health(struct axp288_fg_info *info)
 {
-	int ret, vocv, health = POWER_SUPPLY_HEALTH_UNKNOWN;
-
-	ret = fuel_gauge_get_vocv(info, &vocv);
-	if (ret < 0)
-		goto health_read_fail;
+	int vocv = VOLTAGE_FROM_ADC(info->ocv);
+	int health = POWER_SUPPLY_HEALTH_UNKNOWN;
 
 	if (vocv > info->max_volt)
 		health = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
 	else
 		health = POWER_SUPPLY_HEALTH_GOOD;
 
-health_read_fail:
 	return health;
 }
 
@@ -308,9 +326,14 @@ static int fuel_gauge_get_property(struct power_supply *ps,
 		union power_supply_propval *val)
 {
 	struct axp288_fg_info *info = power_supply_get_drvdata(ps);
-	int ret = 0, value;
+	int ret, value;
 
 	mutex_lock(&info->lock);
+
+	ret = fuel_gauge_update_registers(info);
+	if (ret < 0)
+		goto out;
+
 	switch (prop) {
 	case POWER_SUPPLY_PROP_STATUS:
 		fuel_gauge_get_status(info);
@@ -320,21 +343,19 @@ static int fuel_gauge_get_property(struct power_supply *ps,
 		val->intval = fuel_gauge_battery_health(info);
 		break;
 	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
-		ret = fuel_gauge_get_vbatt(info, &value);
-		if (ret < 0)
-			goto fuel_gauge_read_err;
+		value = VOLTAGE_FROM_ADC(info->bat_volt);
 		val->intval = PROP_VOLT(value);
 		break;
 	case POWER_SUPPLY_PROP_VOLTAGE_OCV:
-		ret = fuel_gauge_get_vocv(info, &value);
-		if (ret < 0)
-			goto fuel_gauge_read_err;
+		value = VOLTAGE_FROM_ADC(info->ocv);
 		val->intval = PROP_VOLT(value);
 		break;
 	case POWER_SUPPLY_PROP_CURRENT_NOW:
-		ret = fuel_gauge_get_current(info, &value);
-		if (ret < 0)
-			goto fuel_gauge_read_err;
+		if (info->d_curr > 0)
+			value = -1 * info->d_curr;
+		else
+			value = info->c_curr;
+
 		val->intval = PROP_CURR(value);
 		break;
 	case POWER_SUPPLY_PROP_PRESENT:
@@ -344,13 +365,9 @@ static int fuel_gauge_get_property(struct power_supply *ps,
 			val->intval = 0;
 		break;
 	case POWER_SUPPLY_PROP_CAPACITY:
-		ret = fuel_gauge_reg_readb(info, AXP20X_FG_RES);
-		if (ret < 0)
-			goto fuel_gauge_read_err;
-
-		if (!(ret & FG_REP_CAP_VALID))
+		if (!(info->fg_res & FG_REP_CAP_VALID))
 			dev_err(info->dev, "capacity measurement not valid\n");
-		val->intval = (ret & FG_REP_CAP_VAL_MASK);
+		val->intval = (info->fg_res & FG_REP_CAP_VAL_MASK);
 		break;
 	case POWER_SUPPLY_PROP_CAPACITY_ALERT_MIN:
 		val->intval = (info->low_cap & 0x0f);
@@ -359,31 +376,19 @@ static int fuel_gauge_get_property(struct power_supply *ps,
 		val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
 		break;
 	case POWER_SUPPLY_PROP_CHARGE_NOW:
-		ret = fuel_gauge_read_15bit_word(info, AXP288_FG_CC_MTR1_REG);
-		if (ret < 0)
-			goto fuel_gauge_read_err;
-
-		val->intval = ret * FG_DES_CAP_RES_LSB;
+		val->intval = info->fg_cc_mtr1 * FG_DES_CAP_RES_LSB;
 		break;
 	case POWER_SUPPLY_PROP_CHARGE_FULL:
-		ret = fuel_gauge_read_15bit_word(info, AXP288_FG_DES_CAP1_REG);
-		if (ret < 0)
-			goto fuel_gauge_read_err;
-
-		val->intval = ret * FG_DES_CAP_RES_LSB;
+		val->intval = info->fg_des_cap1 * FG_DES_CAP_RES_LSB;
 		break;
 	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
 		val->intval = PROP_VOLT(info->max_volt);
 		break;
 	default:
-		mutex_unlock(&info->lock);
-		return -EINVAL;
+		ret = -EINVAL;
 	}
 
-	mutex_unlock(&info->lock);
-	return 0;
-
-fuel_gauge_read_err:
+out:
 	mutex_unlock(&info->lock);
 	return ret;
 }
@@ -472,6 +477,8 @@ static irqreturn_t fuel_gauge_thread_handler(int irq, void *dev)
 		dev_warn(info->dev, "Spurious Interrupt!!!\n");
 	}
 
+	info->valid = 0; /* Force updating of the cached registers */
+
 	power_supply_changed(info->bat);
 	return IRQ_HANDLED;
 }
@@ -480,6 +487,7 @@ static void fuel_gauge_external_power_changed(struct power_supply *psy)
 {
 	struct axp288_fg_info *info = power_supply_get_drvdata(psy);
 
+	info->valid = 0; /* Force updating of the cached registers */
 	power_supply_changed(info->bat);
 }
 
@@ -637,6 +645,7 @@ static int axp288_fuel_gauge_probe(struct platform_device *pdev)
 	info->regmap = axp20x->regmap;
 	info->regmap_irqc = axp20x->regmap_irqc;
 	info->status = POWER_SUPPLY_STATUS_UNKNOWN;
+	info->valid = 0;
 
 	platform_set_drvdata(pdev, info);
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 09/10] power: supply: axp288_fuel_gauge: Move the AXP20X_CC_CTRL check together with the other checks
  2021-07-30  9:55 [PATCH v2 00/10] power: supply: axp288_fuel_gauge: Reduce number of register accesses + cleanups Hans de Goede
                   ` (7 preceding siblings ...)
  2021-07-30  9:56 ` [PATCH v2 08/10] power: supply: axp288_fuel_gauge: Refresh all registers in one go Hans de Goede
@ 2021-07-30  9:56 ` Hans de Goede
  2021-07-30  9:56 ` [PATCH v2 10/10] power: supply: axp288_fuel_gauge: Take the P-Unit semaphore only once during probe() Hans de Goede
  9 siblings, 0 replies; 17+ messages in thread
From: Hans de Goede @ 2021-07-30  9:56 UTC (permalink / raw)
  To: Sebastian Reichel; +Cc: Hans de Goede, Andrejus Basovas, linux-pm

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>
---
 drivers/power/supply/axp288_fuel_gauge.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/power/supply/axp288_fuel_gauge.c b/drivers/power/supply/axp288_fuel_gauge.c
index 8db8ab0827e4..016d8d6bec40 100644
--- a/drivers/power/supply/axp288_fuel_gauge.c
+++ b/drivers/power/supply/axp288_fuel_gauge.c
@@ -627,16 +627,6 @@ static int axp288_fuel_gauge_probe(struct platform_device *pdev)
 	if (dmi_check_system(axp288_no_battery_list))
 		return -ENODEV;
 
-	/*
-	 * On some devices the fuelgauge and charger parts of the axp288 are
-	 * not used, check that the fuelgauge is enabled (CC_CTRL != 0).
-	 */
-	ret = regmap_read(axp20x->regmap, AXP20X_CC_CTRL, &val);
-	if (ret < 0)
-		return ret;
-	if (val == 0)
-		return -ENODEV;
-
 	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
 	if (!info)
 		return -ENOMEM;
@@ -671,6 +661,18 @@ static int axp288_fuel_gauge_probe(struct platform_device *pdev)
 		}
 	}
 
+	/*
+	 * On some devices the fuelgauge and charger parts of the axp288 are
+	 * not used, check that the fuelgauge is enabled (CC_CTRL != 0).
+	 */
+	ret = regmap_read(axp20x->regmap, AXP20X_CC_CTRL, &val);
+	if (ret < 0)
+		goto out_free_iio_chan;
+	if (val == 0) {
+		ret = -ENODEV;
+		goto out_free_iio_chan;
+	}
+
 	ret = fuel_gauge_reg_readb(info, AXP288_FG_DES_CAP1_REG);
 	if (ret < 0)
 		goto out_free_iio_chan;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 10/10] power: supply: axp288_fuel_gauge: Take the P-Unit semaphore only once during probe()
  2021-07-30  9:55 [PATCH v2 00/10] power: supply: axp288_fuel_gauge: Reduce number of register accesses + cleanups Hans de Goede
                   ` (8 preceding siblings ...)
  2021-07-30  9:56 ` [PATCH v2 09/10] power: supply: axp288_fuel_gauge: Move the AXP20X_CC_CTRL check together with the other checks Hans de Goede
@ 2021-07-30  9:56 ` Hans de Goede
  9 siblings, 0 replies; 17+ messages in thread
From: Hans de Goede @ 2021-07-30  9:56 UTC (permalink / raw)
  To: Sebastian Reichel; +Cc: Hans de Goede, Andrejus Basovas, linux-pm

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>
---
 drivers/power/supply/axp288_fuel_gauge.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/power/supply/axp288_fuel_gauge.c b/drivers/power/supply/axp288_fuel_gauge.c
index 016d8d6bec40..c1da217fdb0e 100644
--- a/drivers/power/supply/axp288_fuel_gauge.c
+++ b/drivers/power/supply/axp288_fuel_gauge.c
@@ -661,31 +661,35 @@ static int axp288_fuel_gauge_probe(struct platform_device *pdev)
 		}
 	}
 
+	ret = iosf_mbi_block_punit_i2c_access();
+	if (ret < 0)
+		goto out_free_iio_chan;
+
 	/*
 	 * On some devices the fuelgauge and charger parts of the axp288 are
 	 * not used, check that the fuelgauge is enabled (CC_CTRL != 0).
 	 */
 	ret = regmap_read(axp20x->regmap, AXP20X_CC_CTRL, &val);
 	if (ret < 0)
-		goto out_free_iio_chan;
+		goto unblock_punit_i2c_access;
 	if (val == 0) {
 		ret = -ENODEV;
-		goto out_free_iio_chan;
+		goto unblock_punit_i2c_access;
 	}
 
 	ret = fuel_gauge_reg_readb(info, AXP288_FG_DES_CAP1_REG);
 	if (ret < 0)
-		goto out_free_iio_chan;
+		goto unblock_punit_i2c_access;
 
 	if (!(ret & FG_DES_CAP1_VALID)) {
 		dev_err(&pdev->dev, "axp288 not configured by firmware\n");
 		ret = -ENODEV;
-		goto out_free_iio_chan;
+		goto unblock_punit_i2c_access;
 	}
 
 	ret = fuel_gauge_reg_readb(info, AXP20X_CHRG_CTRL1);
 	if (ret < 0)
-		goto out_free_iio_chan;
+		goto unblock_punit_i2c_access;
 	switch ((ret & CHRG_CCCV_CV_MASK) >> CHRG_CCCV_CV_BIT_POS) {
 	case CHRG_CCCV_CV_4100MV:
 		info->max_volt = 4100;
@@ -703,14 +707,20 @@ static int axp288_fuel_gauge_probe(struct platform_device *pdev)
 
 	ret = fuel_gauge_reg_readb(info, AXP20X_PWR_OP_MODE);
 	if (ret < 0)
-		goto out_free_iio_chan;
+		goto unblock_punit_i2c_access;
 	info->pwr_op = ret;
 
 	ret = fuel_gauge_reg_readb(info, AXP288_FG_LOW_CAP_REG);
 	if (ret < 0)
-		goto out_free_iio_chan;
+		goto unblock_punit_i2c_access;
 	info->low_cap = ret;
 
+unblock_punit_i2c_access:
+	iosf_mbi_unblock_punit_i2c_access();
+	/* In case we arrive here by goto because of a register access error */
+	if (ret < 0)
+		goto out_free_iio_chan;
+
 	psy_cfg.drv_data = info;
 	info->bat = power_supply_register(&pdev->dev, &fuel_gauge_desc, &psy_cfg);
 	if (IS_ERR(info->bat)) {
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 08/10] power: supply: axp288_fuel_gauge: Refresh all registers in one go
  2021-07-30  9:56 ` [PATCH v2 08/10] power: supply: axp288_fuel_gauge: Refresh all registers in one go Hans de Goede
@ 2021-07-30 13:39     ` kernel test robot
  2021-07-30 16:14     ` kernel test robot
  1 sibling, 0 replies; 17+ messages in thread
From: kernel test robot @ 2021-07-30 13:39 UTC (permalink / raw)
  To: Hans de Goede, Sebastian Reichel
  Cc: kbuild-all, Hans de Goede, Andrejus Basovas, linux-pm

[-- Attachment #1: Type: text/plain, Size: 4053 bytes --]

Hi Hans,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on power-supply/for-next]
[also build test ERROR on v5.14-rc3 next-20210729]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Hans-de-Goede/power-supply-axp288_fuel_gauge-Reduce-number-of-register-accesses-cleanups/20210730-175716
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
config: i386-randconfig-a005-20210730 (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/1ff192ccce54fdfce899447999a60a195537460c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Hans-de-Goede/power-supply-axp288_fuel_gauge-Reduce-number-of-register-accesses-cleanups/20210730-175716
        git checkout 1ff192ccce54fdfce899447999a60a195537460c
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/power/supply/axp288_fuel_gauge.c: In function 'fuel_gauge_update_registers':
>> drivers/power/supply/axp288_fuel_gauge.c:219:8: error: implicit declaration of function 'iosf_mbi_block_punit_i2c_access' [-Werror=implicit-function-declaration]
     219 |  ret = iosf_mbi_block_punit_i2c_access();
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/power/supply/axp288_fuel_gauge.c:268:2: error: implicit declaration of function 'iosf_mbi_unblock_punit_i2c_access' [-Werror=implicit-function-declaration]
     268 |  iosf_mbi_unblock_punit_i2c_access();
         |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/iosf_mbi_block_punit_i2c_access +219 drivers/power/supply/axp288_fuel_gauge.c

   209	
   210	static int fuel_gauge_update_registers(struct axp288_fg_info *info)
   211	{
   212		int ret;
   213	
   214		if (info->valid && time_before(jiffies, info->last_updated + AXP288_REG_UPDATE_INTERVAL))
   215			return 0;
   216	
   217		dev_dbg(info->dev, "Fuel Gauge updating register values...\n");
   218	
 > 219		ret = iosf_mbi_block_punit_i2c_access();
   220		if (ret < 0)
   221			return ret;
   222	
   223		ret = fuel_gauge_reg_readb(info, AXP20X_PWR_INPUT_STATUS);
   224		if (ret < 0)
   225			goto out;
   226		info->pwr_stat = ret;
   227	
   228		ret = fuel_gauge_reg_readb(info, AXP20X_FG_RES);
   229		if (ret < 0)
   230			goto out;
   231		info->fg_res = ret;
   232	
   233		ret = iio_read_channel_raw(info->iio_channel[BAT_VOLT], &info->bat_volt);
   234		if (ret < 0)
   235			goto out;
   236	
   237		if (info->pwr_stat & PS_STAT_BAT_CHRG_DIR) {
   238			info->d_curr = 0;
   239			ret = iio_read_channel_raw(info->iio_channel[BAT_CHRG_CURR], &info->c_curr);
   240			if (ret < 0)
   241				goto out;
   242		} else {
   243			info->c_curr = 0;
   244			ret = iio_read_channel_raw(info->iio_channel[BAT_D_CURR], &info->d_curr);
   245			if (ret < 0)
   246				goto out;
   247		}
   248	
   249		ret = fuel_gauge_read_12bit_word(info, AXP288_FG_OCVH_REG);
   250		if (ret < 0)
   251			goto out;
   252		info->ocv = ret;
   253	
   254		ret = fuel_gauge_read_15bit_word(info, AXP288_FG_CC_MTR1_REG);
   255		if (ret < 0)
   256			goto out;
   257		info->fg_cc_mtr1 = ret;
   258	
   259		ret = fuel_gauge_read_15bit_word(info, AXP288_FG_DES_CAP1_REG);
   260		if (ret < 0)
   261			goto out;
   262		info->fg_des_cap1 = ret;
   263	
   264		info->last_updated = jiffies;
   265		info->valid = 1;
   266		ret = 0;
   267	out:
 > 268		iosf_mbi_unblock_punit_i2c_access();
   269		return ret;
   270	}
   271	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35990 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 08/10] power: supply: axp288_fuel_gauge: Refresh all registers in one go
@ 2021-07-30 13:39     ` kernel test robot
  0 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2021-07-30 13:39 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4161 bytes --]

Hi Hans,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on power-supply/for-next]
[also build test ERROR on v5.14-rc3 next-20210729]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Hans-de-Goede/power-supply-axp288_fuel_gauge-Reduce-number-of-register-accesses-cleanups/20210730-175716
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
config: i386-randconfig-a005-20210730 (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/1ff192ccce54fdfce899447999a60a195537460c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Hans-de-Goede/power-supply-axp288_fuel_gauge-Reduce-number-of-register-accesses-cleanups/20210730-175716
        git checkout 1ff192ccce54fdfce899447999a60a195537460c
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/power/supply/axp288_fuel_gauge.c: In function 'fuel_gauge_update_registers':
>> drivers/power/supply/axp288_fuel_gauge.c:219:8: error: implicit declaration of function 'iosf_mbi_block_punit_i2c_access' [-Werror=implicit-function-declaration]
     219 |  ret = iosf_mbi_block_punit_i2c_access();
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/power/supply/axp288_fuel_gauge.c:268:2: error: implicit declaration of function 'iosf_mbi_unblock_punit_i2c_access' [-Werror=implicit-function-declaration]
     268 |  iosf_mbi_unblock_punit_i2c_access();
         |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/iosf_mbi_block_punit_i2c_access +219 drivers/power/supply/axp288_fuel_gauge.c

   209	
   210	static int fuel_gauge_update_registers(struct axp288_fg_info *info)
   211	{
   212		int ret;
   213	
   214		if (info->valid && time_before(jiffies, info->last_updated + AXP288_REG_UPDATE_INTERVAL))
   215			return 0;
   216	
   217		dev_dbg(info->dev, "Fuel Gauge updating register values...\n");
   218	
 > 219		ret = iosf_mbi_block_punit_i2c_access();
   220		if (ret < 0)
   221			return ret;
   222	
   223		ret = fuel_gauge_reg_readb(info, AXP20X_PWR_INPUT_STATUS);
   224		if (ret < 0)
   225			goto out;
   226		info->pwr_stat = ret;
   227	
   228		ret = fuel_gauge_reg_readb(info, AXP20X_FG_RES);
   229		if (ret < 0)
   230			goto out;
   231		info->fg_res = ret;
   232	
   233		ret = iio_read_channel_raw(info->iio_channel[BAT_VOLT], &info->bat_volt);
   234		if (ret < 0)
   235			goto out;
   236	
   237		if (info->pwr_stat & PS_STAT_BAT_CHRG_DIR) {
   238			info->d_curr = 0;
   239			ret = iio_read_channel_raw(info->iio_channel[BAT_CHRG_CURR], &info->c_curr);
   240			if (ret < 0)
   241				goto out;
   242		} else {
   243			info->c_curr = 0;
   244			ret = iio_read_channel_raw(info->iio_channel[BAT_D_CURR], &info->d_curr);
   245			if (ret < 0)
   246				goto out;
   247		}
   248	
   249		ret = fuel_gauge_read_12bit_word(info, AXP288_FG_OCVH_REG);
   250		if (ret < 0)
   251			goto out;
   252		info->ocv = ret;
   253	
   254		ret = fuel_gauge_read_15bit_word(info, AXP288_FG_CC_MTR1_REG);
   255		if (ret < 0)
   256			goto out;
   257		info->fg_cc_mtr1 = ret;
   258	
   259		ret = fuel_gauge_read_15bit_word(info, AXP288_FG_DES_CAP1_REG);
   260		if (ret < 0)
   261			goto out;
   262		info->fg_des_cap1 = ret;
   263	
   264		info->last_updated = jiffies;
   265		info->valid = 1;
   266		ret = 0;
   267	out:
 > 268		iosf_mbi_unblock_punit_i2c_access();
   269		return ret;
   270	}
   271	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 35990 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 08/10] power: supply: axp288_fuel_gauge: Refresh all registers in one go
  2021-07-30  9:56 ` [PATCH v2 08/10] power: supply: axp288_fuel_gauge: Refresh all registers in one go Hans de Goede
@ 2021-07-30 16:14     ` kernel test robot
  2021-07-30 16:14     ` kernel test robot
  1 sibling, 0 replies; 17+ messages in thread
From: kernel test robot @ 2021-07-30 16:14 UTC (permalink / raw)
  To: Hans de Goede, Sebastian Reichel
  Cc: clang-built-linux, kbuild-all, Hans de Goede, Andrejus Basovas, linux-pm

[-- Attachment #1: Type: text/plain, Size: 4421 bytes --]

Hi Hans,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on power-supply/for-next]
[also build test ERROR on v5.14-rc3 next-20210729]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Hans-de-Goede/power-supply-axp288_fuel_gauge-Reduce-number-of-register-accesses-cleanups/20210730-175716
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
config: x86_64-buildonly-randconfig-r001-20210730 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 4f71f59bf3d9914188a11d0c41bedbb339d36ff5)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1ff192ccce54fdfce899447999a60a195537460c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Hans-de-Goede/power-supply-axp288_fuel_gauge-Reduce-number-of-register-accesses-cleanups/20210730-175716
        git checkout 1ff192ccce54fdfce899447999a60a195537460c
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/power/supply/axp288_fuel_gauge.c:219:8: error: implicit declaration of function 'iosf_mbi_block_punit_i2c_access' [-Werror,-Wimplicit-function-declaration]
           ret = iosf_mbi_block_punit_i2c_access();
                 ^
>> drivers/power/supply/axp288_fuel_gauge.c:268:2: error: implicit declaration of function 'iosf_mbi_unblock_punit_i2c_access' [-Werror,-Wimplicit-function-declaration]
           iosf_mbi_unblock_punit_i2c_access();
           ^
   drivers/power/supply/axp288_fuel_gauge.c:268:2: note: did you mean 'iosf_mbi_block_punit_i2c_access'?
   drivers/power/supply/axp288_fuel_gauge.c:219:8: note: 'iosf_mbi_block_punit_i2c_access' declared here
           ret = iosf_mbi_block_punit_i2c_access();
                 ^
   2 errors generated.


vim +/iosf_mbi_block_punit_i2c_access +219 drivers/power/supply/axp288_fuel_gauge.c

   209	
   210	static int fuel_gauge_update_registers(struct axp288_fg_info *info)
   211	{
   212		int ret;
   213	
   214		if (info->valid && time_before(jiffies, info->last_updated + AXP288_REG_UPDATE_INTERVAL))
   215			return 0;
   216	
   217		dev_dbg(info->dev, "Fuel Gauge updating register values...\n");
   218	
 > 219		ret = iosf_mbi_block_punit_i2c_access();
   220		if (ret < 0)
   221			return ret;
   222	
   223		ret = fuel_gauge_reg_readb(info, AXP20X_PWR_INPUT_STATUS);
   224		if (ret < 0)
   225			goto out;
   226		info->pwr_stat = ret;
   227	
   228		ret = fuel_gauge_reg_readb(info, AXP20X_FG_RES);
   229		if (ret < 0)
   230			goto out;
   231		info->fg_res = ret;
   232	
   233		ret = iio_read_channel_raw(info->iio_channel[BAT_VOLT], &info->bat_volt);
   234		if (ret < 0)
   235			goto out;
   236	
   237		if (info->pwr_stat & PS_STAT_BAT_CHRG_DIR) {
   238			info->d_curr = 0;
   239			ret = iio_read_channel_raw(info->iio_channel[BAT_CHRG_CURR], &info->c_curr);
   240			if (ret < 0)
   241				goto out;
   242		} else {
   243			info->c_curr = 0;
   244			ret = iio_read_channel_raw(info->iio_channel[BAT_D_CURR], &info->d_curr);
   245			if (ret < 0)
   246				goto out;
   247		}
   248	
   249		ret = fuel_gauge_read_12bit_word(info, AXP288_FG_OCVH_REG);
   250		if (ret < 0)
   251			goto out;
   252		info->ocv = ret;
   253	
   254		ret = fuel_gauge_read_15bit_word(info, AXP288_FG_CC_MTR1_REG);
   255		if (ret < 0)
   256			goto out;
   257		info->fg_cc_mtr1 = ret;
   258	
   259		ret = fuel_gauge_read_15bit_word(info, AXP288_FG_DES_CAP1_REG);
   260		if (ret < 0)
   261			goto out;
   262		info->fg_des_cap1 = ret;
   263	
   264		info->last_updated = jiffies;
   265		info->valid = 1;
   266		ret = 0;
   267	out:
 > 268		iosf_mbi_unblock_punit_i2c_access();
   269		return ret;
   270	}
   271	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 49545 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 08/10] power: supply: axp288_fuel_gauge: Refresh all registers in one go
@ 2021-07-30 16:14     ` kernel test robot
  0 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2021-07-30 16:14 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4534 bytes --]

Hi Hans,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on power-supply/for-next]
[also build test ERROR on v5.14-rc3 next-20210729]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Hans-de-Goede/power-supply-axp288_fuel_gauge-Reduce-number-of-register-accesses-cleanups/20210730-175716
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
config: x86_64-buildonly-randconfig-r001-20210730 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 4f71f59bf3d9914188a11d0c41bedbb339d36ff5)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1ff192ccce54fdfce899447999a60a195537460c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Hans-de-Goede/power-supply-axp288_fuel_gauge-Reduce-number-of-register-accesses-cleanups/20210730-175716
        git checkout 1ff192ccce54fdfce899447999a60a195537460c
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/power/supply/axp288_fuel_gauge.c:219:8: error: implicit declaration of function 'iosf_mbi_block_punit_i2c_access' [-Werror,-Wimplicit-function-declaration]
           ret = iosf_mbi_block_punit_i2c_access();
                 ^
>> drivers/power/supply/axp288_fuel_gauge.c:268:2: error: implicit declaration of function 'iosf_mbi_unblock_punit_i2c_access' [-Werror,-Wimplicit-function-declaration]
           iosf_mbi_unblock_punit_i2c_access();
           ^
   drivers/power/supply/axp288_fuel_gauge.c:268:2: note: did you mean 'iosf_mbi_block_punit_i2c_access'?
   drivers/power/supply/axp288_fuel_gauge.c:219:8: note: 'iosf_mbi_block_punit_i2c_access' declared here
           ret = iosf_mbi_block_punit_i2c_access();
                 ^
   2 errors generated.


vim +/iosf_mbi_block_punit_i2c_access +219 drivers/power/supply/axp288_fuel_gauge.c

   209	
   210	static int fuel_gauge_update_registers(struct axp288_fg_info *info)
   211	{
   212		int ret;
   213	
   214		if (info->valid && time_before(jiffies, info->last_updated + AXP288_REG_UPDATE_INTERVAL))
   215			return 0;
   216	
   217		dev_dbg(info->dev, "Fuel Gauge updating register values...\n");
   218	
 > 219		ret = iosf_mbi_block_punit_i2c_access();
   220		if (ret < 0)
   221			return ret;
   222	
   223		ret = fuel_gauge_reg_readb(info, AXP20X_PWR_INPUT_STATUS);
   224		if (ret < 0)
   225			goto out;
   226		info->pwr_stat = ret;
   227	
   228		ret = fuel_gauge_reg_readb(info, AXP20X_FG_RES);
   229		if (ret < 0)
   230			goto out;
   231		info->fg_res = ret;
   232	
   233		ret = iio_read_channel_raw(info->iio_channel[BAT_VOLT], &info->bat_volt);
   234		if (ret < 0)
   235			goto out;
   236	
   237		if (info->pwr_stat & PS_STAT_BAT_CHRG_DIR) {
   238			info->d_curr = 0;
   239			ret = iio_read_channel_raw(info->iio_channel[BAT_CHRG_CURR], &info->c_curr);
   240			if (ret < 0)
   241				goto out;
   242		} else {
   243			info->c_curr = 0;
   244			ret = iio_read_channel_raw(info->iio_channel[BAT_D_CURR], &info->d_curr);
   245			if (ret < 0)
   246				goto out;
   247		}
   248	
   249		ret = fuel_gauge_read_12bit_word(info, AXP288_FG_OCVH_REG);
   250		if (ret < 0)
   251			goto out;
   252		info->ocv = ret;
   253	
   254		ret = fuel_gauge_read_15bit_word(info, AXP288_FG_CC_MTR1_REG);
   255		if (ret < 0)
   256			goto out;
   257		info->fg_cc_mtr1 = ret;
   258	
   259		ret = fuel_gauge_read_15bit_word(info, AXP288_FG_DES_CAP1_REG);
   260		if (ret < 0)
   261			goto out;
   262		info->fg_des_cap1 = ret;
   263	
   264		info->last_updated = jiffies;
   265		info->valid = 1;
   266		ret = 0;
   267	out:
 > 268		iosf_mbi_unblock_punit_i2c_access();
   269		return ret;
   270	}
   271	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 49545 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 08/10] power: supply: axp288_fuel_gauge: Refresh all registers in one go
  2021-07-30 16:14     ` kernel test robot
@ 2021-08-01 11:23       ` Hans de Goede
  -1 siblings, 0 replies; 17+ messages in thread
From: Hans de Goede @ 2021-08-01 11:23 UTC (permalink / raw)
  To: kernel test robot, Sebastian Reichel
  Cc: clang-built-linux, kbuild-all, Andrejus Basovas, linux-pm

Hi,

On 7/30/21 6:14 PM, kernel test robot wrote:
> Hi Hans,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on power-supply/for-next]
> [also build test ERROR on v5.14-rc3 next-20210729]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Hans-de-Goede/power-supply-axp288_fuel_gauge-Reduce-number-of-register-accesses-cleanups/20210730-175716
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
> config: x86_64-buildonly-randconfig-r001-20210730 (attached as .config)
> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 4f71f59bf3d9914188a11d0c41bedbb339d36ff5)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/0day-ci/linux/commit/1ff192ccce54fdfce899447999a60a195537460c
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Hans-de-Goede/power-supply-axp288_fuel_gauge-Reduce-number-of-register-accesses-cleanups/20210730-175716
>         git checkout 1ff192ccce54fdfce899447999a60a195537460c
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>>> drivers/power/supply/axp288_fuel_gauge.c:219:8: error: implicit declaration of function 'iosf_mbi_block_punit_i2c_access' [-Werror,-Wimplicit-function-declaration]
>            ret = iosf_mbi_block_punit_i2c_access();
>                  ^
>>> drivers/power/supply/axp288_fuel_gauge.c:268:2: error: implicit declaration of function 'iosf_mbi_unblock_punit_i2c_access' [-Werror,-Wimplicit-function-declaration]
>            iosf_mbi_unblock_punit_i2c_access();
>            ^
>    drivers/power/supply/axp288_fuel_gauge.c:268:2: note: did you mean 'iosf_mbi_block_punit_i2c_access'?
>    drivers/power/supply/axp288_fuel_gauge.c:219:8: note: 'iosf_mbi_block_punit_i2c_access' declared here
>            ret = iosf_mbi_block_punit_i2c_access();
>                  ^
>    2 errors generated.

Ugh, so I guess that it is possible to build a x86 kernel without iosf_mbi support
enabled. Given how the I2C bus to PMIC is special on devices with an AXP288 PMIC
and special attention must be made to coordinate accesses with the SoC-s
P-Unit, allowing building of the AXP288 code without IOSF_MBi makes little sense
(this will lead to a very unreliable kernel build) so I'll do a v3 changing the new
depends on from X86 to IOSF_MBI.

Sorry about all the churn because of this Kconfig depends issue.

Regards,

Hans


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 08/10] power: supply: axp288_fuel_gauge: Refresh all registers in one go
@ 2021-08-01 11:23       ` Hans de Goede
  0 siblings, 0 replies; 17+ messages in thread
From: Hans de Goede @ 2021-08-01 11:23 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3079 bytes --]

Hi,

On 7/30/21 6:14 PM, kernel test robot wrote:
> Hi Hans,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on power-supply/for-next]
> [also build test ERROR on v5.14-rc3 next-20210729]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Hans-de-Goede/power-supply-axp288_fuel_gauge-Reduce-number-of-register-accesses-cleanups/20210730-175716
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
> config: x86_64-buildonly-randconfig-r001-20210730 (attached as .config)
> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 4f71f59bf3d9914188a11d0c41bedbb339d36ff5)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/0day-ci/linux/commit/1ff192ccce54fdfce899447999a60a195537460c
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Hans-de-Goede/power-supply-axp288_fuel_gauge-Reduce-number-of-register-accesses-cleanups/20210730-175716
>         git checkout 1ff192ccce54fdfce899447999a60a195537460c
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>>> drivers/power/supply/axp288_fuel_gauge.c:219:8: error: implicit declaration of function 'iosf_mbi_block_punit_i2c_access' [-Werror,-Wimplicit-function-declaration]
>            ret = iosf_mbi_block_punit_i2c_access();
>                  ^
>>> drivers/power/supply/axp288_fuel_gauge.c:268:2: error: implicit declaration of function 'iosf_mbi_unblock_punit_i2c_access' [-Werror,-Wimplicit-function-declaration]
>            iosf_mbi_unblock_punit_i2c_access();
>            ^
>    drivers/power/supply/axp288_fuel_gauge.c:268:2: note: did you mean 'iosf_mbi_block_punit_i2c_access'?
>    drivers/power/supply/axp288_fuel_gauge.c:219:8: note: 'iosf_mbi_block_punit_i2c_access' declared here
>            ret = iosf_mbi_block_punit_i2c_access();
>                  ^
>    2 errors generated.

Ugh, so I guess that it is possible to build a x86 kernel without iosf_mbi support
enabled. Given how the I2C bus to PMIC is special on devices with an AXP288 PMIC
and special attention must be made to coordinate accesses with the SoC-s
P-Unit, allowing building of the AXP288 code without IOSF_MBi makes little sense
(this will lead to a very unreliable kernel build) so I'll do a v3 changing the new
depends on from X86 to IOSF_MBI.

Sorry about all the churn because of this Kconfig depends issue.

Regards,

Hans

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-08-01 11:23 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-30  9:55 [PATCH v2 00/10] power: supply: axp288_fuel_gauge: Reduce number of register accesses + cleanups Hans de Goede
2021-07-30  9:55 ` [PATCH v2 01/10] power: supply: axp288_fuel_gauge: Fix define alignment Hans de Goede
2021-07-30  9:55 ` [PATCH v2 02/10] power: supply: axp288_fuel_gauge: Remove debugfs support Hans de Goede
2021-07-30  9:56 ` [PATCH v2 03/10] power: supply: axp288_fuel_gauge: Silence the chatty IRQ mapping code Hans de Goede
2021-07-30  9:56 ` [PATCH v2 04/10] power: supply: axp288_fuel_gauge: Report register-address on readb / writeb errors Hans de Goede
2021-07-30  9:56 ` [PATCH v2 05/10] power: supply: axp288_fuel_gauge: Drop retry logic from fuel_gauge_reg_readb() Hans de Goede
2021-07-30  9:56 ` [PATCH v2 06/10] power: supply: axp288_fuel_gauge: Store struct device pointer in axp288_fg_info Hans de Goede
2021-07-30  9:56 ` [PATCH v2 07/10] power: supply: axp288_fuel_gauge: Only read PWR_OP_MODE, FG_LOW_CAP_REG regs once Hans de Goede
2021-07-30  9:56 ` [PATCH v2 08/10] power: supply: axp288_fuel_gauge: Refresh all registers in one go Hans de Goede
2021-07-30 13:39   ` kernel test robot
2021-07-30 13:39     ` kernel test robot
2021-07-30 16:14   ` kernel test robot
2021-07-30 16:14     ` kernel test robot
2021-08-01 11:23     ` Hans de Goede
2021-08-01 11:23       ` Hans de Goede
2021-07-30  9:56 ` [PATCH v2 09/10] power: supply: axp288_fuel_gauge: Move the AXP20X_CC_CTRL check together with the other checks Hans de Goede
2021-07-30  9:56 ` [PATCH v2 10/10] power: supply: axp288_fuel_gauge: Take the P-Unit semaphore only once during probe() Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.