[0/3] Fix RT5033 battery device tree probing

Message

Stephan Gerhold May 17, 2021, 10:35 a.m. UTC

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/[e-mail]/
[2]: https://lore.kernel.org/lkml/[e-mail]/

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

Comments

Stephan Gerhold May 17, 2021, 10:46 a.m. UTC | #1

On Mon, May 17, 2021 at 12:35:53PM +0200, Stephan Gerhold wrote:
> 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_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.
> 
> Cc: Beomho Seo <[e-mail]>
> Cc: Chanwoo Choi <[e-mail]>
> Fixes: b847dd96e659 ("power: rt5033_battery: Add RT5033 Fuel gauge device driver")
> Signed-off-by: Stephan Gerhold <[e-mail]>
> ---
>  drivers/power/supply/Kconfig          | 3 ++-
>  drivers/power/supply/rt5033_battery.c | 7 +++++++
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index e696364126f1..20a2f93252f9 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -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
> diff --git a/drivers/power/supply/rt5033_battery.c b/drivers/power/supply/rt5033_battery.c
> index f330452341f0..11eb9ad66ea9 100644
> --- a/drivers/power/supply/rt5033_battery.c
> +++ b/drivers/power/supply/rt5033_battery.c
> @@ -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