| Message ID | 20220609083213.1795019-14-claudiu.beznea@microchip.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: at91-sama5d2_adc: add support for temperature sensor | expand |
On Thu, 9 Jun 2022 11:32:10 +0300 Claudiu Beznea <claudiu.beznea@microchip.com> wrote: > SAMA7G5 has a temperature sensor embedded that is connected to channel 31 > of ADC. Temperature sensor provides 2 outputs: VTEMP and VBG. VTEMP is > proportional to the absolute temperature voltage, VBG is quasi-temperature > independent voltage. The calibration data for temperature sensor are > retrieved from OTP memory specific to SAMA7G5. The formula to calculate > the junction temperature is as follows: > > P1 + (Vref * (Vtemp - P6 - P4 * Vbg)) / (Vbg * VTEMP_DT) > > where Pi are calibration data retrieved from OTP memory. > > For better resolution before reading the temperature certain settings > for oversampling ratio, sample frequency, EMR.TRACKX, MR.TRACKTIM are > applied. The initial settings are reapplied at the end of temperature > reading. Current support is not integrated with trigger buffers. > > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> This is a complex driver, so I got a bit lost figuring out what happens about buffered capture of this channel. What ends up in the buffer? Most processed channels are not useable with that mode (and hence have a scanindex == -1 which ensures they aren't exposed as an option for userspace to enable). Other comments inline. Thanks, Jonathan > --- > drivers/iio/adc/at91-sama5d2_adc.c | 252 ++++++++++++++++++++++++++++- > 1 file changed, 247 insertions(+), 5 deletions(-) > > diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c > index 8f8fef42de84..67ced1369754 100644 > --- a/drivers/iio/adc/at91-sama5d2_adc.c > +++ b/drivers/iio/adc/at91-sama5d2_adc.c > @@ -26,9 +26,12 @@ > #include <linux/iio/trigger.h> > #include <linux/iio/trigger_consumer.h> > #include <linux/iio/triggered_buffer.h> > +#include <linux/nvmem-consumer.h> > #include <linux/pinctrl/consumer.h> > #include <linux/regulator/consumer.h> > > +#include <dt-bindings/iio/adc/at91-sama5d2_adc.h> > + > struct at91_adc_reg_layout { > /* Control Register */ > u16 CR; > @@ -73,10 +76,13 @@ struct at91_adc_reg_layout { > /* Startup Time */ > #define AT91_SAMA5D2_MR_STARTUP(v) ((v) << 16) > #define AT91_SAMA5D2_MR_STARTUP_MASK GENMASK(19, 16) > +/* Minimum startup time for temperature sensor */ > +#define AT91_SAMA5D2_MR_STARTUP_TS_MIN (50) > /* Analog Change */ > #define AT91_SAMA5D2_MR_ANACH BIT(23) > /* Tracking Time */ > #define AT91_SAMA5D2_MR_TRACKTIM(v) ((v) << 24) > +#define AT91_SAMA5D2_MR_TRACKTIM_TS 6 > #define AT91_SAMA5D2_MR_TRACKTIM_MAX 0xf > /* Transfer Time */ > #define AT91_SAMA5D2_MR_TRANSFER(v) ((v) << 28) > @@ -149,6 +155,9 @@ struct at91_adc_reg_layout { > #define AT91_SAMA5D2_TRACKX_MASK GENMASK(23, 22) > #define AT91_SAMA5D2_TRACKX(x) (((x) << 22) & \ > AT91_SAMA5D2_TRACKX_MASK) > +/* TRACKX for temperature sensor. */ > +#define AT91_SAMA5D2_TRACKX_TS (1) > + > /* Extended Mode Register - Averaging on single trigger event */ > #define AT91_SAMA5D2_EMR_ASTE(V) ((V) << 20) > > @@ -164,6 +173,8 @@ struct at91_adc_reg_layout { > u16 ACR; > /* Analog Control Register - Pen detect sensitivity mask */ > #define AT91_SAMA5D2_ACR_PENDETSENS_MASK GENMASK(1, 0) > +/* Analog Control Register - Source last channel */ > +#define AT91_SAMA5D2_ACR_SRCLCH BIT(16) > > /* Touchscreen Mode Register */ > u16 TSMR; > @@ -231,6 +242,10 @@ struct at91_adc_reg_layout { > u16 WPSR; > /* Version Register */ > u16 VERSION; > +/* Temperature Sensor Mode Register */ > + u16 TEMPMR; > +/* Temperature Sensor Mode - Temperature sensor on */ > +#define AT91_SAMA5D2_TEMPMR_TEMPON BIT(0) > }; > > static const struct at91_adc_reg_layout sama5d2_layout = { > @@ -285,6 +300,7 @@ static const struct at91_adc_reg_layout sama7g5_layout = { > .EOC_IDR = 0x38, > .EOC_IMR = 0x3c, > .EOC_ISR = 0x40, > + .TEMPMR = 0x44, > .OVER = 0x4c, > .EMR = 0x50, > .CWR = 0x54, > @@ -390,6 +406,23 @@ static const struct at91_adc_reg_layout sama7g5_layout = { > .datasheet_name = name, \ > } > > +#define AT91_SAMA5D2_CHAN_TEMP(num, name, addr) \ > + { \ > + .type = IIO_TEMP, \ > + .channel = num, \ > + .address = addr, \ > + .scan_index = num, \ > + .scan_type = { \ > + .sign = 'u', \ > + .realbits = 16, \ > + .storagebits = 16, \ So this is unusual. Normally a processed channel isn't suitable for buffered capture because they tend not to fit in nice compact storage. In this case what actually goes in the buffer? Perhaps a comment would be useful here. > + }, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \ > + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_PROCESSED) | \ > + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),\ > + .datasheet_name = name, \ > + } > + > #define at91_adc_readl(st, reg) \ > readl_relaxed((st)->base + (st)->soc_info.platform->layout->reg) > #define at91_adc_read_chan(st, reg) \ > @@ -413,6 +446,8 @@ static const struct at91_adc_reg_layout sama7g5_layout = { > * @osr_mask: oversampling ratio bitmask on EMR register > * @osr_vals: available oversampling rates > * @chan_realbits: realbits for registered channels > + * @temp_chan: temperature channel index > + * @temp_sensor: temperature sensor supported > */ > struct at91_adc_platform { > const struct at91_adc_reg_layout *layout; > @@ -427,20 +462,54 @@ struct at91_adc_platform { > unsigned int osr_mask; > unsigned int osr_vals; > unsigned int chan_realbits; > + unsigned int temp_chan; > + bool temp_sensor; > +}; > + > +/** > + * struct at91_adc_temp_sensor_clb - at91-sama5d2 temperature sensor > + * calibration data structure > + * @p1: P1 calibration temperature > + * @p4: P4 calibration voltage > + * @p6: P6 calibration voltage > + */ > +struct at91_adc_temp_sensor_clb { > + u32 p1; > + u32 p4; > + u32 p6; > +}; > + > +/** > + * enum at91_adc_ts_clb_idx - calibration indexes in NVMEM buffer > + * @AT91_ADC_TS_CLB_IDX_P1: index for P1 > + * @AT91_ADC_TS_CLB_IDX_P4: index for P4 > + * @AT91_ADC_TS_CLB_IDX_P6: index for P6 > + * @AT91_ADC_TS_CLB_IDX_MAX: max index for temperature calibration packet in OTP > + */ > +enum at91_adc_ts_clb_idx { > + AT91_ADC_TS_CLB_IDX_P1 = 2, > + AT91_ADC_TS_CLB_IDX_P4 = 5, > + AT91_ADC_TS_CLB_IDX_P6 = 7, > + AT91_ADC_TS_CLB_IDX_MAX = 19, > }; > > +/* Temperature sensor calibration - Vtemp voltage sensitivity to temperature. */ > +#define AT91_ADC_TS_VTEMP_DT (2080U) > + > /** > * struct at91_adc_soc_info - at91-sama5d2 soc information struct > * @startup_time: device startup time > * @min_sample_rate: minimum sample rate in Hz > * @max_sample_rate: maximum sample rate in Hz > * @platform: pointer to the platform structure > + * @temp_sensor_clb: temperature sensor calibration data structure > */ > struct at91_adc_soc_info { > unsigned startup_time; > unsigned min_sample_rate; > unsigned max_sample_rate; > const struct at91_adc_platform *platform; > + struct at91_adc_temp_sensor_clb temp_sensor_clb; > }; > > struct at91_adc_trigger { > @@ -488,6 +557,20 @@ struct at91_adc_touch { > struct work_struct workq; > }; > > +/** > + * struct at91_adc_temp - at91-sama5d2 temperature information structure > + * @sample_period_val: sample period value > + * @saved_sample_rate: saved sample rate > + * @saved_oversampling: saved oversampling > + * @init: temperature sensor initialized > + */ > +struct at91_adc_temp { > + u16 sample_period_val; > + u16 saved_sample_rate; > + u16 saved_oversampling; > + bool init; > +}; > + > /* > * Buffer size requirements: > * No channels * bytes_per_channel(2) + timestamp bytes (8) > @@ -515,6 +598,7 @@ struct at91_adc_state { > wait_queue_head_t wq_data_available; > struct at91_adc_dma dma_st; > struct at91_adc_touch touch_st; > + struct at91_adc_temp temp_st; > struct iio_dev *indio_dev; > /* Ensure naturally aligned timestamp */ > u16 buffer[AT91_BUFFER_MAX_HWORDS] __aligned(8); > @@ -604,6 +688,7 @@ static const struct iio_chan_spec at91_sama7g5_adc_channels[] = { > AT91_SAMA5D2_CHAN_DIFF(22, 12, 13, 0x90), > AT91_SAMA5D2_CHAN_DIFF(23, 14, 15, 0x98), > IIO_CHAN_SOFT_TIMESTAMP(24), > + AT91_SAMA5D2_CHAN_TEMP(AT91_SAMA7G5_ADC_TEMP_CHANNEL, "temp", 0xdc), > }; > > static const struct at91_adc_platform sama5d2_platform = { > @@ -637,10 +722,13 @@ static const struct at91_adc_platform sama7g5_platform = { > .adc_channels = &at91_sama7g5_adc_channels, > #define AT91_SAMA7G5_SINGLE_CHAN_CNT 16 > #define AT91_SAMA7G5_DIFF_CHAN_CNT 8 > +#define AT91_SAMA7G5_TEMP_CHAN_CNT 1 > .nr_channels = AT91_SAMA7G5_SINGLE_CHAN_CNT + > - AT91_SAMA7G5_DIFF_CHAN_CNT, > + AT91_SAMA7G5_DIFF_CHAN_CNT + > + AT91_SAMA7G5_TEMP_CHAN_CNT, > #define AT91_SAMA7G5_MAX_CHAN_IDX (AT91_SAMA7G5_SINGLE_CHAN_CNT + \ > - AT91_SAMA7G5_DIFF_CHAN_CNT) > + AT91_SAMA7G5_DIFF_CHAN_CNT + \ > + AT91_SAMA7G5_TEMP_CHAN_CNT) > .max_channels = ARRAY_SIZE(at91_sama7g5_adc_channels), > .max_index = AT91_SAMA7G5_MAX_CHAN_IDX, > #define AT91_SAMA7G5_HW_TRIG_CNT 3 > @@ -652,6 +740,8 @@ static const struct at91_adc_platform sama7g5_platform = { > BIT(AT91_SAMA5D2_EMR_OSR_64SAMPLES) | > BIT(AT91_SAMA5D2_EMR_OSR_256SAMPLES), > .chan_realbits = 16, > + .temp_sensor = true, > + .temp_chan = AT91_SAMA7G5_ADC_TEMP_CHANNEL, > }; > > static int at91_adc_chan_xlate(struct iio_dev *indio_dev, int chan) > @@ -1193,7 +1283,8 @@ static int at91_adc_buffer_prepare(struct iio_dev *indio_dev) > continue; > /* these channel types cannot be handled by this trigger */ > if (chan->type == IIO_POSITIONRELATIVE || > - chan->type == IIO_PRESSURE) > + chan->type == IIO_PRESSURE || > + chan->type == IIO_TEMP) > continue; > > at91_adc_cor(st, chan); > @@ -1235,7 +1326,8 @@ static int at91_adc_buffer_postdisable(struct iio_dev *indio_dev) > continue; > /* these channel types are virtual, no need to do anything */ > if (chan->type == IIO_POSITIONRELATIVE || > - chan->type == IIO_PRESSURE) > + chan->type == IIO_PRESSURE || > + chan->type == IIO_TEMP) > continue; > > at91_adc_writel(st, CHDR, BIT(chan->channel)); > @@ -1617,12 +1709,19 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, > return ret; > } > > - /* in this case we have a voltage channel */ > + /* in this case we have a voltage or temperature channel */ > > st->chan = chan; > > at91_adc_cor(st, chan); > at91_adc_writel(st, CHER, BIT(chan->channel)); > + /* > + * TEMPMR.TEMPON needs to update after CHER otherwise if none > + * of the channels are enabled and TEMPMR.TEMPON = 1 will > + * trigger DRDY interruption while preparing for temperature read. > + */ > + if (chan->type == IIO_TEMP) > + at91_adc_writel(st, TEMPMR, AT91_SAMA5D2_TEMPMR_TEMPON); > at91_adc_eoc_ena(st, chan->channel); > at91_adc_writel(st, CR, AT91_SAMA5D2_CR_START); > > @@ -1642,6 +1741,8 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, > } > > at91_adc_eoc_dis(st, st->chan->channel); > + if (chan->type == IIO_TEMP) > + at91_adc_writel(st, TEMPMR, 0U); > at91_adc_writel(st, CHDR, BIT(chan->channel)); > > /* Needed to ACK the DRDY interruption */ > @@ -1654,6 +1755,91 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, > return ret; > } > > +static void at91_adc_temp_sensor_configure(struct at91_adc_state *st, > + bool start) > +{ > + u32 sample_rate, oversampling_ratio; > + u32 startup_time, tracktim, trackx; > + > + if (start) { > + /* > + * Configure the sensor for best accuracy: 10MHz frequency, > + * oversampling rate of 256, tracktim=0xf and trackx=1. > + */ > + sample_rate = 10000000U; > + oversampling_ratio = AT91_OSR_256SAMPLES; > + startup_time = AT91_SAMA5D2_MR_STARTUP_TS_MIN; > + tracktim = AT91_SAMA5D2_MR_TRACKTIM_TS; > + trackx = AT91_SAMA5D2_TRACKX_TS; > + > + st->temp_st.saved_sample_rate = st->current_sample_rate; > + st->temp_st.saved_oversampling = st->oversampling_ratio; > + } else { > + /* Go back to previous settings. */ > + sample_rate = st->temp_st.saved_sample_rate; > + oversampling_ratio = st->temp_st.saved_oversampling; > + startup_time = st->soc_info.startup_time; > + tracktim = 0; > + trackx = 0; > + } > + > + at91_adc_setup_samp_freq(st->indio_dev, sample_rate, startup_time, > + tracktim); > + at91_adc_config_emr(st, oversampling_ratio, trackx); > +} > + > +static int at91_adc_read_temp(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int *val) > +{ > + struct at91_adc_state *st = iio_priv(indio_dev); > + struct at91_adc_temp_sensor_clb *clb = &st->soc_info.temp_sensor_clb; > + u64 div1, div2; > + u32 tmp; > + int ret, vbg, vtemp; > + > + if (!st->soc_info.platform->temp_sensor || !st->temp_st.init) > + return -EPERM; You shouldn't register the channel if it's not readable. Hence this should never happen. > + > + if (iio_buffer_enabled(indio_dev)) > + return -EBUSY; claim_direct - never check the buffer is enabled as it's racy - nothing stops it being enabled immediately after you check it (only exception is before interfaces are exposed such as _resume() where it should be safe to check). Which fixes the locking issue in the previous patch. Move the iio_device_claim_direct() and mutex_lock() to all callers and you won't have a problem any more. > + > + mutex_lock(&st->lock); > + > + at91_adc_temp_sensor_configure(st, true); > + > + /* Read VBG. */ > + tmp = at91_adc_readl(st, ACR); > + tmp |= AT91_SAMA5D2_ACR_SRCLCH; > + at91_adc_writel(st, ACR, tmp); > + ret = at91_adc_read_info_raw(indio_dev, chan, &vbg, false); > + if (ret < 0) > + goto unlock; > + > + /* Read VTEMP. */ > + tmp &= ~AT91_SAMA5D2_ACR_SRCLCH; > + at91_adc_writel(st, ACR, tmp); > + ret = at91_adc_read_info_raw(indio_dev, chan, &vtemp, false); > + > +unlock: > + /* Revert previous settings. */ > + at91_adc_temp_sensor_configure(st, false); > + mutex_unlock(&st->lock); > + if (ret < 0) > + return ret; > + > + /* > + * Temp[milli] = p1[milli] + (vtemp * clb->p6 - clb->p4 * vbg)/ > + * (vbg * AT91_ADC_TS_VTEMP_DT) > + */ > + div1 = DIV_ROUND_CLOSEST_ULL(((u64)vtemp * clb->p6), vbg); > + div1 = DIV_ROUND_CLOSEST_ULL((div1 * 1000), AT91_ADC_TS_VTEMP_DT); > + div2 = DIV_ROUND_CLOSEST_ULL((u64)clb->p4, AT91_ADC_TS_VTEMP_DT); > + div2 *= 1000; > + *val = clb->p1 + (int)div1 - (int)div2; > + > + return ret; > +} > + > static int at91_adc_read_raw(struct iio_dev *indio_dev, > struct iio_chan_spec const *chan, > int *val, int *val2, long mask) > @@ -1671,6 +1857,11 @@ static int at91_adc_read_raw(struct iio_dev *indio_dev, > *val2 = chan->scan_type.realbits; > return IIO_VAL_FRACTIONAL_LOG2; > > + case IIO_CHAN_INFO_PROCESSED: > + if (chan->type != IIO_TEMP) > + return -EINVAL; > + return at91_adc_read_temp(indio_dev, chan, val); > + > case IIO_CHAN_INFO_SAMP_FREQ: > mutex_lock(&st->lock); > *val = at91_adc_get_sample_freq(st); > @@ -1987,6 +2178,55 @@ static int at91_adc_buffer_and_trigger_init(struct device *dev, > return 0; > } > > +static void at91_adc_temp_sensor_init(struct at91_adc_state *st, > + struct device *dev) > +{ > + struct at91_adc_temp_sensor_clb *clb = &st->soc_info.temp_sensor_clb; > + struct nvmem_cell *temp_calib; > + u32 *buf; > + size_t len; > + > + if (!st->soc_info.platform->temp_sensor) > + return; > + > + st->temp_st.init = false; that structure is kzalloc so this isn't really needed unless you think it's providing 'documentation'. > + > + /* Get the calibration data from NVMEM. */ > + temp_calib = devm_nvmem_cell_get(dev, "temperature_calib"); > + if (IS_ERR(temp_calib)) { > + if (PTR_ERR(temp_calib) != -ENOENT) > + dev_err(dev, "Failed to get temperature_calib cell!\n"); > + return; > + } > + > + buf = nvmem_cell_read(temp_calib, &len); > + if (IS_ERR(buf)) { > + dev_err(dev, "Failed to read calibration data!\n"); > + return; > + } > + if (len < AT91_ADC_TS_CLB_IDX_MAX * 4) { > + dev_err(dev, "Invalid calibration data!\n"); > + goto free_buf; > + } > + > + /* Store calibration data for later use. */ > + clb->p1 = buf[AT91_ADC_TS_CLB_IDX_P1]; > + clb->p4 = buf[AT91_ADC_TS_CLB_IDX_P4]; > + clb->p6 = buf[AT91_ADC_TS_CLB_IDX_P6]; > + > + /* > + * We prepare here the conversion to milli and also add constant > + * factor (5 degrees Celsius) to p1 here to avoid doing it on > + * hotpath. > + */ > + clb->p1 = clb->p1 * 1000 + 5000; > + > + st->temp_st.init = true; > + > +free_buf: > + kfree(buf); > +} > +
On 11.06.2022 21:15, 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:10 +0300 > Claudiu Beznea <claudiu.beznea@microchip.com> wrote: > >> SAMA7G5 has a temperature sensor embedded that is connected to channel 31 >> of ADC. Temperature sensor provides 2 outputs: VTEMP and VBG. VTEMP is >> proportional to the absolute temperature voltage, VBG is quasi-temperature >> independent voltage. The calibration data for temperature sensor are >> retrieved from OTP memory specific to SAMA7G5. The formula to calculate >> the junction temperature is as follows: >> >> P1 + (Vref * (Vtemp - P6 - P4 * Vbg)) / (Vbg * VTEMP_DT) >> >> where Pi are calibration data retrieved from OTP memory. >> >> For better resolution before reading the temperature certain settings >> for oversampling ratio, sample frequency, EMR.TRACKX, MR.TRACKTIM are >> applied. The initial settings are reapplied at the end of temperature >> reading. Current support is not integrated with trigger buffers. >> >> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> > > This is a complex driver, so I got a bit lost figuring out what happens > about buffered capture of this channel. What ends up in the buffer? With this implementation nothing should end up in the buffer as the patch skips channel enable/disable if its about an IIO_TEMP channel (see functions functions at91_adc_buffer_prepare(), at91_adc_buffer_postdisable()). More details above. According to datasheet the temperature channel behaves the same as the other channels. On temperature channel are multiplexed both VBG and VTEMP. ` | \ +-----+ VBG --->| |ch31 | | Vtemp --->| |---->| ADC | | / +-----+ | / . And both are needed to be measured in order to determine the correct in SoC temperature. At a moment of time only one of these could be measured, the selection being done with bit SRCLCH bit of ACR register. According to datasheet there is no special treatment for channel 31 of ADC in case triggers are enabled. So, in case of a buffer capture the buffer for channel 31 will contain either the converted value for VBG or VTEMP (depending on the value of bit SRCLCH bit of ACR register), if channel 31 is enabled for that. But on this implementation we skip the enable/disable of IIO_TEMP channels (functions at91_adc_buffer_prepare(), at91_adc_buffer_postdisable()). Hardware implements a special mechanism for reading the temperature when triggers are enabled as follows: the hardware provides a RTC trigger wich fires every 1 second and starts a conversion on channel 31. This could permit to have a temperature value once every 2 seconds (in the 1st RTC trigger VBG could be read, in the 2nd RTC trigger Vtemp could be read, or the other way arround). For this, configuration needs to be done propertly in the RTC memory spaces and on Linux side something should be implemented for the interaction b/w RTC and IIO subsystems. But this is for future development. > Most processed channels are not useable with that mode (and hence have > a scanindex == -1 which ensures they aren't exposed as an option for > userspace to enable). OK, I haven't been aware of that. I only did some basic research on how this could be achieved. As we are using the thermal support on SAMA7G5 with driver at drivers/thermal/thermal-generic-adc.c which reads processed value at periodic intervals one idea was to take advantage of the RTC trigger mechanism for channel 31 and have the converted values of VBG and VTEMP kept only inside the at91-sama5d2_adc.c thus when receiving requests from themal-generic-adc.c and buffers are enabled to use those cached values in computation formula. > > Other comments inline. > > Thanks, > > Jonathan > >> --- >> drivers/iio/adc/at91-sama5d2_adc.c | 252 ++++++++++++++++++++++++++++- >> 1 file changed, 247 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c >> index 8f8fef42de84..67ced1369754 100644 >> --- a/drivers/iio/adc/at91-sama5d2_adc.c >> +++ b/drivers/iio/adc/at91-sama5d2_adc.c >> @@ -26,9 +26,12 @@ >> #include <linux/iio/trigger.h> >> #include <linux/iio/trigger_consumer.h> >> #include <linux/iio/triggered_buffer.h> >> +#include <linux/nvmem-consumer.h> >> #include <linux/pinctrl/consumer.h> >> #include <linux/regulator/consumer.h> >> >> +#include <dt-bindings/iio/adc/at91-sama5d2_adc.h> >> + >> struct at91_adc_reg_layout { >> /* Control Register */ >> u16 CR; >> @@ -73,10 +76,13 @@ struct at91_adc_reg_layout { >> /* Startup Time */ >> #define AT91_SAMA5D2_MR_STARTUP(v) ((v) << 16) >> #define AT91_SAMA5D2_MR_STARTUP_MASK GENMASK(19, 16) >> +/* Minimum startup time for temperature sensor */ >> +#define AT91_SAMA5D2_MR_STARTUP_TS_MIN (50) >> /* Analog Change */ >> #define AT91_SAMA5D2_MR_ANACH BIT(23) >> /* Tracking Time */ >> #define AT91_SAMA5D2_MR_TRACKTIM(v) ((v) << 24) >> +#define AT91_SAMA5D2_MR_TRACKTIM_TS 6 >> #define AT91_SAMA5D2_MR_TRACKTIM_MAX 0xf >> /* Transfer Time */ >> #define AT91_SAMA5D2_MR_TRANSFER(v) ((v) << 28) >> @@ -149,6 +155,9 @@ struct at91_adc_reg_layout { >> #define AT91_SAMA5D2_TRACKX_MASK GENMASK(23, 22) >> #define AT91_SAMA5D2_TRACKX(x) (((x) << 22) & \ >> AT91_SAMA5D2_TRACKX_MASK) >> +/* TRACKX for temperature sensor. */ >> +#define AT91_SAMA5D2_TRACKX_TS (1) >> + >> /* Extended Mode Register - Averaging on single trigger event */ >> #define AT91_SAMA5D2_EMR_ASTE(V) ((V) << 20) >> >> @@ -164,6 +173,8 @@ struct at91_adc_reg_layout { >> u16 ACR; >> /* Analog Control Register - Pen detect sensitivity mask */ >> #define AT91_SAMA5D2_ACR_PENDETSENS_MASK GENMASK(1, 0) >> +/* Analog Control Register - Source last channel */ >> +#define AT91_SAMA5D2_ACR_SRCLCH BIT(16) >> >> /* Touchscreen Mode Register */ >> u16 TSMR; >> @@ -231,6 +242,10 @@ struct at91_adc_reg_layout { >> u16 WPSR; >> /* Version Register */ >> u16 VERSION; >> +/* Temperature Sensor Mode Register */ >> + u16 TEMPMR; >> +/* Temperature Sensor Mode - Temperature sensor on */ >> +#define AT91_SAMA5D2_TEMPMR_TEMPON BIT(0) >> }; >> >> static const struct at91_adc_reg_layout sama5d2_layout = { >> @@ -285,6 +300,7 @@ static const struct at91_adc_reg_layout sama7g5_layout = { >> .EOC_IDR = 0x38, >> .EOC_IMR = 0x3c, >> .EOC_ISR = 0x40, >> + .TEMPMR = 0x44, >> .OVER = 0x4c, >> .EMR = 0x50, >> .CWR = 0x54, >> @@ -390,6 +406,23 @@ static const struct at91_adc_reg_layout sama7g5_layout = { >> .datasheet_name = name, \ >> } >> >> +#define AT91_SAMA5D2_CHAN_TEMP(num, name, addr) \ >> + { \ >> + .type = IIO_TEMP, \ >> + .channel = num, \ >> + .address = addr, \ >> + .scan_index = num, \ >> + .scan_type = { \ >> + .sign = 'u', \ >> + .realbits = 16, \ >> + .storagebits = 16, \ > > So this is unusual. Normally a processed channel isn't suitable for buffered > capture because they tend not to fit in nice compact storage. In this case > what actually goes in the buffer? Perhaps a comment would be useful here. At the moment we don't allow the enabling of channel 31 when enabling the buffers (we skip IIO_TEMP channels in at91_adc_buffer_prepare(), at91_adc_buffer_postdisable()). At the moment when buffers are enabled the IIO_TEMP consumer (drivers/thermal/thermal-generic-adc.c) will fail on readings due to iio_buffer_enabled() in at91_adc_read_temp() or iio_device_claim_direct_mode() in at91_adc_read_info_raw(). > >> + }, \ >> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \ >> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_PROCESSED) | \ >> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),\ >> + .datasheet_name = name, \ >> + } >> + >> #define at91_adc_readl(st, reg) \ >> readl_relaxed((st)->base + (st)->soc_info.platform->layout->reg) >> #define at91_adc_read_chan(st, reg) \ >> @@ -413,6 +446,8 @@ static const struct at91_adc_reg_layout sama7g5_layout = { >> * @osr_mask: oversampling ratio bitmask on EMR register >> * @osr_vals: available oversampling rates >> * @chan_realbits: realbits for registered channels >> + * @temp_chan: temperature channel index >> + * @temp_sensor: temperature sensor supported >> */ >> struct at91_adc_platform { >> const struct at91_adc_reg_layout *layout; >> @@ -427,20 +462,54 @@ struct at91_adc_platform { >> unsigned int osr_mask; >> unsigned int osr_vals; >> unsigned int chan_realbits; >> + unsigned int temp_chan; >> + bool temp_sensor; >> +}; >> + >> +/** >> + * struct at91_adc_temp_sensor_clb - at91-sama5d2 temperature sensor >> + * calibration data structure >> + * @p1: P1 calibration temperature >> + * @p4: P4 calibration voltage >> + * @p6: P6 calibration voltage >> + */ >> +struct at91_adc_temp_sensor_clb { >> + u32 p1; >> + u32 p4; >> + u32 p6; >> +}; >> + >> +/** >> + * enum at91_adc_ts_clb_idx - calibration indexes in NVMEM buffer >> + * @AT91_ADC_TS_CLB_IDX_P1: index for P1 >> + * @AT91_ADC_TS_CLB_IDX_P4: index for P4 >> + * @AT91_ADC_TS_CLB_IDX_P6: index for P6 >> + * @AT91_ADC_TS_CLB_IDX_MAX: max index for temperature calibration packet in OTP >> + */ >> +enum at91_adc_ts_clb_idx { >> + AT91_ADC_TS_CLB_IDX_P1 = 2, >> + AT91_ADC_TS_CLB_IDX_P4 = 5, >> + AT91_ADC_TS_CLB_IDX_P6 = 7, >> + AT91_ADC_TS_CLB_IDX_MAX = 19, >> }; >> >> +/* Temperature sensor calibration - Vtemp voltage sensitivity to temperature. */ >> +#define AT91_ADC_TS_VTEMP_DT (2080U) >> + >> /** >> * struct at91_adc_soc_info - at91-sama5d2 soc information struct >> * @startup_time: device startup time >> * @min_sample_rate: minimum sample rate in Hz >> * @max_sample_rate: maximum sample rate in Hz >> * @platform: pointer to the platform structure >> + * @temp_sensor_clb: temperature sensor calibration data structure >> */ >> struct at91_adc_soc_info { >> unsigned startup_time; >> unsigned min_sample_rate; >> unsigned max_sample_rate; >> const struct at91_adc_platform *platform; >> + struct at91_adc_temp_sensor_clb temp_sensor_clb; >> }; >> >> struct at91_adc_trigger { >> @@ -488,6 +557,20 @@ struct at91_adc_touch { >> struct work_struct workq; >> }; >> >> +/** >> + * struct at91_adc_temp - at91-sama5d2 temperature information structure >> + * @sample_period_val: sample period value >> + * @saved_sample_rate: saved sample rate >> + * @saved_oversampling: saved oversampling >> + * @init: temperature sensor initialized >> + */ >> +struct at91_adc_temp { >> + u16 sample_period_val; >> + u16 saved_sample_rate; >> + u16 saved_oversampling; >> + bool init; >> +}; >> + >> /* >> * Buffer size requirements: >> * No channels * bytes_per_channel(2) + timestamp bytes (8) >> @@ -515,6 +598,7 @@ struct at91_adc_state { >> wait_queue_head_t wq_data_available; >> struct at91_adc_dma dma_st; >> struct at91_adc_touch touch_st; >> + struct at91_adc_temp temp_st; >> struct iio_dev *indio_dev; >> /* Ensure naturally aligned timestamp */ >> u16 buffer[AT91_BUFFER_MAX_HWORDS] __aligned(8); >> @@ -604,6 +688,7 @@ static const struct iio_chan_spec at91_sama7g5_adc_channels[] = { >> AT91_SAMA5D2_CHAN_DIFF(22, 12, 13, 0x90), >> AT91_SAMA5D2_CHAN_DIFF(23, 14, 15, 0x98), >> IIO_CHAN_SOFT_TIMESTAMP(24), >> + AT91_SAMA5D2_CHAN_TEMP(AT91_SAMA7G5_ADC_TEMP_CHANNEL, "temp", 0xdc), >> }; >> >> static const struct at91_adc_platform sama5d2_platform = { >> @@ -637,10 +722,13 @@ static const struct at91_adc_platform sama7g5_platform = { >> .adc_channels = &at91_sama7g5_adc_channels, >> #define AT91_SAMA7G5_SINGLE_CHAN_CNT 16 >> #define AT91_SAMA7G5_DIFF_CHAN_CNT 8 >> +#define AT91_SAMA7G5_TEMP_CHAN_CNT 1 >> .nr_channels = AT91_SAMA7G5_SINGLE_CHAN_CNT + >> - AT91_SAMA7G5_DIFF_CHAN_CNT, >> + AT91_SAMA7G5_DIFF_CHAN_CNT + >> + AT91_SAMA7G5_TEMP_CHAN_CNT, >> #define AT91_SAMA7G5_MAX_CHAN_IDX (AT91_SAMA7G5_SINGLE_CHAN_CNT + \ >> - AT91_SAMA7G5_DIFF_CHAN_CNT) >> + AT91_SAMA7G5_DIFF_CHAN_CNT + \ >> + AT91_SAMA7G5_TEMP_CHAN_CNT) >> .max_channels = ARRAY_SIZE(at91_sama7g5_adc_channels), >> .max_index = AT91_SAMA7G5_MAX_CHAN_IDX, >> #define AT91_SAMA7G5_HW_TRIG_CNT 3 >> @@ -652,6 +740,8 @@ static const struct at91_adc_platform sama7g5_platform = { >> BIT(AT91_SAMA5D2_EMR_OSR_64SAMPLES) | >> BIT(AT91_SAMA5D2_EMR_OSR_256SAMPLES), >> .chan_realbits = 16, >> + .temp_sensor = true, >> + .temp_chan = AT91_SAMA7G5_ADC_TEMP_CHANNEL, >> }; >> >> static int at91_adc_chan_xlate(struct iio_dev *indio_dev, int chan) >> @@ -1193,7 +1283,8 @@ static int at91_adc_buffer_prepare(struct iio_dev *indio_dev) >> continue; >> /* these channel types cannot be handled by this trigger */ >> if (chan->type == IIO_POSITIONRELATIVE || >> - chan->type == IIO_PRESSURE) >> + chan->type == IIO_PRESSURE || >> + chan->type == IIO_TEMP) >> continue; >> >> at91_adc_cor(st, chan); >> @@ -1235,7 +1326,8 @@ static int at91_adc_buffer_postdisable(struct iio_dev *indio_dev) >> continue; >> /* these channel types are virtual, no need to do anything */ >> if (chan->type == IIO_POSITIONRELATIVE || >> - chan->type == IIO_PRESSURE) >> + chan->type == IIO_PRESSURE || >> + chan->type == IIO_TEMP) >> continue; >> >> at91_adc_writel(st, CHDR, BIT(chan->channel)); >> @@ -1617,12 +1709,19 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, >> return ret; >> } >> >> - /* in this case we have a voltage channel */ >> + /* in this case we have a voltage or temperature channel */ >> >> st->chan = chan; >> >> at91_adc_cor(st, chan); >> at91_adc_writel(st, CHER, BIT(chan->channel)); >> + /* >> + * TEMPMR.TEMPON needs to update after CHER otherwise if none >> + * of the channels are enabled and TEMPMR.TEMPON = 1 will >> + * trigger DRDY interruption while preparing for temperature read. >> + */ >> + if (chan->type == IIO_TEMP) >> + at91_adc_writel(st, TEMPMR, AT91_SAMA5D2_TEMPMR_TEMPON); >> at91_adc_eoc_ena(st, chan->channel); >> at91_adc_writel(st, CR, AT91_SAMA5D2_CR_START); >> >> @@ -1642,6 +1741,8 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, >> } >> >> at91_adc_eoc_dis(st, st->chan->channel); >> + if (chan->type == IIO_TEMP) >> + at91_adc_writel(st, TEMPMR, 0U); >> at91_adc_writel(st, CHDR, BIT(chan->channel)); >> >> /* Needed to ACK the DRDY interruption */ >> @@ -1654,6 +1755,91 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, >> return ret; >> } >> >> +static void at91_adc_temp_sensor_configure(struct at91_adc_state *st, >> + bool start) >> +{ >> + u32 sample_rate, oversampling_ratio; >> + u32 startup_time, tracktim, trackx; >> + >> + if (start) { >> + /* >> + * Configure the sensor for best accuracy: 10MHz frequency, >> + * oversampling rate of 256, tracktim=0xf and trackx=1. >> + */ >> + sample_rate = 10000000U; >> + oversampling_ratio = AT91_OSR_256SAMPLES; >> + startup_time = AT91_SAMA5D2_MR_STARTUP_TS_MIN; >> + tracktim = AT91_SAMA5D2_MR_TRACKTIM_TS; >> + trackx = AT91_SAMA5D2_TRACKX_TS; >> + >> + st->temp_st.saved_sample_rate = st->current_sample_rate; >> + st->temp_st.saved_oversampling = st->oversampling_ratio; >> + } else { >> + /* Go back to previous settings. */ >> + sample_rate = st->temp_st.saved_sample_rate; >> + oversampling_ratio = st->temp_st.saved_oversampling; >> + startup_time = st->soc_info.startup_time; >> + tracktim = 0; >> + trackx = 0; >> + } >> + >> + at91_adc_setup_samp_freq(st->indio_dev, sample_rate, startup_time, >> + tracktim); >> + at91_adc_config_emr(st, oversampling_ratio, trackx); >> +} >> + >> +static int at91_adc_read_temp(struct iio_dev *indio_dev, >> + struct iio_chan_spec const *chan, int *val) >> +{ >> + struct at91_adc_state *st = iio_priv(indio_dev); >> + struct at91_adc_temp_sensor_clb *clb = &st->soc_info.temp_sensor_clb; >> + u64 div1, div2; >> + u32 tmp; >> + int ret, vbg, vtemp; >> + >> + if (!st->soc_info.platform->temp_sensor || !st->temp_st.init) >> + return -EPERM; > > You shouldn't register the channel if it's not readable. Hence this > should never happen. I kept this as an indicator for temperature consumer that something wrong happend on probing path of temperature sensor. In function at91_adc_temp_sensor_init() the following could fail: - devm_nvmem_cell_get() - nvmem_cell_read() - invalid length for NVMEM cell Thus to keep the ADC probe going on in case temperature sensor probing init failed I've added this st->temp_st.init. On the field there might be devices that don't have proper information in NVMEM memory for temperature sensor. > >> + >> + if (iio_buffer_enabled(indio_dev)) >> + return -EBUSY; > > claim_direct - never check the buffer is enabled as it's racy - nothing > stops it being enabled immediately after you check it (only exception > is before interfaces are exposed such as _resume() where it should be safe > to check). > Which fixes the locking issue in the previous patch. Move the > iio_device_claim_direct() and mutex_lock() to all callers and you won't > have a problem any more. OK > > >> + >> + mutex_lock(&st->lock); >> + >> + at91_adc_temp_sensor_configure(st, true); >> + >> + /* Read VBG. */ >> + tmp = at91_adc_readl(st, ACR); >> + tmp |= AT91_SAMA5D2_ACR_SRCLCH; >> + at91_adc_writel(st, ACR, tmp); >> + ret = at91_adc_read_info_raw(indio_dev, chan, &vbg, false); >> + if (ret < 0) >> + goto unlock; >> + >> + /* Read VTEMP. */ >> + tmp &= ~AT91_SAMA5D2_ACR_SRCLCH; >> + at91_adc_writel(st, ACR, tmp); >> + ret = at91_adc_read_info_raw(indio_dev, chan, &vtemp, false); >> + >> +unlock: >> + /* Revert previous settings. */ >> + at91_adc_temp_sensor_configure(st, false); >> + mutex_unlock(&st->lock); >> + if (ret < 0) >> + return ret; >> + >> + /* >> + * Temp[milli] = p1[milli] + (vtemp * clb->p6 - clb->p4 * vbg)/ >> + * (vbg * AT91_ADC_TS_VTEMP_DT) >> + */ >> + div1 = DIV_ROUND_CLOSEST_ULL(((u64)vtemp * clb->p6), vbg); >> + div1 = DIV_ROUND_CLOSEST_ULL((div1 * 1000), AT91_ADC_TS_VTEMP_DT); >> + div2 = DIV_ROUND_CLOSEST_ULL((u64)clb->p4, AT91_ADC_TS_VTEMP_DT); >> + div2 *= 1000; >> + *val = clb->p1 + (int)div1 - (int)div2; >> + >> + return ret; >> +} >> + >> static int at91_adc_read_raw(struct iio_dev *indio_dev, >> struct iio_chan_spec const *chan, >> int *val, int *val2, long mask) >> @@ -1671,6 +1857,11 @@ static int at91_adc_read_raw(struct iio_dev *indio_dev, >> *val2 = chan->scan_type.realbits; >> return IIO_VAL_FRACTIONAL_LOG2; >> >> + case IIO_CHAN_INFO_PROCESSED: >> + if (chan->type != IIO_TEMP) >> + return -EINVAL; >> + return at91_adc_read_temp(indio_dev, chan, val); >> + >> case IIO_CHAN_INFO_SAMP_FREQ: >> mutex_lock(&st->lock); >> *val = at91_adc_get_sample_freq(st); >> @@ -1987,6 +2178,55 @@ static int at91_adc_buffer_and_trigger_init(struct device *dev, >> return 0; >> } >> >> +static void at91_adc_temp_sensor_init(struct at91_adc_state *st, >> + struct device *dev) >> +{ >> + struct at91_adc_temp_sensor_clb *clb = &st->soc_info.temp_sensor_clb; >> + struct nvmem_cell *temp_calib; >> + u32 *buf; >> + size_t len; >> + >> + if (!st->soc_info.platform->temp_sensor) >> + return; >> + >> + st->temp_st.init = false; > > that structure is kzalloc so this isn't really needed unless you think it's > providing 'documentation'. As mentioned above, I keep this as an indicator for temperature consumer that temperature sendor was correctly probed. > >> + >> + /* Get the calibration data from NVMEM. */ >> + temp_calib = devm_nvmem_cell_get(dev, "temperature_calib"); >> + if (IS_ERR(temp_calib)) { >> + if (PTR_ERR(temp_calib) != -ENOENT) >> + dev_err(dev, "Failed to get temperature_calib cell!\n"); >> + return; >> + } >> + >> + buf = nvmem_cell_read(temp_calib, &len); >> + if (IS_ERR(buf)) { >> + dev_err(dev, "Failed to read calibration data!\n"); >> + return; >> + } >> + if (len < AT91_ADC_TS_CLB_IDX_MAX * 4) { >> + dev_err(dev, "Invalid calibration data!\n"); >> + goto free_buf; >> + } >> + >> + /* Store calibration data for later use. */ >> + clb->p1 = buf[AT91_ADC_TS_CLB_IDX_P1]; >> + clb->p4 = buf[AT91_ADC_TS_CLB_IDX_P4]; >> + clb->p6 = buf[AT91_ADC_TS_CLB_IDX_P6]; >> + >> + /* >> + * We prepare here the conversion to milli and also add constant >> + * factor (5 degrees Celsius) to p1 here to avoid doing it on >> + * hotpath. >> + */ >> + clb->p1 = clb->p1 * 1000 + 5000; >> + >> + st->temp_st.init = true; >> + >> +free_buf: >> + kfree(buf); >> +} >> + >
On Tue, 14 Jun 2022 10:13:17 +0000 <Claudiu.Beznea@microchip.com> wrote: > On 11.06.2022 21:15, 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:10 +0300 > > Claudiu Beznea <claudiu.beznea@microchip.com> wrote: > > > >> SAMA7G5 has a temperature sensor embedded that is connected to channel 31 > >> of ADC. Temperature sensor provides 2 outputs: VTEMP and VBG. VTEMP is > >> proportional to the absolute temperature voltage, VBG is quasi-temperature > >> independent voltage. The calibration data for temperature sensor are > >> retrieved from OTP memory specific to SAMA7G5. The formula to calculate > >> the junction temperature is as follows: > >> > >> P1 + (Vref * (Vtemp - P6 - P4 * Vbg)) / (Vbg * VTEMP_DT) > >> > >> where Pi are calibration data retrieved from OTP memory. > >> > >> For better resolution before reading the temperature certain settings > >> for oversampling ratio, sample frequency, EMR.TRACKX, MR.TRACKTIM are > >> applied. The initial settings are reapplied at the end of temperature > >> reading. Current support is not integrated with trigger buffers. > >> > >> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> > > > > This is a complex driver, so I got a bit lost figuring out what happens > > about buffered capture of this channel. What ends up in the buffer? > > With this implementation nothing should end up in the buffer as the patch > skips channel enable/disable if its about an IIO_TEMP channel (see > functions functions at91_adc_buffer_prepare(), > at91_adc_buffer_postdisable()). More details above. > > According to datasheet the temperature channel behaves the same as the > other channels. On temperature channel are multiplexed both VBG and VTEMP. > > ` > | \ +-----+ > VBG --->| |ch31 | | > Vtemp --->| |---->| ADC | > | / +-----+ > | / > . > > And both are needed to be measured in order to determine the correct in SoC > temperature. > > At a moment of time only one of these could be measured, the selection > being done with bit SRCLCH bit of ACR register. > > According to datasheet there is no special treatment for channel 31 of ADC > in case triggers are enabled. So, in case of a buffer capture the buffer > for channel 31 will contain either the converted value for VBG or VTEMP > (depending on the value of bit SRCLCH bit of ACR register), if channel 31 > is enabled for that. But on this implementation we skip the enable/disable > of IIO_TEMP channels (functions at91_adc_buffer_prepare(), > at91_adc_buffer_postdisable()). Great explanation. Perhaps we can capture some of it either as comments, or patch description etc. > > Hardware implements a special mechanism for reading the temperature when > triggers are enabled as follows: the hardware provides a RTC trigger wich > fires every 1 second and starts a conversion on channel 31. This could > permit to have a temperature value once every 2 seconds (in the 1st RTC > trigger VBG could be read, in the 2nd RTC trigger Vtemp could be read, or > the other way arround). For this, configuration needs to be done propertly > in the RTC memory spaces and on Linux side something should be implemented > for the interaction b/w RTC and IIO subsystems. But this is for future > development. > > > Most processed channels are not useable with that mode (and hence have > > a scanindex == -1 which ensures they aren't exposed as an option for > > userspace to enable). > > OK, I haven't been aware of that. I only did some basic research on how > this could be achieved. As we are using the thermal support on SAMA7G5 with > driver at drivers/thermal/thermal-generic-adc.c which reads processed value > at periodic intervals one idea was to take advantage of the RTC trigger > mechanism for channel 31 and have the converted values of VBG and VTEMP > kept only inside the at91-sama5d2_adc.c thus when receiving requests from > themal-generic-adc.c and buffers are enabled to use those cached values in > computation formula. Sure, that might work ok. It's a bit of a hack, but would let you keep the more interesting stuff hidden way in one place. > ... > >> +#define AT91_SAMA5D2_CHAN_TEMP(num, name, addr) \ > >> + { \ > >> + .type = IIO_TEMP, \ > >> + .channel = num, \ > >> + .address = addr, \ > >> + .scan_index = num, \ > >> + .scan_type = { \ > >> + .sign = 'u', \ > >> + .realbits = 16, \ > >> + .storagebits = 16, \ > > > > So this is unusual. Normally a processed channel isn't suitable for buffered > > capture because they tend not to fit in nice compact storage. In this case > > what actually goes in the buffer? Perhaps a comment would be useful here. > > At the moment we don't allow the enabling of channel 31 when enabling the > buffers (we skip IIO_TEMP channels in at91_adc_buffer_prepare(), > at91_adc_buffer_postdisable()). At the moment when buffers are enabled the > IIO_TEMP consumer (drivers/thermal/thermal-generic-adc.c) will fail on > readings due to iio_buffer_enabled() in at91_adc_read_temp() or > iio_device_claim_direct_mode() in at91_adc_read_info_raw(). I suspected as much. If so, a bunch of this makes not sense. Channels that can't be in that buffer have magic scan_index = -1 and .scan_type is probably not used (occasionally it is helpful for non buffered paths, though rarely all the info in there). > > >> +static int at91_adc_read_temp(struct iio_dev *indio_dev, > >> + struct iio_chan_spec const *chan, int *val) > >> +{ > >> + struct at91_adc_state *st = iio_priv(indio_dev); > >> + struct at91_adc_temp_sensor_clb *clb = &st->soc_info.temp_sensor_clb; > >> + u64 div1, div2; > >> + u32 tmp; > >> + int ret, vbg, vtemp; > >> + > >> + if (!st->soc_info.platform->temp_sensor || !st->temp_st.init) > >> + return -EPERM; > > > > You shouldn't register the channel if it's not readable. Hence this > > should never happen. > > I kept this as an indicator for temperature consumer that something wrong > happend on probing path of temperature sensor. In function > at91_adc_temp_sensor_init() the following could fail: > - devm_nvmem_cell_get() > - nvmem_cell_read() > - invalid length for NVMEM cell > Thus to keep the ADC probe going on in case temperature sensor probing init > failed I've added this st->temp_st.init. On the field there might be > devices that don't have proper information in NVMEM memory for temperature > sensor. If those fail, don't register the channel. It should just be invisible to userspace / in kernel consumers. That may require reorganizing how you register channels to know if these worked or not before the point of registering channels. Thanks, Jonathan
diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c index 8f8fef42de84..67ced1369754 100644 --- a/drivers/iio/adc/at91-sama5d2_adc.c +++ b/drivers/iio/adc/at91-sama5d2_adc.c @@ -26,9 +26,12 @@ #include <linux/iio/trigger.h> #include <linux/iio/trigger_consumer.h> #include <linux/iio/triggered_buffer.h> +#include <linux/nvmem-consumer.h> #include <linux/pinctrl/consumer.h> #include <linux/regulator/consumer.h> +#include <dt-bindings/iio/adc/at91-sama5d2_adc.h> + struct at91_adc_reg_layout { /* Control Register */ u16 CR; @@ -73,10 +76,13 @@ struct at91_adc_reg_layout { /* Startup Time */ #define AT91_SAMA5D2_MR_STARTUP(v) ((v) << 16) #define AT91_SAMA5D2_MR_STARTUP_MASK GENMASK(19, 16) +/* Minimum startup time for temperature sensor */ +#define AT91_SAMA5D2_MR_STARTUP_TS_MIN (50) /* Analog Change */ #define AT91_SAMA5D2_MR_ANACH BIT(23) /* Tracking Time */ #define AT91_SAMA5D2_MR_TRACKTIM(v) ((v) << 24) +#define AT91_SAMA5D2_MR_TRACKTIM_TS 6 #define AT91_SAMA5D2_MR_TRACKTIM_MAX 0xf /* Transfer Time */ #define AT91_SAMA5D2_MR_TRANSFER(v) ((v) << 28) @@ -149,6 +155,9 @@ struct at91_adc_reg_layout { #define AT91_SAMA5D2_TRACKX_MASK GENMASK(23, 22) #define AT91_SAMA5D2_TRACKX(x) (((x) << 22) & \ AT91_SAMA5D2_TRACKX_MASK) +/* TRACKX for temperature sensor. */ +#define AT91_SAMA5D2_TRACKX_TS (1) + /* Extended Mode Register - Averaging on single trigger event */ #define AT91_SAMA5D2_EMR_ASTE(V) ((V) << 20) @@ -164,6 +173,8 @@ struct at91_adc_reg_layout { u16 ACR; /* Analog Control Register - Pen detect sensitivity mask */ #define AT91_SAMA5D2_ACR_PENDETSENS_MASK GENMASK(1, 0) +/* Analog Control Register - Source last channel */ +#define AT91_SAMA5D2_ACR_SRCLCH BIT(16) /* Touchscreen Mode Register */ u16 TSMR; @@ -231,6 +242,10 @@ struct at91_adc_reg_layout { u16 WPSR; /* Version Register */ u16 VERSION; +/* Temperature Sensor Mode Register */ + u16 TEMPMR; +/* Temperature Sensor Mode - Temperature sensor on */ +#define AT91_SAMA5D2_TEMPMR_TEMPON BIT(0) }; static const struct at91_adc_reg_layout sama5d2_layout = { @@ -285,6 +300,7 @@ static const struct at91_adc_reg_layout sama7g5_layout = { .EOC_IDR = 0x38, .EOC_IMR = 0x3c, .EOC_ISR = 0x40, + .TEMPMR = 0x44, .OVER = 0x4c, .EMR = 0x50, .CWR = 0x54, @@ -390,6 +406,23 @@ static const struct at91_adc_reg_layout sama7g5_layout = { .datasheet_name = name, \ } +#define AT91_SAMA5D2_CHAN_TEMP(num, name, addr) \ + { \ + .type = IIO_TEMP, \ + .channel = num, \ + .address = addr, \ + .scan_index = num, \ + .scan_type = { \ + .sign = 'u', \ + .realbits = 16, \ + .storagebits = 16, \ + }, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \ + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_PROCESSED) | \ + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),\ + .datasheet_name = name, \ + } + #define at91_adc_readl(st, reg) \ readl_relaxed((st)->base + (st)->soc_info.platform->layout->reg) #define at91_adc_read_chan(st, reg) \ @@ -413,6 +446,8 @@ static const struct at91_adc_reg_layout sama7g5_layout = { * @osr_mask: oversampling ratio bitmask on EMR register * @osr_vals: available oversampling rates * @chan_realbits: realbits for registered channels + * @temp_chan: temperature channel index + * @temp_sensor: temperature sensor supported */ struct at91_adc_platform { const struct at91_adc_reg_layout *layout; @@ -427,20 +462,54 @@ struct at91_adc_platform { unsigned int osr_mask; unsigned int osr_vals; unsigned int chan_realbits; + unsigned int temp_chan; + bool temp_sensor; +}; + +/** + * struct at91_adc_temp_sensor_clb - at91-sama5d2 temperature sensor + * calibration data structure + * @p1: P1 calibration temperature + * @p4: P4 calibration voltage + * @p6: P6 calibration voltage + */ +struct at91_adc_temp_sensor_clb { + u32 p1; + u32 p4; + u32 p6; +}; + +/** + * enum at91_adc_ts_clb_idx - calibration indexes in NVMEM buffer + * @AT91_ADC_TS_CLB_IDX_P1: index for P1 + * @AT91_ADC_TS_CLB_IDX_P4: index for P4 + * @AT91_ADC_TS_CLB_IDX_P6: index for P6 + * @AT91_ADC_TS_CLB_IDX_MAX: max index for temperature calibration packet in OTP + */ +enum at91_adc_ts_clb_idx { + AT91_ADC_TS_CLB_IDX_P1 = 2, + AT91_ADC_TS_CLB_IDX_P4 = 5, + AT91_ADC_TS_CLB_IDX_P6 = 7, + AT91_ADC_TS_CLB_IDX_MAX = 19, }; +/* Temperature sensor calibration - Vtemp voltage sensitivity to temperature. */ +#define AT91_ADC_TS_VTEMP_DT (2080U) + /** * struct at91_adc_soc_info - at91-sama5d2 soc information struct * @startup_time: device startup time * @min_sample_rate: minimum sample rate in Hz * @max_sample_rate: maximum sample rate in Hz * @platform: pointer to the platform structure + * @temp_sensor_clb: temperature sensor calibration data structure */ struct at91_adc_soc_info { unsigned startup_time; unsigned min_sample_rate; unsigned max_sample_rate; const struct at91_adc_platform *platform; + struct at91_adc_temp_sensor_clb temp_sensor_clb; }; struct at91_adc_trigger { @@ -488,6 +557,20 @@ struct at91_adc_touch { struct work_struct workq; }; +/** + * struct at91_adc_temp - at91-sama5d2 temperature information structure + * @sample_period_val: sample period value + * @saved_sample_rate: saved sample rate + * @saved_oversampling: saved oversampling + * @init: temperature sensor initialized + */ +struct at91_adc_temp { + u16 sample_period_val; + u16 saved_sample_rate; + u16 saved_oversampling; + bool init; +}; + /* * Buffer size requirements: * No channels * bytes_per_channel(2) + timestamp bytes (8) @@ -515,6 +598,7 @@ struct at91_adc_state { wait_queue_head_t wq_data_available; struct at91_adc_dma dma_st; struct at91_adc_touch touch_st; + struct at91_adc_temp temp_st; struct iio_dev *indio_dev; /* Ensure naturally aligned timestamp */ u16 buffer[AT91_BUFFER_MAX_HWORDS] __aligned(8); @@ -604,6 +688,7 @@ static const struct iio_chan_spec at91_sama7g5_adc_channels[] = { AT91_SAMA5D2_CHAN_DIFF(22, 12, 13, 0x90), AT91_SAMA5D2_CHAN_DIFF(23, 14, 15, 0x98), IIO_CHAN_SOFT_TIMESTAMP(24), + AT91_SAMA5D2_CHAN_TEMP(AT91_SAMA7G5_ADC_TEMP_CHANNEL, "temp", 0xdc), }; static const struct at91_adc_platform sama5d2_platform = { @@ -637,10 +722,13 @@ static const struct at91_adc_platform sama7g5_platform = { .adc_channels = &at91_sama7g5_adc_channels, #define AT91_SAMA7G5_SINGLE_CHAN_CNT 16 #define AT91_SAMA7G5_DIFF_CHAN_CNT 8 +#define AT91_SAMA7G5_TEMP_CHAN_CNT 1 .nr_channels = AT91_SAMA7G5_SINGLE_CHAN_CNT + - AT91_SAMA7G5_DIFF_CHAN_CNT, + AT91_SAMA7G5_DIFF_CHAN_CNT + + AT91_SAMA7G5_TEMP_CHAN_CNT, #define AT91_SAMA7G5_MAX_CHAN_IDX (AT91_SAMA7G5_SINGLE_CHAN_CNT + \ - AT91_SAMA7G5_DIFF_CHAN_CNT) + AT91_SAMA7G5_DIFF_CHAN_CNT + \ + AT91_SAMA7G5_TEMP_CHAN_CNT) .max_channels = ARRAY_SIZE(at91_sama7g5_adc_channels), .max_index = AT91_SAMA7G5_MAX_CHAN_IDX, #define AT91_SAMA7G5_HW_TRIG_CNT 3 @@ -652,6 +740,8 @@ static const struct at91_adc_platform sama7g5_platform = { BIT(AT91_SAMA5D2_EMR_OSR_64SAMPLES) | BIT(AT91_SAMA5D2_EMR_OSR_256SAMPLES), .chan_realbits = 16, + .temp_sensor = true, + .temp_chan = AT91_SAMA7G5_ADC_TEMP_CHANNEL, }; static int at91_adc_chan_xlate(struct iio_dev *indio_dev, int chan) @@ -1193,7 +1283,8 @@ static int at91_adc_buffer_prepare(struct iio_dev *indio_dev) continue; /* these channel types cannot be handled by this trigger */ if (chan->type == IIO_POSITIONRELATIVE || - chan->type == IIO_PRESSURE) + chan->type == IIO_PRESSURE || + chan->type == IIO_TEMP) continue; at91_adc_cor(st, chan); @@ -1235,7 +1326,8 @@ static int at91_adc_buffer_postdisable(struct iio_dev *indio_dev) continue; /* these channel types are virtual, no need to do anything */ if (chan->type == IIO_POSITIONRELATIVE || - chan->type == IIO_PRESSURE) + chan->type == IIO_PRESSURE || + chan->type == IIO_TEMP) continue; at91_adc_writel(st, CHDR, BIT(chan->channel)); @@ -1617,12 +1709,19 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, return ret; } - /* in this case we have a voltage channel */ + /* in this case we have a voltage or temperature channel */ st->chan = chan; at91_adc_cor(st, chan); at91_adc_writel(st, CHER, BIT(chan->channel)); + /* + * TEMPMR.TEMPON needs to update after CHER otherwise if none + * of the channels are enabled and TEMPMR.TEMPON = 1 will + * trigger DRDY interruption while preparing for temperature read. + */ + if (chan->type == IIO_TEMP) + at91_adc_writel(st, TEMPMR, AT91_SAMA5D2_TEMPMR_TEMPON); at91_adc_eoc_ena(st, chan->channel); at91_adc_writel(st, CR, AT91_SAMA5D2_CR_START); @@ -1642,6 +1741,8 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, } at91_adc_eoc_dis(st, st->chan->channel); + if (chan->type == IIO_TEMP) + at91_adc_writel(st, TEMPMR, 0U); at91_adc_writel(st, CHDR, BIT(chan->channel)); /* Needed to ACK the DRDY interruption */ @@ -1654,6 +1755,91 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, return ret; } +static void at91_adc_temp_sensor_configure(struct at91_adc_state *st, + bool start) +{ + u32 sample_rate, oversampling_ratio; + u32 startup_time, tracktim, trackx; + + if (start) { + /* + * Configure the sensor for best accuracy: 10MHz frequency, + * oversampling rate of 256, tracktim=0xf and trackx=1. + */ + sample_rate = 10000000U; + oversampling_ratio = AT91_OSR_256SAMPLES; + startup_time = AT91_SAMA5D2_MR_STARTUP_TS_MIN; + tracktim = AT91_SAMA5D2_MR_TRACKTIM_TS; + trackx = AT91_SAMA5D2_TRACKX_TS; + + st->temp_st.saved_sample_rate = st->current_sample_rate; + st->temp_st.saved_oversampling = st->oversampling_ratio; + } else { + /* Go back to previous settings. */ + sample_rate = st->temp_st.saved_sample_rate; + oversampling_ratio = st->temp_st.saved_oversampling; + startup_time = st->soc_info.startup_time; + tracktim = 0; + trackx = 0; + } + + at91_adc_setup_samp_freq(st->indio_dev, sample_rate, startup_time, + tracktim); + at91_adc_config_emr(st, oversampling_ratio, trackx); +} + +static int at91_adc_read_temp(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, int *val) +{ + struct at91_adc_state *st = iio_priv(indio_dev); + struct at91_adc_temp_sensor_clb *clb = &st->soc_info.temp_sensor_clb; + u64 div1, div2; + u32 tmp; + int ret, vbg, vtemp; + + if (!st->soc_info.platform->temp_sensor || !st->temp_st.init) + return -EPERM; + + if (iio_buffer_enabled(indio_dev)) + return -EBUSY; + + mutex_lock(&st->lock); + + at91_adc_temp_sensor_configure(st, true); + + /* Read VBG. */ + tmp = at91_adc_readl(st, ACR); + tmp |= AT91_SAMA5D2_ACR_SRCLCH; + at91_adc_writel(st, ACR, tmp); + ret = at91_adc_read_info_raw(indio_dev, chan, &vbg, false); + if (ret < 0) + goto unlock; + + /* Read VTEMP. */ + tmp &= ~AT91_SAMA5D2_ACR_SRCLCH; + at91_adc_writel(st, ACR, tmp); + ret = at91_adc_read_info_raw(indio_dev, chan, &vtemp, false); + +unlock: + /* Revert previous settings. */ + at91_adc_temp_sensor_configure(st, false); + mutex_unlock(&st->lock); + if (ret < 0) + return ret; + + /* + * Temp[milli] = p1[milli] + (vtemp * clb->p6 - clb->p4 * vbg)/ + * (vbg * AT91_ADC_TS_VTEMP_DT) + */ + div1 = DIV_ROUND_CLOSEST_ULL(((u64)vtemp * clb->p6), vbg); + div1 = DIV_ROUND_CLOSEST_ULL((div1 * 1000), AT91_ADC_TS_VTEMP_DT); + div2 = DIV_ROUND_CLOSEST_ULL((u64)clb->p4, AT91_ADC_TS_VTEMP_DT); + div2 *= 1000; + *val = clb->p1 + (int)div1 - (int)div2; + + return ret; +} + static int at91_adc_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, int *val, int *val2, long mask) @@ -1671,6 +1857,11 @@ static int at91_adc_read_raw(struct iio_dev *indio_dev, *val2 = chan->scan_type.realbits; return IIO_VAL_FRACTIONAL_LOG2; + case IIO_CHAN_INFO_PROCESSED: + if (chan->type != IIO_TEMP) + return -EINVAL; + return at91_adc_read_temp(indio_dev, chan, val); + case IIO_CHAN_INFO_SAMP_FREQ: mutex_lock(&st->lock); *val = at91_adc_get_sample_freq(st); @@ -1987,6 +2178,55 @@ static int at91_adc_buffer_and_trigger_init(struct device *dev, return 0; } +static void at91_adc_temp_sensor_init(struct at91_adc_state *st, + struct device *dev) +{ + struct at91_adc_temp_sensor_clb *clb = &st->soc_info.temp_sensor_clb; + struct nvmem_cell *temp_calib; + u32 *buf; + size_t len; + + if (!st->soc_info.platform->temp_sensor) + return; + + st->temp_st.init = false; + + /* Get the calibration data from NVMEM. */ + temp_calib = devm_nvmem_cell_get(dev, "temperature_calib"); + if (IS_ERR(temp_calib)) { + if (PTR_ERR(temp_calib) != -ENOENT) + dev_err(dev, "Failed to get temperature_calib cell!\n"); + return; + } + + buf = nvmem_cell_read(temp_calib, &len); + if (IS_ERR(buf)) { + dev_err(dev, "Failed to read calibration data!\n"); + return; + } + if (len < AT91_ADC_TS_CLB_IDX_MAX * 4) { + dev_err(dev, "Invalid calibration data!\n"); + goto free_buf; + } + + /* Store calibration data for later use. */ + clb->p1 = buf[AT91_ADC_TS_CLB_IDX_P1]; + clb->p4 = buf[AT91_ADC_TS_CLB_IDX_P4]; + clb->p6 = buf[AT91_ADC_TS_CLB_IDX_P6]; + + /* + * We prepare here the conversion to milli and also add constant + * factor (5 degrees Celsius) to p1 here to avoid doing it on + * hotpath. + */ + clb->p1 = clb->p1 * 1000 + 5000; + + st->temp_st.init = true; + +free_buf: + kfree(buf); +} + static int at91_adc_probe(struct platform_device *pdev) { struct iio_dev *indio_dev; @@ -2120,6 +2360,8 @@ static int at91_adc_probe(struct platform_device *pdev) if (ret) goto vref_disable; + at91_adc_temp_sensor_init(st, &pdev->dev); + at91_adc_hw_init(indio_dev); platform_set_drvdata(pdev, indio_dev);
SAMA7G5 has a temperature sensor embedded that is connected to channel 31 of ADC. Temperature sensor provides 2 outputs: VTEMP and VBG. VTEMP is proportional to the absolute temperature voltage, VBG is quasi-temperature independent voltage. The calibration data for temperature sensor are retrieved from OTP memory specific to SAMA7G5. The formula to calculate the junction temperature is as follows: P1 + (Vref * (Vtemp - P6 - P4 * Vbg)) / (Vbg * VTEMP_DT) where Pi are calibration data retrieved from OTP memory. For better resolution before reading the temperature certain settings for oversampling ratio, sample frequency, EMR.TRACKX, MR.TRACKTIM are applied. The initial settings are reapplied at the end of temperature reading. Current support is not integrated with trigger buffers. Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> --- drivers/iio/adc/at91-sama5d2_adc.c | 252 ++++++++++++++++++++++++++++- 1 file changed, 247 insertions(+), 5 deletions(-)