| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 23 Dec 2019 12:28:38 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: <Eugen.Hristev@...rochip.com>
Cc: <robh+dt@...nel.org>, <alexandre.belloni@...tlin.com>,
<Nicolas.Ferre@...rochip.com>, <linux-iio@...r.kernel.org>,
<devicetree@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>, <linux-rtc@...r.kernel.org>,
<a.zummo@...ertech.it>, <Ludovic.Desroches@...rochip.com>
Subject: Re: [PATCH 08/10] iio: adc: at91-sama5d2_adc: implement RTC
triggering
On Wed, 18 Dec 2019 16:24:02 +0000
<Eugen.Hristev@...rochip.com> wrote:
> From: Eugen Hristev <eugen.hristev@...rochip.com>
>
> Implement the property atmel,rtc-trigger which provides a phandle
> to a RTC trigger.
> To make it work, one has to check at buffer_postenable if the trigger
> the device is using is the one we provide using the phandle link.
> The trigger mode must be selected accordingly in the trigger mode selection
> register.
> The RTC trigger will use our IRQ. Dedicated hardware line inside the SoC
> will actually trigger the ADC to make the conversion, and EOC irqs are fired
> when conversion is done.
>
> Signed-off-by: Eugen Hristev <eugen.hristev@...rochip.com>
Minor points inline.
Thanks,
Jonathan
> ---
> drivers/iio/adc/at91-sama5d2_adc.c | 109 +++++++++++++++++++++++++++++++++++--
> 1 file changed, 104 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> index ccffa48..ac97f4a 100644
> --- a/drivers/iio/adc/at91-sama5d2_adc.c
> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
> @@ -58,6 +58,8 @@
> #define AT91_SAMA5D2_MR_TRGSEL_TRIG6 6
> /* RTCOUT0 */
> #define AT91_SAMA5D2_MR_TRGSEL_TRIG7 7
> +/* TRGSEL mask */
> +#define AT91_SAMA5D2_MR_TRGSEL_MASK GENMASK(3, 1)
> /* Sleep Mode */
> #define AT91_SAMA5D2_MR_SLEEP BIT(5)
> /* Fast Wake Up */
> @@ -195,6 +197,8 @@
> #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 RTC - must be any of the above 3 values */
> +#define AT91_SAMA5D2_TRGR_TRGMOD_RTC AT91_SAMA5D2_TRGR_TRGMOD_EXT_TRIG_RISE
> /* Trigger Mode internal periodic */
> #define AT91_SAMA5D2_TRGR_TRGMOD_PERIODIC 5
> /* Trigger Mode - trigger period mask */
> @@ -407,6 +411,8 @@ struct at91_adc_state {
> struct mutex lock;
> struct work_struct workq;
> s64 timestamp;
> + struct device *rtc_trig_dev;
> + bool rtc_triggered;
> };
>
> static const struct at91_adc_trigger at91_adc_trigger_list[] = {
> @@ -737,6 +743,42 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
> /* set/unset hw trigger */
> at91_adc_writel(st, AT91_SAMA5D2_TRGR, status);
>
> + status = at91_adc_readl(st, AT91_SAMA5D2_MR);
> +
> + status &= ~AT91_SAMA5D2_MR_TRGSEL_MASK;
> +
> + /* set/unset TRGSEL to ADTRG */
> + if (state)
> + status |= AT91_SAMA5D2_MR_TRGSEL(AT91_SAMA5D2_MR_TRGSEL_TRIG0);
> +
> + at91_adc_writel(st, AT91_SAMA5D2_MR, status);
> +
> + return 0;
> +}
> +
> +static int at91_adc_rtc_configure_trigger(struct at91_adc_state *st, bool state)
> +{
> + u32 status = at91_adc_readl(st, AT91_SAMA5D2_TRGR);
> +
> + /* clear TRGMOD */
> + status &= ~AT91_SAMA5D2_TRGR_TRGMOD_MASK;
> +
> + if (state)
> + status |= AT91_SAMA5D2_TRGR_TRGMOD_RTC;
> +
> + /* set/unset hw trigger */
> + at91_adc_writel(st, AT91_SAMA5D2_TRGR, status);
> +
> + status = at91_adc_readl(st, AT91_SAMA5D2_MR);
> +
> + status &= ~AT91_SAMA5D2_MR_TRGSEL_MASK;
> +
> + /* set/unset TRGSEL to RTCOUT0 */
> + if (state)
> + status |= AT91_SAMA5D2_MR_TRGSEL(AT91_SAMA5D2_MR_TRGSEL_TRIG7);
> +
> + at91_adc_writel(st, AT91_SAMA5D2_MR, status);
> +
> return 0;
> }
>
> @@ -866,7 +908,8 @@ static int at91_adc_dma_start(struct iio_dev *indio_dev)
> if (st->dma_st.dma_chan) \
> use_irq = false; \
> /* if the trigger is not ours, then it has its own IRQ */ \
> - if (iio_trigger_validate_own_device(indio->trig, indio)) \
> + if (iio_trigger_validate_own_device(indio->trig, indio) && \
This increasingly feels like it should be a function with clearly
passed parameters rather than macro fun.
> + !st->rtc_triggered) \
> use_irq = false; \
> }
>
> @@ -884,6 +927,18 @@ static int at91_adc_buffer_postenable(struct iio_dev *indio)
> /* touchscreen enabling */
> return at91_adc_configure_touch(st, true);
> }
> +
> + /*
> + * If our rtc trigger link is identical to the current trigger,
> + * then we are rtc-triggered.
> + * Configure accordingly.
> + */
> + if (!IS_ERR_OR_NULL(st->rtc_trig_dev) &&
> + st->rtc_trig_dev == indio->trig->dev.parent) {
> + at91_adc_rtc_configure_trigger(st, true);
> + st->rtc_triggered = true;
> + }
> +
> /* if we are not in triggered mode, we cannot enable the buffer. */
> if (!(indio->currentmode & INDIO_ALL_TRIGGERED_MODES))
> return -EINVAL;
> @@ -947,6 +1002,17 @@ static int at91_adc_buffer_predisable(struct iio_dev *indio)
> if (!(indio->currentmode & INDIO_ALL_TRIGGERED_MODES))
> return -EINVAL;
>
> + /*
> + * If our rtc trigger link is identical to the current trigger,
> + * then we are rtc-triggered.
> + * Unconfigure accordingly.
> + */
> + if (!IS_ERR_OR_NULL(st->rtc_trig_dev) &&
> + st->rtc_trig_dev == indio->trig->dev.parent) {
> + at91_adc_rtc_configure_trigger(st, false);
> + st->rtc_triggered = false;
> + }
> +
> AT91_ADC_BUFFER_CHECK_USE_IRQ(use_irq);
> /*
> * For each enable channel we must disable it in hardware.
> @@ -1153,8 +1219,15 @@ static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
> else
> ret = at91_adc_trigger_handler_nodma(indio_dev, pf);
>
> - if (!ret)
> + if (!ret) {
> iio_trigger_notify_done(indio_dev->trig);
> + /*
> + * RTC trigger does not know how to reenable our IRQ.
> + * So, we must do it.
> + */
> + if (st->rtc_triggered)
> + enable_irq(st->irq);
Hmm. This is a bit nasty but I guess we can't avoid it.
> + }
>
> return IRQ_HANDLED;
> }
> @@ -1166,10 +1239,13 @@ irqreturn_t at91_adc_pollfunc(int irq, void *p)
> struct at91_adc_state *st = iio_priv(indio_dev);
>
> /*
> - * If it's not our trigger, start a conversion now, as we are
> - * actually polling the trigger now.
> + * We need to start a software trigger if we are not using a trigger
> + * that uses our own IRQ.
> + * External trigger and RTC trigger do not not need software start
External trigger is a bit of a generic name - sounds like one coming from
'somewhere else'. Perhaps "External trigger in the ADC .." or similar?
> + * However the other triggers do.
> */
> - if (iio_trigger_validate_own_device(indio_dev->trig, indio_dev))
> + if (iio_trigger_validate_own_device(indio_dev->trig, indio_dev) &&
> + !st->rtc_triggered)
> at91_adc_writel(st, AT91_SAMA5D2_CR, AT91_SAMA5D2_CR_START);
>
> return iio_pollfunc_store_time(irq, p);
> @@ -1307,6 +1383,12 @@ static void at91_adc_workq_handler(struct work_struct *workq)
>
> at91_adc_read_and_push_channels(indio_dev, st->timestamp);
> iio_trigger_notify_done(indio_dev->trig);
> + /*
> + * RTC trigger does not know how to reenable our IRQ.
> + * So, we must do it.
> + */
> + if (st->rtc_triggered)
> + enable_irq(st->irq);
> } else {
> iio_push_to_buffers(indio_dev, st->buffer);
> }
> @@ -1712,6 +1794,7 @@ static int at91_adc_probe(struct platform_device *pdev)
> struct iio_dev *indio_dev;
> struct at91_adc_state *st;
> struct resource *res;
> + struct device_node *rtc_trig_np;
> int ret, i;
> u32 edge_type = IRQ_TYPE_NONE;
>
> @@ -1737,6 +1820,8 @@ static int at91_adc_probe(struct platform_device *pdev)
>
> st->oversampling_ratio = AT91_OSR_1SAMPLES;
>
> + st->rtc_trig_dev = ERR_PTR(-EINVAL);
> +
> ret = of_property_read_u32(pdev->dev.of_node,
> "atmel,min-sample-rate-hz",
> &st->soc_info.min_sample_rate);
> @@ -1784,6 +1869,20 @@ static int at91_adc_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> + rtc_trig_np = of_parse_phandle(pdev->dev.of_node, "atmel,rtc-trigger",
> + 0);
> + if (rtc_trig_np) {
> + struct platform_device *rtc_trig_plat_dev;
> +
> + rtc_trig_plat_dev = of_find_device_by_node(rtc_trig_np);
> + if (rtc_trig_plat_dev) {
> + st->rtc_trig_dev = &rtc_trig_plat_dev->dev;
> + dev_info(&pdev->dev,
> + "RTC trigger link set-up with %s\n",
> + dev_name(st->rtc_trig_dev));
> + }
> + }
> +
> init_waitqueue_head(&st->wq_data_available);
> mutex_init(&st->lock);
> INIT_WORK(&st->workq, at91_adc_workq_handler);
Powered by blists - more mailing lists