| Message ID | 20220609083213.1795019-1-claudiu.beznea@microchip.com |
|---|---|
| Headers | show |
| Series | iio: adc: at91-sama5d2_adc: add support for temperature sensor | expand |
On Thu, 9 Jun 2022 11:32:00 +0300 Claudiu Beznea <claudiu.beznea@microchip.com> wrote: > When buffers are enabled conversion may start asynchronously thus > allowing changes on actual hardware could lead to bad behavior. Thus > do not allow changing oversampling ratio and sample frequency when > buffers are enabled. Less than desirable behavior perhaps, but broken? I don't see this as a fix from what you have mentioned - though I'm not against it. (just drop the fixes tag) It is an ABI change, but unlikely to be one any sane code hits. > > Fixes: 5e1a1da0f8c9 ("iio: adc: at91-sama5d2_adc: add hw trigger and buffer support") > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> > --- > drivers/iio/adc/at91-sama5d2_adc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c > index a672a520cdc0..b76328da0cb2 100644 > --- a/drivers/iio/adc/at91-sama5d2_adc.c > +++ b/drivers/iio/adc/at91-sama5d2_adc.c > @@ -1644,6 +1644,9 @@ static int at91_adc_write_raw(struct iio_dev *indio_dev, > { > struct at91_adc_state *st = iio_priv(indio_dev); > > + if (iio_buffer_enabled(indio_dev)) > + return -EBUSY; This is racy as nothing stops buffers being enabled after this point. Use the iio_device_claim_direct_mode() and release for this as they protect against the race. > + > switch (mask) { > case IIO_CHAN_INFO_OVERSAMPLING_RATIO: > if ((val != AT91_OSR_1SAMPLES) && (val != AT91_OSR_4SAMPLES) &&
On Thu, 9 Jun 2022 11:32:03 +0300 Claudiu Beznea <claudiu.beznea@microchip.com> wrote: > Add 64 and 256 oversampling ratio support. It is necessary for temperature > sensor. > > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> > --- > drivers/iio/adc/at91-sama5d2_adc.c | 31 +++++++++++++++++++++++++++--- > 1 file changed, 28 insertions(+), 3 deletions(-) > > diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c > index 7321a4b519af..b52f1020feaf 100644 > --- a/drivers/iio/adc/at91-sama5d2_adc.c > +++ b/drivers/iio/adc/at91-sama5d2_adc.c > @@ -142,6 +142,8 @@ struct at91_adc_reg_layout { > #define AT91_SAMA5D2_EMR_OSR_1SAMPLES 0 > #define AT91_SAMA5D2_EMR_OSR_4SAMPLES 1 > #define AT91_SAMA5D2_EMR_OSR_16SAMPLES 2 > +#define AT91_SAMA5D2_EMR_OSR_64SAMPLES 3 > +#define AT91_SAMA5D2_EMR_OSR_256SAMPLES 4 > > /* Extended Mode Register - Averaging on single trigger event */ > #define AT91_SAMA5D2_EMR_ASTE(V) ((V) << 20) > @@ -308,6 +310,8 @@ static const struct at91_adc_reg_layout sama7g5_layout = { > #define AT91_OSR_1SAMPLES 1 > #define AT91_OSR_4SAMPLES 4 > #define AT91_OSR_16SAMPLES 16 > +#define AT91_OSR_64SAMPLES 64 > +#define AT91_OSR_256SAMPLES 256 These defines seems a bit silly. Better to use the values inline than to have these. > > #define AT91_SAMA5D2_CHAN_SINGLE(index, num, addr) \ > { \ > @@ -640,7 +644,9 @@ static const struct at91_adc_platform sama7g5_platform = { > .osr_mask = GENMASK(18, 16), > .osr_vals = BIT(AT91_SAMA5D2_EMR_OSR_1SAMPLES) | > BIT(AT91_SAMA5D2_EMR_OSR_4SAMPLES) | > - BIT(AT91_SAMA5D2_EMR_OSR_16SAMPLES), > + BIT(AT91_SAMA5D2_EMR_OSR_16SAMPLES) | > + BIT(AT91_SAMA5D2_EMR_OSR_64SAMPLES) | > + BIT(AT91_SAMA5D2_EMR_OSR_256SAMPLES), > .chan_realbits = 16, > }; > > @@ -774,6 +780,18 @@ static int at91_adc_config_emr(struct at91_adc_state *st, > emr |= AT91_SAMA5D2_EMR_OSR(AT91_SAMA5D2_EMR_OSR_16SAMPLES, > osr_mask); > break; > + case AT91_OSR_64SAMPLES: > + if (!(osr_vals & BIT(AT91_SAMA5D2_EMR_OSR_64SAMPLES))) > + return -EINVAL; > + emr |= AT91_SAMA5D2_EMR_OSR(AT91_SAMA5D2_EMR_OSR_64SAMPLES, > + osr_mask); > + break; > + case AT91_OSR_256SAMPLES: > + if (!(osr_vals & BIT(AT91_SAMA5D2_EMR_OSR_256SAMPLES))) > + return -EINVAL; > + emr |= AT91_SAMA5D2_EMR_OSR(AT91_SAMA5D2_EMR_OSR_256SAMPLES, > + osr_mask); > + break; > } > > at91_adc_writel(st, EMR, emr); > @@ -791,6 +809,10 @@ static int at91_adc_adjust_val_osr(struct at91_adc_state *st, int *val) > nbits = 13; > else if (st->oversampling_ratio == AT91_OSR_16SAMPLES) > nbits = 14; > + else if (st->oversampling_ratio == AT91_OSR_64SAMPLES) > + nbits = 15; > + else if (st->oversampling_ratio == AT91_OSR_256SAMPLES) > + nbits = 16; > > /* > * We have nbits of real data and channel is registered as > @@ -1679,7 +1701,8 @@ static int at91_adc_write_raw(struct iio_dev *indio_dev, > switch (mask) { > case IIO_CHAN_INFO_OVERSAMPLING_RATIO: > if ((val != AT91_OSR_1SAMPLES) && (val != AT91_OSR_4SAMPLES) && > - (val != AT91_OSR_16SAMPLES)) > + (val != AT91_OSR_16SAMPLES) && (val != AT91_OSR_64SAMPLES) && > + (val != AT91_OSR_256SAMPLES)) Dropping this partial validity check and moving into a default in the switch statement in config_emr() would be nice cleanup (I also replied to earlier patch based on what is visible here). > return -EINVAL; > /* if no change, optimize out */ > mutex_lock(&st->lock); > @@ -1897,7 +1920,9 @@ static IIO_CONST_ATTR(hwfifo_watermark_max, AT91_HWFIFO_MAX_SIZE_STR); > static IIO_CONST_ATTR(oversampling_ratio_available, > __stringify(AT91_OSR_1SAMPLES) " " > __stringify(AT91_OSR_4SAMPLES) " " > - __stringify(AT91_OSR_16SAMPLES)); > + __stringify(AT91_OSR_16SAMPLES) " " > + __stringify(AT91_OSR_64SAMPLES) " " > + __stringify(AT91_OSR_256SAMPLES)); At somepoint it would be good to move this over to the read_avail() callback rather than hand rolling it. We are slowly working through doing this for all the IIO drivers but it will take a long time yet! > > static struct attribute *at91_adc_attributes[] = { > &iio_const_attr_oversampling_ratio_available.dev_attr.attr,
On Thu, 9 Jun 2022 11:31:57 +0300 Claudiu Beznea <claudiu.beznea@microchip.com> wrote: > Hi, > > The following series add support for temperature sensor available on > SAMA7G5. > > Temperature sensor available on SAMA7G5 provides 2 outputs VTEMP and VBG. > VTEMP is proportional to the absolute temperature voltage and VBG is a > quasi-temperature independent voltage. Both are necessary in computing > the temperature (for better accuracy). Also, for better accuracy the > following settings were imposed when measusing the temperature: > oversampling rate of 256, sampling frequency of 10MHz, a startup time of > 512 ticks, MR.tracktim=0xf, EMR.trackx=0x3. > > For computing the temperature measured by ADC calibration data is > necessary. This is provided via OTP memory available on SAMA7G5. > > Patches 1/16-3/16 provides some fixes. > Patches 3/16-12/16 prepares for the addition of temperature sensor > support. > Patch 13/16 adds the temperature sensor support. > > Along with temperature sensor support I took the chance and added > runtime PM support in this series, too (handled in patch 15/16). > > The rest of patches in this series are minor cleanups. > > Thank you, > Claudiu Beznea Hi CLaudiu, Those patches I haven't replied to individually look good to me. Thanks, Jonathan > > Claudiu Beznea (16): > iio: adc: at91-sama5d2_adc: fix AT91_SAMA5D2_MR_TRACKTIM_MAX > iio: adc: at91-sama5d2_adc: lock around oversampling and sample freq > iio: adc: at91-sama5d2_adc: exit from write_raw() when buffers are > enabled > iio: adc: at91-sama5d2_adc: handle different EMR.OSR for different hw > versions > iio: adc: at91-sama5d2_adc: adjust osr based on specific platform data > iio: adc: at91-sama5d2_adc: add 64 and 256 oversampling ratio > iio: adc: at91-sama5d2_adc: simplify the code in > at91_adc_read_info_raw() > iio: adc: at91-sama5d2_adc: move oversampling storage in its function > iio: adc: at91-sama5d2_adc: update trackx on emr > iio: adc: at91-sama5d2_adc: add startup and tracktim as parameter for > at91_adc_setup_samp_freq() > iio: adc: at91-sama5d2_adc: add locking parameter to > at91_adc_read_info_raw() > dt-bindings: iio: adc: at91-sama5d2_adc: add id for temperature > channel > iio: adc: at91-sama5d2_adc: add support for temperature sensor > iio: adc: at91-sama5d2_adc: add empty line after functions > iio: adc: at91-sama5d2_adc: add runtime pm support > iio: adc: at91-sama5d2_adc: use pm_ptr() > > drivers/iio/adc/at91-sama5d2_adc.c | 633 +++++++++++++++--- > .../dt-bindings/iio/adc/at91-sama5d2_adc.h | 3 + > 2 files changed, 548 insertions(+), 88 deletions(-) >
On 11.06.2022 20:46, Jonathan Cameron wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Thu, 9 Jun 2022 11:32:01 +0300 > Claudiu Beznea <claudiu.beznea@microchip.com> wrote: > >> SAMA7G5 introduces 64 and 256 oversampling rates. Due to this EMR.OSR is 3 >> bits long. Change the code to reflect this. Commit prepares the code >> for the addition of 64 and 256 oversampling rates. >> >> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> >> --- >> drivers/iio/adc/at91-sama5d2_adc.c | 55 ++++++++++++++++++++++-------- >> 1 file changed, 40 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c >> index b76328da0cb2..1ceab097335c 100644 >> --- a/drivers/iio/adc/at91-sama5d2_adc.c >> +++ b/drivers/iio/adc/at91-sama5d2_adc.c >> @@ -138,8 +138,7 @@ struct at91_adc_reg_layout { >> /* Extended Mode Register */ >> u16 EMR; >> /* Extended Mode Register - Oversampling rate */ >> -#define AT91_SAMA5D2_EMR_OSR(V) ((V) << 16) >> -#define AT91_SAMA5D2_EMR_OSR_MASK GENMASK(17, 16) >> +#define AT91_SAMA5D2_EMR_OSR(V, M) (((V) << 16) & (M)) >> #define AT91_SAMA5D2_EMR_OSR_1SAMPLES 0 >> #define AT91_SAMA5D2_EMR_OSR_4SAMPLES 1 >> #define AT91_SAMA5D2_EMR_OSR_16SAMPLES 2 >> @@ -403,6 +402,8 @@ static const struct at91_adc_reg_layout sama7g5_layout = { >> * @max_index: highest channel index (highest index may be higher >> * than the total channel number) >> * @hw_trig_cnt: number of possible hardware triggers >> + * @osr_mask: oversampling ratio bitmask on EMR register >> + * @osr_vals: available oversampling rates >> */ >> struct at91_adc_platform { >> const struct at91_adc_reg_layout *layout; >> @@ -414,6 +415,8 @@ struct at91_adc_platform { >> unsigned int max_channels; >> unsigned int max_index; >> unsigned int hw_trig_cnt; >> + unsigned int osr_mask; >> + unsigned int osr_vals; >> }; >> >> /** >> @@ -612,6 +615,10 @@ static const struct at91_adc_platform sama5d2_platform = { >> .max_index = AT91_SAMA5D2_MAX_CHAN_IDX, >> #define AT91_SAMA5D2_HW_TRIG_CNT 3 >> .hw_trig_cnt = AT91_SAMA5D2_HW_TRIG_CNT, >> + .osr_mask = GENMASK(17, 16), >> + .osr_vals = BIT(AT91_SAMA5D2_EMR_OSR_1SAMPLES) | >> + BIT(AT91_SAMA5D2_EMR_OSR_4SAMPLES) | >> + BIT(AT91_SAMA5D2_EMR_OSR_16SAMPLES), >> }; >> >> static const struct at91_adc_platform sama7g5_platform = { >> @@ -627,6 +634,10 @@ static const struct at91_adc_platform sama7g5_platform = { >> .max_index = AT91_SAMA7G5_MAX_CHAN_IDX, >> #define AT91_SAMA7G5_HW_TRIG_CNT 3 >> .hw_trig_cnt = AT91_SAMA7G5_HW_TRIG_CNT, >> + .osr_mask = GENMASK(18, 16), >> + .osr_vals = BIT(AT91_SAMA5D2_EMR_OSR_1SAMPLES) | >> + BIT(AT91_SAMA5D2_EMR_OSR_4SAMPLES) | >> + BIT(AT91_SAMA5D2_EMR_OSR_16SAMPLES), >> }; >> >> static int at91_adc_chan_xlate(struct iio_dev *indio_dev, int chan) >> @@ -725,34 +736,45 @@ static void at91_adc_eoc_ena(struct at91_adc_state *st, unsigned int channel) >> at91_adc_writel(st, EOC_IER, BIT(channel)); >> } >> >> -static void at91_adc_config_emr(struct at91_adc_state *st) >> +static int at91_adc_config_emr(struct at91_adc_state *st, >> + u32 oversampling_ratio) >> { >> /* configure the extended mode register */ >> unsigned int emr = at91_adc_readl(st, EMR); >> + unsigned int osr_mask = st->soc_info.platform->osr_mask; >> + unsigned int osr_vals = st->soc_info.platform->osr_vals; >> >> /* select oversampling per single trigger event */ >> emr |= AT91_SAMA5D2_EMR_ASTE(1); >> >> /* delete leftover content if it's the case */ >> - emr &= ~AT91_SAMA5D2_EMR_OSR_MASK; >> + emr &= ~osr_mask; >> >> /* select oversampling ratio from configuration */ >> - switch (st->oversampling_ratio) { >> + switch (oversampling_ratio) { >> case AT91_OSR_1SAMPLES: >> - emr |= AT91_SAMA5D2_EMR_OSR(AT91_SAMA5D2_EMR_OSR_1SAMPLES) & >> - AT91_SAMA5D2_EMR_OSR_MASK; >> + if (!(osr_vals & BIT(AT91_SAMA5D2_EMR_OSR_1SAMPLES))) >> + return -EINVAL; >> + emr |= AT91_SAMA5D2_EMR_OSR(AT91_SAMA5D2_EMR_OSR_1SAMPLES, >> + osr_mask); >> break; >> case AT91_OSR_4SAMPLES: >> - emr |= AT91_SAMA5D2_EMR_OSR(AT91_SAMA5D2_EMR_OSR_4SAMPLES) & >> - AT91_SAMA5D2_EMR_OSR_MASK; >> + if (!(osr_vals & BIT(AT91_SAMA5D2_EMR_OSR_4SAMPLES))) >> + return -EINVAL; >> + emr |= AT91_SAMA5D2_EMR_OSR(AT91_SAMA5D2_EMR_OSR_4SAMPLES, >> + osr_mask); >> break; >> case AT91_OSR_16SAMPLES: >> - emr |= AT91_SAMA5D2_EMR_OSR(AT91_SAMA5D2_EMR_OSR_16SAMPLES) & >> - AT91_SAMA5D2_EMR_OSR_MASK; >> + if (!(osr_vals & BIT(AT91_SAMA5D2_EMR_OSR_16SAMPLES))) >> + return -EINVAL; >> + emr |= AT91_SAMA5D2_EMR_OSR(AT91_SAMA5D2_EMR_OSR_16SAMPLES, >> + osr_mask); >> break; >> } >> >> at91_adc_writel(st, EMR, emr); >> + >> + return 0; >> } >> >> static int at91_adc_adjust_val_osr(struct at91_adc_state *st, int *val) >> @@ -1643,6 +1665,7 @@ static int at91_adc_write_raw(struct iio_dev *indio_dev, >> int val, int val2, long mask) >> { >> struct at91_adc_state *st = iio_priv(indio_dev); >> + int ret = 0; >> >> if (iio_buffer_enabled(indio_dev)) >> return -EBUSY; >> @@ -1656,12 +1679,14 @@ static int at91_adc_write_raw(struct iio_dev *indio_dev, >> mutex_lock(&st->lock); >> if (val == st->oversampling_ratio) >> goto unlock; >> - st->oversampling_ratio = val; >> /* update ratio */ >> - at91_adc_config_emr(st); >> + ret = at91_adc_config_emr(st, val); >> + if (ret) >> + goto unlock; >> + st->oversampling_ratio = val; > > Good. I looked at the old ordering when reviewing earlier patch and thought > that doesn't look good :) > > However, now you hae the value passed to at91_adc_config_emr() perhaps > you can drop the checking that it is a possible value from above this call > and move it to the default case on the switch statement in there? > (noticed on later patch, where that context is visible). I'll check it and adapt it in next version. > >> unlock: >> mutex_unlock(&st->lock); >> - return 0; >> + return ret; >> case IIO_CHAN_INFO_SAMP_FREQ: >> if (val < st->soc_info.min_sample_rate || >> val > st->soc_info.max_sample_rate) >> @@ -1834,7 +1859,7 @@ static void at91_adc_hw_init(struct iio_dev *indio_dev) >> at91_adc_setup_samp_freq(indio_dev, st->soc_info.min_sample_rate); >> >> /* configure extended mode register */ >> - at91_adc_config_emr(st); >> + at91_adc_config_emr(st, st->oversampling_ratio); >> } >> >> static ssize_t at91_adc_get_fifo_state(struct device *dev, >
On 11.06.2022 20:47, Jonathan Cameron wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Thu, 9 Jun 2022 11:32:03 +0300 > Claudiu Beznea <claudiu.beznea@microchip.com> wrote: > >> Add 64 and 256 oversampling ratio support. It is necessary for temperature >> sensor. >> >> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> >> --- >> drivers/iio/adc/at91-sama5d2_adc.c | 31 +++++++++++++++++++++++++++--- >> 1 file changed, 28 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c >> index 7321a4b519af..b52f1020feaf 100644 >> --- a/drivers/iio/adc/at91-sama5d2_adc.c >> +++ b/drivers/iio/adc/at91-sama5d2_adc.c >> @@ -142,6 +142,8 @@ struct at91_adc_reg_layout { >> #define AT91_SAMA5D2_EMR_OSR_1SAMPLES 0 >> #define AT91_SAMA5D2_EMR_OSR_4SAMPLES 1 >> #define AT91_SAMA5D2_EMR_OSR_16SAMPLES 2 >> +#define AT91_SAMA5D2_EMR_OSR_64SAMPLES 3 >> +#define AT91_SAMA5D2_EMR_OSR_256SAMPLES 4 >> >> /* Extended Mode Register - Averaging on single trigger event */ >> #define AT91_SAMA5D2_EMR_ASTE(V) ((V) << 20) >> @@ -308,6 +310,8 @@ static const struct at91_adc_reg_layout sama7g5_layout = { >> #define AT91_OSR_1SAMPLES 1 >> #define AT91_OSR_4SAMPLES 4 >> #define AT91_OSR_16SAMPLES 16 >> +#define AT91_OSR_64SAMPLES 64 >> +#define AT91_OSR_256SAMPLES 256 > > These defines seems a bit silly. Better to use the values inline than > to have these. > >> >> #define AT91_SAMA5D2_CHAN_SINGLE(index, num, addr) \ >> { \ >> @@ -640,7 +644,9 @@ static const struct at91_adc_platform sama7g5_platform = { >> .osr_mask = GENMASK(18, 16), >> .osr_vals = BIT(AT91_SAMA5D2_EMR_OSR_1SAMPLES) | >> BIT(AT91_SAMA5D2_EMR_OSR_4SAMPLES) | >> - BIT(AT91_SAMA5D2_EMR_OSR_16SAMPLES), >> + BIT(AT91_SAMA5D2_EMR_OSR_16SAMPLES) | >> + BIT(AT91_SAMA5D2_EMR_OSR_64SAMPLES) | >> + BIT(AT91_SAMA5D2_EMR_OSR_256SAMPLES), >> .chan_realbits = 16, >> }; >> >> @@ -774,6 +780,18 @@ static int at91_adc_config_emr(struct at91_adc_state *st, >> emr |= AT91_SAMA5D2_EMR_OSR(AT91_SAMA5D2_EMR_OSR_16SAMPLES, >> osr_mask); >> break; >> + case AT91_OSR_64SAMPLES: >> + if (!(osr_vals & BIT(AT91_SAMA5D2_EMR_OSR_64SAMPLES))) >> + return -EINVAL; >> + emr |= AT91_SAMA5D2_EMR_OSR(AT91_SAMA5D2_EMR_OSR_64SAMPLES, >> + osr_mask); >> + break; >> + case AT91_OSR_256SAMPLES: >> + if (!(osr_vals & BIT(AT91_SAMA5D2_EMR_OSR_256SAMPLES))) >> + return -EINVAL; >> + emr |= AT91_SAMA5D2_EMR_OSR(AT91_SAMA5D2_EMR_OSR_256SAMPLES, >> + osr_mask); >> + break; >> } >> >> at91_adc_writel(st, EMR, emr); >> @@ -791,6 +809,10 @@ static int at91_adc_adjust_val_osr(struct at91_adc_state *st, int *val) >> nbits = 13; >> else if (st->oversampling_ratio == AT91_OSR_16SAMPLES) >> nbits = 14; >> + else if (st->oversampling_ratio == AT91_OSR_64SAMPLES) >> + nbits = 15; >> + else if (st->oversampling_ratio == AT91_OSR_256SAMPLES) >> + nbits = 16; >> >> /* >> * We have nbits of real data and channel is registered as >> @@ -1679,7 +1701,8 @@ static int at91_adc_write_raw(struct iio_dev *indio_dev, >> switch (mask) { >> case IIO_CHAN_INFO_OVERSAMPLING_RATIO: >> if ((val != AT91_OSR_1SAMPLES) && (val != AT91_OSR_4SAMPLES) && >> - (val != AT91_OSR_16SAMPLES)) >> + (val != AT91_OSR_16SAMPLES) && (val != AT91_OSR_64SAMPLES) && >> + (val != AT91_OSR_256SAMPLES)) > Dropping this partial validity check and moving into a default in the switch statement > in config_emr() would be nice cleanup (I also replied to earlier patch based on what > is visible here). Sure, I'll check it. > >> return -EINVAL; >> /* if no change, optimize out */ >> mutex_lock(&st->lock); >> @@ -1897,7 +1920,9 @@ static IIO_CONST_ATTR(hwfifo_watermark_max, AT91_HWFIFO_MAX_SIZE_STR); >> static IIO_CONST_ATTR(oversampling_ratio_available, >> __stringify(AT91_OSR_1SAMPLES) " " >> __stringify(AT91_OSR_4SAMPLES) " " >> - __stringify(AT91_OSR_16SAMPLES)); >> + __stringify(AT91_OSR_16SAMPLES) " " >> + __stringify(AT91_OSR_64SAMPLES) " " >> + __stringify(AT91_OSR_256SAMPLES)); > > At somepoint it would be good to move this over to the read_avail() callback rather than > hand rolling it. We are slowly working through doing this for all the IIO drivers > but it will take a long time yet! I'll check this, too. > >> >> static struct attribute *at91_adc_attributes[] = { >> &iio_const_attr_oversampling_ratio_available.dev_attr.attr, >
On 11.06.2022 21:16, Jonathan Cameron wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Thu, 9 Jun 2022 11:31:57 +0300 > Claudiu Beznea <claudiu.beznea@microchip.com> wrote: > >> Hi, >> >> The following series add support for temperature sensor available on >> SAMA7G5. >> >> Temperature sensor available on SAMA7G5 provides 2 outputs VTEMP and VBG. >> VTEMP is proportional to the absolute temperature voltage and VBG is a >> quasi-temperature independent voltage. Both are necessary in computing >> the temperature (for better accuracy). Also, for better accuracy the >> following settings were imposed when measusing the temperature: >> oversampling rate of 256, sampling frequency of 10MHz, a startup time of >> 512 ticks, MR.tracktim=0xf, EMR.trackx=0x3. >> >> For computing the temperature measured by ADC calibration data is >> necessary. This is provided via OTP memory available on SAMA7G5. >> >> Patches 1/16-3/16 provides some fixes. >> Patches 3/16-12/16 prepares for the addition of temperature sensor >> support. >> Patch 13/16 adds the temperature sensor support. >> >> Along with temperature sensor support I took the chance and added >> runtime PM support in this series, too (handled in patch 15/16). >> >> The rest of patches in this series are minor cleanups. >> >> Thank you, >> Claudiu Beznea > > Hi CLaudiu, > > Those patches I haven't replied to individually look good to me. Hi, Jonathan, Thank you for your review! > > Thanks, > > Jonathan > >> >> Claudiu Beznea (16): >> iio: adc: at91-sama5d2_adc: fix AT91_SAMA5D2_MR_TRACKTIM_MAX >> iio: adc: at91-sama5d2_adc: lock around oversampling and sample freq >> iio: adc: at91-sama5d2_adc: exit from write_raw() when buffers are >> enabled >> iio: adc: at91-sama5d2_adc: handle different EMR.OSR for different hw >> versions >> iio: adc: at91-sama5d2_adc: adjust osr based on specific platform data >> iio: adc: at91-sama5d2_adc: add 64 and 256 oversampling ratio >> iio: adc: at91-sama5d2_adc: simplify the code in >> at91_adc_read_info_raw() >> iio: adc: at91-sama5d2_adc: move oversampling storage in its function >> iio: adc: at91-sama5d2_adc: update trackx on emr >> iio: adc: at91-sama5d2_adc: add startup and tracktim as parameter for >> at91_adc_setup_samp_freq() >> iio: adc: at91-sama5d2_adc: add locking parameter to >> at91_adc_read_info_raw() >> dt-bindings: iio: adc: at91-sama5d2_adc: add id for temperature >> channel >> iio: adc: at91-sama5d2_adc: add support for temperature sensor >> iio: adc: at91-sama5d2_adc: add empty line after functions >> iio: adc: at91-sama5d2_adc: add runtime pm support >> iio: adc: at91-sama5d2_adc: use pm_ptr() >> >> drivers/iio/adc/at91-sama5d2_adc.c | 633 +++++++++++++++--- >> .../dt-bindings/iio/adc/at91-sama5d2_adc.h | 3 + >> 2 files changed, 548 insertions(+), 88 deletions(-) >> >
Hi, The following series add support for temperature sensor available on SAMA7G5. Temperature sensor available on SAMA7G5 provides 2 outputs VTEMP and VBG. VTEMP is proportional to the absolute temperature voltage and VBG is a quasi-temperature independent voltage. Both are necessary in computing the temperature (for better accuracy). Also, for better accuracy the following settings were imposed when measusing the temperature: oversampling rate of 256, sampling frequency of 10MHz, a startup time of 512 ticks, MR.tracktim=0xf, EMR.trackx=0x3. For computing the temperature measured by ADC calibration data is necessary. This is provided via OTP memory available on SAMA7G5. Patches 1/16-3/16 provides some fixes. Patches 3/16-12/16 prepares for the addition of temperature sensor support. Patch 13/16 adds the temperature sensor support. Along with temperature sensor support I took the chance and added runtime PM support in this series, too (handled in patch 15/16). The rest of patches in this series are minor cleanups. Thank you, Claudiu Beznea Claudiu Beznea (16): iio: adc: at91-sama5d2_adc: fix AT91_SAMA5D2_MR_TRACKTIM_MAX iio: adc: at91-sama5d2_adc: lock around oversampling and sample freq iio: adc: at91-sama5d2_adc: exit from write_raw() when buffers are enabled iio: adc: at91-sama5d2_adc: handle different EMR.OSR for different hw versions iio: adc: at91-sama5d2_adc: adjust osr based on specific platform data iio: adc: at91-sama5d2_adc: add 64 and 256 oversampling ratio iio: adc: at91-sama5d2_adc: simplify the code in at91_adc_read_info_raw() iio: adc: at91-sama5d2_adc: move oversampling storage in its function iio: adc: at91-sama5d2_adc: update trackx on emr iio: adc: at91-sama5d2_adc: add startup and tracktim as parameter for at91_adc_setup_samp_freq() iio: adc: at91-sama5d2_adc: add locking parameter to at91_adc_read_info_raw() dt-bindings: iio: adc: at91-sama5d2_adc: add id for temperature channel iio: adc: at91-sama5d2_adc: add support for temperature sensor iio: adc: at91-sama5d2_adc: add empty line after functions iio: adc: at91-sama5d2_adc: add runtime pm support iio: adc: at91-sama5d2_adc: use pm_ptr() drivers/iio/adc/at91-sama5d2_adc.c | 633 +++++++++++++++--- .../dt-bindings/iio/adc/at91-sama5d2_adc.h | 3 + 2 files changed, 548 insertions(+), 88 deletions(-)