Message-ID: <CA+U=Dsrt8bJgLJ_tvsf9p07-7kG4rEWvYBa02movJkWU6sfZ_w@mail.gmail.com>
Date:   Fri, 19 Feb 2021 10:49:19 +0200
From:   Alexandru Ardelean <ardeleanalex@...il.com>
To:     Rob Herring <robh+dt@...nel.org>
Cc:     Jonathan Cameron <jic23@...nel.org>,
        Alexandru Ardelean <alexandru.ardelean@...log.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "open list:IIO SUBSYSTEM AND DRIVERS" <linux-iio@...r.kernel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Michael Hennerich <Michael.Hennerich@...log.com>,
        Nuno Sá <nuno.sa@...log.com>,
        Dragos Bogdan <dragos.bogdan@...log.com>,
        Mircea Caprioru <mircea.caprioru@...log.com>,
        Mihail Chindris <Mihail.Chindris@...log.com>
Subject: Re: [PATCH v2 5/5] iio: dac: ad5686: Add PWM as a trigger source

On Thu, Feb 18, 2021 at 5:31 PM Rob Herring <robh+dt@...nel.org> wrote:
>
> On Thu, Feb 18, 2021 at 8:05 AM Jonathan Cameron <jic23@...nel.org> wrote:
> >
> > On Wed, 17 Feb 2021 10:34:38 +0200
> > Alexandru Ardelean <alexandru.ardelean@...log.com> wrote:
> >
> > > From: Mircea Caprioru <mircea.caprioru@...log.com>
> > >
> > > A PWM signal will be used as a trigger source to have a deterministic
> > > sampling frequency since this family of DAC has no hardware interrupt
> > > source.
> > >
> > > This feature is made optional however, as there are some board setups where
> > > this isn't used.
> > >
> >
> > So this is taking a very generic setup, but then implementing it
> > as a bit of a hack within the driver.
> >
> > It's effectively a PWM connected up to an instance
> > of iio/triggers/iio-trig-interrupt.c
> >
> > Now, I've not looked at that trigger driver for a while, so you may well
> > need to figure out how to add a binding to instantiate it.
> > (looks like no one has used it since board file days, or via instantiation
> > from another driver).
> >
> > It's a slightly odd corner case as what it reflects is that we have
> > an interrupt available that is intended to drive some sort of data
> > capture or output (it's a trigger signal) - but exactly what is done
> > is a runtime configurable.  In this particular case that interrupt
> > is hooked up to a PWM and we also want to represent that.
> >
> > The fact it's being driven via a PWM is interesting but we should be
> > able to extend that trigger driver to optionally accept a pwm provider
> > and if it has one provide frequency control.
> >

So, the main intent here was to provide a user for this new output kfifo.
I don't think I have time to re-spin this into a proper solution.
Someone else may come about and do it.

I'll drop this from the series [for now].

> > Binding might look something like the following..
> >
> > interrupt-trigger {
> >    interrupts = <>;
> >    pwms = <&pwm 0 4000 PWM_POLARITY_INVERTED>;
> > };
> >
> > @Rob, what do you think of this odd beast?
>
> So a PWM routed back to a GPIO interrupt? It needs a compatible, but
> otherwise I wouldn't object to the binding if that's what the h/w
> looks like. But from an OS perspective, I don't think you need it.
>
> > So all in all, this generic facility needs a generic implementation, not
> > one buried in a driver.
> >
> > Another open question here is whether you really can't just use an hrtimer
> > to get similar precision?  Way back at the dawn of time in IIO we had
> > code to use the RTC periodic ticks as a trigger with the theory that they
> > would give very precise and even timing.  In the end it turned out that
> > hrtimers worked just as well (and RTCs drivers emulated the periodic
> > ticks via hrtimers, dropping their use of the hardware periodic timers).
>
> +100
>
> A hrtimer is likely going to be more precise. IIRC, timers are
> serviced first. Either way, you're going to have some amount of
> interrupt service latency, so any precision you think you are gaining
> by 'doing it in h/w' isn't really there.
>
> Rob

Powered by blists - more mailing lists