All of lore.kernel.org
 help / color / mirror / Atom feed
From: Clemens Gruber <clemens.gruber@pqgruber.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"D, Lakshmi Sowjanya" <lakshmi.sowjanya.d@intel.com>,
	"thierry.reding@gmail.com" <thierry.reding@gmail.com>,
	"Lee Jones" <lee.jones@linaro.org>,
	"open list:PWM SUBSYSTEM" <linux-pwm@vger.kernel.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	"Bartosz Golaszewski" <bgolaszewski@baylibre.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	"Mark Gross" <mgross@linux.intel.com>,
	"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
	"Saha, Tamal" <tamal.saha@intel.com>,
	bala.senthil@intel.com, "Dipen Patel" <dipenp@nvidia.com>
Subject: Re: [RFC PATCH v1 07/20] gpio: Add output event generation method to GPIOLIB and PMC Driver
Date: Sun, 19 Sep 2021 23:21:22 +0200	[thread overview]
Message-ID: <YUep0gtZkc6D3ukt@workstation.tuxnet> (raw)
In-Reply-To: <CACRpkdZmjWQ_mNw_JOZnkvvU15qS26gB3GL_9k=Vao3m=w_N9w@mail.gmail.com>

On Sun, Sep 19, 2021 at 09:38:58PM +0200, Linus Walleij wrote:
> On Fri, Sep 17, 2021 at 9:27 AM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.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@intel.com> wrote:
> > >
> > > > From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.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@intel.com>
> > > > Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
> > > > Signed-off-by: Tamal Saha <tamal.saha@intel.com>
> > > > Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> > > > Reviewed-by: Mark Gross <mgross@linux.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.

This would be interesting. Maybe even more abstract, not just supporting
GPIO patterns but also PWM patterns.

E.g. Set gpiochip1 line 2 to 1, wait 5ms, set it to 0
Or set pwmchip1 pwm 2 to 100%, wait 250ms, set it back to 50% duty cycle

This subsystem could then implement the patterns with hrtimers and be
usable with every GPIO or PWM device supported in Linux, and for
special hardware like the Intel Timed I/O, it could configure it to
output the pattern itself.

One usecase besides stepper motors and Robotics would be solenoid
valves: You often have different sequences for opening, closing and
maintenance. E.g. for liquid valves, especially if the liquid is
viscuous, you have to first use 100% duty cycle PWM for e.g. 250ms to
get it open and then dial back to 50% to keep it open without
overheating it.

Of course this can be done in userspace.. but it may also be useful to
have some kind of pattern generator in the kernel. What do you think?

Clemens

  reply	other threads:[~2021-09-19 21:30 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 01/20] gpio: Add basic GPIO driver for Intel PMC Timed I/O device lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 02/20] gpio: Add GPIO polling interface to GPIO lib lakshmi.sowjanya.d
2021-09-22 10:03   ` Bartosz Golaszewski
2021-10-07  2:14     ` Kent Gibson
2021-08-24 16:47 ` [RFC PATCH v1 03/20] arch: x86: Add ART support function to tsc code lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 04/20] gpio: Add input code to Intel PMC Timed I/O Driver lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 05/20] tools: gpio: Add additional polling support to gpio-event-mon lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 06/20] gpio: Add set_input and polling interface to GPIO lib code lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 07/20] gpio: Add output event generation method to GPIOLIB and PMC Driver lakshmi.sowjanya.d
2021-09-16 21:42   ` Linus Walleij
2021-09-17  7:27     ` Uwe Kleine-König
2021-09-19 19:38       ` Linus Walleij
2021-09-19 21:21         ` Clemens Gruber [this message]
2021-09-20  7:14           ` Uwe Kleine-König
2021-08-24 16:47 ` [RFC PATCH v1 08/20] kernel: time: Add system time to system counter translation lakshmi.sowjanya.d
2021-09-16 21:48   ` Linus Walleij
2021-08-24 16:47 ` [RFC PATCH v1 09/20] arch: x86: Add TSC to ART translation lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 10/20] tools: gpio: Add GPIO output generation user application lakshmi.sowjanya.d
2021-09-16 21:52   ` Linus Walleij
2021-08-24 16:47 ` [RFC PATCH v1 11/20] gpio: Add event count interface to gpiolib lakshmi.sowjanya.d
2021-09-22  9:53   ` Bartosz Golaszewski
2021-08-24 16:47 ` [RFC PATCH v1 12/20] gpio: Add event count to Intel(R) PMC Timed I/O driver lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 13/20] tools: gpio: Add event count capability to event monitor application lakshmi.sowjanya.d
2021-09-16 21:57   ` Linus Walleij
2021-08-24 16:47 ` [RFC PATCH v1 14/20] arch/x86: Add ART nanosecond to ART conversion lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 15/20] pwm: Add capability for PWM Driver managed state lakshmi.sowjanya.d
2021-09-16 22:00   ` Linus Walleij
2021-08-24 16:47 ` [RFC PATCH v1 16/20] gpio: Add PWM capabilities to Intel(R) PMC TIO driver lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 17/20] pwm: Add second alignment to the core PWM interface lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 18/20] gpio: Add PWM alignment support to the Intel(R) PMC Timed I/O driver lakshmi.sowjanya.d
2021-08-24 16:48 ` [RFC PATCH v1 19/20] gpio: Add GPIO monitor line to Intel(R) Timed I/O Driver lakshmi.sowjanya.d
2021-08-24 16:48 ` [RFC PATCH v1 20/20] tools: gpio: Add PWM monitor application lakshmi.sowjanya.d
2021-09-16 21:21 ` [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC Linus Walleij
2021-10-11 21:14   ` Dipen Patel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YUep0gtZkc6D3ukt@workstation.tuxnet \
    --to=clemens.gruber@pqgruber.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=bala.senthil@intel.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=dipenp@nvidia.com \
    --cc=lakshmi.sowjanya.d@intel.com \
    --cc=lee.jones@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=mgross@linux.intel.com \
    --cc=tamal.saha@intel.com \
    --cc=thierry.reding@gmail.com \
    --cc=u.kleine-koenig@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.