On 09/27/2017 11:55 AM, Darren Hart wrote:
On Wed, Sep 27, 2017 at 09:31:47PM +0300, Andy Shevchenko wrote:
On Wed, Sep 27, 2017 at 9:27 PM, <Mario.Limonciello@xxxxxxxx> wrote:
Darren, Andy, any comments? I'm not quite sure if such API is suitable
for long term in kernel.
I would try to avoid sysfs interfaces for some particular devices.
Besides we are creating a character device. Would it be suitable there?
If the character device having 2 different ioctls for different needs is
acceptable I'm happy to adjust the series to do this instead.
One piece of feedback I had re the char device was to see if we could avoid the
need for the IOCTL altogether, I'd like to have that discussion before we add
another.
My original design was sysfs files for everything but it was raised by several folks
that you run into the potential of two userspace processes stomping on each
other's data when they run the ACPI call. That's why I need to have a mutex to
protect and make sure that userspace calls get the right results.
Basically tokens are list of tuples <id, location, value> with
possibility to active them, right?
I didn't add a way to activate them through this, it was only for
reading purpose. Activating them should be possible through the
SMBIOS calling interface though.
These are read-only as I understood it, and only with the right privileges.
Sysfs seemed appropriate for this to me.
Andy S was against having this data as another sysfs file. From a userspace
perspective I think it's simpler to just parse a sysfs file with read only static
data as root. With the current ioctl based solution it requires userspace to run
an ioctl to determine how many tokens exist, then allocate a chunk of memory
big enough to hold all the token data and then run another ioctl to get all the tokens.
Andy S, given this change between v1 and v2 what do you feel is better?
I have no strong opinion on this. That's why I recommended to listen to Andy L.
+Andy Lutomirski
Andy L, any preference on your part regarding exporting these tokens via sysfs
or through an additional IOCTL in the chardev?
Not really. If this is indeed static data that is potentially useful
for scripts and such, than sysfs is kind of nice.
