On Tue, 27 Mar 2018 15:32:39 +0300 Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx> wrote: > This implements the support for position and pressure for the included > touchscreen support in the SAMA5D2 SOC ADC block. > Two position channels are added and one for pressure. > They can be read in raw format, or through a buffer. > A normal use case is for a consumer driver to register a callback buffer > for these channels. > When the touchscreen channels are in the active scan mask, > the driver will start the touchscreen sampling and push the data to the > buffer. > > Some parts of this patch are based on initial original work by > Mohamed Jamsheeth Hajanajubudeen and Bandaru Venkateswara Swamy > > Signed-off-by: Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx> Looks pretty good to me. A few minor comments inline. Jonathan > --- > Changes in v2: > - the support is now based on callback buffer. > > drivers/iio/adc/at91-sama5d2_adc.c | 452 ++++++++++++++++++++++++++++++++++++- > 1 file changed, 443 insertions(+), 9 deletions(-) > > diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c > index 8729d65..29a7fb9 100644 > --- a/drivers/iio/adc/at91-sama5d2_adc.c > +++ b/drivers/iio/adc/at91-sama5d2_adc.c > @@ -102,14 +102,26 @@ > #define AT91_SAMA5D2_LCDR 0x20 > /* Interrupt Enable Register */ > #define AT91_SAMA5D2_IER 0x24 > +/* Interrupt Enable Register - TS X measurement ready */ > +#define AT91_SAMA5D2_IER_XRDY BIT(20) > +/* Interrupt Enable Register - TS Y measurement ready */ > +#define AT91_SAMA5D2_IER_YRDY BIT(21) > +/* Interrupt Enable Register - TS pressure measurement ready */ > +#define AT91_SAMA5D2_IER_PRDY BIT(22) > /* Interrupt Enable Register - general overrun error */ > #define AT91_SAMA5D2_IER_GOVRE BIT(25) > +/* Interrupt Enable Register - Pen detect */ > +#define AT91_SAMA5D2_IER_PEN BIT(29) > +/* Interrupt Enable Register - No pen detect */ > +#define AT91_SAMA5D2_IER_NOPEN BIT(30) > /* Interrupt Disable Register */ > #define AT91_SAMA5D2_IDR 0x28 > /* Interrupt Mask Register */ > #define AT91_SAMA5D2_IMR 0x2c > /* Interrupt Status Register */ > #define AT91_SAMA5D2_ISR 0x30 > +/* Interrupt Status Register - Pen touching sense status */ > +#define AT91_SAMA5D2_ISR_PENS BIT(31) > /* Last Channel Trigger Mode Register */ > #define AT91_SAMA5D2_LCTMR 0x34 > /* Last Channel Compare Window Register */ > @@ -131,8 +143,38 @@ > #define AT91_SAMA5D2_CDR0 0x50 > /* Analog Control Register */ > #define AT91_SAMA5D2_ACR 0x94 > +/* Analog Control Register - Pen detect sensitivity mask */ > +#define AT91_SAMA5D2_ACR_PENDETSENS_MASK GENMASK(1, 0) > + > /* Touchscreen Mode Register */ > #define AT91_SAMA5D2_TSMR 0xb0 > +/* Touchscreen Mode Register - No touch mode */ > +#define AT91_SAMA5D2_TSMR_TSMODE_NONE 0 > +/* Touchscreen Mode Register - 4 wire screen, no pressure measurement */ > +#define AT91_SAMA5D2_TSMR_TSMODE_4WIRE_NO_PRESS 1 > +/* Touchscreen Mode Register - 4 wire screen, pressure measurement */ > +#define AT91_SAMA5D2_TSMR_TSMODE_4WIRE_PRESS 2 > +/* Touchscreen Mode Register - 5 wire screen */ > +#define AT91_SAMA5D2_TSMR_TSMODE_5WIRE 3 > +/* Touchscreen Mode Register - Average samples mask */ > +#define AT91_SAMA5D2_TSMR_TSAV_MASK GENMASK(5, 4) > +/* Touchscreen Mode Register - Average samples */ > +#define AT91_SAMA5D2_TSMR_TSAV(x) ((x) << 4) > +/* Touchscreen Mode Register - Touch/trigger frequency ratio mask */ > +#define AT91_SAMA5D2_TSMR_TSFREQ_MASK GENMASK(11, 8) > +/* Touchscreen Mode Register - Touch/trigger frequency ratio */ > +#define AT91_SAMA5D2_TSMR_TSFREQ(x) ((x) << 8) > +/* Touchscreen Mode Register - Pen Debounce Time mask */ > +#define AT91_SAMA5D2_TSMR_PENDBC_MASK GENMASK(31, 28) > +/* Touchscreen Mode Register - Pen Debounce Time */ > +#define AT91_SAMA5D2_TSMR_PENDBC(x) ((x) << 28) > +/* Touchscreen Mode Register - No DMA for touch measurements */ > +#define AT91_SAMA5D2_TSMR_NOTSDMA BIT(22) > +/* Touchscreen Mode Register - Disable pen detection */ > +#define AT91_SAMA5D2_TSMR_PENDET_DIS (0 << 24) > +/* Touchscreen Mode Register - Enable pen detection */ > +#define AT91_SAMA5D2_TSMR_PENDET_ENA BIT(24) > + > /* Touchscreen X Position Register */ > #define AT91_SAMA5D2_XPOSR 0xb4 > /* Touchscreen Y Position Register */ > @@ -151,6 +193,12 @@ > #define AT91_SAMA5D2_TRGR_TRGMOD_EXT_TRIG_FALL 2 > /* Trigger Mode external trigger any edge */ > #define AT91_SAMA5D2_TRGR_TRGMOD_EXT_TRIG_ANY 3 > +/* Trigger Mode internal periodic */ > +#define AT91_SAMA5D2_TRGR_TRGMOD_PERIODIC 5 > +/* Trigger Mode - trigger period mask */ > +#define AT91_SAMA5D2_TRGR_TRGPER_MASK GENMASK(31, 16) > +/* Trigger Mode - trigger period */ > +#define AT91_SAMA5D2_TRGR_TRGPER(x) ((x) << 16) > > /* Correction Select Register */ > #define AT91_SAMA5D2_COSR 0xd0 > @@ -169,6 +217,22 @@ > #define AT91_SAMA5D2_SINGLE_CHAN_CNT 12 > #define AT91_SAMA5D2_DIFF_CHAN_CNT 6 > > +#define AT91_SAMA5D2_TIMESTAMP_CHAN_IDX (AT91_SAMA5D2_SINGLE_CHAN_CNT + \ > + AT91_SAMA5D2_DIFF_CHAN_CNT + 1) > + > +#define AT91_SAMA5D2_TOUCH_X_CHAN_IDX (AT91_SAMA5D2_SINGLE_CHAN_CNT + \ > + AT91_SAMA5D2_DIFF_CHAN_CNT * 2) > +#define AT91_SAMA5D2_TOUCH_Y_CHAN_IDX (AT91_SAMA5D2_TOUCH_X_CHAN_IDX + 1) > +#define AT91_SAMA5D2_TOUCH_P_CHAN_IDX (AT91_SAMA5D2_TOUCH_Y_CHAN_IDX + 1) > +#define AT91_SAMA5D2_MAX_CHAN_IDX AT91_SAMA5D2_TOUCH_P_CHAN_IDX > + > +#define TOUCH_SAMPLE_PERIOD_US 2000 /* 2ms */ > +#define TOUCH_PEN_DETECT_DEBOUNCE_US 200 > + > +#define XYZ_MASK GENMASK(11, 0) > + > +#define MAX_POS_BITS 12 > + Please prefix these defines as the chances of a clash with these generic names is very high. > /* > * Maximum number of bytes to hold conversion from all channels > * without the timestamp. > @@ -222,6 +286,37 @@ > .indexed = 1, \ > } > > +#define AT91_SAMA5D2_CHAN_TOUCH(num, name, mod) \ > + { \ > + .type = IIO_POSITIONRELATIVE, \ > + .modified = 1, \ > + .channel = num, \ > + .channel2 = mod, \ > + .scan_index = num, \ > + .scan_type = { \ > + .sign = 'u', \ > + .realbits = 12, \ > + .storagebits = 16, \ > + }, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\ > + .datasheet_name = name, \ > + } > +#define AT91_SAMA5D2_CHAN_PRESSURE(num, name) \ > + { \ > + .type = IIO_PRESSURE, \ > + .channel = num, \ > + .scan_index = num, \ > + .scan_type = { \ > + .sign = 'u', \ > + .realbits = 12, \ > + .storagebits = 16, \ > + }, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\ > + .datasheet_name = name, \ > + } > + > #define at91_adc_readl(st, reg) readl_relaxed(st->base + reg) > #define at91_adc_writel(st, reg, val) writel_relaxed(val, st->base + reg) > > @@ -260,6 +355,22 @@ struct at91_adc_dma { > s64 dma_ts; > }; > > +/** > + * at91_adc_touch - at91-sama5d2 touchscreen information struct > + * @sample_period_val: the value for periodic trigger interval > + * @touching: is the pen touching the screen or not > + * @x_pos: temporary placeholder for pressure computation > + * @channels_bitmask: bitmask with the touchscreen channels enabled > + * @workq: workqueue for buffer data pushing > + */ > +struct at91_adc_touch { > + u16 sample_period_val; > + bool touching; > + u16 x_pos; > + unsigned long channels_bitmask; > + struct work_struct workq; > +}; > + > struct at91_adc_state { > void __iomem *base; > int irq; > @@ -267,6 +378,7 @@ struct at91_adc_state { > struct regulator *reg; > struct regulator *vref; > int vref_uv; > + unsigned int current_sample_rate; > struct iio_trigger *trig; > const struct at91_adc_trigger *selected_trig; > const struct iio_chan_spec *chan; > @@ -275,6 +387,7 @@ struct at91_adc_state { > struct at91_adc_soc_info soc_info; > wait_queue_head_t wq_data_available; > struct at91_adc_dma dma_st; > + struct at91_adc_touch touch_st; > u16 buffer[AT91_BUFFER_MAX_HWORDS]; > /* > * lock to prevent concurrent 'single conversion' requests through > @@ -329,8 +442,10 @@ static const struct iio_chan_spec at91_adc_channels[] = { > AT91_SAMA5D2_CHAN_DIFF(6, 7, 0x68), > AT91_SAMA5D2_CHAN_DIFF(8, 9, 0x70), > AT91_SAMA5D2_CHAN_DIFF(10, 11, 0x78), > - IIO_CHAN_SOFT_TIMESTAMP(AT91_SAMA5D2_SINGLE_CHAN_CNT > - + AT91_SAMA5D2_DIFF_CHAN_CNT + 1), > + IIO_CHAN_SOFT_TIMESTAMP(AT91_SAMA5D2_TIMESTAMP_CHAN_IDX), > + AT91_SAMA5D2_CHAN_TOUCH(AT91_SAMA5D2_TOUCH_X_CHAN_IDX, "x", IIO_MOD_X), > + AT91_SAMA5D2_CHAN_TOUCH(AT91_SAMA5D2_TOUCH_Y_CHAN_IDX, "y", IIO_MOD_Y), > + AT91_SAMA5D2_CHAN_PRESSURE(AT91_SAMA5D2_TOUCH_P_CHAN_IDX, "pressure"), > }; > > static int at91_adc_chan_xlate(struct iio_dev *indio_dev, int chan) > @@ -354,6 +469,162 @@ at91_adc_chan_get(struct iio_dev *indio_dev, int chan) > return indio_dev->channels + index; > } > > +static inline int at91_adc_of_xlate(struct iio_dev *indio_dev, > + const struct of_phandle_args *iiospec) > +{ > + return at91_adc_chan_xlate(indio_dev, iiospec->args[0]); > +} > + > +static int at91_adc_configure_touch(struct at91_adc_state *st, bool state) > +{ > + u32 clk_khz = st->current_sample_rate / 1000; > + int i = 0; > + u16 pendbc; > + u32 tsmr, acr; > + > + if (!state) { > + /* disabling touch IRQs and setting mode to no touch enabled */ > + at91_adc_writel(st, AT91_SAMA5D2_IDR, > + AT91_SAMA5D2_IER_PEN | AT91_SAMA5D2_IER_NOPEN); > + at91_adc_writel(st, AT91_SAMA5D2_TSMR, 0); > + return 0; > + } > + /* > + * debounce time is in microseconds, we need it in milliseconds to > + * multiply with kilohertz, so, divide by 1000, but after the multiply. > + * round up to make sure pendbc is at least 1 > + */ > + pendbc = round_up(TOUCH_PEN_DETECT_DEBOUNCE_US * clk_khz / 1000, 1); > + > + /* get the required exponent */ > + while (pendbc >> i++) > + ; > + > + pendbc = i; > + > + tsmr = AT91_SAMA5D2_TSMR_TSMODE_4WIRE_PRESS; > + > + tsmr |= AT91_SAMA5D2_TSMR_TSAV(2) & AT91_SAMA5D2_TSMR_TSAV_MASK; > + tsmr |= AT91_SAMA5D2_TSMR_PENDBC(pendbc) & > + AT91_SAMA5D2_TSMR_PENDBC_MASK; > + tsmr |= AT91_SAMA5D2_TSMR_NOTSDMA; > + tsmr |= AT91_SAMA5D2_TSMR_PENDET_ENA; > + tsmr |= AT91_SAMA5D2_TSMR_TSFREQ(2) & AT91_SAMA5D2_TSMR_TSFREQ_MASK; > + > + at91_adc_writel(st, AT91_SAMA5D2_TSMR, tsmr); > + > + acr = at91_adc_readl(st, AT91_SAMA5D2_ACR); > + acr &= ~AT91_SAMA5D2_ACR_PENDETSENS_MASK; > + acr |= 0x02 & AT91_SAMA5D2_ACR_PENDETSENS_MASK; > + at91_adc_writel(st, AT91_SAMA5D2_ACR, acr); > + > + /* Sample Period Time = (TRGPER + 1) / ADCClock */ > + st->touch_st.sample_period_val = round_up((TOUCH_SAMPLE_PERIOD_US * > + clk_khz / 1000) - 1, 1); > + /* enable pen detect IRQ */ > + at91_adc_writel(st, AT91_SAMA5D2_IER, AT91_SAMA5D2_IER_PEN); > + > + return 0; > +} > + > +static u16 at91_adc_touch_x_pos(struct at91_adc_state *st) > +{ > + u32 val; > + u32 xscale, x, xpos; > + > + /* x position = (x / xscale) * max, max = 2^MAX_POS_BITS - 1 */ > + val = at91_adc_readl(st, AT91_SAMA5D2_XPOSR); > + if (!val) > + dev_dbg(&iio_priv_to_dev(st)->dev, "x_pos is 0\n"); > + > + xpos = val & XYZ_MASK; > + x = (xpos << MAX_POS_BITS) - xpos; > + xscale = (val >> 16) & XYZ_MASK; > + if (xscale == 0) { > + dev_err(&iio_priv_to_dev(st)->dev, "xscale is 0\n"); > + return 0; > + } > + x /= xscale; > + st->touch_st.x_pos = x; > + > + return x; > +} > + > +static u16 at91_adc_touch_y_pos(struct at91_adc_state *st) > +{ > + u32 val; > + u32 yscale, y, ypos; > + > + /* y position = (y / yscale) * max, max = 2^MAX_POS_BITS - 1 */ > + val = at91_adc_readl(st, AT91_SAMA5D2_YPOSR); > + if (!val) > + dev_dbg(&iio_priv_to_dev(st)->dev, "y_pos is 0\n"); > + > + ypos = val & XYZ_MASK; > + y = (ypos << MAX_POS_BITS) - ypos; > + yscale = (val >> 16) & XYZ_MASK; > + > + if (yscale == 0) { > + dev_err(&iio_priv_to_dev(st)->dev, "yscale is 0\n"); > + return 0; > + } > + > + y /= yscale; > + > + return y; This is obviously very similar to the above and feels like you need some wrappers around a single function? Just the xpos store needs to be outside the shared function. > +} > + > +static u16 at91_adc_touch_pressure(struct at91_adc_state *st) > +{ > + u32 val; > + u32 z1, z2; > + u32 pres; > + u32 rxp = 1; > + u32 factor = 1000; > + > + /* calculate the pressure */ > + val = at91_adc_readl(st, AT91_SAMA5D2_PRESSR); > + z1 = val & XYZ_MASK; > + z2 = (val >> 16) & XYZ_MASK; > + > + if (z1 != 0) > + pres = rxp * (st->touch_st.x_pos * factor / 1024) * > + (z2 * factor / z1 - factor) / > + factor; > + else > + pres = 0xFFFF; /* no pen contact */ Would a pressure value of 0 not make more sense? > + > + return pres; > +} > + > +static int at91_adc_read_position(struct at91_adc_state *st, int chan, u16 *val) > +{ > + *val = 0; > + if (!st->touch_st.touching) > + return -ENODATA; > + if (chan == AT91_SAMA5D2_TOUCH_X_CHAN_IDX) > + *val = at91_adc_touch_x_pos(st); > + else if (chan == AT91_SAMA5D2_TOUCH_Y_CHAN_IDX) > + *val = at91_adc_touch_y_pos(st); > + else > + return -ENODATA; > + > + return IIO_VAL_INT; > +} > + > +static int at91_adc_read_pressure(struct at91_adc_state *st, int chan, u16 *val) > +{ > + *val = 0; > + if (!st->touch_st.touching) > + return -ENODATA; > + if (chan == AT91_SAMA5D2_TOUCH_P_CHAN_IDX) > + *val = at91_adc_touch_pressure(st); > + else > + return -ENODATA; > + > + return IIO_VAL_INT; > +} > + > static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state) > { > struct iio_dev *indio = iio_trigger_get_drvdata(trig); > @@ -373,7 +644,7 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state) > for_each_set_bit(bit, indio->active_scan_mask, indio->num_channels) { > struct iio_chan_spec const *chan = at91_adc_chan_get(indio, bit); > > - if (!chan) > + if (!chan || chan->type != IIO_VOLTAGE) I think this would be more robust if we had it matching what it was rather than what it wasn't. Otherwise we'll get a temperature sensor on this part at somepoint and it'll break. > continue; > if (state) { > at91_adc_writel(st, AT91_SAMA5D2_CHER, > @@ -520,7 +791,17 @@ static int at91_adc_dma_start(struct iio_dev *indio_dev) > static int at91_adc_buffer_postenable(struct iio_dev *indio_dev) > { > int ret; > + struct at91_adc_state *st = iio_priv(indio_dev); > > + /* check if we are enabling triggered buffer or the touchscreen */ > + if (bitmap_subset(indio_dev->active_scan_mask, > + &st->touch_st.channels_bitmask, > + AT91_SAMA5D2_MAX_CHAN_IDX + 1)) { > + /* touchscreen enabling */ > + at91_adc_configure_touch(st, true); Given you have that returning a value (be it always 0 I think) better to pass that on here as well. > + return 0; > + } > + /* we continue with the triggered buffer */ > ret = at91_adc_dma_start(indio_dev); > if (ret) { > dev_err(&indio_dev->dev, "buffer postenable failed\n"); > @@ -536,6 +817,16 @@ static int at91_adc_buffer_predisable(struct iio_dev *indio_dev) > int ret; > u8 bit; > > + /* check if we are disabling triggered buffer or the touchscreen */ > + if (bitmap_subset(indio_dev->active_scan_mask, > + &st->touch_st.channels_bitmask, > + AT91_SAMA5D2_MAX_CHAN_IDX + 1)) { > + /* touchscreen disable */ > + at91_adc_configure_touch(st, false); > + return 0; > + } > + > + /* continue with the triggered buffer */ > ret = iio_triggered_buffer_predisable(indio_dev); > if (ret < 0) > dev_err(&indio_dev->dev, "buffer predisable failed\n"); > @@ -556,7 +847,7 @@ static int at91_adc_buffer_predisable(struct iio_dev *indio_dev) > struct iio_chan_spec const *chan = > at91_adc_chan_get(indio_dev, bit); > > - if (!chan) > + if (!chan && chan->type != IIO_VOLTAGE) > continue; > if (st->dma_st.dma_chan) > at91_adc_readl(st, chan->address); > @@ -622,7 +913,10 @@ static void at91_adc_trigger_handler_nodma(struct iio_dev *indio_dev, > > if (!chan) > continue; > - st->buffer[i] = at91_adc_readl(st, chan->address); > + if (chan->type == IIO_VOLTAGE) > + st->buffer[i] = at91_adc_readl(st, chan->address); > + else > + st->buffer[i] = 0; Please explain this one with a comment. > i++; > } > iio_push_to_buffers_with_timestamp(indio_dev, st->buffer, > @@ -736,6 +1030,7 @@ static void at91_adc_setup_samp_freq(struct at91_adc_state *st, unsigned freq) > > dev_dbg(&indio_dev->dev, "freq: %u, startup: %u, prescal: %u\n", > freq, startup, prescal); > + st->current_sample_rate = freq; > } > > static unsigned at91_adc_get_sample_freq(struct at91_adc_state *st) > @@ -751,23 +1046,109 @@ static unsigned at91_adc_get_sample_freq(struct at91_adc_state *st) If st->current_sample_rate is always valid, then can we just use that for the get call? > return f_adc; > } > > +static void at91_adc_touch_data_handler(struct iio_dev *indio_dev) > +{ > + struct at91_adc_state *st = iio_priv(indio_dev); > + u8 bit; > + u16 val; > + int i = 0; > + > + for_each_set_bit(bit, indio_dev->active_scan_mask, > + AT91_SAMA5D2_MAX_CHAN_IDX + 1) { > + struct iio_chan_spec const *chan = > + at91_adc_chan_get(indio_dev, bit); > + > + if (chan->type == IIO_POSITIONRELATIVE) > + at91_adc_read_position(st, chan->channel, &val); > + else if (chan->type == IIO_PRESSURE) > + at91_adc_read_pressure(st, chan->channel, &val); > + else > + continue; > + st->buffer[i] = val; > + i++; > + } > + /* schedule work to push to buffers */ I would expand the comment to say why you don't just do it directly. > + schedule_work(&st->touch_st.workq); > +} > + > +static void at91_adc_pen_detect_interrupt(struct at91_adc_state *st) > +{ > + at91_adc_writel(st, AT91_SAMA5D2_IDR, AT91_SAMA5D2_IER_PEN); > + at91_adc_writel(st, AT91_SAMA5D2_IER, AT91_SAMA5D2_IER_NOPEN | > + AT91_SAMA5D2_IER_XRDY | AT91_SAMA5D2_IER_YRDY | > + AT91_SAMA5D2_IER_PRDY); > + at91_adc_writel(st, AT91_SAMA5D2_TRGR, > + AT91_SAMA5D2_TRGR_TRGMOD_PERIODIC | > + AT91_SAMA5D2_TRGR_TRGPER(st->touch_st.sample_period_val)); > + st->touch_st.touching = true; > +} > + > +static void at91_adc_no_pen_detect_interrupt(struct at91_adc_state *st) > +{ > + struct iio_dev *indio_dev = iio_priv_to_dev(st); > + > + at91_adc_writel(st, AT91_SAMA5D2_TRGR, > + AT91_SAMA5D2_TRGR_TRGMOD_NO_TRIGGER); > + at91_adc_writel(st, AT91_SAMA5D2_IDR, AT91_SAMA5D2_IER_NOPEN | > + AT91_SAMA5D2_IER_XRDY | AT91_SAMA5D2_IER_YRDY | > + AT91_SAMA5D2_IER_PRDY); > + st->touch_st.touching = false; > + > + at91_adc_touch_data_handler(indio_dev); > + > + at91_adc_writel(st, AT91_SAMA5D2_IER, AT91_SAMA5D2_IER_PEN); > +} > + > +static void at91_adc_workq_handler(struct work_struct *workq) > +{ > + struct at91_adc_touch *touch_st = container_of(workq, > + struct at91_adc_touch, workq); > + struct at91_adc_state *st = container_of(touch_st, > + struct at91_adc_state, touch_st); > + struct iio_dev *indio_dev = iio_priv_to_dev(st); > + > + iio_push_to_buffers(indio_dev, st->buffer); > +} > + > static irqreturn_t at91_adc_interrupt(int irq, void *private) > { > struct iio_dev *indio = private; > struct at91_adc_state *st = iio_priv(indio); > u32 status = at91_adc_readl(st, AT91_SAMA5D2_ISR); > u32 imr = at91_adc_readl(st, AT91_SAMA5D2_IMR); > + u32 rdy_mask = AT91_SAMA5D2_IER_XRDY | AT91_SAMA5D2_IER_YRDY | > + AT91_SAMA5D2_IER_PRDY; > > if (!(status & imr)) > return IRQ_NONE; > - > - if (iio_buffer_enabled(indio) && !st->dma_st.dma_chan) { > + if (status & AT91_SAMA5D2_IER_PEN) { > + /* pen detected IRQ */ > + at91_adc_pen_detect_interrupt(st); > + } else if ((status & AT91_SAMA5D2_IER_NOPEN)) { > + /* nopen detected IRQ */ > + at91_adc_no_pen_detect_interrupt(st); > + } else if ((status & AT91_SAMA5D2_ISR_PENS) && > + ((status & rdy_mask) == rdy_mask)) { > + /* periodic trigger IRQ - during pen sense */ > + at91_adc_touch_data_handler(indio); > + } else if (status & AT91_SAMA5D2_ISR_PENS) { > + /* > + * touching, but the measurements are not ready yet. > + * read and ignore. > + */ > + status = at91_adc_readl(st, AT91_SAMA5D2_XPOSR); > + status = at91_adc_readl(st, AT91_SAMA5D2_YPOSR); > + status = at91_adc_readl(st, AT91_SAMA5D2_PRESSR); > + } else if (iio_buffer_enabled(indio) && !st->dma_st.dma_chan) { > + /* triggered buffer without DMA */ > disable_irq_nosync(irq); > iio_trigger_poll(indio->trig); > } else if (iio_buffer_enabled(indio) && st->dma_st.dma_chan) { > + /* triggered buffer with DMA - should not happen */ > disable_irq_nosync(irq); > WARN(true, "Unexpected irq occurred\n"); > } else if (!iio_buffer_enabled(indio)) { > + /* software requested conversion */ > st->conversion_value = at91_adc_readl(st, st->chan->address); > st->conversion_done = true; > wake_up_interruptible(&st->wq_data_available); > @@ -791,6 +1172,21 @@ static int at91_adc_read_raw(struct iio_dev *indio_dev, > return ret; > mutex_lock(&st->lock); > > + if (chan->type == IIO_POSITIONRELATIVE) { > + ret = at91_adc_read_position(st, > + chan->channel, (u16 *)val); Odd line breaking, I'd expect chan->channel on the first line.. These exit paths are a bit ugly. Perhaps it is time to pull the INFO_RAW block out as a utility function. Then the indenting will be less allowing a simple single exit point with the unlocking all in one place. > + mutex_unlock(&st->lock); > + iio_device_release_direct_mode(indio_dev); > + return ret; > + } > + if (chan->type == IIO_PRESSURE) { > + ret = at91_adc_read_pressure(st, > + chan->channel, (u16 *)val); > + mutex_unlock(&st->lock); > + iio_device_release_direct_mode(indio_dev); > + return ret; > + } > + > st->chan = chan; > > if (chan->differential) > @@ -974,9 +1370,29 @@ static int at91_adc_set_watermark(struct iio_dev *indio_dev, unsigned int val) > return 0; > } > > +static int at91_adc_update_scan_mode(struct iio_dev *indio_dev, > + const unsigned long *scan_mask) > +{ > + struct at91_adc_state *st = iio_priv(indio_dev); > + > + if (bitmap_subset(scan_mask, &st->touch_st.channels_bitmask, > + AT91_SAMA5D2_MAX_CHAN_IDX + 1)) > + return 0; > + /* > + * if the new bitmap is a combination of touchscreen and regular > + * channels, then we are not fine > + */ > + if (bitmap_intersects(&st->touch_st.channels_bitmask, scan_mask, > + AT91_SAMA5D2_MAX_CHAN_IDX + 1)) > + return -EBUSY; > + return 0; > +} > + > static const struct iio_info at91_adc_info = { > .read_raw = &at91_adc_read_raw, > .write_raw = &at91_adc_write_raw, > + .update_scan_mode = &at91_adc_update_scan_mode, > + .of_xlate = &at91_adc_of_xlate, > .hwfifo_set_watermark = &at91_adc_set_watermark, > }; > > @@ -1044,13 +1460,20 @@ static int at91_adc_probe(struct platform_device *pdev) > > indio_dev->dev.parent = &pdev->dev; > indio_dev->name = dev_name(&pdev->dev); > - indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE; > indio_dev->info = &at91_adc_info; > indio_dev->channels = at91_adc_channels; > indio_dev->num_channels = ARRAY_SIZE(at91_adc_channels); > > st = iio_priv(indio_dev); > > + bitmap_set(&st->touch_st.channels_bitmask, > + AT91_SAMA5D2_TOUCH_X_CHAN_IDX, 1); > + bitmap_set(&st->touch_st.channels_bitmask, > + AT91_SAMA5D2_TOUCH_Y_CHAN_IDX, 1); > + bitmap_set(&st->touch_st.channels_bitmask, > + AT91_SAMA5D2_TOUCH_P_CHAN_IDX, 1); > + > ret = of_property_read_u32(pdev->dev.of_node, > "atmel,min-sample-rate-hz", > &st->soc_info.min_sample_rate); > @@ -1100,6 +1523,7 @@ static int at91_adc_probe(struct platform_device *pdev) > > init_waitqueue_head(&st->wq_data_available); > mutex_init(&st->lock); > + INIT_WORK(&st->touch_st.workq, at91_adc_workq_handler); > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > if (!res) > @@ -1272,8 +1696,18 @@ static __maybe_unused int at91_adc_resume(struct device *dev) > at91_adc_hw_init(st); > > /* reconfiguring trigger hardware state */ > - if (iio_buffer_enabled(indio_dev)) > + if (!iio_buffer_enabled(indio_dev)) > + return 0; > + > + /* check if we are enabling triggered buffer or the touchscreen */ > + if (bitmap_subset(indio_dev->active_scan_mask, > + &st->touch_st.channels_bitmask, > + AT91_SAMA5D2_MAX_CHAN_IDX + 1)) { > + /* touchscreen enabling */ > + at91_adc_configure_touch(st, true); > + } else { > at91_adc_configure_trigger(st->trig, true); > + } > > return 0; > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html