Message ID | 20221014172427.128512-1-marex@denx.de |
---|---|
State | Accepted |
Commit | ef1ca2102e9c546a507ed43994f5dd022f7a80d3 |
Headers | show |
Series | [v2,1/7] power: supply: bq25890: Document POWER_SUPPLY_PROP_CURRENT_NOW | expand |
Hi, On 10/14/22 19:24, Marek Vasut wrote: > The chip is capable of reporting Vbus voltage, add .get_voltage > implementation to Vbus regulator to report current Vbus voltage. > This requires for the Vbus regulator to be registered always > instead of the current state where the regulator is registered > only in case USB PHY is not found. > > Do not provide Vbus regulator enable/disable ops in case USB PHY > is present, as they would race with USB PHY notifier which is also > used to toggle OTG boost mode. > > Signed-off-by: Marek Vasut <marex@denx.de> > --- > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Hans de Goede <hdegoede@redhat.com> > Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl> > Cc: Sebastian Reichel <sebastian.reichel@collabora.com> > To: linux-pm@vger.kernel.org > --- > V2: Simplify the Vbus regulator registration, quoting Hans: > " > AFAIK if the Vboost regulator is not referenced in dt because > it is controller through the usb-phy framework then valid_ops_mask > will be empty, so the 2 sets of ops + 2 descs are not necessary > I believe. > So I believe this can be simplified to just adding > bq25890_vbus_get_voltage to the ops, dropping .fixed_uV and > .n_voltages from the desc, and just completely dropping > the IS_ERR_OR_NULL(bq->usb_phy) check. > " Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > drivers/power/supply/bq25890_charger.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c > index dad98b782a2f8..ad5811304f88a 100644 > --- a/drivers/power/supply/bq25890_charger.c > +++ b/drivers/power/supply/bq25890_charger.c > @@ -1095,10 +1095,18 @@ static int bq25890_vbus_is_enabled(struct regulator_dev *rdev) > return bq25890_field_read(bq, F_OTG_CFG); > } > > +static int bq25890_vbus_get_voltage(struct regulator_dev *rdev) > +{ > + struct bq25890_device *bq = rdev_get_drvdata(rdev); > + > + return bq25890_get_vbus_voltage(bq); > +} > + > static const struct regulator_ops bq25890_vbus_ops = { > .enable = bq25890_vbus_enable, > .disable = bq25890_vbus_disable, > .is_enabled = bq25890_vbus_is_enabled, > + .get_voltage = bq25890_vbus_get_voltage, > }; > > static const struct regulator_desc bq25890_vbus_desc = { > @@ -1107,8 +1115,6 @@ static const struct regulator_desc bq25890_vbus_desc = { > .type = REGULATOR_VOLTAGE, > .owner = THIS_MODULE, > .ops = &bq25890_vbus_ops, > - .fixed_uV = 5000000, > - .n_voltages = 1, > }; > > static int bq25890_register_regulator(struct bq25890_device *bq) > @@ -1120,9 +1126,6 @@ static int bq25890_register_regulator(struct bq25890_device *bq) > }; > struct regulator_dev *reg; > > - if (!IS_ERR_OR_NULL(bq->usb_phy)) > - return 0; > - > if (pdata) > cfg.init_data = pdata->regulator_init_data; >
Hi, On Fri, Oct 14, 2022 at 07:24:21PM +0200, Marek Vasut wrote: > Document that POWER_SUPPLY_PROP_CURRENT_NOW really does refer to ADC-sampled > immediate battery charge current I_BAT , since the meaning is not clear with > all the currents which might be measured by charger chips. > > Reviewed-by: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Marek Vasut <marex@denx.de> > --- > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Hans de Goede <hdegoede@redhat.com> > Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl> > Cc: Sebastian Reichel <sebastian.reichel@collabora.com> > To: linux-pm@vger.kernel.org > --- > V2: Add RB from Hans > --- Thanks, I queued the whole series. -- Sebastian
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c index 6020b58c641d2..1298d5720aa4b 100644 --- a/drivers/power/supply/bq25890_charger.c +++ b/drivers/power/supply/bq25890_charger.c @@ -588,7 +588,14 @@ static int bq25890_power_supply_get_property(struct power_supply *psy, val->intval = 2304000 + ret * 20000; break; - case POWER_SUPPLY_PROP_CURRENT_NOW: + case POWER_SUPPLY_PROP_CURRENT_NOW: /* I_BAT now */ + /* + * This is ADC-sampled immediate charge current supplied + * from charger to battery. The property name is confusing, + * for clarification refer to: + * Documentation/ABI/testing/sysfs-class-power + * /sys/class/power_supply/<supply_name>/current_now + */ ret = bq25890_field_read(bq, F_ICHGR); /* read measured value */ if (ret < 0) return ret;