On Mon, Mar 29, 2021 at 4:46 PM Greg Kroah-Hartman
[off-list ref] wrote:

On Mon, Mar 29, 2021 at 04:01:18PM +0300, Andy Shevchenko wrote:

quoted

On Sun, Mar 28, 2021 at 03:02:24PM +0200, Greg Kroah-Hartman wrote:

quoted

On Sun, Mar 28, 2021 at 03:56:26PM +0300, Andy Shevchenko wrote:

quoted

On Sun, Mar 28, 2021 at 9:47 AM Greg Kroah-Hartman
[off-list ref] wrote:

quoted

On Sun, Mar 28, 2021 at 12:20:07AM +0200, Andy Shevchenko wrote:

quoted

It's very convenient to see what properties and their values
are currently being assigned in the registered software nodes.

...

quoted

quoted

quoted

quoted

quoted

+             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?

quoted

quoted

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.

quoted

The idea behind that this is following what ACPI and DT provides to the users
via /sys/firmware/ (however, in binary format). I can re-do to provide a
binary, and it will effectively make software nodes in align with the rest.

binary files in sysfs are only to be used as a "pass through" from
hardware to userspace.  That does not seem relevant here.

This makes sense.

sorry, please keep this out of sysfs for now.

OK. For now I will resubmit the rest with addressed comments.
Thanks!

-- 
With Best Regards,
Andy Shevchenko