| Message ID | 20230506155435.3005-8-jahau@rocketmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add RT5033 charger device driver | expand |
Hi, On Sat, May 06, 2023 at 05:54:34PM +0200, Jakob Hauser wrote: > The rt5033-battery fuelgauge can't get a status by itself. The rt5033-charger > can, let's get this value. > > Tested-by: Raymond Hackley <raymondhackley@protonmail.com> > Signed-off-by: Jakob Hauser <jahau@rocketmail.com> > --- > drivers/power/supply/rt5033_battery.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/power/supply/rt5033_battery.c b/drivers/power/supply/rt5033_battery.c > index 5c04cf305219..a6520716d813 100644 > --- a/drivers/power/supply/rt5033_battery.c > +++ b/drivers/power/supply/rt5033_battery.c > @@ -12,6 +12,26 @@ > #include <linux/mfd/rt5033-private.h> > #include <linux/mfd/rt5033.h> > > +static int rt5033_battery_get_status(struct i2c_client *client) > +{ > + struct power_supply *charger; > + union power_supply_propval val; > + int ret; > + > + charger = power_supply_get_by_name("rt5033-charger"); > + if (!charger) > + return POWER_SUPPLY_STATUS_UNKNOWN; > + > + ret = power_supply_get_property(charger, POWER_SUPPLY_PROP_STATUS, &val); > + if (ret) { > + power_supply_put(charger); > + return POWER_SUPPLY_STATUS_UNKNOWN; > + } struct rt5033_battery *battery = i2c_get_clientdata(client); ret = power_supply_get_property_from_supplier(battery->psy, POWER_SUPPLY_PROP_STATUS, &val); if (ret) val.intval = POWER_SUPPLY_STATUS_UNKNOWN; > + > + power_supply_put(charger); > + return val.intval; > +} > + > static int rt5033_battery_get_capacity(struct i2c_client *client) > { > struct rt5033_battery *battery = i2c_get_clientdata(client); > @@ -84,6 +104,9 @@ static int rt5033_battery_get_property(struct power_supply *psy, > case POWER_SUPPLY_PROP_CAPACITY: > val->intval = rt5033_battery_get_capacity(battery->client); > break; > + case POWER_SUPPLY_PROP_STATUS: > + val->intval = rt5033_battery_get_status(battery->client); > + break; > default: > return -EINVAL; > } > @@ -96,6 +119,7 @@ static enum power_supply_property rt5033_battery_props[] = { > POWER_SUPPLY_PROP_VOLTAGE_OCV, > POWER_SUPPLY_PROP_PRESENT, > POWER_SUPPLY_PROP_CAPACITY, > + POWER_SUPPLY_PROP_STATUS, > }; > > static const struct regmap_config rt5033_battery_regmap_config = { > -- > 2.39.2 > Otherwise LGTM. -- Sebastian
Hi Sebastian, On 08.05.23 13:35, Sebastian Reichel wrote: > Hi, > > On Sat, May 06, 2023 at 05:54:34PM +0200, Jakob Hauser wrote: >> The rt5033-battery fuelgauge can't get a status by itself. The rt5033-charger >> can, let's get this value. >> >> Tested-by: Raymond Hackley <raymondhackley@protonmail.com> >> Signed-off-by: Jakob Hauser <jahau@rocketmail.com> >> --- >> drivers/power/supply/rt5033_battery.c | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/drivers/power/supply/rt5033_battery.c b/drivers/power/supply/rt5033_battery.c >> index 5c04cf305219..a6520716d813 100644 >> --- a/drivers/power/supply/rt5033_battery.c >> +++ b/drivers/power/supply/rt5033_battery.c >> @@ -12,6 +12,26 @@ >> #include <linux/mfd/rt5033-private.h> >> #include <linux/mfd/rt5033.h> >> >> +static int rt5033_battery_get_status(struct i2c_client *client) >> +{ >> + struct power_supply *charger; >> + union power_supply_propval val; >> + int ret; >> + >> + charger = power_supply_get_by_name("rt5033-charger"); >> + if (!charger) >> + return POWER_SUPPLY_STATUS_UNKNOWN; >> + >> + ret = power_supply_get_property(charger, POWER_SUPPLY_PROP_STATUS, &val); >> + if (ret) { >> + power_supply_put(charger); >> + return POWER_SUPPLY_STATUS_UNKNOWN; >> + } > > struct rt5033_battery *battery = i2c_get_clientdata(client); > ret = power_supply_get_property_from_supplier(battery->psy, POWER_SUPPLY_PROP_STATUS, &val); > if (ret) > val.intval = POWER_SUPPLY_STATUS_UNKNOWN; I don't think this works. There is no direct relationship between rt5033-charger and rt5033-battery. They operate independently from each other. I had a short try and the status property of rt5033-battery was "unknown". Just for the record, the full function I tried was: static int rt5033_battery_get_status(struct i2c_client *client) { struct rt5033_battery *battery = i2c_get_clientdata(client); union power_supply_propval val; int ret; ret = power_supply_get_property_from_supplier(battery->psy, POWER_SUPPLY_PROP_STATUS, &val); if (ret) val.intval = POWER_SUPPLY_STATUS_UNKNOWN; return val.intval; } Later on I added a read-out of the "ret" value. It is "-19". I guess that's the "return -ENODEV;" from function power_supply_get_property_from_supplier(). [2] [2] https://github.com/torvalds/linux/blob/v6.4-rc1/drivers/power/supply/power_supply_core.c#L397-L421 >> + >> + power_supply_put(charger); >> + return val.intval; >> +} >> + >> static int rt5033_battery_get_capacity(struct i2c_client *client) >> { >> struct rt5033_battery *battery = i2c_get_clientdata(client); >> @@ -84,6 +104,9 @@ static int rt5033_battery_get_property(struct power_supply *psy, >> case POWER_SUPPLY_PROP_CAPACITY: >> val->intval = rt5033_battery_get_capacity(battery->client); >> break; >> + case POWER_SUPPLY_PROP_STATUS: >> + val->intval = rt5033_battery_get_status(battery->client); >> + break; >> default: >> return -EINVAL; >> } >> @@ -96,6 +119,7 @@ static enum power_supply_property rt5033_battery_props[] = { >> POWER_SUPPLY_PROP_VOLTAGE_OCV, >> POWER_SUPPLY_PROP_PRESENT, >> POWER_SUPPLY_PROP_CAPACITY, >> + POWER_SUPPLY_PROP_STATUS, >> }; >> >> static const struct regmap_config rt5033_battery_regmap_config = { >> -- >> 2.39.2 >> > > Otherwise LGTM. > > -- Sebastian Kind regards, Jakob
Hi, On Mon, May 08, 2023 at 11:18:28PM +0200, Jakob Hauser wrote: > On 08.05.23 13:35, Sebastian Reichel wrote: > > On Sat, May 06, 2023 at 05:54:34PM +0200, Jakob Hauser wrote: > > > The rt5033-battery fuelgauge can't get a status by itself. The rt5033-charger > > > can, let's get this value. > > > > > > Tested-by: Raymond Hackley <raymondhackley@protonmail.com> > > > Signed-off-by: Jakob Hauser <jahau@rocketmail.com> > > > --- > > > drivers/power/supply/rt5033_battery.c | 24 ++++++++++++++++++++++++ > > > 1 file changed, 24 insertions(+) > > > > > > diff --git a/drivers/power/supply/rt5033_battery.c b/drivers/power/supply/rt5033_battery.c > > > index 5c04cf305219..a6520716d813 100644 > > > --- a/drivers/power/supply/rt5033_battery.c > > > +++ b/drivers/power/supply/rt5033_battery.c > > > @@ -12,6 +12,26 @@ > > > #include <linux/mfd/rt5033-private.h> > > > #include <linux/mfd/rt5033.h> > > > +static int rt5033_battery_get_status(struct i2c_client *client) > > > +{ > > > + struct power_supply *charger; > > > + union power_supply_propval val; > > > + int ret; > > > + > > > + charger = power_supply_get_by_name("rt5033-charger"); > > > + if (!charger) > > > + return POWER_SUPPLY_STATUS_UNKNOWN; > > > + > > > + ret = power_supply_get_property(charger, POWER_SUPPLY_PROP_STATUS, &val); > > > + if (ret) { > > > + power_supply_put(charger); > > > + return POWER_SUPPLY_STATUS_UNKNOWN; > > > + } > > > > struct rt5033_battery *battery = i2c_get_clientdata(client); > > ret = power_supply_get_property_from_supplier(battery->psy, POWER_SUPPLY_PROP_STATUS, &val); > > if (ret) > > val.intval = POWER_SUPPLY_STATUS_UNKNOWN; > > I don't think this works. There is no direct relationship between > rt5033-charger and rt5033-battery. They operate independently from each > other. That should be fine as long as the supply dependency is properly declared. > I had a short try and the status property of rt5033-battery was "unknown". > > Just for the record, the full function I tried was: > > static int rt5033_battery_get_status(struct i2c_client *client) > { > struct rt5033_battery *battery = i2c_get_clientdata(client); > union power_supply_propval val; > int ret; > > ret = power_supply_get_property_from_supplier(battery->psy, > POWER_SUPPLY_PROP_STATUS, > &val); > if (ret) > val.intval = POWER_SUPPLY_STATUS_UNKNOWN; > > return val.intval; > } > > Later on I added a read-out of the "ret" value. It is "-19". I guess that's > the "return -ENODEV;" from function > power_supply_get_property_from_supplier(). [2] > > [2] https://github.com/torvalds/linux/blob/v6.4-rc1/drivers/power/supply/power_supply_core.c#L397-L421 I suppose your DT is missing the connection between the charger and the battery: rt5033_charger: charger { compatible = "rt5033-charger"; ... } fuel-gauge { compatible = "rt5033-battery"; ... power-supplies = <&rt5033_charger>; // you are probably missing this }; See also Documentation/devicetree/bindings/power/supply/power-supply.yaml -- Sebastian > > > > + > > > + power_supply_put(charger); > > > + return val.intval; > > > +} > > > + > > > static int rt5033_battery_get_capacity(struct i2c_client *client) > > > { > > > struct rt5033_battery *battery = i2c_get_clientdata(client); > > > @@ -84,6 +104,9 @@ static int rt5033_battery_get_property(struct power_supply *psy, > > > case POWER_SUPPLY_PROP_CAPACITY: > > > val->intval = rt5033_battery_get_capacity(battery->client); > > > break; > > > + case POWER_SUPPLY_PROP_STATUS: > > > + val->intval = rt5033_battery_get_status(battery->client); > > > + break; > > > default: > > > return -EINVAL; > > > } > > > @@ -96,6 +119,7 @@ static enum power_supply_property rt5033_battery_props[] = { > > > POWER_SUPPLY_PROP_VOLTAGE_OCV, > > > POWER_SUPPLY_PROP_PRESENT, > > > POWER_SUPPLY_PROP_CAPACITY, > > > + POWER_SUPPLY_PROP_STATUS, > > > }; > > > static const struct regmap_config rt5033_battery_regmap_config = { > > > -- > > > 2.39.2 > > > > > > > Otherwise LGTM. > > > > -- Sebastian > > Kind regards, > Jakob
Hi Sebastian, On 09.05.23 00:06, Sebastian Reichel wrote: > Hi, > > On Mon, May 08, 2023 at 11:18:28PM +0200, Jakob Hauser wrote: >> On 08.05.23 13:35, Sebastian Reichel wrote: >>> On Sat, May 06, 2023 at 05:54:34PM +0200, Jakob Hauser wrote: >>>> The rt5033-battery fuelgauge can't get a status by itself. The rt5033-charger >>>> can, let's get this value. >>>> >>>> Tested-by: Raymond Hackley <raymondhackley@protonmail.com> >>>> Signed-off-by: Jakob Hauser <jahau@rocketmail.com> >>>> --- >>>> drivers/power/supply/rt5033_battery.c | 24 ++++++++++++++++++++++++ >>>> 1 file changed, 24 insertions(+) >>>> >>>> diff --git a/drivers/power/supply/rt5033_battery.c b/drivers/power/supply/rt5033_battery.c >>>> index 5c04cf305219..a6520716d813 100644 >>>> --- a/drivers/power/supply/rt5033_battery.c >>>> +++ b/drivers/power/supply/rt5033_battery.c >>>> @@ -12,6 +12,26 @@ >>>> #include <linux/mfd/rt5033-private.h> >>>> #include <linux/mfd/rt5033.h> >>>> +static int rt5033_battery_get_status(struct i2c_client *client) >>>> +{ >>>> + struct power_supply *charger; >>>> + union power_supply_propval val; >>>> + int ret; >>>> + >>>> + charger = power_supply_get_by_name("rt5033-charger"); >>>> + if (!charger) >>>> + return POWER_SUPPLY_STATUS_UNKNOWN; >>>> + >>>> + ret = power_supply_get_property(charger, POWER_SUPPLY_PROP_STATUS, &val); >>>> + if (ret) { >>>> + power_supply_put(charger); >>>> + return POWER_SUPPLY_STATUS_UNKNOWN; >>>> + } >>> >>> struct rt5033_battery *battery = i2c_get_clientdata(client); >>> ret = power_supply_get_property_from_supplier(battery->psy, POWER_SUPPLY_PROP_STATUS, &val); >>> if (ret) >>> val.intval = POWER_SUPPLY_STATUS_UNKNOWN; >> >> I don't think this works. There is no direct relationship between >> rt5033-charger and rt5033-battery. They operate independently from each >> other. > > That should be fine as long as the supply dependency is properly declared. > >> I had a short try and the status property of rt5033-battery was "unknown". >> >> Just for the record, the full function I tried was: >> >> static int rt5033_battery_get_status(struct i2c_client *client) >> { >> struct rt5033_battery *battery = i2c_get_clientdata(client); >> union power_supply_propval val; >> int ret; >> >> ret = power_supply_get_property_from_supplier(battery->psy, >> POWER_SUPPLY_PROP_STATUS, >> &val); >> if (ret) >> val.intval = POWER_SUPPLY_STATUS_UNKNOWN; >> >> return val.intval; >> } >> >> Later on I added a read-out of the "ret" value. It is "-19". I guess that's >> the "return -ENODEV;" from function >> power_supply_get_property_from_supplier(). [2] >> >> [2] https://github.com/torvalds/linux/blob/v6.4-rc1/drivers/power/supply/power_supply_core.c#L397-L421 > > I suppose your DT is missing the connection between the charger and > the battery: > > rt5033_charger: charger { > compatible = "rt5033-charger"; > ... > } > > fuel-gauge { > compatible = "rt5033-battery"; > ... > power-supplies = <&rt5033_charger>; // you are probably missing this > }; > > See also Documentation/devicetree/bindings/power/supply/power-supply.yaml ... Thanks for the hints. This leads to updating the dt-bindings because adding the "power-supplies" property is important to be aware of. Btw. first it didn't work. It took me quite some time to debug. I needed to add "psy_cfg.of_node = client->dev.of_node;" to the rt5033-battery probe function. Now it works. However, there is a new problem. The battery driver gets probed first. The charger driver a bit later. In the meantime the battery driver spams dmesg with several "Failed to register power supply" because the charger driver isn't available yet. Once the charger driver is there, it works fine and dmesg becomes silent. With the current state of the patchset: dmesg | grep rt5033 [ 13.628030] rt5033 6-0034: Device found (rev. 6) [ 13.633552] rt5033-led: Failed to locate of_node [id: -1] [ 13.790478] rt5033-charger rt5033-charger: DMA mask not set With the changes discussed here: dmesg | grep rt5033 [ 15.741915] rt5033-battery 4-0035: Failed to register power supply [ 15.752894] rt5033-battery 4-0035: Failed to register power supply [ 15.795458] rt5033-battery 4-0035: Failed to register power supply [ 15.910760] rt5033-battery 4-0035: Failed to register power supply [ 15.913187] rt5033 6-0034: Device found (rev. 6) [ 15.914341] rt5033-led: Failed to locate of_node [id: -1] [ 15.920052] rt5033-battery 4-0035: Failed to register power supply [ 15.927262] rt5033-battery 4-0035: Failed to register power supply [ 16.017131] rt5033-battery 4-0035: Failed to register power supply [ 16.017401] rt5033-charger rt5033-charger: DMA mask not set The message is comming from the rt5033-battery probe function, it's the power_supply_register() that fails. Any ideas what could be done about this? Kind regards, Jakob
Hi, On Tue, May 09, 2023 at 03:01:32AM +0200, Jakob Hauser wrote: > On 09.05.23 00:06, Sebastian Reichel wrote: > > On Mon, May 08, 2023 at 11:18:28PM +0200, Jakob Hauser wrote: > > > On 08.05.23 13:35, Sebastian Reichel wrote: > > > > On Sat, May 06, 2023 at 05:54:34PM +0200, Jakob Hauser wrote: > > > > > The rt5033-battery fuelgauge can't get a status by itself. The rt5033-charger > > > > > can, let's get this value. > > > > > > > > > > Tested-by: Raymond Hackley <raymondhackley@protonmail.com> > > > > > Signed-off-by: Jakob Hauser <jahau@rocketmail.com> > > > > > --- > > > > > drivers/power/supply/rt5033_battery.c | 24 ++++++++++++++++++++++++ > > > > > 1 file changed, 24 insertions(+) > > > > > > > > > > diff --git a/drivers/power/supply/rt5033_battery.c b/drivers/power/supply/rt5033_battery.c > > > > > index 5c04cf305219..a6520716d813 100644 > > > > > --- a/drivers/power/supply/rt5033_battery.c > > > > > +++ b/drivers/power/supply/rt5033_battery.c > > > > > @@ -12,6 +12,26 @@ > > > > > #include <linux/mfd/rt5033-private.h> > > > > > #include <linux/mfd/rt5033.h> > > > > > +static int rt5033_battery_get_status(struct i2c_client *client) > > > > > +{ > > > > > + struct power_supply *charger; > > > > > + union power_supply_propval val; > > > > > + int ret; > > > > > + > > > > > + charger = power_supply_get_by_name("rt5033-charger"); > > > > > + if (!charger) > > > > > + return POWER_SUPPLY_STATUS_UNKNOWN; > > > > > + > > > > > + ret = power_supply_get_property(charger, POWER_SUPPLY_PROP_STATUS, &val); > > > > > + if (ret) { > > > > > + power_supply_put(charger); > > > > > + return POWER_SUPPLY_STATUS_UNKNOWN; > > > > > + } > > > > > > > > struct rt5033_battery *battery = i2c_get_clientdata(client); > > > > ret = power_supply_get_property_from_supplier(battery->psy, POWER_SUPPLY_PROP_STATUS, &val); > > > > if (ret) > > > > val.intval = POWER_SUPPLY_STATUS_UNKNOWN; > > > > > > I don't think this works. There is no direct relationship between > > > rt5033-charger and rt5033-battery. They operate independently from each > > > other. > > > > That should be fine as long as the supply dependency is properly declared. > > > > > I had a short try and the status property of rt5033-battery was "unknown". > > > > > > Just for the record, the full function I tried was: > > > > > > static int rt5033_battery_get_status(struct i2c_client *client) > > > { > > > struct rt5033_battery *battery = i2c_get_clientdata(client); > > > union power_supply_propval val; > > > int ret; > > > > > > ret = power_supply_get_property_from_supplier(battery->psy, > > > POWER_SUPPLY_PROP_STATUS, > > > &val); > > > if (ret) > > > val.intval = POWER_SUPPLY_STATUS_UNKNOWN; > > > > > > return val.intval; > > > } > > > > > > Later on I added a read-out of the "ret" value. It is "-19". I guess that's > > > the "return -ENODEV;" from function > > > power_supply_get_property_from_supplier(). [2] > > > > > > [2] https://github.com/torvalds/linux/blob/v6.4-rc1/drivers/power/supply/power_supply_core.c#L397-L421 > > > > I suppose your DT is missing the connection between the charger and > > the battery: > > > > rt5033_charger: charger { > > compatible = "rt5033-charger"; > > ... > > } > > > > fuel-gauge { > > compatible = "rt5033-battery"; > > ... > > power-supplies = <&rt5033_charger>; // you are probably missing this > > }; > > > > See also Documentation/devicetree/bindings/power/supply/power-supply.yaml > > ... > > Thanks for the hints. > > This leads to updating the dt-bindings because adding the "power-supplies" > property is important to be aware of. It should already be part of the binding, because richtek,rt5033-battery.yaml has allOf: - $ref: power-supply.yaml# > Btw. first it didn't work. It took me quite some time to debug. I needed to > add "psy_cfg.of_node = client->dev.of_node;" to the rt5033-battery probe > function. > > Now it works. However, there is a new problem. The battery driver gets > probed first. The charger driver a bit later. In the meantime the battery > driver spams dmesg with several "Failed to register power supply" because > the charger driver isn't available yet. Once the charger driver is there, it > works fine and dmesg becomes silent. > > With the current state of the patchset: > dmesg | grep rt5033 > [ 13.628030] rt5033 6-0034: Device found (rev. 6) > [ 13.633552] rt5033-led: Failed to locate of_node [id: -1] > [ 13.790478] rt5033-charger rt5033-charger: DMA mask not set > > With the changes discussed here: > dmesg | grep rt5033 > [ 15.741915] rt5033-battery 4-0035: Failed to register power supply > [ 15.752894] rt5033-battery 4-0035: Failed to register power supply > [ 15.795458] rt5033-battery 4-0035: Failed to register power supply > [ 15.910760] rt5033-battery 4-0035: Failed to register power supply > [ 15.913187] rt5033 6-0034: Device found (rev. 6) > [ 15.914341] rt5033-led: Failed to locate of_node [id: -1] > [ 15.920052] rt5033-battery 4-0035: Failed to register power supply > [ 15.927262] rt5033-battery 4-0035: Failed to register power supply > [ 16.017131] rt5033-battery 4-0035: Failed to register power supply > [ 16.017401] rt5033-charger rt5033-charger: DMA mask not set > > The message is comming from the rt5033-battery probe function, it's the > power_supply_register() that fails. > > Any ideas what could be done about this? Replace the dev_err() with dev_err_probe(): if (IS_ERR(battery->psy)) return dev_err_probe(&client->dev, PTR_ERR(battery->psy), "Failed to register power supply\n"); That will avoid printing an error for -EPROBE_DEFER. Greetings, -- Sebastian
Hi Sebastian, On 09.05.23 09:25, Sebastian Reichel wrote: > On Tue, May 09, 2023 at 03:01:32AM +0200, Jakob Hauser wrote: >> On 09.05.23 00:06, Sebastian Reichel wrote: ... >>> I suppose your DT is missing the connection between the charger and >>> the battery: >>> >>> rt5033_charger: charger { >>> compatible = "rt5033-charger"; >>> ... >>> } >>> >>> fuel-gauge { >>> compatible = "rt5033-battery"; >>> ... >>> power-supplies = <&rt5033_charger>; // you are probably missing this >>> }; >>> >>> See also Documentation/devicetree/bindings/power/supply/power-supply.yaml >> >> ... >> >> Thanks for the hints. >> >> This leads to updating the dt-bindings because adding the "power-supplies" >> property is important to be aware of. > > It should already be part of the binding, because richtek,rt5033-battery.yaml has > > allOf: > - $ref: power-supply.yaml# Uh, I see, you're two steps ahead ;) >> Btw. first it didn't work. It took me quite some time to debug. I needed to >> add "psy_cfg.of_node = client->dev.of_node;" to the rt5033-battery probe >> function. >> >> Now it works. However, there is a new problem. The battery driver gets >> probed first. The charger driver a bit later. In the meantime the battery >> driver spams dmesg with several "Failed to register power supply" because >> the charger driver isn't available yet. Once the charger driver is there, it >> works fine and dmesg becomes silent. >> >> With the current state of the patchset: >> dmesg | grep rt5033 >> [ 13.628030] rt5033 6-0034: Device found (rev. 6) >> [ 13.633552] rt5033-led: Failed to locate of_node [id: -1] >> [ 13.790478] rt5033-charger rt5033-charger: DMA mask not set >> >> With the changes discussed here: >> dmesg | grep rt5033 >> [ 15.741915] rt5033-battery 4-0035: Failed to register power supply >> [ 15.752894] rt5033-battery 4-0035: Failed to register power supply >> [ 15.795458] rt5033-battery 4-0035: Failed to register power supply >> [ 15.910760] rt5033-battery 4-0035: Failed to register power supply >> [ 15.913187] rt5033 6-0034: Device found (rev. 6) >> [ 15.914341] rt5033-led: Failed to locate of_node [id: -1] >> [ 15.920052] rt5033-battery 4-0035: Failed to register power supply >> [ 15.927262] rt5033-battery 4-0035: Failed to register power supply >> [ 16.017131] rt5033-battery 4-0035: Failed to register power supply >> [ 16.017401] rt5033-charger rt5033-charger: DMA mask not set >> >> The message is comming from the rt5033-battery probe function, it's the >> power_supply_register() that fails. >> >> Any ideas what could be done about this? > > Replace the dev_err() with dev_err_probe(): > > if (IS_ERR(battery->psy)) > return dev_err_probe(&client->dev, PTR_ERR(battery->psy), "Failed to register power supply\n"); > > That will avoid printing an error for -EPROBE_DEFER. Confirming, that works. Thanks! Kind regards, Jakob
diff --git a/drivers/power/supply/rt5033_battery.c b/drivers/power/supply/rt5033_battery.c index 5c04cf305219..a6520716d813 100644 --- a/drivers/power/supply/rt5033_battery.c +++ b/drivers/power/supply/rt5033_battery.c @@ -12,6 +12,26 @@ #include <linux/mfd/rt5033-private.h> #include <linux/mfd/rt5033.h> +static int rt5033_battery_get_status(struct i2c_client *client) +{ + struct power_supply *charger; + union power_supply_propval val; + int ret; + + charger = power_supply_get_by_name("rt5033-charger"); + if (!charger) + return POWER_SUPPLY_STATUS_UNKNOWN; + + ret = power_supply_get_property(charger, POWER_SUPPLY_PROP_STATUS, &val); + if (ret) { + power_supply_put(charger); + return POWER_SUPPLY_STATUS_UNKNOWN; + } + + power_supply_put(charger); + return val.intval; +} + static int rt5033_battery_get_capacity(struct i2c_client *client) { struct rt5033_battery *battery = i2c_get_clientdata(client); @@ -84,6 +104,9 @@ static int rt5033_battery_get_property(struct power_supply *psy, case POWER_SUPPLY_PROP_CAPACITY: val->intval = rt5033_battery_get_capacity(battery->client); break; + case POWER_SUPPLY_PROP_STATUS: + val->intval = rt5033_battery_get_status(battery->client); + break; default: return -EINVAL; } @@ -96,6 +119,7 @@ static enum power_supply_property rt5033_battery_props[] = { POWER_SUPPLY_PROP_VOLTAGE_OCV, POWER_SUPPLY_PROP_PRESENT, POWER_SUPPLY_PROP_CAPACITY, + POWER_SUPPLY_PROP_STATUS, }; static const struct regmap_config rt5033_battery_regmap_config = {