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@xxxxxxxxx> 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
