| Message ID | 20210705113637.28908-3-andreas@kemnade.info |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mfd: rn5t618: Extend ADC support | expand |
On Mon, 5 Jul 2021 13:36:37 +0200 Andreas Kemnade <andreas@kemnade.info> wrote: > Read voltage_now via IIO and provide the property. > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info> > Reported-by: kernel test robot <lkp@intel.com> Huh? Seems unlikely it pointed out that this patch was necessary in general. If highlighting a particular fix in an earlier version, then state what it was in the commit message. Note for fixes that get rolled into patches, it's often just mentioned in the change log and we skip the tag because it can cause confusion. One other comment inline but it's up to you whether you care or not! These could go via the IIO tree or power. I don't mind which, but unless someone shouts, I'm assuming power. Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Jonathan > --- > Changes in v2: > - different error handling needed for iio_map usage > - fix dependencies in Kconfig > > drivers/power/supply/Kconfig | 2 ++ > drivers/power/supply/rn5t618_power.c | 40 ++++++++++++++++++++++++++++ > 2 files changed, 42 insertions(+) > > diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig > index e696364126f1..b2910d950929 100644 > --- a/drivers/power/supply/Kconfig > +++ b/drivers/power/supply/Kconfig > @@ -790,6 +790,8 @@ config CHARGER_WILCO > config RN5T618_POWER > tristate "RN5T618 charger/fuel gauge support" > depends on MFD_RN5T618 > + depends on RN5T618_ADC > + depends on IIO > help > Say Y here to have support for RN5T618 PMIC family fuel gauge and charger. > This driver can also be built as a module. If so, the module will be > diff --git a/drivers/power/supply/rn5t618_power.c b/drivers/power/supply/rn5t618_power.c > index 819061918b2a..bca3fd86c14d 100644 > --- a/drivers/power/supply/rn5t618_power.c > +++ b/drivers/power/supply/rn5t618_power.c > @@ -9,10 +9,12 @@ > #include <linux/device.h> > #include <linux/bitops.h> > #include <linux/errno.h> > +#include <linux/iio/consumer.h> > #include <linux/init.h> > #include <linux/interrupt.h> > #include <linux/module.h> > #include <linux/mfd/rn5t618.h> > +#include <linux/of_device.h> > #include <linux/platform_device.h> > #include <linux/power_supply.h> > #include <linux/regmap.h> > @@ -64,6 +66,8 @@ struct rn5t618_power_info { > struct power_supply *battery; > struct power_supply *usb; > struct power_supply *adp; > + struct iio_channel *channel_vusb; > + struct iio_channel *channel_vadp; > int irq; > }; > > @@ -77,6 +81,7 @@ static enum power_supply_usb_type rn5t618_usb_types[] = { > static enum power_supply_property rn5t618_usb_props[] = { > /* input current limit is not very accurate */ > POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT, > + POWER_SUPPLY_PROP_VOLTAGE_NOW, > POWER_SUPPLY_PROP_STATUS, > POWER_SUPPLY_PROP_USB_TYPE, > POWER_SUPPLY_PROP_ONLINE, > @@ -85,6 +90,7 @@ static enum power_supply_property rn5t618_usb_props[] = { > static enum power_supply_property rn5t618_adp_props[] = { > /* input current limit is not very accurate */ > POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT, > + POWER_SUPPLY_PROP_VOLTAGE_NOW, > POWER_SUPPLY_PROP_STATUS, > POWER_SUPPLY_PROP_ONLINE, > }; > @@ -464,6 +470,16 @@ static int rn5t618_adp_get_property(struct power_supply *psy, > > val->intval = FROM_CUR_REG(regval); > break; > + case POWER_SUPPLY_PROP_VOLTAGE_NOW: > + if (!info->channel_vadp) > + return -ENODATA; > + > + ret = iio_read_channel_processed(info->channel_vadp, &val->intval); > + if (ret < 0) > + return ret; > + > + val->intval *= 1000; > + break; > default: > return -EINVAL; > } > @@ -589,6 +605,16 @@ static int rn5t618_usb_get_property(struct power_supply *psy, > val->intval = FROM_CUR_REG(regval); > } > break; > + case POWER_SUPPLY_PROP_VOLTAGE_NOW: > + if (!info->channel_vusb) > + return -ENODATA; > + > + ret = iio_read_channel_processed(info->channel_vusb, &val->intval); > + if (ret < 0) > + return ret; > + > + val->intval *= 1000; It's a recent addition, but we now have an iio_read_channel_processed_scale() function that can retain a little more precision because, in a fractional scale case like with the adc here, it will multiply by 1000 before doing the division. May make a negligable difference though depending on noise level of the ADC etc. > + break; > default: > return -EINVAL; > } > @@ -711,6 +737,20 @@ static int rn5t618_power_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, info); > > + info->channel_vusb = devm_iio_channel_get(&pdev->dev, "vusb"); > + if (IS_ERR(info->channel_vusb)) { > + if (PTR_ERR(info->channel_vusb) == -ENODEV) > + return -EPROBE_DEFER; > + return PTR_ERR(info->channel_vusb); > + } > + > + info->channel_vadp = devm_iio_channel_get(&pdev->dev, "vadp"); > + if (IS_ERR(info->channel_vadp)) { > + if (PTR_ERR(info->channel_vadp) == -ENODEV) > + return -EPROBE_DEFER; > + return PTR_ERR(info->channel_vadp); > + } > + > ret = regmap_read(info->rn5t618->regmap, RN5T618_CONTROL, &v); > if (ret) > return ret;
On Sun, 11 Jul 2021 11:20:39 +0100 Jonathan Cameron <jic23@kernel.org> wrote: > On Mon, 5 Jul 2021 13:36:37 +0200 > Andreas Kemnade <andreas@kemnade.info> wrote: > > > Read voltage_now via IIO and provide the property. > > > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info> > > Reported-by: kernel test robot <lkp@intel.com> > Huh? Seems unlikely it pointed out that this patch was necessary in general. > If highlighting a particular fix in an earlier version, then state what it was > in the commit message. Note for fixes that get rolled into patches, it's > often just mentioned in the change log and we skip the tag because it can > cause confusion. > The robot found a problem in v1 (missing depends on IIO). It is fixed now. The message from the bot tells to add a tag. It seems not to make sense. But probably the bot is also running on public branches (which will not be rebase) and uses the same message where it actually makes sense. I will send a v3 with that tag removed and the other comment addressed. Regards, Andreas
On Mon, 12 Jul 2021 09:11:30 +0200 Andreas Kemnade <andreas@kemnade.info> wrote: > On Sun, 11 Jul 2021 11:20:39 +0100 > Jonathan Cameron <jic23@kernel.org> wrote: > > > On Mon, 5 Jul 2021 13:36:37 +0200 > > Andreas Kemnade <andreas@kemnade.info> wrote: > > > > > Read voltage_now via IIO and provide the property. > > > > > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info> > > > Reported-by: kernel test robot <lkp@intel.com> > > Huh? Seems unlikely it pointed out that this patch was necessary in general. > > If highlighting a particular fix in an earlier version, then state what it was > > in the commit message. Note for fixes that get rolled into patches, it's > > often just mentioned in the change log and we skip the tag because it can > > cause confusion. > > > The robot found a problem in v1 (missing depends on IIO). It is fixed > now. The message from the bot tells to add a tag. It seems not to make > sense. But probably the bot is also running on public branches (which > will not be rebase) and uses the same message where it actually makes > sense. Yup. It might be helpful if they modified that message to suggest commented format if the fix is rolled into an existing patch. I've seen things like. Reported-by: kernel test robot <lkp@intel.com> # Fix something interesting. Which makes it clear what is going on. Jonathan > > I will send a v3 with that tag removed and the other comment addressed. > > Regards, > Andreas
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig index e696364126f1..b2910d950929 100644 --- a/drivers/power/supply/Kconfig +++ b/drivers/power/supply/Kconfig @@ -790,6 +790,8 @@ config CHARGER_WILCO config RN5T618_POWER tristate "RN5T618 charger/fuel gauge support" depends on MFD_RN5T618 + depends on RN5T618_ADC + depends on IIO help Say Y here to have support for RN5T618 PMIC family fuel gauge and charger. This driver can also be built as a module. If so, the module will be diff --git a/drivers/power/supply/rn5t618_power.c b/drivers/power/supply/rn5t618_power.c index 819061918b2a..bca3fd86c14d 100644 --- a/drivers/power/supply/rn5t618_power.c +++ b/drivers/power/supply/rn5t618_power.c @@ -9,10 +9,12 @@ #include <linux/device.h> #include <linux/bitops.h> #include <linux/errno.h> +#include <linux/iio/consumer.h> #include <linux/init.h> #include <linux/interrupt.h> #include <linux/module.h> #include <linux/mfd/rn5t618.h> +#include <linux/of_device.h> #include <linux/platform_device.h> #include <linux/power_supply.h> #include <linux/regmap.h> @@ -64,6 +66,8 @@ struct rn5t618_power_info { struct power_supply *battery; struct power_supply *usb; struct power_supply *adp; + struct iio_channel *channel_vusb; + struct iio_channel *channel_vadp; int irq; }; @@ -77,6 +81,7 @@ static enum power_supply_usb_type rn5t618_usb_types[] = { static enum power_supply_property rn5t618_usb_props[] = { /* input current limit is not very accurate */ POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT, + POWER_SUPPLY_PROP_VOLTAGE_NOW, POWER_SUPPLY_PROP_STATUS, POWER_SUPPLY_PROP_USB_TYPE, POWER_SUPPLY_PROP_ONLINE, @@ -85,6 +90,7 @@ static enum power_supply_property rn5t618_usb_props[] = { static enum power_supply_property rn5t618_adp_props[] = { /* input current limit is not very accurate */ POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT, + POWER_SUPPLY_PROP_VOLTAGE_NOW, POWER_SUPPLY_PROP_STATUS, POWER_SUPPLY_PROP_ONLINE, }; @@ -464,6 +470,16 @@ static int rn5t618_adp_get_property(struct power_supply *psy, val->intval = FROM_CUR_REG(regval); break; + case POWER_SUPPLY_PROP_VOLTAGE_NOW: + if (!info->channel_vadp) + return -ENODATA; + + ret = iio_read_channel_processed(info->channel_vadp, &val->intval); + if (ret < 0) + return ret; + + val->intval *= 1000; + break; default: return -EINVAL; } @@ -589,6 +605,16 @@ static int rn5t618_usb_get_property(struct power_supply *psy, val->intval = FROM_CUR_REG(regval); } break; + case POWER_SUPPLY_PROP_VOLTAGE_NOW: + if (!info->channel_vusb) + return -ENODATA; + + ret = iio_read_channel_processed(info->channel_vusb, &val->intval); + if (ret < 0) + return ret; + + val->intval *= 1000; + break; default: return -EINVAL; } @@ -711,6 +737,20 @@ static int rn5t618_power_probe(struct platform_device *pdev) platform_set_drvdata(pdev, info); + info->channel_vusb = devm_iio_channel_get(&pdev->dev, "vusb"); + if (IS_ERR(info->channel_vusb)) { + if (PTR_ERR(info->channel_vusb) == -ENODEV) + return -EPROBE_DEFER; + return PTR_ERR(info->channel_vusb); + } + + info->channel_vadp = devm_iio_channel_get(&pdev->dev, "vadp"); + if (IS_ERR(info->channel_vadp)) { + if (PTR_ERR(info->channel_vadp) == -ENODEV) + return -EPROBE_DEFER; + return PTR_ERR(info->channel_vadp); + } + ret = regmap_read(info->rn5t618->regmap, RN5T618_CONTROL, &v); if (ret) return ret;
Read voltage_now via IIO and provide the property. Signed-off-by: Andreas Kemnade <andreas@kemnade.info> Reported-by: kernel test robot <lkp@intel.com> --- Changes in v2: - different error handling needed for iio_map usage - fix dependencies in Kconfig drivers/power/supply/Kconfig | 2 ++ drivers/power/supply/rn5t618_power.c | 40 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+)