public inbox for linux-iio@vger.kernel.org
 help / color / mirror / Atom feed
From: Kent Gibson <warthog618@gmail.com>
To: "Hall, Christopher S" <christopher.s.hall@intel.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	"william.gray@linaro.org" <william.gray@linaro.org>,
	Bartosz Golaszewski <brgl@bgdev.pl>,
	"Shevchenko, Andriy" <andriy.shevchenko@intel.com>,
	Dipen Patel <dipenp@nvidia.com>,
	"N, Pandith" <pandith.n@intel.com>,
	"D, Lakshmi Sowjanya" <lakshmi.sowjanya.d@intel.com>,
	Lars-Peter Clausen <lars@metafoo.de>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
	"vilhelm.gray@gmail.com" <vilhelm.gray@gmail.com>,
	"jic23@kernel.org" <jic23@kernel.org>,
	"Sangannavar,
	Mallikarjunappa"  <mallikarjunappa.sangannavar@intel.com>,
	"T R, Thejesh Reddy" <thejesh.reddy.t.r@intel.com>
Subject: Re: Intel Timed-IO driver in IIO/Counter subsystem
Date: Thu, 7 Jul 2022 11:21:33 +0800	[thread overview]
Message-ID: <20220707032133.GC7250@sol> (raw)
In-Reply-To: <CACRpkdYxverx-KsG9URrh5qkEFXDknZKCE6RM555mjOuC--vMg@mail.gmail.com>

On Thu, Jul 07, 2022 at 01:05:16AM +0200, Linus Walleij wrote:
> On Wed, Jul 6, 2022 at 7:52 AM Hall, Christopher S
> <christopher.s.hall@intel.com> wrote:
> 
> > > My first thought was that you could extend the SET_VALUES ioctl but,
> > > while we have reserved space for future use in most of the ioctls it
> > > turns out we overlooked sets and gets, so you would be looking at a new
> > > ioctl.  And you need to keep in mind how the SET_VALUES ioctl would
> > > interact with it (Linus' point).
> >
> > I think we worked around this in a previous patchset by disallowing
> > (return error) the 'set' method. The pin/pad is assigned (by mux
> > configured in BIOS) to either GPIO or Timed I/O logic and both cannot be
> > used simultaneously.
> 
> And we know it will always be like that? What about other systems
> that are not your specific x86 box and that go and implement this
> API? I don't think this should be handled in the driver but in the
> gpiolib.
> 

Strongly agree with Linus on this - the ABI needs to be general, not
in any way specific to your hardware - but something that can be
implemented using your hardware.  Ideally you have other example
hardware, or two.

GPIO or Timed may not be used simultaneously, but might it be selectable
at runtime?  If so the user would specify an output mode as part of the
line request, which could be an addition to the existing drive modes.

It still isn't clear to me what happens if I request a set for a past
time.  Does it set immediately, do nothing, or return an error?

Same applies for while one is pending, particularly how it is determined
that it is pending.  I have a bad feeling about race conditions here.

Also be aware that the GPIO uAPI line requests operate on a set of lines,
so your ioctl needs to identify which of the lines in the request it is
operating on. e.g. the mask in gpio_v2_line_values.

And you should use fixed sized fields in ioctl structs [1].
For timestamps the GPIO uAPI uses u64.  How that is interpreted depends
on the selected clock.

Also agree with Linus on the periodic trigger - that seems to have
jumped outside of GPIO scope.

Cheers,
Kent.

[1] https://www.kernel.org/doc/html/latest/driver-api/ioctl.html

  reply	other threads:[~2022-07-07  3:21 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-17  6:37 Intel Timed-IO driver in IIO/Counter subsystem N, Pandith
2022-06-17  7:22 ` Lars-Peter Clausen
2022-06-17  9:51   ` Shevchenko, Andriy
2022-06-17 11:39     ` Linus Walleij
2022-06-18  2:01       ` Hall, Christopher S
2022-06-23 12:21         ` Linus Walleij
2022-07-05  3:16           ` Kent Gibson
2022-07-06  5:52             ` Hall, Christopher S
2022-07-06 23:05               ` Linus Walleij
2022-07-07  3:21                 ` Kent Gibson [this message]
2022-06-17 14:03 ` William Breathitt Gray
2022-06-17 17:15   ` Jonathan Cameron
2022-06-18  1:01 ` Hall, Christopher S
2022-06-21 13:58   ` William Breathitt Gray

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=20220707032133.GC7250@sol \
    --to=warthog618@gmail.com \
    --cc=andriy.shevchenko@intel.com \
    --cc=brgl@bgdev.pl \
    --cc=christopher.s.hall@intel.com \
    --cc=dipenp@nvidia.com \
    --cc=jic23@kernel.org \
    --cc=lakshmi.sowjanya.d@intel.com \
    --cc=lars@metafoo.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=mallikarjunappa.sangannavar@intel.com \
    --cc=pandith.n@intel.com \
    --cc=thejesh.reddy.t.r@intel.com \
    --cc=vilhelm.gray@gmail.com \
    --cc=william.gray@linaro.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox