Fix RT5033 battery device tree probing
Subsystems (3)
cover Fix RT5033 battery device tree probing
At the moment, the RT5033 MFD and battery driver suggest that the battery driver should probe as a sub-device of the MFD driver. However, this does not make any sense since the fuel gauge part of RT5033 has its own I2C device and interrupt line.
It was also documented as separate I2C device in the original device tree bindings [1] (that were never finished up and merged) but for some reason the code does not match the documentation (and reality). :/
Given other fairly critical mistakes like setting the wrong bits in the regulator driver (see [2]), unfortunately I get the feeling that none of the RT5033 drivers were ever tested properly. :(
This patch sets adds a proper of_match_table to rt5033-battery and removes the rt5033-battery sub-device from the MFD driver. There is no compile/runtime dependency of the power supply / MFD patch so they can just be applied separately through the power supply / MFD tree.
With these changes, rt5033-battery seems to work fine on the Samsung Galaxy A5 (2015) at least (it reports a reasonable battery percentage).
[1]: https://lore.kernel.org/linux-pm/[email protected]/
[2]: https://lore.kernel.org/lkml/[email protected]/
Stephan Gerhold (3):
dt-bindings: power: supply: Add DT schema for richtek,rt5033-battery
power: supply: rt5033_battery: Fix device tree enumeration
mfd: rt5033: Drop rt5033-battery sub-device
.../power/supply/richtek,rt5033-battery.yaml | 54 +++++++++++++++++++ drivers/mfd/rt5033.c | 3 -- drivers/power/supply/Kconfig | 3 +- drivers/power/supply/rt5033_battery.c | 7 +++ 4 files changed, 63 insertions(+), 4 deletions(-) create mode 100644 Documentation/devicetree/bindings/power/supply/richtek,rt5033-battery.yaml
Signature · 1 line
2.31.1
PATCH 1/3 dt-bindings: power: supply: Add DT schema for richtek,rt5033-battery
The RT5033 PMIC provides a simple fuel gauge via I2C. Add a DT schema to describe how to set it up in the device tree.
Note that although RT5033 is a MFD with lots of functionality (also charger, regulator, LEDs, ...) the fuel gauge has a separate I2C bus and is not part of the MFD.
--- .../power/supply/richtek,rt5033-battery.yaml | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/richtek,rt5033-battery.yaml
Documentation/devicetree/bindings/power/supply/richtek,rt5033-battery.yaml new file +54 -0
@@ -0,0 +1,54 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/power/supply/richtek,rt5033-battery.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Richtek RT5033 PMIC Fuel Gauge + +maintainers: + - Stephan Gerhold <[email protected]> + +allOf: + - $ref: power-supply.yaml# + +properties: + compatible: + const: richtek,rt5033-battery + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + battery@35 { + compatible = "richtek,rt5033-battery"; + reg = <0x35>; + }; + }; + - | + #include <dt-bindings/interrupt-controller/irq.h> + i2c { + #address-cells = <1>; + #size-cells = <0>; + + battery@35 { + compatible = "richtek,rt5033-battery"; + reg = <0x35>; + interrupt-parent = <&msmgpio>; + interrupts = <121 IRQ_TYPE_EDGE_FALLING>; + }; + };
Signature · 1 line
2.31.1
PATCH 2/3 power: supply: rt5033_battery: Fix device tree enumeration
In reply to Stephan GerholdThe fuel gauge in the RT5033 PMIC has its own I2C bus and interrupt line. Therefore, it is not actually part of the RT5033 MFD and needs its own of_match_table to probe properly.
Also, given that it's independent of the MFD, there is actually no need to make the Kconfig depend on MFD_RT5033. Although the driver uses the shared <linux/mfd/rt5033.h> header, there is no compile or runtime dependency on the RT5033 MFD driver.
--- drivers/power/supply/Kconfig | 3 ++- drivers/power/supply/rt5033_battery.c | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-)
drivers/power/supply/Kconfig +2 -1
@@ -712,7 +712,8 @@ config BATTERY_GOLDFISH config BATTERY_RT5033 tristate "RT5033 fuel gauge support" - depends on MFD_RT5033 + depends on I2C + select REGMAP_I2C help This adds support for battery fuel gauge in Richtek RT5033 PMIC. The fuelgauge calculates and determines the battery state of charge
drivers/power/supply/rt5033_battery.c +7 -0
@@ -164,9 +164,16 @@ static const struct i2c_device_id rt5033_battery_id[] = { };MODULE_DEVICE_TABLE (i2c, rt5033_battery_id); +static const struct of_device_id rt5033_battery_of_match[] = { + { .compatible ="richtek,rt5033-battery" , }, + { } +}; +MODULE_DEVICE_TABLE (of, rt5033_battery_dt_match); +static struct i2c_driver rt5033_battery_driver = { .driver = { .name ="rt5033-battery" , + .of_match_table = rt5033_battery_of_match, }, .probe = rt5033_battery_probe, .remove = rt5033_battery_remove,
Signature · 1 line
2.31.1
Re: [PATCH 2/3] power: supply: rt5033_battery: Fix device tree enumeration
Quoted 37 lines · Stephan Gerhold
The fuel gauge in the RT5033 PMIC has its own I2C bus and interrupt line. Therefore, it is not actually part of the RT5033 MFD and needs its own
of_match_tableto probe properly.Also, given that it's independent of the MFD, there is actually no need to make the
Kconfigdepend onMFD_RT5033. Although the driver uses the shared <linux/mfd/rt5033.h> header, there is no compile or runtime dependency on the RT5033 MFD driver.Cc Beomho Seo <[email protected]>Cc Chanwoo Choi <[email protected]>Fixes b847dd96e659 ("power: rt5033_battery: Add RT5033 Fuel gauge device driver")Signed-off-by Stephan Gerhold <[email protected]>--- drivers/power/supply/Kconfig | 3 ++- drivers/power/supply/rt5033_battery.c | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-)drivers/power/supply/Kconfig +2
-1@@ -712,7 +712,8 @@ config BATTERY_GOLDFISH config BATTERY_RT5033 tristate "RT5033 fuel gauge support" - depends on MFD_RT5033 + depends on I2C + select REGMAP_I2C help This adds support for battery fuel gauge in Richtek RT5033 PMIC. The fuelgauge calculates and determines the battery state of chargedrivers/power/supply/rt5033_battery.c +0
-0
drivers/power/supply/rt5033_battery.c static const struct i2c_device_id rt5033_battery_id[] = { · line 171
Quoted 8 lines
drivers/power/supply/rt5033_battery.c +5
-0@@ -164,9 +164,16 @@ static const struct i2c_device_id rt5033_battery_id[] = { };MODULE_DEVICE_TABLE (i2c, rt5033_battery_id); +static const struct of_device_id rt5033_battery_of_match[] = { + { .compatible ="richtek,rt5033-battery" , }, + { } +}; +MODULE_DEVICE_TABLE (of, rt5033_battery_dt_match);
s/dt_match/of_match
Ugh, I shouldn't do any last-minute renames and then only compile-test with modules disabled. :(
Sorry, please ignore this one, will send a v2 shortly...
Stephan
PATCH 3/3 mfd: rt5033: Drop rt5033-battery sub-device
In reply to Stephan GerholdThe fuel gauge in the RT5033 PMIC (rt5033-battery) has its own I2C bus and interrupt lines. Therefore, it is not part of the MFD device and needs to be specified separately in the device tree.
--- drivers/mfd/rt5033.c | 3 --- 1 file changed, 3 deletions(-)
drivers/mfd/rt5033.c +0 -3
@@ -41,9 +41,6 @@ static const struct mfd_cell rt5033_devs[] = { { .name = "rt5033-charger", .of_compatible = "richtek,rt5033-charger", - }, { - .name = "rt5033-battery", - .of_compatible = "richtek,rt5033-battery", }, { .name = "rt5033-led", .of_compatible = "richtek,rt5033-led",
Signature · 1 line
2.31.1