linux-kernel - Re: [RFC PATCH v1 07/20] gpio: Add output event generation method to GPIOLIB and PMC Driver

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 19 Sep 2021 21:38:58 +0200
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
Cc:     "D, Lakshmi Sowjanya" <lakshmi.sowjanya.d@...el.com>,
        "thierry.reding@...il.com" <thierry.reding@...il.com>,
        Lee Jones <lee.jones@...aro.org>,
        "open list:PWM SUBSYSTEM" <linux-pwm@...r.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Mark Gross <mgross@...ux.intel.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        "Saha, Tamal" <tamal.saha@...el.com>, bala.senthil@...el.com,
        Dipen Patel <dipenp@...dia.com>
Subject: Re: [RFC PATCH v1 07/20] gpio: Add output event generation method to
 GPIOLIB and PMC Driver

On Fri, Sep 17, 2021 at 9:27 AM Uwe Kleine-König
<u.kleine-koenig@...gutronix.de> wrote:
> On Thu, Sep 16, 2021 at 11:42:04PM +0200, Linus Walleij wrote:
> > On Tue, Aug 24, 2021 at 6:48 PM <lakshmi.sowjanya.d@...el.com> wrote:
> >
> > > From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@...el.com>
> > >
> > > Intel Timed I/O hardware supports output scheduled in hardware. Enable
> > > this functionality using GPIOlib
> > >
> > > Adds GPIOlib generate_output() hook into the driver. The driver is
> > > supplied with a timestamp in terms of realtime system clock (the same
> > > used for input timestamping). The driver must know how to translate this
> > > into a timebase meaningful for the hardware.
> > >
> > > Adds userspace write() interface. Output can be selected using the line
> > > event create ioctl. The write() interface takes a single timestamp
> > > event request parameter. An output edge rising or falling is generated
> > > for each event request.
> > >
> > > The user application supplies a trigger time in terms of the realtime
> > > clock the driver converts this into the corresponding ART clock value
> > > that is used to 'arm' the output.
> > >
> > > Work around device quirk that doesn't allow the output to be explicitly
> > > set. Instead, count the output edges and insert an additional edge as
> > > needed to reset the output to zero.
> > >
> > > Co-developed-by: Christopher Hall <christopher.s.hall@...el.com>
> > > Signed-off-by: Christopher Hall <christopher.s.hall@...el.com>
> > > Signed-off-by: Tamal Saha <tamal.saha@...el.com>
> > > Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@...el.com>
> > > Reviewed-by: Mark Gross <mgross@...ux.intel.com>
> >
> > So this is some street organ machine that generates sequences
> > with determined timing between positive and negative edges
> > right?
> >
> > I can't see how this hardware is different from a PWM, or well
> > I do to some extent, you can control the period of several
> > subsequent waves, but that is really just an elaborate version
> > of PWM in my book.
>
> From looking in the patch I think this is more versatile than the PWM
> framework abstracts. I wonder if there is a usecase for the
> functionality that cannot be expressed using pwm_apply_state?!
>
> I remember we had approaches before that implemented repeating patterns
> (something like: active for 5ms, inactive for 10 ms, active for 30 ms,
> inactive for 10 ms, repeat) and limiting the number of periods
> (something like: .duty_cycle = 5ms, .period = 20ms, after 5 periods go
> into inactive state). These were considered to be too special to be
> abstracted in drivers/pwm.
>
> > It seems to me that this part of the functionality belongs in the
> > PWM subsystem which already has interfaces for similar
> > things, and you should probably extend PWM to handle
> > random waveforms rather than trying to shoehorn this
> > into the GPIO subsystem.
>
> I agree that GPIO is a worse candidate than PWM to abstract that. But
> I'm not convinced (yet?) that it's a good idea to extend PWM
> accordingly.

Yeah it is a bit unfortunate.

I think we need to fully understand the intended usecase before
we can deal with this: exactly what was this hardware constructed
to handle? Sound? Robotic stepper motors? It must be something
and apparently there are users.

Maybe even a new subsystem is needed, like a
drivers/gpio-patterns or drivers/stepper-motor or whatever this
is supposed to drive.

Yours,
Linus Walleij