[PATCH 0/5] ROHM bd71828 power-supply fixes

[PATCH 0/5] ROHM bd71828 power-supply fixes

[Matti Vaittinen]

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

Fix a few issues from PSU drivers for ROHM components.

I hired couple of gnomes to work for me. :) (Ran AI reviews).
Unsurprizingly some bugs were spotted. Time to try fix mess (mostly) I
have (originally) authored.

Bugs were found by AI review but fixes are made by hand. Hence no
AI-tooling tags are added. Please, let me know if one is needed.

---

NOTE: Only _very_ shallow testing is done. Some of the fixes are not
tested in the hardware at all. All reviewing and testing is appreciated
as usual.

Matti Vaittinen (5):
  power: supply: bd71815: Fix temperature reading
  power: supply: bd71828: Fix current direction
  power: supply: bd71828: Drop duplicate power-supply property
  power: supply: bd71828: Do not hide errors
  power: supply: bd99954: Drop bad register fields

 drivers/power/supply/bd71828-power.c   | 20 ++++++++++++--------
 drivers/power/supply/bd99954-charger.h | 23 +----------------------
 2 files changed, 13 insertions(+), 30 deletions(-)


base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 1/5] power: supply: bd71815: Fix temperature reading

[Matti Vaittinen]

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

From: Matti Vaittinen <mazziesaccount@gmail.com>

The temperature read from the hardware is never returned to caller.
Furthermore, the check for temperature validity is wrong. This yields
garbage value to be returned to caller, and also detection of
bad values read from hardware to fail.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
Fixes: 5bff79dad20a ("power: supply: Add bd718(15/28/78) charger driver")
---
 drivers/power/supply/bd71828-power.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/bd71828-power.c b/drivers/power/supply/bd71828-power.c
index b671563ead79..e1ae5fe52e1d 100644
--- a/drivers/power/supply/bd71828-power.c
+++ b/drivers/power/supply/bd71828-power.c
@@ -225,13 +225,13 @@ static int bd71815_get_temp(struct bd71828_power *pwr, int *temp)
 	if (ret)
 		return ret;
 
-	t = 200 - t;
-
 	if (t > 200) {
 		dev_err(pwr->dev, "Failed to read battery temperature\n");
 		return -ENODATA;
 	}
 
+	*temp = 200 - t;
+
 	return 0;
 }
 
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 2/5] power: supply: bd71828: Fix current direction

[Matti Vaittinen]

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

From: Matti Vaittinen <mazziesaccount@gmail.com>

Both the current and the average current registers contain same
'direction' -bit to denote the direction of the current. The code
reading these registers incorrectly caches the direction read from the
first register, and uses it also for the second.

Fix this by initializing the direction bit for both register reads.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
Fixes: 5bff79dad20a ("power: supply: Add bd718(15/28/78) charger driver")
---
 drivers/power/supply/bd71828-power.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/bd71828-power.c b/drivers/power/supply/bd71828-power.c
index e1ae5fe52e1d..9927a7e8934a 100644
--- a/drivers/power/supply/bd71828-power.c
+++ b/drivers/power/supply/bd71828-power.c
@@ -192,12 +192,13 @@ static int bd71828_get_current_ds_adc(struct bd71828_power *pwr, int *curr, int
 {
 	__be16 tmp_curr;
 	char *tmp = (char *)&tmp_curr;
-	int dir = 1;
 	int regs[] = { pwr->regs->ibat, pwr->regs->ibat_avg };
 	int *vals[] = { curr, curr_avg };
 	int ret, i;
 
-	for (dir = 1, i = 0; i < ARRAY_SIZE(regs); i++) {
+	for (i = 0; i < ARRAY_SIZE(regs); i++) {
+		int dir = 1;
+
 		ret = regmap_bulk_read(pwr->regmap, regs[i], &tmp_curr,
 				       sizeof(tmp_curr));
 		if (ret)
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 3/5] power: supply: bd71828: Drop duplicate power-supply property

[Matti Vaittinen]

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

From: Matti Vaittinen <mazziesaccount@gmail.com>

The POWER_SUPPLY_PROP_HEALTH is added into property-array twice.

Drop the duplicate property.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
Fixes: 5bff79dad20a ("power: supply: Add bd718(15/28/78) charger driver")
---
 drivers/power/supply/bd71828-power.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/power/supply/bd71828-power.c b/drivers/power/supply/bd71828-power.c
index 9927a7e8934a..a564ad7210be 100644
--- a/drivers/power/supply/bd71828-power.c
+++ b/drivers/power/supply/bd71828-power.c
@@ -640,7 +640,6 @@ static const enum power_supply_property bd71828_charger_props[] = {
 
 static const enum power_supply_property bd71828_battery_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
-	POWER_SUPPLY_PROP_HEALTH,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_HEALTH,
 	POWER_SUPPLY_PROP_PRESENT,
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 4/5] power: supply: bd71828: Do not hide errors

[Matti Vaittinen]

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

From: Matti Vaittinen <mazziesaccount@gmail.com>

The bd71828 reports success / continues operation even when some regmap
operations fail. This can lead incorrect data to be reported.

Return appropriate errors when operations fail.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
Fixes: 5bff79dad20a ("power: supply: Add bd718(15/28/78) charger driver")
---
 drivers/power/supply/bd71828-power.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/power/supply/bd71828-power.c b/drivers/power/supply/bd71828-power.c
index a564ad7210be..589e12225cb2 100644
--- a/drivers/power/supply/bd71828-power.c
+++ b/drivers/power/supply/bd71828-power.c
@@ -456,8 +456,10 @@ static int bd71828_charger_get_property(struct power_supply *psy,
 	switch (psp) {
 	case POWER_SUPPLY_PROP_ONLINE:
 		ret = get_chg_online(pwr, &online);
-		if (!ret)
-			val->intval = online;
+		if (ret)
+			return ret;
+
+		val->intval = online;
 		break;
 	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
 		ret = bd7182x_read16_himask(pwr, pwr->regs->vdcin,
@@ -1218,7 +1220,9 @@ static int bd71828_power_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, ret, "sense resistor missing\n");
 
 	dev_set_drvdata(&pdev->dev, pwr);
-	bd71828_init_hardware(pwr);
+	ret = bd71828_init_hardware(pwr);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "hardware init failed\n");
 
 	bat_cfg.drv_data	= pwr;
 	bat_cfg.fwnode		= dev_fwnode(&pdev->dev);
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 5/5] power: supply: bd99954: Drop bad register fields

[Matti Vaittinen]

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

From: Matti Vaittinen <mazziesaccount@gmail.com>

The bd99954 driver creates regmap fields for all the registers. All the
fields for VBUS_UCD_FCTRL_SET (extended command 0x33) are accidentally
created for VCC_UCD_FCTRL_SET (extended command 0x2b), causing all
reads/writes for the fields to access wrong register. Luckily the fields
for VBUS_UCD_FCTRL_SET are unused.

Taking a look at the data-sheet:
https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/battery_management/bd99954xxx-e.pdf

all the fields in the VBUS_UCD_FCTRL_SET are marked as:
"In normal operation, please don’t set these registers" with not much of
additional documentation. As the fields are wrong and because there seems
to be no users of the fields - it is better to just drop them. This will
also decrease the memory footprint.

Drop incorrect VBUS_UCD_FCTRL_SET fields.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
Fixes: 0902f8366491 ("power: supply: Support ROHM bd99954 charger")
---
 drivers/power/supply/bd99954-charger.h | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/drivers/power/supply/bd99954-charger.h b/drivers/power/supply/bd99954-charger.h
index f58897925383..d053ac901274 100644
--- a/drivers/power/supply/bd99954-charger.h
+++ b/drivers/power/supply/bd99954-charger.h
@@ -279,17 +279,7 @@ enum bd9995x_fields {
 	F_VBUS_EXTID,
 	F_VBUS_IDRDET,
 	F_VBUS_INDO,
-	F_VBUS_UCDSWEN,
-	F_VBUS_RREF_EN,
-	F_VBUS_DPPU_EN,
-	F_VBUS_DPREF_EN,
-	F_VBUS_DMREF_EN,
-	F_VBUS_DPDET_EN,
-	F_VBUS_DMDET_EN,
-	F_VBUS_DPSINK_EN,
-	F_VBUS_DMSINK_EN,
-	F_VBUS_DP_BUFF_EN,
-	F_VBUS_DM_BUFF_EN,
+
 	F_VBUS_EXTCLKENBL,
 	F_VBUS_PLSTESTEN,
 	F_VBUS_UCDSWEN_TSTENB,
@@ -626,17 +616,6 @@ static const struct reg_field bd9995x_reg_fields[] = {
 	    [F_VBUS_EXTID] = REG_FIELD(VBUS_IDD_STATUS, 5, 5),
 	    [F_VBUS_IDRDET] = REG_FIELD(VBUS_IDD_STATUS, 4, 4),
 	    [F_VBUS_INDO] = REG_FIELD(VBUS_IDD_STATUS, 0, 3),
-	    [F_VBUS_UCDSWEN] = REG_FIELD(VCC_UCD_FCTRL_SET, 10, 10),
-	    [F_VBUS_RREF_EN] = REG_FIELD(VCC_UCD_FCTRL_SET, 9, 9),
-	    [F_VBUS_DPPU_EN] = REG_FIELD(VCC_UCD_FCTRL_SET, 8, 8),
-	    [F_VBUS_DPREF_EN] = REG_FIELD(VCC_UCD_FCTRL_SET, 7, 7),
-	    [F_VBUS_DMREF_EN] = REG_FIELD(VCC_UCD_FCTRL_SET, 6, 6),
-	    [F_VBUS_DPDET_EN] = REG_FIELD(VCC_UCD_FCTRL_SET, 5, 5),
-	    [F_VBUS_DMDET_EN] = REG_FIELD(VCC_UCD_FCTRL_SET, 4, 4),
-	    [F_VBUS_DPSINK_EN] = REG_FIELD(VCC_UCD_FCTRL_SET, 3, 3),
-	    [F_VBUS_DMSINK_EN] = REG_FIELD(VCC_UCD_FCTRL_SET, 2, 2),
-	    [F_VBUS_DP_BUFF_EN] = REG_FIELD(VCC_UCD_FCTRL_SET, 1, 1),
-	    [F_VBUS_DM_BUFF_EN] = REG_FIELD(VCC_UCD_FCTRL_SET, 0, 0),
 
 	    [F_VBUS_EXTCLKENBL] = REG_FIELD(VBUS_UCD_FCTRL_EN, 15, 15),
 	    [F_VBUS_PLSTESTEN] = REG_FIELD(VBUS_UCD_FCTRL_EN, 14, 14),
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/5] power: supply: bd71828: Fix current direction

[Andy Shevchenko]

On Mon, Aug 10, 2026 at 01:09:07PM +0300, Matti Vaittinen wrote:

> Both the current and the average current registers contain same
> 'direction' -bit to denote the direction of the current. The code
> reading these registers incorrectly caches the direction read from the
> first register, and uses it also for the second.
> 
> Fix this by initializing the direction bit for both register reads.

...

> static int bd71828_get_current_ds_adc(struct bd71828_power *pwr, int *curr, int

>  {
>  	__be16 tmp_curr;
>  	char *tmp = (char *)&tmp_curr;
> -	int dir = 1;
>  	int regs[] = { pwr->regs->ibat, pwr->regs->ibat_avg };
>  	int *vals[] = { curr, curr_avg };
>  	int ret, i;

> -	for (dir = 1, i = 0; i < ARRAY_SIZE(regs); i++) {
> +	for (i = 0; i < ARRAY_SIZE(regs); i++) {


While at it

	for (unsigned int i = 0; i < ARRAY_SIZE(regs); i++) {

> +		int dir = 1;


...and drop it from the top declaration.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/5] power: supply: bd71828: Drop duplicate power-supply property

[Andy Shevchenko]

On Mon, Aug 10, 2026 at 01:09:25PM +0300, Matti Vaittinen wrote:

> The POWER_SUPPLY_PROP_HEALTH is added into property-array twice.
> 
> Drop the duplicate property.

From this commit message is not obvious why is this a fix. Perhaps elaborate
that the index is somehow related (or what is the reason behind)?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 5/5] power: supply: bd99954: Drop bad register fields

[Andy Shevchenko]

On Mon, Aug 10, 2026 at 01:10:03PM +0300, Matti Vaittinen wrote:

> The bd99954 driver creates regmap fields for all the registers. All the
> fields for VBUS_UCD_FCTRL_SET (extended command 0x33) are accidentally
> created for VCC_UCD_FCTRL_SET (extended command 0x2b), causing all
> reads/writes for the fields to access wrong register. Luckily the fields
> for VBUS_UCD_FCTRL_SET are unused.
> 
> Taking a look at the data-sheet:
> https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/battery_management/bd99954xxx-e.pdf

Make it a Link or Datasheet tag.

> all the fields in the VBUS_UCD_FCTRL_SET are marked as:
> "In normal operation, please don’t set these registers" with not much of
> additional documentation. As the fields are wrong and because there seems
> to be no users of the fields - it is better to just drop them. This will
> also decrease the memory footprint.
> 
> Drop incorrect VBUS_UCD_FCTRL_SET fields.

-- 
With Best Regards,
Andy Shevchenko