[v3,13/29] dts: Add a binding for hid-over-i2c

Add this binding from Linux v5.4.

Signed-off-by: Simon Glass <sjg at chromium.org>
---

Changes in v3:
- Split out hid-over-i2c into its own patch

Changes in v2: None

 .../input/hid-over-i2c.txt                    | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 doc/device-tree-bindings/input/hid-over-i2c.txt

Hi Simon,

-----"Simon Glass" <sjg at chromium.org> schrieb: -----

>An: u-boot at lists.denx.de
>Von: "Simon Glass" <sjg at chromium.org>
>Datum: 31.03.2020 01:14
>Kopie: "Andy Shevchenko" <andriy.shevchenko at linux.intel.com>,
>"Wolfgang Wallner" <wolfgang.wallner at br-automation.com>, "Leif
>Lindholm" <leif at nuviainc.com>, "Simon Glass" <sjg at chromium.org>
>Betreff: [PATCH v3 14/29] acpi: Add a binding for ACPI settings in
>the device tree
>
> Devices need to report various identifiers in the ACPI tables. Rather than
> hard-coding these in drivers it is typically better to put them in the
> device tree.
>
> Add a binding file to describe this.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> Changes in v3:
> - Add a pointer to information about acpi,compatible
> - Change the example to ELAN
> - Correct description of acpi,probed
> - Drop hid-descr-addr
> - Drop mention of PRIC

I understand now where the term "PRIC" came from.
The property "acpi,has-power-resource" triggers the function
acpi_device_add_power_res(), which writes a PowerResource ('PowerResource'
as specified in section 7.2 of ACPI 6.3). This function comes from Coreboot,
and that implementation uses "PRIC" as the hardcoded device name. That's it,
it is an arbitrary chosen device name (probably meaning "power IC" ... ?).

Anyway, dropping the term "PRIC" makes the description easier to understand.
The important information is that "acpi,has-power-resource" leads to the
addition of a PowerResource entry.

> - Just add the device.txt binding file in this patch
> - Rename acpi,desc to acpi,ddn
>
> Changes in v2:
> - Add the hid-over-i2c binding document
> - Fix definition of HID
> - Infer hid-over-i2c CID value
>
>  doc/device-tree-bindings/device.txt | 37 +++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 doc/device-tree-bindings/device.txt
>
> diff --git a/doc/device-tree-bindings/device.txt b/doc/device-tree-bindings/device.txt
> new file mode 100644
> index 00000000000..06c2b84b6d5
> --- /dev/null
> +++ b/doc/device-tree-bindings/device.txt
> @@ -0,0 +1,37 @@
> +Devices
> +=======
> +
> +Device bindings are described by their own individual binding files.
> +
> +U-Boot provides for some optional properties which are documented here. See
> +also hid-over-i2c.txt which describes HID devices. See also
> +Documentation/firmware-guide/acpi/enumeration.rst in the Linux kernel for
> +the acpi,compatible property.
> +
> + - acpi,has-power-resource : (boolean) true if this device has a power resource.
> +    This causes an ACPI PowerResource to be written containing the properties
> +    provided by this binding, to describe how to handle powering the device up
> +    and down using GPIOs
> + - acpi,compatible : compatible string to report

I still don't see a use case for a new "acpi,compatible" property.
I will take a step back, and explain my understanding of the involved pieces
and why I think adding "acpi,compatible" is of no benefit.

Summary:
As far as I understand the proposed "acpi, compatible" property, the following
would happen:
We have "acpi,compatible" in Devicetree, which results in a "compatible"-entry
in ACPI's _DSD method, which is then again interpreted as the
"compatible"-property of Devicetree. IMHO it would be strange for "compatible"
and "acpi,compatible" to be different, so we could drop "acpi,compatible" and
use the existing "compatible" instead.

The _DSD-method for "PRP0001"-devices in ACPI allows to use Devicetree
properties inside ACPI, especially it allows to re-use Devicetree's
"compatible"-property. But this is for a different use case (using Devicetree
properties inside ACPI, not add ACPI properties in Devicetree).

Detailed explanation:

1) ACPI Constraint: Devices need to have either _HID or _ADR

   ACPI puts constraints on what properties a device ("device" here means a
   "Device()" entry in ASL code (DSDT or SSDT)) has to have. It has to have
   either an _HID or an _ADR property depending on whether it is on an
   enumerable bus:

    * if it is on an enumerable bus, it has to have an _ADR (address)
      property (e.g. a PCI device on a PCI bus)
    * if it is not on an enumerable bus, it has to have a _HID (hardware ID)
      property (e.g. the GPIO controllers on the Apollo Lake SoC). The legal
      values for _HID are specified and allow the type of the device to be
      identified (a similar concept as the "compatible" property in devicetree)

   These contraints are specified in section 6.1 of ACPI 6.3 [1].

2) ACPI's _DSD Method

   The Device Specific Data (_DSD) method provides a way to provide additional
   device properties to device drivers. It returns a package of "Device Data
   Descriptors", each consisting of a UUID and a package in a format defined
   by the provided UUID.
   
   The details are specified in section 6.2.5 of ACPI 6.3 [1].

3) Special UUID value for _DSD: daffd814-...

   The document "Device Properties UUID For _DSD" [2] specifies a special UUID
   value:    daffd814-6eba-4d8c-8a91-bc9bbf4aa301
   
   When this UUID is used in the package returned by in a _DSD method,
   it means the format of the package after the UUID consits of packages
   each of size 2.
   The first entry in these packages always has to be a string, the second
   entry can be an integer, a string, a reference or again a package. The
   value of the string defines the type and allowed values of the second entry.
   
   The typical use case for this UUID is to return some kind of
   key/value-pairs.
   
   The specification in [2] does not specify which strings are legal as
   key names. This depends on the _HID of the device that implements the
   _DSD method.

4) Special _HID value: PRP0001

   When the _HID property has the value "PRP0001", the _DSD method is expected
   to provide data with the "daffd814"-UUID which contains a "compatible"
   property.
   
   This interpreted similar to the "compatible" property known from Devicetree.

5) Linux device-property API

   Linux provides a "device-property" API (define in include/linux/property.h)
   which can be used instead of a Devicetree specific API.
   E.g. using device_property_read_u32() instead of of_property_read_u32().
   
Putting these pieces together:

   Suppose the following use case:
   There is an existing driver for Devicetree, but it should be used under
   x86, where devices are usually described via ACPI.
   
   To avoid having to register a new _HID that would have the exact same
   meaning in ACPI as an already existing Devicetree "compatible" string, the
   possibilities desbribed by 2-4) allow to solve this use case while
   respecting the contraints described in 1).
   
   Additionally, using the API described in 5) allows to add other Devicetree
   properties  (not only the "compatible" property) to the ACPI description of
   the device. This allows to have a single device driver that can get its
   device description from either Devicetree or ACPI, and the description is
   basically the same in both worlds. This is described e.g. in the
   presentation in [4].

[1] https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
[2] http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
[3] Documentation/firmware-guide/acpi/enumeration.rst in the Linux kernel
[4] https://elinux.org/images/2/2d/Device_tree_acpi_compatibility-david_woodhouse-kernel_recipes_2015.pdf

> + - acpi,ddn : Contains the string to use as the _DDN (DOS (Disk Operating
> +    System) Device Name)
> + - acpi,hid : Contains the string to use as the HID (Hardware ID)
> +    identifier _HID
> + - acpi,probed : Tells U-Boot to add 'linux,probed' to the ACPI tables so that
> +    Linux will only load the driver if the device can be detected (e.g. on I2C
> +    bus)

Will support for 'linux,probed' be mainlined? Otherwise the description
should IMHO mention that it is an out-of-tree feature.

> + - acpi,uid : _UID value for device
> +
> +
> +Example
> +-------
> +
> +elan_touchscreen: elan-touchscreen at 10 {
> + compatible = "i2c-chip";

Why would you use a generic compatible string in this case?
According to drivers/input/touchscreen/elants_i2c.c in the Linux kernel
the ACPI _HID "ELAN0001" refers to the same device as the Devicetree
compatible string "elan,ekth3500".

Again, because of the direct relationship between "compatible" string and
_HID I think we could move that knowledge into a (stub-) driver for
"elan,ekth3500" and then we could avoid the need for "acpi,hid" here.

> + reg = <0x10>;
> + acpi,hid = "ELAN0001";
> + acpi,ddn = "ELAN Touchscreen";
> + interrupts-extended = <&acpi_gpe GPIO_21_IRQ
> + IRQ_TYPE_EDGE_FALLING>;
> + acpi,probed;
> +};
> --
> 2.26.0.rc2.310.g2932bb562d-goog

regards, Wolfgang