| Message ID | 20221201181635.3522962-1-andreas@kemnade.info (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | iio:adc:twl6030: Enable measurements of VUSB, VBAT and others | expand |
On Thu, 1 Dec 2022 19:16:35 +0100 Andreas Kemnade <andreas@kemnade.info> wrote: > Some inputs need to be wired up to produce proper measurements, > without this change only near zero values are reported. > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info> Sounds like a fix to me. If so, Fixes tag? Anything in here we should be turning off again if the driver is removed or toggling on suspend? If not, other than the space below this looks fine to me. Jonathan > --- > drivers/iio/adc/twl6030-gpadc.c | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/drivers/iio/adc/twl6030-gpadc.c b/drivers/iio/adc/twl6030-gpadc.c > index f53e8558b560c..40438e5b49702 100644 > --- a/drivers/iio/adc/twl6030-gpadc.c > +++ b/drivers/iio/adc/twl6030-gpadc.c > @@ -57,6 +57,18 @@ > #define TWL6030_GPADCS BIT(1) > #define TWL6030_GPADCR BIT(0) > > +#define USB_VBUS_CTRL_SET 0x04 > +#define USB_ID_CTRL_SET 0x06 > + > +#define TWL6030_MISC1 0xE4 > +#define VBUS_MEAS 0x01 > +#define ID_MEAS 0x01 > + > +#define VAC_MEAS 0x04 > +#define VBAT_MEAS 0x02 > +#define BB_MEAS 0x01 > + I'm always of the view one blank line is enough! I'll tidy this up whilst applying. > + > /** > * struct twl6030_chnl_calib - channel calibration > * @gain: slope coefficient for ideal curve > @@ -927,6 +939,26 @@ static int twl6030_gpadc_probe(struct platform_device *pdev) > return ret; > } > > + ret = twl_i2c_write_u8(TWL_MODULE_USB, VBUS_MEAS, USB_VBUS_CTRL_SET); > + if (ret < 0) { > + dev_err(dev, "failed to wire up inputs\n"); > + return ret; > + } > + > + ret = twl_i2c_write_u8(TWL_MODULE_USB, ID_MEAS, USB_ID_CTRL_SET); > + if (ret < 0) { > + dev_err(dev, "failed to wire up inputs\n"); > + return ret; > + } > + > + ret = twl_i2c_write_u8(TWL6030_MODULE_ID0, > + VBAT_MEAS | BB_MEAS | BB_MEAS, > + TWL6030_MISC1); > + if (ret < 0) { > + dev_err(dev, "failed to wire up inputs\n"); > + return ret; > + } > + > indio_dev->name = DRIVER_NAME; > indio_dev->info = &twl6030_gpadc_iio_info; > indio_dev->modes = INDIO_DIRECT_MODE;
On Sun, 4 Dec 2022 15:41:52 +0000 Jonathan Cameron <jic23@kernel.org> wrote: > On Thu, 1 Dec 2022 19:16:35 +0100 > Andreas Kemnade <andreas@kemnade.info> wrote: > > > Some inputs need to be wired up to produce proper measurements, > > without this change only near zero values are reported. > > > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info> > > Sounds like a fix to me. If so, Fixes tag? seems to be there since the beginning, to it would be Fixes: 1696f36482e70 ("iio: twl6030-gpadc: TWL6030, TWL6032 GPADC driver") I think it was just not used with the charger (which is not in mainline yet), so it was probably ignored. > > Anything in here we should be turning off again if the driver is removed > or toggling on suspend? If not, other than the space below this looks fine to me. > I would consider that as configuration, comparing with the nearest relative twl4030, there a similar bit is set in the probe() without disabling it in the remove(). But I think we should set TWL6030_GPADCR in remove() as we do in suspend(), but that is another fix. Regards, Andreas
On Sun, 4 Dec 2022 21:27:51 +0100 Andreas Kemnade <andreas@kemnade.info> wrote: > On Sun, 4 Dec 2022 15:41:52 +0000 > Jonathan Cameron <jic23@kernel.org> wrote: > > > On Thu, 1 Dec 2022 19:16:35 +0100 > > Andreas Kemnade <andreas@kemnade.info> wrote: > > > > > Some inputs need to be wired up to produce proper measurements, > > > without this change only near zero values are reported. > > > > > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info> > > > > Sounds like a fix to me. If so, Fixes tag? > > seems to be there since the beginning, to it would be > Fixes: 1696f36482e70 ("iio: twl6030-gpadc: TWL6030, TWL6032 GPADC driver") > > I think it was just not used with the charger (which is not in mainline yet), > so it was probably ignored. Makes sense. I've applied it at marked it for stable on basis it wants the fixes tag and would get picked up anyway + appears unlikely to have bad side effects. > > > > > Anything in here we should be turning off again if the driver is removed > > or toggling on suspend? If not, other than the space below this looks fine to me. > > > I would consider that as configuration, comparing with the nearest relative twl4030, > there a similar bit is set in the probe() without disabling it in the remove(). > > But I think we should set TWL6030_GPADCR in remove() as we do in suspend(), > but that is another fix. There are always more fixes :) Jonathan > > Regards, > Andreas
diff --git a/drivers/iio/adc/twl6030-gpadc.c b/drivers/iio/adc/twl6030-gpadc.c index f53e8558b560c..40438e5b49702 100644 --- a/drivers/iio/adc/twl6030-gpadc.c +++ b/drivers/iio/adc/twl6030-gpadc.c @@ -57,6 +57,18 @@ #define TWL6030_GPADCS BIT(1) #define TWL6030_GPADCR BIT(0) +#define USB_VBUS_CTRL_SET 0x04 +#define USB_ID_CTRL_SET 0x06 + +#define TWL6030_MISC1 0xE4 +#define VBUS_MEAS 0x01 +#define ID_MEAS 0x01 + +#define VAC_MEAS 0x04 +#define VBAT_MEAS 0x02 +#define BB_MEAS 0x01 + + /** * struct twl6030_chnl_calib - channel calibration * @gain: slope coefficient for ideal curve @@ -927,6 +939,26 @@ static int twl6030_gpadc_probe(struct platform_device *pdev) return ret; } + ret = twl_i2c_write_u8(TWL_MODULE_USB, VBUS_MEAS, USB_VBUS_CTRL_SET); + if (ret < 0) { + dev_err(dev, "failed to wire up inputs\n"); + return ret; + } + + ret = twl_i2c_write_u8(TWL_MODULE_USB, ID_MEAS, USB_ID_CTRL_SET); + if (ret < 0) { + dev_err(dev, "failed to wire up inputs\n"); + return ret; + } + + ret = twl_i2c_write_u8(TWL6030_MODULE_ID0, + VBAT_MEAS | BB_MEAS | BB_MEAS, + TWL6030_MISC1); + if (ret < 0) { + dev_err(dev, "failed to wire up inputs\n"); + return ret; + } + indio_dev->name = DRIVER_NAME; indio_dev->info = &twl6030_gpadc_iio_info; indio_dev->modes = INDIO_DIRECT_MODE;
Some inputs need to be wired up to produce proper measurements, without this change only near zero values are reported. Signed-off-by: Andreas Kemnade <andreas@kemnade.info> --- drivers/iio/adc/twl6030-gpadc.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)