[PATCH v3 13/29] dts: Add a binding for hid-over-i2c
Simon Glass
sjg at chromium.org
Wed Apr 8 04:58:13 CEST 2020
Hi Wolfgang,
On Tue, 31 Mar 2020 at 13:25, Wolfgang Wallner
<wolfgang.wallner at br-automation.com> wrote:
>
> 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.
But the compatible string is "hid-over-i2c". We don't want to have
lots of different compatible strings here, different drivers which do
the same thing. If we end up wanting a touchscreen drivers in U-Boot
that might change, but for now I think a generic driver is easier.
>
> 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().
>
Thank you very much for digging into this. I read the ACPI spec years
ago but clearly need to read it again.
> 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.
OK
>
> > + - 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.
But then we need a driver for the ELAN touchscreen. My goal here is to
have all of these devices serviced by a single driver, using suitable
properties in the DT.
>
> > + 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,
SImon
More information about the U-Boot
mailing list