Message ID | 1384857392-2199-1-git-send-email-maxime.ripard@free-electrons.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
> The mxs LRADC is able to read an internal die temperature sensor. The > temperature has to be calculated from the value read on channel 8 and channel 9. > To be able to expose the result to hwmon, implement iio channel 8 as > (channel 9 - channel 8). Then, implement IIO_CHAN_INFO_SCALE and > IIO_CHAN_INFO_OFFSET so that it can be processed by hwmon through the in kernel > provider/consumer mechanism. wouldn't IIO_CHAN_INFO_PROCESSED be more suitable than IIO_CHAN_INFO_RAW? > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > Reviewed-by: Lars-Peter Clausen <lars@metafoo.de> > --- > drivers/staging/iio/adc/mxs-lradc.c | 91 +++++++++++++++++++++++++++++++------ > 1 file changed, 78 insertions(+), 13 deletions(-) > > diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c > index a08c1736458b..fec76090dae5 100644 > --- a/drivers/staging/iio/adc/mxs-lradc.c > +++ b/drivers/staging/iio/adc/mxs-lradc.c > @@ -231,20 +231,11 @@ struct mxs_lradc { > /* > * Raw I/O operations > */ > -static int mxs_lradc_read_raw(struct iio_dev *iio_dev, > - const struct iio_chan_spec *chan, > - int *val, int *val2, long m) > +static int mxs_lradc_read_single(struct iio_dev *iio_dev, int chan, int *val) > { > struct mxs_lradc *lradc = iio_priv(iio_dev); > int ret; > > - if (m != IIO_CHAN_INFO_RAW) > - return -EINVAL; > - > - /* Check for invalid channel */ > - if (chan->channel > LRADC_MAX_TOTAL_CHANS) > - return -EINVAL; > - > /* > * See if there is no buffered operation in progess. If there is, simply > * bail out. This can be improved to support both buffered and raw IO at > @@ -269,7 +260,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev, > /* Clean the slot's previous content, then set new one. */ > writel(LRADC_CTRL4_LRADCSELECT_MASK(0), > lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR); > - writel(chan->channel, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET); > + writel(chan, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET); > > writel(0, lradc->base + LRADC_CH(0)); > > @@ -298,6 +289,71 @@ err: > return ret; > } > > +static int mxs_lradc_read_temp(struct iio_dev *iio_dev, int *val) > +{ > + int ret, min, max; > + > + ret = mxs_lradc_read_single(iio_dev, 8, &min); > + if (ret != IIO_VAL_INT) > + return ret; > + > + ret = mxs_lradc_read_single(iio_dev, 9, &max); > + if (ret != IIO_VAL_INT) > + return ret; > + > + *val = max - min; > + > + return IIO_VAL_INT; > +} > + > +static int mxs_lradc_read_raw(struct iio_dev *iio_dev, > + const struct iio_chan_spec *chan, > + int *val, int *val2, long m) > +{ > + /* Check for invalid channel */ > + if (chan->channel > LRADC_MAX_TOTAL_CHANS) > + return -EINVAL; > + > + switch (m) { > + case IIO_CHAN_INFO_RAW: > + if (chan->type == IIO_TEMP) > + return mxs_lradc_read_temp(iio_dev, val); > + > + return mxs_lradc_read_single(iio_dev, chan->channel, val); > + > + case IIO_CHAN_INFO_SCALE: > + if (chan->type == IIO_TEMP) { > + /* From the datasheet, we have to multiply by 1.012 and > + * divide by 4 > + */ > + *val = 0; > + *val2 = 253000; > + return IIO_VAL_INT_PLUS_MICRO; > + } > + > + return -EINVAL; > + > + case IIO_CHAN_INFO_OFFSET: > + if (chan->type == IIO_TEMP) { > + /* The calculated value from the ADC is in Kelvin, we > + * want Celsius for hwmon so the offset is > + * -272.15 * scale > + */ > + *val = -1075; > + *val2 = 691699; > + > + return IIO_VAL_INT_PLUS_MICRO; > + } > + > + return -EINVAL; > + > + default: > + break; > + } > + > + return -EINVAL; > +} > + > static const struct iio_info mxs_lradc_iio_info = { > .driver_module = THIS_MODULE, > .read_raw = mxs_lradc_read_raw, > @@ -835,8 +891,17 @@ static const struct iio_chan_spec mxs_lradc_chan_spec[] = { > MXS_ADC_CHAN(5, IIO_VOLTAGE), > MXS_ADC_CHAN(6, IIO_VOLTAGE), > MXS_ADC_CHAN(7, IIO_VOLTAGE), /* VBATT */ > - MXS_ADC_CHAN(8, IIO_TEMP), /* Temp sense 0 */ > - MXS_ADC_CHAN(9, IIO_TEMP), /* Temp sense 1 */ > + /* Combined Temperature sensors */ > + { > + .type = IIO_TEMP, > + .indexed = 1, > + .scan_index = 8, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > + BIT(IIO_CHAN_INFO_OFFSET) | > + BIT(IIO_CHAN_INFO_SCALE), > + .channel = 8, > + .scan_type = {.sign = 'u', .realbits = 18, .storagebits = 32,}, > + }, > MXS_ADC_CHAN(10, IIO_VOLTAGE), /* VDDIO */ > MXS_ADC_CHAN(11, IIO_VOLTAGE), /* VTH */ > MXS_ADC_CHAN(12, IIO_VOLTAGE), /* VDDA */ >
On 11/19/2013 11:51 AM, Peter Meerwald wrote: > >> The mxs LRADC is able to read an internal die temperature sensor. The >> temperature has to be calculated from the value read on channel 8 and channel 9. >> To be able to expose the result to hwmon, implement iio channel 8 as >> (channel 9 - channel 8). Then, implement IIO_CHAN_INFO_SCALE and >> IIO_CHAN_INFO_OFFSET so that it can be processed by hwmon through the in kernel >> provider/consumer mechanism. > > wouldn't IIO_CHAN_INFO_PROCESSED be more suitable than IIO_CHAN_INFO_RAW? No. PROCESSED means it has the right scale and offset. This is still a raw value. - Lars
On 11/19/13 10:36, Maxime Ripard wrote: > From: Alexandre Belloni <alexandre.belloni@free-electrons.com> > > The mxs LRADC is able to read an internal die temperature sensor. The > temperature has to be calculated from the value read on channel 8 and channel 9. > To be able to expose the result to hwmon, implement iio channel 8 as > (channel 9 - channel 8). Then, implement IIO_CHAN_INFO_SCALE and > IIO_CHAN_INFO_OFFSET so that it can be processed by hwmon through the in kernel > provider/consumer mechanism. > > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > Reviewed-by: Lars-Peter Clausen <lars@metafoo.de> As this is still in staging, I think we can get away with the resulting ABI change from this patch. Applied to the togreg branch of iio.git Thanks, Jonathan > --- > drivers/staging/iio/adc/mxs-lradc.c | 91 +++++++++++++++++++++++++++++++------ > 1 file changed, 78 insertions(+), 13 deletions(-) > > diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c > index a08c1736458b..fec76090dae5 100644 > --- a/drivers/staging/iio/adc/mxs-lradc.c > +++ b/drivers/staging/iio/adc/mxs-lradc.c > @@ -231,20 +231,11 @@ struct mxs_lradc { > /* > * Raw I/O operations > */ > -static int mxs_lradc_read_raw(struct iio_dev *iio_dev, > - const struct iio_chan_spec *chan, > - int *val, int *val2, long m) > +static int mxs_lradc_read_single(struct iio_dev *iio_dev, int chan, int *val) > { > struct mxs_lradc *lradc = iio_priv(iio_dev); > int ret; > > - if (m != IIO_CHAN_INFO_RAW) > - return -EINVAL; > - > - /* Check for invalid channel */ > - if (chan->channel > LRADC_MAX_TOTAL_CHANS) > - return -EINVAL; > - > /* > * See if there is no buffered operation in progess. If there is, simply > * bail out. This can be improved to support both buffered and raw IO at > @@ -269,7 +260,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev, > /* Clean the slot's previous content, then set new one. */ > writel(LRADC_CTRL4_LRADCSELECT_MASK(0), > lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR); > - writel(chan->channel, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET); > + writel(chan, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET); > > writel(0, lradc->base + LRADC_CH(0)); > > @@ -298,6 +289,71 @@ err: > return ret; > } > > +static int mxs_lradc_read_temp(struct iio_dev *iio_dev, int *val) > +{ > + int ret, min, max; > + > + ret = mxs_lradc_read_single(iio_dev, 8, &min); > + if (ret != IIO_VAL_INT) > + return ret; > + > + ret = mxs_lradc_read_single(iio_dev, 9, &max); > + if (ret != IIO_VAL_INT) > + return ret; > + > + *val = max - min; > + > + return IIO_VAL_INT; > +} > + > +static int mxs_lradc_read_raw(struct iio_dev *iio_dev, > + const struct iio_chan_spec *chan, > + int *val, int *val2, long m) > +{ > + /* Check for invalid channel */ > + if (chan->channel > LRADC_MAX_TOTAL_CHANS) > + return -EINVAL; > + > + switch (m) { > + case IIO_CHAN_INFO_RAW: > + if (chan->type == IIO_TEMP) > + return mxs_lradc_read_temp(iio_dev, val); > + > + return mxs_lradc_read_single(iio_dev, chan->channel, val); > + > + case IIO_CHAN_INFO_SCALE: > + if (chan->type == IIO_TEMP) { > + /* From the datasheet, we have to multiply by 1.012 and > + * divide by 4 > + */ > + *val = 0; > + *val2 = 253000; > + return IIO_VAL_INT_PLUS_MICRO; > + } > + > + return -EINVAL; > + > + case IIO_CHAN_INFO_OFFSET: > + if (chan->type == IIO_TEMP) { > + /* The calculated value from the ADC is in Kelvin, we > + * want Celsius for hwmon so the offset is > + * -272.15 * scale > + */ > + *val = -1075; > + *val2 = 691699; > + > + return IIO_VAL_INT_PLUS_MICRO; > + } > + > + return -EINVAL; > + > + default: > + break; > + } > + > + return -EINVAL; > +} > + > static const struct iio_info mxs_lradc_iio_info = { > .driver_module = THIS_MODULE, > .read_raw = mxs_lradc_read_raw, > @@ -835,8 +891,17 @@ static const struct iio_chan_spec mxs_lradc_chan_spec[] = { > MXS_ADC_CHAN(5, IIO_VOLTAGE), > MXS_ADC_CHAN(6, IIO_VOLTAGE), > MXS_ADC_CHAN(7, IIO_VOLTAGE), /* VBATT */ > - MXS_ADC_CHAN(8, IIO_TEMP), /* Temp sense 0 */ > - MXS_ADC_CHAN(9, IIO_TEMP), /* Temp sense 1 */ > + /* Combined Temperature sensors */ > + { > + .type = IIO_TEMP, > + .indexed = 1, > + .scan_index = 8, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > + BIT(IIO_CHAN_INFO_OFFSET) | > + BIT(IIO_CHAN_INFO_SCALE), > + .channel = 8, > + .scan_type = {.sign = 'u', .realbits = 18, .storagebits = 32,}, > + }, > MXS_ADC_CHAN(10, IIO_VOLTAGE), /* VDDIO */ > MXS_ADC_CHAN(11, IIO_VOLTAGE), /* VTH */ > MXS_ADC_CHAN(12, IIO_VOLTAGE), /* VDDA */ >
On 11/30/13 12:15, Jonathan Cameron wrote: > On 11/19/13 10:36, Maxime Ripard wrote: >> From: Alexandre Belloni <alexandre.belloni@free-electrons.com> >> >> The mxs LRADC is able to read an internal die temperature sensor. The >> temperature has to be calculated from the value read on channel 8 and channel 9. >> To be able to expose the result to hwmon, implement iio channel 8 as >> (channel 9 - channel 8). Then, implement IIO_CHAN_INFO_SCALE and >> IIO_CHAN_INFO_OFFSET so that it can be processed by hwmon through the in kernel >> provider/consumer mechanism. >> >> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> >> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> >> Reviewed-by: Lars-Peter Clausen <lars@metafoo.de> > As this is still in staging, I think we can get away with the resulting > ABI change from this patch. > > Applied to the togreg branch of iio.git > > Thanks, > > Jonathan Having said that, there is an odd bit inline that I've just dropped whilst applying the patch. Shout if I've done this wrong. >> --- >> drivers/staging/iio/adc/mxs-lradc.c | 91 +++++++++++++++++++++++++++++++------ >> 1 file changed, 78 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c >> index a08c1736458b..fec76090dae5 100644 >> --- a/drivers/staging/iio/adc/mxs-lradc.c >> +++ b/drivers/staging/iio/adc/mxs-lradc.c >> @@ -231,20 +231,11 @@ struct mxs_lradc { >> /* >> * Raw I/O operations >> */ >> -static int mxs_lradc_read_raw(struct iio_dev *iio_dev, >> - const struct iio_chan_spec *chan, >> - int *val, int *val2, long m) >> +static int mxs_lradc_read_single(struct iio_dev *iio_dev, int chan, int *val) >> { >> struct mxs_lradc *lradc = iio_priv(iio_dev); >> int ret; >> >> - if (m != IIO_CHAN_INFO_RAW) >> - return -EINVAL; >> - >> - /* Check for invalid channel */ >> - if (chan->channel > LRADC_MAX_TOTAL_CHANS) >> - return -EINVAL; >> - >> /* >> * See if there is no buffered operation in progess. If there is, simply >> * bail out. This can be improved to support both buffered and raw IO at >> @@ -269,7 +260,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev, >> /* Clean the slot's previous content, then set new one. */ >> writel(LRADC_CTRL4_LRADCSELECT_MASK(0), >> lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR); >> - writel(chan->channel, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET); >> + writel(chan, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET); This doesn't make any sense. I'm guessing it drifted in from a different patch? THere is a lot of fuzz in here in general. Please do check this - for now it'll be pushed to my testing branch for some build testing. >> >> writel(0, lradc->base + LRADC_CH(0)); >> >> @@ -298,6 +289,71 @@ err: >> return ret; >> } >> >> +static int mxs_lradc_read_temp(struct iio_dev *iio_dev, int *val) >> +{ >> + int ret, min, max; >> + >> + ret = mxs_lradc_read_single(iio_dev, 8, &min); >> + if (ret != IIO_VAL_INT) >> + return ret; >> + >> + ret = mxs_lradc_read_single(iio_dev, 9, &max); >> + if (ret != IIO_VAL_INT) >> + return ret; >> + >> + *val = max - min; >> + >> + return IIO_VAL_INT; >> +} >> + >> +static int mxs_lradc_read_raw(struct iio_dev *iio_dev, >> + const struct iio_chan_spec *chan, >> + int *val, int *val2, long m) >> +{ >> + /* Check for invalid channel */ >> + if (chan->channel > LRADC_MAX_TOTAL_CHANS) >> + return -EINVAL; >> + >> + switch (m) { >> + case IIO_CHAN_INFO_RAW: >> + if (chan->type == IIO_TEMP) >> + return mxs_lradc_read_temp(iio_dev, val); >> + >> + return mxs_lradc_read_single(iio_dev, chan->channel, val); >> + >> + case IIO_CHAN_INFO_SCALE: >> + if (chan->type == IIO_TEMP) { >> + /* From the datasheet, we have to multiply by 1.012 and >> + * divide by 4 >> + */ >> + *val = 0; >> + *val2 = 253000; >> + return IIO_VAL_INT_PLUS_MICRO; >> + } >> + >> + return -EINVAL; >> + >> + case IIO_CHAN_INFO_OFFSET: >> + if (chan->type == IIO_TEMP) { >> + /* The calculated value from the ADC is in Kelvin, we >> + * want Celsius for hwmon so the offset is >> + * -272.15 * scale >> + */ >> + *val = -1075; >> + *val2 = 691699; >> + >> + return IIO_VAL_INT_PLUS_MICRO; >> + } >> + >> + return -EINVAL; >> + >> + default: >> + break; >> + } >> + >> + return -EINVAL; >> +} >> + >> static const struct iio_info mxs_lradc_iio_info = { >> .driver_module = THIS_MODULE, >> .read_raw = mxs_lradc_read_raw, >> @@ -835,8 +891,17 @@ static const struct iio_chan_spec mxs_lradc_chan_spec[] = { >> MXS_ADC_CHAN(5, IIO_VOLTAGE), >> MXS_ADC_CHAN(6, IIO_VOLTAGE), >> MXS_ADC_CHAN(7, IIO_VOLTAGE), /* VBATT */ >> - MXS_ADC_CHAN(8, IIO_TEMP), /* Temp sense 0 */ >> - MXS_ADC_CHAN(9, IIO_TEMP), /* Temp sense 1 */ >> + /* Combined Temperature sensors */ >> + { >> + .type = IIO_TEMP, >> + .indexed = 1, >> + .scan_index = 8, >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | >> + BIT(IIO_CHAN_INFO_OFFSET) | >> + BIT(IIO_CHAN_INFO_SCALE), >> + .channel = 8, >> + .scan_type = {.sign = 'u', .realbits = 18, .storagebits = 32,}, >> + }, >> MXS_ADC_CHAN(10, IIO_VOLTAGE), /* VDDIO */ >> MXS_ADC_CHAN(11, IIO_VOLTAGE), /* VTH */ >> MXS_ADC_CHAN(12, IIO_VOLTAGE), /* VDDA */ >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
2013/11/30 Jonathan Cameron <jic23@kernel.org>: > On 11/30/13 12:15, Jonathan Cameron wrote: >> On 11/19/13 10:36, Maxime Ripard wrote: >>> From: Alexandre Belloni <alexandre.belloni@free-electrons.com> >>> >>> The mxs LRADC is able to read an internal die temperature sensor. The >>> temperature has to be calculated from the value read on channel 8 and channel 9. >>> To be able to expose the result to hwmon, implement iio channel 8 as >>> (channel 9 - channel 8). Then, implement IIO_CHAN_INFO_SCALE and >>> IIO_CHAN_INFO_OFFSET so that it can be processed by hwmon through the in kernel >>> provider/consumer mechanism. >>> >>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> >>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> >>> Reviewed-by: Lars-Peter Clausen <lars@metafoo.de> >> As this is still in staging, I think we can get away with the resulting >> ABI change from this patch. >> >> Applied to the togreg branch of iio.git >> >> Thanks, >> >> Jonathan > Having said that, there is an odd bit inline that I've just dropped whilst > applying the patch. Shout if I've done this wrong. >>> --- >>> drivers/staging/iio/adc/mxs-lradc.c | 91 +++++++++++++++++++++++++++++++------ >>> 1 file changed, 78 insertions(+), 13 deletions(-) >>> >>> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c >>> index a08c1736458b..fec76090dae5 100644 >>> --- a/drivers/staging/iio/adc/mxs-lradc.c >>> +++ b/drivers/staging/iio/adc/mxs-lradc.c >>> @@ -231,20 +231,11 @@ struct mxs_lradc { >>> /* >>> * Raw I/O operations >>> */ >>> -static int mxs_lradc_read_raw(struct iio_dev *iio_dev, >>> - const struct iio_chan_spec *chan, >>> - int *val, int *val2, long m) >>> +static int mxs_lradc_read_single(struct iio_dev *iio_dev, int chan, int *val) [...] >>> @@ -269,7 +260,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev, [...] >>> - writel(chan->channel, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET); >>> + writel(chan, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET); > > This doesn't make any sense. I'm guessing it drifted in from a different patch? 'chan' changed type here, and the function got a new name. I would assume thant this won't build if you dropped the part above. Best Regards, Micha? Miros?aw
On 11/30/13 12:24, Micha? Miros?aw wrote: > 2013/11/30 Jonathan Cameron <jic23@kernel.org>: >> On 11/30/13 12:15, Jonathan Cameron wrote: >>> On 11/19/13 10:36, Maxime Ripard wrote: >>>> From: Alexandre Belloni <alexandre.belloni@free-electrons.com> >>>> >>>> The mxs LRADC is able to read an internal die temperature sensor. The >>>> temperature has to be calculated from the value read on channel 8 and channel 9. >>>> To be able to expose the result to hwmon, implement iio channel 8 as >>>> (channel 9 - channel 8). Then, implement IIO_CHAN_INFO_SCALE and >>>> IIO_CHAN_INFO_OFFSET so that it can be processed by hwmon through the in kernel >>>> provider/consumer mechanism. >>>> >>>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> >>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> >>>> Reviewed-by: Lars-Peter Clausen <lars@metafoo.de> >>> As this is still in staging, I think we can get away with the resulting >>> ABI change from this patch. >>> >>> Applied to the togreg branch of iio.git >>> >>> Thanks, >>> >>> Jonathan >> Having said that, there is an odd bit inline that I've just dropped whilst >> applying the patch. Shout if I've done this wrong. >>>> --- >>>> drivers/staging/iio/adc/mxs-lradc.c | 91 +++++++++++++++++++++++++++++++------ >>>> 1 file changed, 78 insertions(+), 13 deletions(-) >>>> >>>> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c >>>> index a08c1736458b..fec76090dae5 100644 >>>> --- a/drivers/staging/iio/adc/mxs-lradc.c >>>> +++ b/drivers/staging/iio/adc/mxs-lradc.c >>>> @@ -231,20 +231,11 @@ struct mxs_lradc { >>>> /* >>>> * Raw I/O operations >>>> */ >>>> -static int mxs_lradc_read_raw(struct iio_dev *iio_dev, >>>> - const struct iio_chan_spec *chan, >>>> - int *val, int *val2, long m) >>>> +static int mxs_lradc_read_single(struct iio_dev *iio_dev, int chan, int *val) > [...] >>>> @@ -269,7 +260,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev, > [...] >>>> - writel(chan->channel, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET); >>>> + writel(chan, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET); >> >> This doesn't make any sense. I'm guessing it drifted in from a different patch? > > 'chan' changed type here, and the function got a new name. I would > assume thant this won't build if you dropped the part above. Right you are... I'll fix this up later if I can. For now I'll drop it from my tree. > > Best Regards, > Micha? Miros?aw > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On 11/30/13 12:27, Jonathan Cameron wrote: > On 11/30/13 12:24, Micha? Miros?aw wrote: >> 2013/11/30 Jonathan Cameron <jic23@kernel.org>: >>> On 11/30/13 12:15, Jonathan Cameron wrote: >>>> On 11/19/13 10:36, Maxime Ripard wrote: >>>>> From: Alexandre Belloni <alexandre.belloni@free-electrons.com> >>>>> >>>>> The mxs LRADC is able to read an internal die temperature sensor. The >>>>> temperature has to be calculated from the value read on channel 8 and channel 9. >>>>> To be able to expose the result to hwmon, implement iio channel 8 as >>>>> (channel 9 - channel 8). Then, implement IIO_CHAN_INFO_SCALE and >>>>> IIO_CHAN_INFO_OFFSET so that it can be processed by hwmon through the in kernel >>>>> provider/consumer mechanism. >>>>> >>>>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> >>>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> >>>>> Reviewed-by: Lars-Peter Clausen <lars@metafoo.de> >>>> As this is still in staging, I think we can get away with the resulting >>>> ABI change from this patch. >>>> >>>> Applied to the togreg branch of iio.git >>>> >>>> Thanks, >>>> >>>> Jonathan >>> Having said that, there is an odd bit inline that I've just dropped whilst >>> applying the patch. Shout if I've done this wrong. Given issues below and the substantial changes that have occured in this driver, could your please rebase it and repost. Thanks, >>>>> --- >>>>> drivers/staging/iio/adc/mxs-lradc.c | 91 +++++++++++++++++++++++++++++++------ >>>>> 1 file changed, 78 insertions(+), 13 deletions(-) >>>>> >>>>> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c >>>>> index a08c1736458b..fec76090dae5 100644 >>>>> --- a/drivers/staging/iio/adc/mxs-lradc.c >>>>> +++ b/drivers/staging/iio/adc/mxs-lradc.c >>>>> @@ -231,20 +231,11 @@ struct mxs_lradc { >>>>> /* >>>>> * Raw I/O operations >>>>> */ >>>>> -static int mxs_lradc_read_raw(struct iio_dev *iio_dev, >>>>> - const struct iio_chan_spec *chan, >>>>> - int *val, int *val2, long m) >>>>> +static int mxs_lradc_read_single(struct iio_dev *iio_dev, int chan, int *val) >> [...] >>>>> @@ -269,7 +260,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev, >> [...] >>>>> - writel(chan->channel, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET); >>>>> + writel(chan, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET); >>> >>> This doesn't make any sense. I'm guessing it drifted in from a different patch? >> >> 'chan' changed type here, and the function got a new name. I would >> assume thant this won't build if you dropped the part above. > Right you are... I'll fix this up later if I can. For now I'll drop it from my tree. >> >> Best Regards, >> Micha? Miros?aw >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-iio" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On Sat, Nov 30, 2013 at 12:29:54PM +0000, Jonathan Cameron wrote: > On 11/30/13 12:27, Jonathan Cameron wrote: > > On 11/30/13 12:24, Micha? Miros?aw wrote: > >> 2013/11/30 Jonathan Cameron <jic23@kernel.org>: > >>> On 11/30/13 12:15, Jonathan Cameron wrote: > >>>> On 11/19/13 10:36, Maxime Ripard wrote: > >>>>> From: Alexandre Belloni <alexandre.belloni@free-electrons.com> > >>>>> > >>>>> The mxs LRADC is able to read an internal die temperature sensor. The > >>>>> temperature has to be calculated from the value read on channel 8 and channel 9. > >>>>> To be able to expose the result to hwmon, implement iio channel 8 as > >>>>> (channel 9 - channel 8). Then, implement IIO_CHAN_INFO_SCALE and > >>>>> IIO_CHAN_INFO_OFFSET so that it can be processed by hwmon through the in kernel > >>>>> provider/consumer mechanism. > >>>>> > >>>>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> > >>>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > >>>>> Reviewed-by: Lars-Peter Clausen <lars@metafoo.de> > >>>> As this is still in staging, I think we can get away with the resulting > >>>> ABI change from this patch. > >>>> > >>>> Applied to the togreg branch of iio.git > >>>> > >>>> Thanks, > >>>> > >>>> Jonathan > >>> Having said that, there is an odd bit inline that I've just dropped whilst > >>> applying the patch. Shout if I've done this wrong. > Given issues below and the substantial changes that have occured in this driver, could > your please rebase it and repost. I will. Thanks, Maxime
diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c index a08c1736458b..fec76090dae5 100644 --- a/drivers/staging/iio/adc/mxs-lradc.c +++ b/drivers/staging/iio/adc/mxs-lradc.c @@ -231,20 +231,11 @@ struct mxs_lradc { /* * Raw I/O operations */ -static int mxs_lradc_read_raw(struct iio_dev *iio_dev, - const struct iio_chan_spec *chan, - int *val, int *val2, long m) +static int mxs_lradc_read_single(struct iio_dev *iio_dev, int chan, int *val) { struct mxs_lradc *lradc = iio_priv(iio_dev); int ret; - if (m != IIO_CHAN_INFO_RAW) - return -EINVAL; - - /* Check for invalid channel */ - if (chan->channel > LRADC_MAX_TOTAL_CHANS) - return -EINVAL; - /* * See if there is no buffered operation in progess. If there is, simply * bail out. This can be improved to support both buffered and raw IO at @@ -269,7 +260,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev, /* Clean the slot's previous content, then set new one. */ writel(LRADC_CTRL4_LRADCSELECT_MASK(0), lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR); - writel(chan->channel, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET); + writel(chan, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET); writel(0, lradc->base + LRADC_CH(0)); @@ -298,6 +289,71 @@ err: return ret; } +static int mxs_lradc_read_temp(struct iio_dev *iio_dev, int *val) +{ + int ret, min, max; + + ret = mxs_lradc_read_single(iio_dev, 8, &min); + if (ret != IIO_VAL_INT) + return ret; + + ret = mxs_lradc_read_single(iio_dev, 9, &max); + if (ret != IIO_VAL_INT) + return ret; + + *val = max - min; + + return IIO_VAL_INT; +} + +static int mxs_lradc_read_raw(struct iio_dev *iio_dev, + const struct iio_chan_spec *chan, + int *val, int *val2, long m) +{ + /* Check for invalid channel */ + if (chan->channel > LRADC_MAX_TOTAL_CHANS) + return -EINVAL; + + switch (m) { + case IIO_CHAN_INFO_RAW: + if (chan->type == IIO_TEMP) + return mxs_lradc_read_temp(iio_dev, val); + + return mxs_lradc_read_single(iio_dev, chan->channel, val); + + case IIO_CHAN_INFO_SCALE: + if (chan->type == IIO_TEMP) { + /* From the datasheet, we have to multiply by 1.012 and + * divide by 4 + */ + *val = 0; + *val2 = 253000; + return IIO_VAL_INT_PLUS_MICRO; + } + + return -EINVAL; + + case IIO_CHAN_INFO_OFFSET: + if (chan->type == IIO_TEMP) { + /* The calculated value from the ADC is in Kelvin, we + * want Celsius for hwmon so the offset is + * -272.15 * scale + */ + *val = -1075; + *val2 = 691699; + + return IIO_VAL_INT_PLUS_MICRO; + } + + return -EINVAL; + + default: + break; + } + + return -EINVAL; +} + static const struct iio_info mxs_lradc_iio_info = { .driver_module = THIS_MODULE, .read_raw = mxs_lradc_read_raw, @@ -835,8 +891,17 @@ static const struct iio_chan_spec mxs_lradc_chan_spec[] = { MXS_ADC_CHAN(5, IIO_VOLTAGE), MXS_ADC_CHAN(6, IIO_VOLTAGE), MXS_ADC_CHAN(7, IIO_VOLTAGE), /* VBATT */ - MXS_ADC_CHAN(8, IIO_TEMP), /* Temp sense 0 */ - MXS_ADC_CHAN(9, IIO_TEMP), /* Temp sense 1 */ + /* Combined Temperature sensors */ + { + .type = IIO_TEMP, + .indexed = 1, + .scan_index = 8, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_OFFSET) | + BIT(IIO_CHAN_INFO_SCALE), + .channel = 8, + .scan_type = {.sign = 'u', .realbits = 18, .storagebits = 32,}, + }, MXS_ADC_CHAN(10, IIO_VOLTAGE), /* VDDIO */ MXS_ADC_CHAN(11, IIO_VOLTAGE), /* VTH */ MXS_ADC_CHAN(12, IIO_VOLTAGE), /* VDDA */