[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <YGIb4z8oHIV7LlWi@kroah.com>
Date: Mon, 29 Mar 2021 20:26:43 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Andy Shevchenko <andy.shevchenko@...il.com>
Cc: Daniel Scally <djrscally@...il.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Linux Media Mailing List <linux-media@...r.kernel.org>,
ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Yong Zhi <yong.zhi@...el.com>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Bingbu Cao <bingbu.cao@...el.com>,
Tianshu Qiu <tian.shu.qiu@...el.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Subject: Re: [PATCH v1 3/8] software node: Show properties and their values
in sysfs
On Mon, Mar 29, 2021 at 05:51:01PM +0300, Andy Shevchenko wrote:
> On Mon, Mar 29, 2021 at 4:46 PM Greg Kroah-Hartman
> <gregkh@...uxfoundation.org> wrote:
> > On Mon, Mar 29, 2021 at 04:01:18PM +0300, Andy Shevchenko wrote:
> > > On Sun, Mar 28, 2021 at 03:02:24PM +0200, Greg Kroah-Hartman wrote:
> > > > On Sun, Mar 28, 2021 at 03:56:26PM +0300, Andy Shevchenko wrote:
> > > > > On Sun, Mar 28, 2021 at 9:47 AM Greg Kroah-Hartman
> > > > > <gregkh@...uxfoundation.org> wrote:
> > > > > > On Sun, Mar 28, 2021 at 12:20:07AM +0200, Andy Shevchenko wrote:
> > > > > > > It's very convenient to see what properties and their values
> > > > > > > are currently being assigned in the registered software nodes.
>
> ...
>
> > > > > > > + for (i = 0; i < prop->length / sizeof(u8); i++)
> > > > > > > + len += sysfs_emit_at(buf, len, "%u,", ((u8 *)pointer)[i]);
> > > > > >
> > > > > > No, sysfs is "one value per file", and that is not what you are showing
> > > > > > here at all :(
> > > > >
> > > > > It is following: it's a "one value" for property in question,
> > > > >
> > > > > As we may read in [1]: "...so it is socially acceptable to express an
> > > > > array of values of the same type."
> > > > >
> > > > > And here is exactly the case: *values of the same type*.
> > > >
> > > > So what is it going to look like exactly?
> > >
> > > Basically we have two approaches (already done in the kernel!) use space or
> > > comma for a separator. So:
> > > - for boolean it will be an empty string (and it's one value always)
> > > - for integers it will be, for example, '0,1,2' (w/o single quotes)
> > > for property array with values 0, 1, and 2
> > > - for plain integers or arrays out of 1 element it will be plain integer
> > > - for strings it will be, for example, '"str1","str2"' (w/o single quotes)
> > > for array of string { "str1", "str2" }
> > > - for single string or array out of 1 element, it will be '"str"' (w/o single
> > > quotes)
> > >
> > > This should be a part of documentation.
> >
> > And I will complain then too, these "lists of values" are not for sysfs,
> > sorry.
>
> How can you recommend showing an array(s) of values of the same type?
I do not think that is something that should be shown in sysfs if at all
possible.
> > > > And what tool is going to be
> > > > there to parse this mess? Who is going to to use it?
> > >
> > > I guess something like hwinfo (needs a patch).
> >
> > If nothing needs this, then why are you adding these?
>
> OK, I will prepare tools first, but I have no idea for the format. So,
> how can I be sure that tools will accept the patch if there is no
> kernel interface available? Seems like a stale case.
Develop both together. Do not think that you will get the kernel format
right without actually knowing what userspace needs/wants. That is
crazy.
thanks,
greg k-h
Powered by blists - more mailing lists