Re: [PATCH v2 1/1] ACPI: properties: expose device properties through sysfs

["Rafael J. Wysocki" <rjw@xxxxxxxxxxxxx>]

Sorry for the delayed response.

On Monday, July 2, 2018 7:44:02 PM CEST Curt Brune wrote:
> This change exposes ACPI device properties via sysfs under
> <device>/firmware_node/properties/*.  This is a logical extension of
> the changes that expose _ADR, _STR, _HID, etc... in the same
> directory.
> 
> Integer properties are represented by a file containing a single hex
> number (printf format 0x%llx).
> 
> String properties are represented by a file containing a single
> string.
> 
> Arrays of integers properties are represented by a file containing a
> space delimited string of hex numbers (printf format 0x%llx).
> 
> Example:
>   0x1 0x2 0x3
> 
> Arrays of strings properties are represented by a file containing a
> newline delimited string of escaped strings (printf format %*pE).
> 
> Example:
>   foo
>   bar
>   has\nnewline
> 
> Device reference properties are represented by a symlink to the
> referenced device plus a <property>_args node containing an integer
> array of reference arguments.  In the case of multiple local
> references, the references at index 1..N are represented the same way
> using the naming convention <property><N>, <property><N>_args.
> 
> Examples:
>   (a property containing a single reference)
> 
>   phy-handle -> ../../../device:00
>   phy-handle_args: 0x0
> 
>   (a property containing three references)
> 
>   ref -> ../device:01
>   ref_args: 0x1 0x2
>   ref1 -> ../device:02
>   ref1_args: 0x3 0x4
>   ref2 -> ../device:03
>   ref2_args: 0x5 0x6
> 
> The bulk of the credit for this idea and implementation goes to Dustin
> Byford <dustin@xxxxxxxxxxxxxxxxxxx>.  I only massaged it a little to
> work with a more recent kernel version.
> 
> Signed-off-by: Curt Brune <curt@xxxxxxxxxxxxxxxxxxx>
> ---
>  Documentation/acpi/dsd/sysfs-properties.txt |  45 ++++
>  drivers/acpi/device_sysfs.c                 | 379 ++++++++++++++++++++++++++++
>  drivers/acpi/internal.h                     |   1 +
>  drivers/acpi/scan.c                         |   2 +
>  include/acpi/acpi_bus.h                     |   1 +
>  5 files changed, 428 insertions(+)
>  create mode 100644 Documentation/acpi/dsd/sysfs-properties.txt
> 
> diff --git a/Documentation/acpi/dsd/sysfs-properties.txt b/Documentation/acpi/dsd/sysfs-properties.txt

Could you instead or in addition to this document the new sysfs attributes
under Documentation/ABI/testing/ ?

> new file mode 100644
> index 000000000000..c70fd5b52d47
> --- /dev/null
> +++ b/Documentation/acpi/dsd/sysfs-properties.txt
> @@ -0,0 +1,45 @@
> +ACPI device properties are exported via sysfs under
> +<device>/firmware_node/properties/*.  This is a logical extension of
> +the changes that expose _ADR, _STR, _HID, etc... in the same
> +directory.
> +
> +Integer properties are represented by a file containing a single hex
> +number (printf format 0x%llx).
> +
> +String properties are represented by a file containing a single
> +string.
> +
> +Arrays of integers properties are represented by a file containing a
> +space delimited string of hex numbers (printf format 0x%llx).
> +
> +Example:
> +  0x1 0x2 0x3
> +
> +Arrays of strings properties are represented by a file containing a
> +newline delimited string of escaped strings (printf format %*pE).
> +
> +Example:
> +  foo
> +  bar
> +  has\nnewline
> +
> +Device reference properties are represented by a symlink to the
> +referenced device plus a <property>_args node containing an integer
> +array of reference arguments.  In the case of multiple local
> +references, the references at index 1..N are represented the same way
> +using the naming convention <property><N>, <property><N>_args.
> +
> +Examples:
> +  (a property containing a single reference)
> +
> +  phy-handle -> ../../../device:00
> +  phy-handle_args: 0x0
> +
> +  (a property containing three references)
> +
> +  ref -> ../device:01
> +  ref_args: 0x1 0x2
> +  ref1 -> ../device:02
> +  ref1_args: 0x3 0x4
> +  ref2 -> ../device:03
> +  ref2_args: 0x5 0x6
> diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
> index 24418932612e..5e98bc5015ab 100644
> --- a/drivers/acpi/device_sysfs.c
> +++ b/drivers/acpi/device_sysfs.c

There are no comments in the code you're adding to this file and some pieces of
it are not entirely obvious.  Care to document what's going on?

At least the data structures used should be described.

Thanks,
Rafael