From: Kent Gibson <warthog618@gmail.com>
To: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>,
LKML <linux-kernel@vger.kernel.org>,
linux-gpio <linux-gpio@vger.kernel.org>,
Linus Walleij <linus.walleij@linaro.org>
Subject: Re: [RFC PATCH] gpio: uapi: v2 proposal
Date: Tue, 9 Jun 2020 17:43:38 +0800 [thread overview]
Message-ID: <20200609094338.GA16448@sol> (raw)
In-Reply-To: <CAMRc=Mdz__0TD8Qa33KRK9PE6jLvxa_O_dDjA54MHBLOeMxWfg@mail.gmail.com>
On Tue, Jun 09, 2020 at 10:03:42AM +0200, Bartosz Golaszewski wrote:
> sob., 6 cze 2020 o 03:56 Kent Gibson <warthog618@gmail.com> napisał(a):
> >
>
> [snip!]
>
> > >
> > > I'd say yes - consolidation and reuse of data structures is always
> > > good and normally they are going to be wrapped in some kind of
> > > low-level user-space library anyway.
> > >
> >
> > Ok, and I've changed the values field name to bitmap, along with the change
> > to a bitmap type, so the stuttering is gone.
> >
> > And, as the change to bitmap substantially reduced the size of
> > gpioline_config, I now embed that in the gpioline_info instead of
> > duplicating all the other fields. The values field will be zeroed
> > when returned within info.
> >
>
> Could you post an example, I'm not sure I follow.
>
The gpioline_info_v2 now looks like this:
/**
* struct gpioline_info_v2 - Information about a certain GPIO line
* @name: the name of this GPIO line, such as the output pin of the line on
* the chip, a rail or a pin header name on a board, as specified by the
* gpio chip, may be empty
* @consumer: a functional name for the consumer of this GPIO line as set
* by whatever is using it, will be empty if there is no current user but
* may also be empty if the consumer doesn't set this up
* @config: the configuration of the line. Note that the values field is
* always zeroed.
* @offset: the local offset on this GPIO device, fill this in when
* requesting the line information from the kernel
* @padding: reserved for future use
*/
struct gpioline_info_v2 {
char name[GPIO_MAX_NAME_SIZE];
char consumer[GPIO_MAX_NAME_SIZE];
struct gpioline_config config;
__u32 offset;
__u32 padding[GPIOLINE_INFO_V2_PAD_SIZE]; /* for future use */
};
Previously that had all the fields from config - other than the values.
When that is populated the config.values will always be zeroed.
[snip!]
>
> > > >
> > > > I'm also kicking around the idea of adding sequence numbers to events,
> > > > one per line and one per handle, so userspace can more easily detect
> > > > mis-ordering or buffer overflows. Does that make any sense?
> > > >
> > >
> > > Hmm, now that you mention it - and in the light of the recent post by
> > > Ryan Lovelett about polling precision - I think it makes sense to have
> > > this. Especially since it's very easy to add.
> > >
> >
> > OK. I was only thinking about the edge events, but you might want it
> > for your line info events on the chip fd as well?
> >
>
> I don't see the need for it now, but you never know. Let's leave it
> out for now and if we ever need it - we now have the appropriate
> padding.
>
OK. It is a trivial change - I've already got the patch for it.
> > > > And would it be useful for userspace to be able to influence the size of
> > > > the event buffer (currently fixed at 16 events per line)?
> > > >
> > >
> > > Good question. I would prefer to not overdo it though. The event
> > > request would need to contain the desired kfifo size and we'd only
> > > allow to set it on request, right?
> > >
> >
> > Yeah, it would only be relevant if edge detection was set and, as per
> > edge detection itself, would only be settable via the request, not
> > via set_config. It would only be a suggestion, as the kfifo size gets
> > rounded up to a power of 2 anyway. It would be capped - I'm open to
> > suggestions for a suitable max value. And the 0 value would mean use
> > the default - currently 16 per line.
> >
>
> This sounds good. How about 512 for max value for now and we can
> always increase it if needed. I don't think we should explicitly cap
> it though - let the user specify any value and just silently limit it
> to 512 in the kernel.
>
It will be an internal cap only - no error if the user requests more.
I was thinking 1024, which corresponds to the maximum default - 16*64.
> > If you want the equivalent for the info watch then I'm not sure where to
> > hook it in. It should be at the chip scope, and there isn't any
> > suitable ioctl to hook it into so it would need a new one - maybe a
> > set_config for the chip? But the buffer size would only be settable up
> > until you add a watch.
> >
>
> I don't think we need this. Status changes are naturally much less
> frequent and the potential for buffer overflow is miniscule here.
>
Agreed.
Cheers,
Kent.
next prev parent reply other threads:[~2020-06-09 9:43 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-16 6:45 [RFC PATCH] gpio: uapi: v2 proposal Kent Gibson
2020-05-25 8:39 ` Linus Walleij
2020-05-25 14:19 ` Kent Gibson
2020-05-27 5:58 ` Linus Walleij
2020-06-04 12:06 ` Bartosz Golaszewski
2020-06-04 14:23 ` Kent Gibson
2020-05-25 16:24 ` Bartosz Golaszewski
2020-05-27 0:35 ` Kent Gibson
2020-05-26 8:04 ` Andy Shevchenko
2020-05-26 12:42 ` Kent Gibson
2020-06-04 12:43 ` Bartosz Golaszewski
2020-06-04 16:00 ` Kent Gibson
2020-06-05 9:53 ` Bartosz Golaszewski
2020-06-06 1:56 ` Kent Gibson
2020-06-09 8:03 ` Bartosz Golaszewski
2020-06-09 9:43 ` Kent Gibson [this message]
2020-06-09 9:57 ` Bartosz Golaszewski
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=20200609094338.GA16448@sol \
--to=warthog618@gmail.com \
--cc=bgolaszewski@baylibre.com \
--cc=brgl@bgdev.pl \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.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;
as well as URLs for NNTP newsgroup(s).