Antwort: [PATCH v5 14/29] acpi: Add a binding for ACPI settings in the device tree

[Wolfgang Wallner]

Hi Simon,

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

>Betreff: [PATCH v5 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 v5: None
>Changes in v4:
>- Put 'interrupts-extended' property on one line
>- Rename acpi-probed to linux,probed
>- Note that linux,probed is an out-of-tree feature
>
>Changes in v3:
>- Drop mention of PRIC
>- Rename acpi,desc to acpi,ddn
>- Correct description of acpi,probed
>- Drop hid-descr-addr
>- Just add the device.txt binding file in this patch
>- Change the example to ELAN
>- Add a pointer to information about acpi,compatible
>
>Changes in v2:
>- Fix definition of HID
>- Infer hid-over-i2c CID value
>- Add the hid-over-i2c binding document
>
> doc/device-tree-bindings/device.txt | 36
>+++++++++++++++++++++++++++++
> 1 file changed, 36 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..27bd3978d98
>--- /dev/null
>+++ b/doc/device-tree-bindings/device.txt
>@@ -0,0 +1,36 @@
>+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
>+ - 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,uid : _UID value for device
>+ - linux,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). Note that this is an out-of-tree Linux feature.
>+
>+
>+Example
>+-------
>+
>+elan_touchscreen: elan-touchscreen at 10 {
>+	compatible = "i2c-chip";
>+	reg = <0x10>;
>+	acpi,hid = "ELAN0001";
>+	acpi,ddn = "ELAN Touchscreen";
>+	interrupts-extended = <&acpi_gpe GPIO_21_IRQ
>IRQ_TYPE_EDGE_FALLING>;
>+	linux,probed;
>+};
>-- 
>2.26.0.292.g33ef6b2f38-goog

I have learned through the previous review discussions aspects about this
binding which are not captured in the current patch. I tried to incorporate
these findings, the modified text is proposed below.

Additionally, I realized that I still don't understand how some parts of the
proposed binding are intended to work. I have highlighted these aspects with
a TODO note. Please have a look.

regards, Wolfgang

--------------------------------------------------------------------------

Devices
=======

Device bindings are described by their own individual binding files.
U-Boot provides for some optional properties which are documented here.

ACPI-related properties:

 - acpi,compatible : ACPI devices may report a _HID or _CID of "PRP0001", in
    which case they are expected to provide a _DSD object with a "compatible"
    property. The value of this "compatible" property is specified by 
    "acpi,compatible".
    See also section 6.2.5 of [1] as well as [2-3] for details.
    
    TODO: will PRP0001 be used as _HID or as _CID?
    
    Note: the value for "acpi,compatible" can be different from the
    "compatible" property used in Devicetree for certain devices, e.g. when
    a custom driver in U-Boot would not make sense. An example would be touch
    screen devices.
 - acpi,ddn : Contains the string to use as the _DDN (DOS (Disk Operating
    System) Device Name)
 - 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
    
    TODO: a list of the required properties for this to work would be helpful
    
 - acpi,hid : Contains the string to use as the HID (Hardware ID)
    identifier _HID
    
    TODO: How does this work togheter with "acpi, compatible"? (assuming
    "acpi,compatible" implies a _HID of "PRP0001")
 
    - acpi,uid : _UID value for the device

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

Other properties:

 - linux,probed : With this property Linux can be told to only load the driver
    if the device can be detected (e.g. on I2C bus).
    When generating ACPI tables, this property is adopted into a device's
    _DSD object.
    Note that this is an out-of-tree Linux feature.

Example
-------

elan_touchscreen: elan-touchscreen at 10 {
	compatible = "i2c-chip";
	reg = <0x10>;
	acpi,hid = "ELAN0001";
	acpi,ddn = "ELAN Touchscreen";
	acpi,uid = 1;
	interrupts-extended = <&acpi_gpe GPIO_21_IRQ IRQ_TYPE_EDGE_FALLING>;
	linux,probed;
};

The example above would result in an ACPI entry similar to the following:

Device(DEV1)    // TODO: how is the name ("DEV1" in this example) selected?
{
    Name (_HID, "ELAN0001")
    Name (_DDN, "ELAN Touchscreen")
    Name (_UID, 1)

    Name (_DSD, Package (0x02)
    {
        ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
        Package (0x01)
        {
            Package (0x02) { "linux,probed", Package (0x01) {1} },
        }
    })
}

--------------------------------------------------------------------------

I have dropped the sentence "See also hid-over-i2c.txt which describes HID
devices.", as I don't think it is still required. HID devices are not mentioned
any more as the previous hid-over-i2c example was dropped.