diff mbox series

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

Message ID 20200330231305.130488-2-sjg@chromium.org
State Superseded
Delegated to: Bin Meng
Headers show
Series dm: Add programmatic generation of ACPI tables (part A) | expand

Commit Message

Simon Glass March 30, 2020, 11:12 p.m. UTC
Add this binding from Linux v5.4.

Signed-off-by: Simon Glass <sjg@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

Comments

Wolfgang Wallner March 31, 2020, 7:25 p.m. UTC | #1
Hi Simon,

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

>An: u-boot@lists.denx.de
>Von: "Simon Glass" <sjg@chromium.org>
>Datum: 31.03.2020 01:14
>Kopie: "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
>"Wolfgang Wallner" <wolfgang.wallner@br-automation.com>, "Leif
>Lindholm" <leif@nuviainc.com>, "Simon Glass" <sjg@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@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@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
Wolfgang Wallner April 1, 2020, 7:39 a.m. UTC | #2
Hi Simon,

-----"U-Boot" <u-boot-bounces@lists.denx.de> schrieb: -----

[snip]

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

I have thought some more about this property.
The Chromium discussions [1] mention that "linux,probed" is intended to be
a property in both Devicetree and ACPI, so a Linux kernel could handle the
device in the expected way whether it boots on a Devicetree platform or an
ACPI platform. The proposed "acpi,probed" property would only solve one such
use case (ACPI-based platforms): U-Boot boots, reads "acpi,probed" from
Devicetree, adds "linux,probed" to the ACPI description and hands that to the
Linux kernel.

Why would we not keep the name "linux,probed" for this property? U-Boot could
still do the same Devicetree -> ACPI translation, but the same property could
then also be useful for Devicetree-based platforms.

[snip]

regards, Wolfgang

[1] https://groups.google.com/a/chromium.org/forum/#!msg/chromium-os-reviews/4HTHl78IGHw/oz82uImnBgAJ
Simon Glass April 8, 2020, 2:57 a.m. UTC | #3
Hi Wolfgang,

On Wed, 1 Apr 2020 at 01:39, Wolfgang Wallner
<wolfgang.wallner@br-automation.com> wrote:
>
> Hi Simon,
>
> -----"U-Boot" <u-boot-bounces@lists.denx.de> schrieb: -----
>
> [snip]
>
> >> + - 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.
>
> I have thought some more about this property.
> The Chromium discussions [1] mention that "linux,probed" is intended to be
> a property in both Devicetree and ACPI, so a Linux kernel could handle the
> device in the expected way whether it boots on a Devicetree platform or an
> ACPI platform. The proposed "acpi,probed" property would only solve one such
> use case (ACPI-based platforms): U-Boot boots, reads "acpi,probed" from
> Devicetree, adds "linux,probed" to the ACPI description and hands that to the
> Linux kernel.
>
> Why would we not keep the name "linux,probed" for this property? U-Boot could
> still do the same Devicetree -> ACPI translation, but the same property could
> then also be useful for Devicetree-based platforms.

OK...oh dear. I will change it back to linux,probed

Regards,
Simon
Simon Glass April 8, 2020, 2:58 a.m. UTC | #4
Hi Wolfgang,

On Tue, 31 Mar 2020 at 13:25, Wolfgang Wallner
<wolfgang.wallner@br-automation.com> wrote:
>
> Hi Simon,
>
> -----"Simon Glass" <sjg@chromium.org> schrieb: -----
>
> >An: u-boot@lists.denx.de
> >Von: "Simon Glass" <sjg@chromium.org>
> >Datum: 31.03.2020 01:14
> >Kopie: "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
> >"Wolfgang Wallner" <wolfgang.wallner@br-automation.com>, "Leif
> >Lindholm" <leif@nuviainc.com>, "Simon Glass" <sjg@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@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@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
Andy Shevchenko April 8, 2020, 5:08 p.m. UTC | #5
On Tue, Apr 07, 2020 at 08:58:13PM -0600, Simon Glass wrote:
> On Tue, 31 Mar 2020 at 13:25, Wolfgang Wallner
> <wolfgang.wallner@br-automation.com> wrote:
> > >An: u-boot@lists.denx.de
> > >Von: "Simon Glass" <sjg@chromium.org>
> > >Datum: 31.03.2020 01:14
> > >Kopie: "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
> > >"Wolfgang Wallner" <wolfgang.wallner@br-automation.com>, "Leif
> > >Lindholm" <leif@nuviainc.com>, "Simon Glass" <sjg@chromium.org>
> > >Betreff: [PATCH v3 14/29] acpi: Add a binding for ACPI settings in
> > >the device tree

> > 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).

Before we are going further with this here is a BIG CAVEAT.

PRP0001   MUST NOT be used in production devices.

This has been derived solely for debugging / pre-production testing / etc
purposes. The real devices must have an official ACPI _HID.
Wolfgang Wallner April 8, 2020, 7:39 p.m. UTC | #6
Hi Andy,

-----"Andy Shevchenko" <andriy.shevchenko@linux.intel.com> schrieb: -----
> 
> Betreff: Re: [PATCH v3 13/29] dts: Add a binding for hid-over-i2c
> 
> On Tue, Apr 07, 2020 at 08:58:13PM -0600, Simon Glass wrote:
> > On Tue, 31 Mar 2020 at 13:25, Wolfgang Wallner
> > <wolfgang.wallner@br-automation.com> wrote:
> > > >An: u-boot@lists.denx.de
> > > >Von: "Simon Glass" <sjg@chromium.org>
> > > >Datum: 31.03.2020 01:14
> > > >Kopie: "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
> > > >"Wolfgang Wallner" <wolfgang.wallner@br-automation.com>, "Leif
> > > >Lindholm" <leif@nuviainc.com>, "Simon Glass" <sjg@chromium.org>
> > > >Betreff: [PATCH v3 14/29] acpi: Add a binding for ACPI settings in
> > > >the device tree
> 
> > > 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).
> 
> Before we are going further with this here is a BIG CAVEAT.
> 
> PRP0001   MUST NOT be used in production devices.
> 
> This has been derived solely for debugging / pre-production testing / etc
> purposes. The real devices must have an official ACPI _HID.

Thanks for pointing this out! I was not aware of this.
I have tried to understand how the PRP0001 is meant to be used, but could not
find sufficient documentation. The best document I could find is
Documentation/firmware-guide/acpi/enumeration.rst in the Linux kernel, and
as far as I can tell the constraint that you mention is also not described
there.

Do you know any other resources regarding PRP0001, e.g. some kind of
speficiation?

If PRP0001 is only for debugging, then I must also have misunderstood the
Linux "device-property" API (define in include/linux/property.h).
There are some presentations available on the internet, e.g. [1], that I
understand like PRP0001 + "device-property" API provide a way do access data
from either Devicetree or ACPI, depending on what kind of platform you are on.

regards, Wolfgang

[1] https://elinux.org/images/2/2d/Device_tree_acpi_compatibility-david_woodhouse-kernel_recipes_2015.pdf
Andy Shevchenko April 8, 2020, 8:40 p.m. UTC | #7
On Wed, Apr 8, 2020 at 10:39 PM Wolfgang Wallner
<wolfgang.wallner@br-automation.com> wrote:
> > On Tue, Apr 07, 2020 at 08:58:13PM -0600, Simon Glass wrote:
> > > On Tue, 31 Mar 2020 at 13:25, Wolfgang Wallner
> > > <wolfgang.wallner@br-automation.com> wrote:
> > > > >An: u-boot@lists.denx.de
> > > > >Von: "Simon Glass" <sjg@chromium.org>
> > > > >Datum: 31.03.2020 01:14
> > > > >Kopie: "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
> > > > >"Wolfgang Wallner" <wolfgang.wallner@br-automation.com>, "Leif
> > > > >Lindholm" <leif@nuviainc.com>, "Simon Glass" <sjg@chromium.org>
> > > > >Betreff: [PATCH v3 14/29] acpi: Add a binding for ACPI settings in
> > > > >the device tree
> >
> > > > 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).
> >
> > Before we are going further with this here is a BIG CAVEAT.
> >
> > PRP0001   MUST NOT be used in production devices.
> >
> > This has been derived solely for debugging / pre-production testing / etc
> > purposes. The real devices must have an official ACPI _HID.
>
> Thanks for pointing this out! I was not aware of this.
> I have tried to understand how the PRP0001 is meant to be used, but could not
> find sufficient documentation. The best document I could find is
> Documentation/firmware-guide/acpi/enumeration.rst in the Linux kernel, and
> as far as I can tell the constraint that you mention is also not described
> there.
>
> Do you know any other resources regarding PRP0001, e.g. some kind of
> speficiation?

I guess the best one is to ask somebody from UEFI Forum / ASWG. PRP is
a PNP ID for UEFI Forum.

> If PRP0001 is only for debugging, then I must also have misunderstood the
> Linux "device-property" API (define in include/linux/property.h).

Not exactly.

> There are some presentations available on the internet, e.g. [1], that I
> understand like PRP0001 + "device-property" API provide a way do access data
> from either Devicetree or ACPI, depending on what kind of platform you are on.

No, these are not hard linked to each other (the relation is that
PRP0001 is a way to enumerate devices, which don't have dedicated ACPI
_HID, by using compatible property [1]). The _DSD per se (i.o.w.
device properties implementation in ACPI) is a different story [2].

And I put [3] here, interesting to read. However, at that time I was
quite far from this topic.

[1]: https://www.kernel.org/doc/html/latest/firmware-guide/acpi/enumeration.html#device-tree-namespace-link-device-id
[2]: https://uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel-1_2-3.htm.
[3]: https://patchwork.kernel.org/patch/7004241/

> [1] https://elinux.org/images/2/2d/Device_tree_acpi_compatibility-david_woodhouse-kernel_recipes_2015.pdf
Andy Shevchenko April 8, 2020, 8:49 p.m. UTC | #8
On Wed, Apr 8, 2020 at 11:40 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Apr 8, 2020 at 10:39 PM Wolfgang Wallner
> <wolfgang.wallner@br-automation.com> wrote:
> > > On Tue, Apr 07, 2020 at 08:58:13PM -0600, Simon Glass wrote:
> > > > On Tue, 31 Mar 2020 at 13:25, Wolfgang Wallner
> > > > <wolfgang.wallner@br-automation.com> wrote:
> > > > > >An: u-boot@lists.denx.de
> > > > > >Von: "Simon Glass" <sjg@chromium.org>
> > > > > >Datum: 31.03.2020 01:14
> > > > > >Kopie: "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
> > > > > >"Wolfgang Wallner" <wolfgang.wallner@br-automation.com>, "Leif
> > > > > >Lindholm" <leif@nuviainc.com>, "Simon Glass" <sjg@chromium.org>
> > > > > >Betreff: [PATCH v3 14/29] acpi: Add a binding for ACPI settings in
> > > > > >the device tree
> > >
> > > > > 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).
> > >
> > > Before we are going further with this here is a BIG CAVEAT.
> > >
> > > PRP0001   MUST NOT be used in production devices.
> > >
> > > This has been derived solely for debugging / pre-production testing / etc
> > > purposes. The real devices must have an official ACPI _HID.
> >
> > Thanks for pointing this out! I was not aware of this.
> > I have tried to understand how the PRP0001 is meant to be used, but could not
> > find sufficient documentation. The best document I could find is
> > Documentation/firmware-guide/acpi/enumeration.rst in the Linux kernel, and
> > as far as I can tell the constraint that you mention is also not described
> > there.
> >
> > Do you know any other resources regarding PRP0001, e.g. some kind of
> > speficiation?
>
> I guess the best one is to ask somebody from UEFI Forum / ASWG. PRP is
> a PNP ID for UEFI Forum.

Basically last message in [3] from Rafael mentions his view on
PRP0001. I guess there is still no document, although as I noticed the
PRP prefix become official in a mean time.

> > If PRP0001 is only for debugging, then I must also have misunderstood the
> > Linux "device-property" API (define in include/linux/property.h).
>
> Not exactly.
>
> > There are some presentations available on the internet, e.g. [1], that I
> > understand like PRP0001 + "device-property" API provide a way do access data
> > from either Devicetree or ACPI, depending on what kind of platform you are on.
>
> No, these are not hard linked to each other (the relation is that
> PRP0001 is a way to enumerate devices, which don't have dedicated ACPI
> _HID, by using compatible property [1]). The _DSD per se (i.o.w.
> device properties implementation in ACPI) is a different story [2].
>
> And I put [3] here, interesting to read. However, at that time I was
> quite far from this topic.
>
> [1]: https://www.kernel.org/doc/html/latest/firmware-guide/acpi/enumeration.html#device-tree-namespace-link-device-id
> [2]: https://uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel-1_2-3.htm.
> [3]: https://patchwork.kernel.org/patch/7004241/
>
> > [1] https://elinux.org/images/2/2d/Device_tree_acpi_compatibility-david_woodhouse-kernel_recipes_2015.pdf
>
> --
> With Best Regards,
> Andy Shevchenko
Wolfgang Wallner April 15, 2020, 2 p.m. UTC | #9
Hi Andy,

-----"Andy Shevchenko" <andy.shevchenko@gmail.com> schrieb: -----
> Betreff: Re: Re: [PATCH v3 13/29] dts: Add a binding for hid-over-i2c
> 
> On Wed, Apr 8, 2020 at 11:40 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Wed, Apr 8, 2020 at 10:39 PM Wolfgang Wallner
> > <wolfgang.wallner@br-automation.com> wrote:
> > > > On Tue, Apr 07, 2020 at 08:58:13PM -0600, Simon Glass wrote:
> > > > > On Tue, 31 Mar 2020 at 13:25, Wolfgang Wallner
> > > > > <wolfgang.wallner@br-automation.com> wrote:
> > > > > > >An: u-boot@lists.denx.de
> > > > > > >Von: "Simon Glass" <sjg@chromium.org>
> > > > > > >Datum: 31.03.2020 01:14
> > > > > > >Kopie: "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
> > > > > > >"Wolfgang Wallner" <wolfgang.wallner@br-automation.com>, "Leif
> > > > > > >Lindholm" <leif@nuviainc.com>, "Simon Glass" <sjg@chromium.org>
> > > > > > >Betreff: [PATCH v3 14/29] acpi: Add a binding for ACPI settings in
> > > > > > >the device tree
> > > >
> > > > > > 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).
> > > >
> > > > Before we are going further with this here is a BIG CAVEAT.
> > > >
> > > > PRP0001   MUST NOT be used in production devices.
> > > >
> > > > This has been derived solely for debugging / pre-production testing / etc
> > > > purposes. The real devices must have an official ACPI _HID.
> > >
> > > Thanks for pointing this out! I was not aware of this.
> > > I have tried to understand how the PRP0001 is meant to be used, but could not
> > > find sufficient documentation. The best document I could find is
> > > Documentation/firmware-guide/acpi/enumeration.rst in the Linux kernel, and
> > > as far as I can tell the constraint that you mention is also not described
> > > there.
> > >
> > > Do you know any other resources regarding PRP0001, e.g. some kind of
> > > speficiation?
> >
> > I guess the best one is to ask somebody from UEFI Forum / ASWG. PRP is
> > a PNP ID for UEFI Forum.
> 
> Basically last message in [3] from Rafael mentions his view on
> PRP0001. I guess there is still no document, although as I noticed the
> PRP prefix become official in a mean time.

Thanks for the references. I have read them carefully, especially the thread 
in [3].

My understanding up to now was based on presentations by David Woodhouse,
so it matches with his viewpoint in the thread at [3]. And as far as I can
tell Rafael Wysocki agrees with him in this thread.

What I could not find in either of the references is that PRP0001 is only
for debugging, I only know about this constraint from your mail. Could you
point me to any source for this?
 
> > > If PRP0001 is only for debugging, then I must also have misunderstood the
> > > Linux "device-property" API (define in include/linux/property.h).
> >
> > Not exactly.
> >
> > > There are some presentations available on the internet, e.g. [1], that I
> > > understand like PRP0001 + "device-property" API provide a way do access data
> > > from either Devicetree or ACPI, depending on what kind of platform you are on.
> >
> > No, these are not hard linked to each other (the relation is that
> > PRP0001 is a way to enumerate devices, which don't have dedicated ACPI
> > _HID, by using compatible property [1]). The _DSD per se (i.o.w.
> > device properties implementation in ACPI) is a different story [2].
> >
> > And I put [3] here, interesting to read. However, at that time I was
> > quite far from this topic.
> >
> > [1]: https://www.kernel.org/doc/html/latest/firmware-guide/acpi/enumeration.html#device-tree-namespace-link-device-id
> > [2]: https://uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel-1_2-3.htm.
> > [3]: https://patchwork.kernel.org/patch/7004241/
> >
> > > [1] https://elinux.org/images/2/2d/Device_tree_acpi_compatibility-david_woodhouse-kernel_recipes_2015.pdf
> >
> > --
> > With Best Regards,
> > Andy Shevchenko

regards, Wolfgang

PS: I only realized now that I had mixed up the email-subject in my first
reply and now the complete thread is under the wrong topic. Sorry for that.
Andy Shevchenko April 15, 2020, 2:25 p.m. UTC | #10
On Wed, Apr 15, 2020 at 5:00 PM Wolfgang Wallner
<wolfgang.wallner@br-automation.com> wrote:
> -----"Andy Shevchenko" <andy.shevchenko@gmail.com> schrieb: -----
> > Betreff: Re: Re: [PATCH v3 13/29] dts: Add a binding for hid-over-i2c
> >
> > On Wed, Apr 8, 2020 at 11:40 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Wed, Apr 8, 2020 at 10:39 PM Wolfgang Wallner
> > > <wolfgang.wallner@br-automation.com> wrote:
> > > > > On Tue, Apr 07, 2020 at 08:58:13PM -0600, Simon Glass wrote:
> > > > > > On Tue, 31 Mar 2020 at 13:25, Wolfgang Wallner
> > > > > > <wolfgang.wallner@br-automation.com> wrote:
> > > > > > > >An: u-boot@lists.denx.de
> > > > > > > >Von: "Simon Glass" <sjg@chromium.org>
> > > > > > > >Datum: 31.03.2020 01:14
> > > > > > > >Kopie: "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
> > > > > > > >"Wolfgang Wallner" <wolfgang.wallner@br-automation.com>, "Leif
> > > > > > > >Lindholm" <leif@nuviainc.com>, "Simon Glass" <sjg@chromium.org>
> > > > > > > >Betreff: [PATCH v3 14/29] acpi: Add a binding for ACPI settings in
> > > > > > > >the device tree
> > > > >
> > > > > > > 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).
> > > > >
> > > > > Before we are going further with this here is a BIG CAVEAT.
> > > > >
> > > > > PRP0001   MUST NOT be used in production devices.
> > > > >
> > > > > This has been derived solely for debugging / pre-production testing / etc
> > > > > purposes. The real devices must have an official ACPI _HID.
> > > >
> > > > Thanks for pointing this out! I was not aware of this.
> > > > I have tried to understand how the PRP0001 is meant to be used, but could not
> > > > find sufficient documentation. The best document I could find is
> > > > Documentation/firmware-guide/acpi/enumeration.rst in the Linux kernel, and
> > > > as far as I can tell the constraint that you mention is also not described
> > > > there.
> > > >
> > > > Do you know any other resources regarding PRP0001, e.g. some kind of
> > > > speficiation?
> > >
> > > I guess the best one is to ask somebody from UEFI Forum / ASWG. PRP is
> > > a PNP ID for UEFI Forum.
> >
> > Basically last message in [3] from Rafael mentions his view on
> > PRP0001. I guess there is still no document, although as I noticed the
> > PRP prefix become official in a mean time.
>
> Thanks for the references. I have read them carefully, especially the thread
> in [3].
>
> My understanding up to now was based on presentations by David Woodhouse,
> so it matches with his viewpoint in the thread at [3]. And as far as I can
> tell Rafael Wysocki agrees with him in this thread.
>
> What I could not find in either of the references is that PRP0001 is only
> for debugging, I only know about this constraint from your mail. Could you
> point me to any source for this?

From [3], Rafael said:
"Let alone the fact that PRP0001 is actually quite useful at the
prototyping stage when it is premature to allocate a new device ID just
yet.  Taking that to the extreme, if someone whittles a board in his or
her garage and wants to use it to drive their homemade grass watering
system, having to invent a new device ID and put it into the existing
driver that otherwise doesn't require any modifications is ... you know
what I mean."

It implies that the process should have included the allocation of an
official ACPI ID.

You always can ask ASWG for the clarification. Maybe I learn something
new about PRP0001 :-)

> > > > If PRP0001 is only for debugging, then I must also have misunderstood the
> > > > Linux "device-property" API (define in include/linux/property.h).
> > >
> > > Not exactly.
> > >
> > > > There are some presentations available on the internet, e.g. [1], that I
> > > > understand like PRP0001 + "device-property" API provide a way do access data
> > > > from either Devicetree or ACPI, depending on what kind of platform you are on.
> > >
> > > No, these are not hard linked to each other (the relation is that
> > > PRP0001 is a way to enumerate devices, which don't have dedicated ACPI
> > > _HID, by using compatible property [1]). The _DSD per se (i.o.w.
> > > device properties implementation in ACPI) is a different story [2].
> > >
> > > And I put [3] here, interesting to read. However, at that time I was
> > > quite far from this topic.
> > >
> > > [1]: https://www.kernel.org/doc/html/latest/firmware-guide/acpi/enumeration.html#device-tree-namespace-link-device-id
> > > [2]: https://uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel-1_2-3.htm.
> > > [3]: https://patchwork.kernel.org/patch/7004241/
> > >
> > > > [1] https://elinux.org/images/2/2d/Device_tree_acpi_compatibility-david_woodhouse-kernel_recipes_2015.pdf
Wolfgang Wallner April 15, 2020, 2:57 p.m. UTC | #11
Hi Andy,

-----"Andy Shevchenko" <andy.shevchenko@gmail.com> schrieb: -----

> On Wed, Apr 15, 2020 at 5:00 PM Wolfgang Wallner
> <wolfgang.wallner@br-automation.com> wrote:
> > -----"Andy Shevchenko" <andy.shevchenko@gmail.com> schrieb: -----
> > > Betreff: Re: Re: [PATCH v3 13/29] dts: Add a binding for hid-over-i2c
> > >
> > > On Wed, Apr 8, 2020 at 11:40 PM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Wed, Apr 8, 2020 at 10:39 PM Wolfgang Wallner
> > > > <wolfgang.wallner@br-automation.com> wrote:
> > > > > > On Tue, Apr 07, 2020 at 08:58:13PM -0600, Simon Glass wrote:
> > > > > > > On Tue, 31 Mar 2020 at 13:25, Wolfgang Wallner
> > > > > > > <wolfgang.wallner@br-automation.com> wrote:
> > > > > > > > >An: u-boot@lists.denx.de
> > > > > > > > >Von: "Simon Glass" <sjg@chromium.org>
> > > > > > > > >Datum: 31.03.2020 01:14
> > > > > > > > >Kopie: "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
> > > > > > > > >"Wolfgang Wallner" <wolfgang.wallner@br-automation.com>, "Leif
> > > > > > > > >Lindholm" <leif@nuviainc.com>, "Simon Glass" <sjg@chromium.org>
> > > > > > > > >Betreff: [PATCH v3 14/29] acpi: Add a binding for ACPI settings in
> > > > > > > > >the device tree
> > > > > >
> > > > > > > > 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).
> > > > > >
> > > > > > Before we are going further with this here is a BIG CAVEAT.
> > > > > >
> > > > > > PRP0001   MUST NOT be used in production devices.
> > > > > >
> > > > > > This has been derived solely for debugging / pre-production testing / etc
> > > > > > purposes. The real devices must have an official ACPI _HID.
> > > > >
> > > > > Thanks for pointing this out! I was not aware of this.
> > > > > I have tried to understand how the PRP0001 is meant to be used, but could not
> > > > > find sufficient documentation. The best document I could find is
> > > > > Documentation/firmware-guide/acpi/enumeration.rst in the Linux kernel, and
> > > > > as far as I can tell the constraint that you mention is also not described
> > > > > there.
> > > > >
> > > > > Do you know any other resources regarding PRP0001, e.g. some kind of
> > > > > speficiation?
> > > >
> > > > I guess the best one is to ask somebody from UEFI Forum / ASWG. PRP is
> > > > a PNP ID for UEFI Forum.
> > >
> > > Basically last message in [3] from Rafael mentions his view on
> > > PRP0001. I guess there is still no document, although as I noticed the
> > > PRP prefix become official in a mean time.
> >
> > Thanks for the references. I have read them carefully, especially the thread
> > in [3].
> >
> > My understanding up to now was based on presentations by David Woodhouse,
> > so it matches with his viewpoint in the thread at [3]. And as far as I can
> > tell Rafael Wysocki agrees with him in this thread.
> >
> > What I could not find in either of the references is that PRP0001 is only
> > for debugging, I only know about this constraint from your mail. Could you
> > point me to any source for this?
> 
> From [3], Rafael said:
> "Let alone the fact that PRP0001 is actually quite useful at the
> prototyping stage when it is premature to allocate a new device ID just
> yet.  Taking that to the extreme, if someone whittles a board in his or
> her garage and wants to use it to drive their homemade grass watering
> system, having to invent a new device ID and put it into the existing
> driver that otherwise doesn't require any modifications is ... you know
> what I mean."
> 
> It implies that the process should have included the allocation of an
> official ACPI ID.

I understand the quoted paragraph differently. In the paragraphs above Rafael
argues that PRP0001 is useful, as with PRP0001 we can avoid registering
redundant ACPI IDs. In the quoted paragraph he then gives another example
where PRP0001 is also useful: for debugging.
But I don't understand it in a way that PRP0001 would be limited to only
debugging.

A few posts before in [4] he explains in more detail, and I think it matches
my understanding of the last post, especially the following sentence:

"It would be a failure if people had to write separate drivers for 
ACPI-based and DT-based platforms to handle the same hardware, at least 
at the leaf driver level."

[4] https://patchwork.kernel.org/comment/15339311/

> You always can ask ASWG for the clarification. Maybe I learn something
> new about PRP0001 :-)

I had no interaction with ASWG before so I'm not sure what the process is,
but I will look it up.

> > > > > If PRP0001 is only for debugging, then I must also have misunderstood the
> > > > > Linux "device-property" API (define in include/linux/property.h).
> > > >
> > > > Not exactly.
> > > >
> > > > > There are some presentations available on the internet, e.g. [1], that I
> > > > > understand like PRP0001 + "device-property" API provide a way do access data
> > > > > from either Devicetree or ACPI, depending on what kind of platform you are on.
> > > >
> > > > No, these are not hard linked to each other (the relation is that
> > > > PRP0001 is a way to enumerate devices, which don't have dedicated ACPI
> > > > _HID, by using compatible property [1]). The _DSD per se (i.o.w.
> > > > device properties implementation in ACPI) is a different story [2].
> > > >
> > > > And I put [3] here, interesting to read. However, at that time I was
> > > > quite far from this topic.
> > > >
> > > > [1]: https://www.kernel.org/doc/html/latest/firmware-guide/acpi/enumeration.html#device-tree-namespace-link-device-id
> > > > [2]: https://uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel-1_2-3.htm.
> > > > [3]: https://patchwork.kernel.org/patch/7004241/
> > > >
> > > > > [1] https://elinux.org/images/2/2d/Device_tree_acpi_compatibility-david_woodhouse-kernel_recipes_2015.pdf

regards, Wolfgang
Andy Shevchenko April 15, 2020, 3:15 p.m. UTC | #12
On Wed, Apr 15, 2020 at 04:57:33PM +0200, Wolfgang Wallner wrote:
> > On Wed, Apr 15, 2020 at 5:00 PM Wolfgang Wallner
> > <wolfgang.wallner@br-automation.com> wrote:
> > > -----"Andy Shevchenko" <andy.shevchenko@gmail.com> schrieb: -----
> > > > Betreff: Re: Re: [PATCH v3 13/29] dts: Add a binding for hid-over-i2c
> > > >
> > > > On Wed, Apr 8, 2020 at 11:40 PM Andy Shevchenko
> > > > <andy.shevchenko@gmail.com> wrote:
> > > > > On Wed, Apr 8, 2020 at 10:39 PM Wolfgang Wallner
> > > > > <wolfgang.wallner@br-automation.com> wrote:
> > > > > > > On Tue, Apr 07, 2020 at 08:58:13PM -0600, Simon Glass wrote:
> > > > > > > > On Tue, 31 Mar 2020 at 13:25, Wolfgang Wallner
> > > > > > > > <wolfgang.wallner@br-automation.com> wrote:
> > > > > > > > > >An: u-boot@lists.denx.de
> > > > > > > > > >Von: "Simon Glass" <sjg@chromium.org>
> > > > > > > > > >Datum: 31.03.2020 01:14
> > > > > > > > > >Kopie: "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
> > > > > > > > > >"Wolfgang Wallner" <wolfgang.wallner@br-automation.com>, "Leif
> > > > > > > > > >Lindholm" <leif@nuviainc.com>, "Simon Glass" <sjg@chromium.org>
> > > > > > > > > >Betreff: [PATCH v3 14/29] acpi: Add a binding for ACPI settings in
> > > > > > > > > >the device tree
> > > > > > >
> > > > > > > > > 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).
> > > > > > >
> > > > > > > Before we are going further with this here is a BIG CAVEAT.
> > > > > > >
> > > > > > > PRP0001   MUST NOT be used in production devices.
> > > > > > >
> > > > > > > This has been derived solely for debugging / pre-production testing / etc
> > > > > > > purposes. The real devices must have an official ACPI _HID.
> > > > > >
> > > > > > Thanks for pointing this out! I was not aware of this.
> > > > > > I have tried to understand how the PRP0001 is meant to be used, but could not
> > > > > > find sufficient documentation. The best document I could find is
> > > > > > Documentation/firmware-guide/acpi/enumeration.rst in the Linux kernel, and
> > > > > > as far as I can tell the constraint that you mention is also not described
> > > > > > there.
> > > > > >
> > > > > > Do you know any other resources regarding PRP0001, e.g. some kind of
> > > > > > speficiation?
> > > > >
> > > > > I guess the best one is to ask somebody from UEFI Forum / ASWG. PRP is
> > > > > a PNP ID for UEFI Forum.
> > > >
> > > > Basically last message in [3] from Rafael mentions his view on
> > > > PRP0001. I guess there is still no document, although as I noticed the
> > > > PRP prefix become official in a mean time.
> > >
> > > Thanks for the references. I have read them carefully, especially the thread
> > > in [3].
> > >
> > > My understanding up to now was based on presentations by David Woodhouse,
> > > so it matches with his viewpoint in the thread at [3]. And as far as I can
> > > tell Rafael Wysocki agrees with him in this thread.
> > >
> > > What I could not find in either of the references is that PRP0001 is only
> > > for debugging, I only know about this constraint from your mail. Could you
> > > point me to any source for this?
> > 
> > From [3], Rafael said:
> > "Let alone the fact that PRP0001 is actually quite useful at the
> > prototyping stage when it is premature to allocate a new device ID just
> > yet.  Taking that to the extreme, if someone whittles a board in his or
> > her garage and wants to use it to drive their homemade grass watering
> > system, having to invent a new device ID and put it into the existing
> > driver that otherwise doesn't require any modifications is ... you know
> > what I mean."
> > 
> > It implies that the process should have included the allocation of an
> > official ACPI ID.
> 
> I understand the quoted paragraph differently. In the paragraphs above Rafael
> argues that PRP0001 is useful, as with PRP0001 we can avoid registering
> redundant ACPI IDs. In the quoted paragraph he then gives another example
> where PRP0001 is also useful: for debugging.
> But I don't understand it in a way that PRP0001 would be limited to only
> debugging.
> 
> A few posts before in [4] he explains in more detail, and I think it matches
> my understanding of the last post, especially the following sentence:
> 
> "It would be a failure if people had to write separate drivers for 
> ACPI-based and DT-based platforms to handle the same hardware, at least 
> at the leaf driver level."
> 

Maybe better to ask him directly?

> [4] https://patchwork.kernel.org/comment/15339311/
> 
> > You always can ask ASWG for the clarification. Maybe I learn something
> > new about PRP0001 :-)
> 
> I had no interaction with ASWG before so I'm not sure what the process is,
> but I will look it up.
> 
> > > > > > If PRP0001 is only for debugging, then I must also have misunderstood the
> > > > > > Linux "device-property" API (define in include/linux/property.h).
> > > > >
> > > > > Not exactly.
> > > > >
> > > > > > There are some presentations available on the internet, e.g. [1], that I
> > > > > > understand like PRP0001 + "device-property" API provide a way do access data
> > > > > > from either Devicetree or ACPI, depending on what kind of platform you are on.
> > > > >
> > > > > No, these are not hard linked to each other (the relation is that
> > > > > PRP0001 is a way to enumerate devices, which don't have dedicated ACPI
> > > > > _HID, by using compatible property [1]). The _DSD per se (i.o.w.
> > > > > device properties implementation in ACPI) is a different story [2].
> > > > >
> > > > > And I put [3] here, interesting to read. However, at that time I was
> > > > > quite far from this topic.
> > > > >
> > > > > [1]: https://www.kernel.org/doc/html/latest/firmware-guide/acpi/enumeration.html#device-tree-namespace-link-device-id
> > > > > [2]: https://uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel-1_2-3.htm.
> > > > > [3]: https://patchwork.kernel.org/patch/7004241/
> > > > >
> > > > > > [1] https://elinux.org/images/2/2d/Device_tree_acpi_compatibility-david_woodhouse-kernel_recipes_2015.pdf
> 
> regards, Wolfgang
Andy Shevchenko April 15, 2020, 3:17 p.m. UTC | #13
On Wed, Apr 15, 2020 at 06:15:39PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 15, 2020 at 04:57:33PM +0200, Wolfgang Wallner wrote:
> > > On Wed, Apr 15, 2020 at 5:00 PM Wolfgang Wallner
> > > <wolfgang.wallner@br-automation.com> wrote:
> > > > -----"Andy Shevchenko" <andy.shevchenko@gmail.com> schrieb: -----
> > > > > Betreff: Re: Re: [PATCH v3 13/29] dts: Add a binding for hid-over-i2c
> > > > >
> > > > > On Wed, Apr 8, 2020 at 11:40 PM Andy Shevchenko
> > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > On Wed, Apr 8, 2020 at 10:39 PM Wolfgang Wallner
> > > > > > <wolfgang.wallner@br-automation.com> wrote:
> > > > > > > > On Tue, Apr 07, 2020 at 08:58:13PM -0600, Simon Glass wrote:
> > > > > > > > > On Tue, 31 Mar 2020 at 13:25, Wolfgang Wallner
> > > > > > > > > <wolfgang.wallner@br-automation.com> wrote:
> > > > > > > > > > >An: u-boot@lists.denx.de
> > > > > > > > > > >Von: "Simon Glass" <sjg@chromium.org>
> > > > > > > > > > >Datum: 31.03.2020 01:14
> > > > > > > > > > >Kopie: "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
> > > > > > > > > > >"Wolfgang Wallner" <wolfgang.wallner@br-automation.com>, "Leif
> > > > > > > > > > >Lindholm" <leif@nuviainc.com>, "Simon Glass" <sjg@chromium.org>
> > > > > > > > > > >Betreff: [PATCH v3 14/29] acpi: Add a binding for ACPI settings in
> > > > > > > > > > >the device tree
> > > > > > > >
> > > > > > > > > > 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).
> > > > > > > >
> > > > > > > > Before we are going further with this here is a BIG CAVEAT.
> > > > > > > >
> > > > > > > > PRP0001   MUST NOT be used in production devices.
> > > > > > > >
> > > > > > > > This has been derived solely for debugging / pre-production testing / etc
> > > > > > > > purposes. The real devices must have an official ACPI _HID.
> > > > > > >
> > > > > > > Thanks for pointing this out! I was not aware of this.
> > > > > > > I have tried to understand how the PRP0001 is meant to be used, but could not
> > > > > > > find sufficient documentation. The best document I could find is
> > > > > > > Documentation/firmware-guide/acpi/enumeration.rst in the Linux kernel, and
> > > > > > > as far as I can tell the constraint that you mention is also not described
> > > > > > > there.
> > > > > > >
> > > > > > > Do you know any other resources regarding PRP0001, e.g. some kind of
> > > > > > > speficiation?
> > > > > >
> > > > > > I guess the best one is to ask somebody from UEFI Forum / ASWG. PRP is
> > > > > > a PNP ID for UEFI Forum.
> > > > >
> > > > > Basically last message in [3] from Rafael mentions his view on
> > > > > PRP0001. I guess there is still no document, although as I noticed the
> > > > > PRP prefix become official in a mean time.
> > > >
> > > > Thanks for the references. I have read them carefully, especially the thread
> > > > in [3].
> > > >
> > > > My understanding up to now was based on presentations by David Woodhouse,
> > > > so it matches with his viewpoint in the thread at [3]. And as far as I can
> > > > tell Rafael Wysocki agrees with him in this thread.
> > > >
> > > > What I could not find in either of the references is that PRP0001 is only
> > > > for debugging, I only know about this constraint from your mail. Could you
> > > > point me to any source for this?
> > > 
> > > From [3], Rafael said:
> > > "Let alone the fact that PRP0001 is actually quite useful at the
> > > prototyping stage when it is premature to allocate a new device ID just
> > > yet.  Taking that to the extreme, if someone whittles a board in his or
> > > her garage and wants to use it to drive their homemade grass watering
> > > system, having to invent a new device ID and put it into the existing
> > > driver that otherwise doesn't require any modifications is ... you know
> > > what I mean."
> > > 
> > > It implies that the process should have included the allocation of an
> > > official ACPI ID.
> > 
> > I understand the quoted paragraph differently. In the paragraphs above Rafael
> > argues that PRP0001 is useful, as with PRP0001 we can avoid registering
> > redundant ACPI IDs. In the quoted paragraph he then gives another example
> > where PRP0001 is also useful: for debugging.
> > But I don't understand it in a way that PRP0001 would be limited to only
> > debugging.
> > 
> > A few posts before in [4] he explains in more detail, and I think it matches
> > my understanding of the last post, especially the following sentence:
> > 
> > "It would be a failure if people had to write separate drivers for 
> > ACPI-based and DT-based platforms to handle the same hardware, at least 
> > at the leaf driver level."
> > 
> 
> Maybe better to ask him directly?

I guess here is a root of misunderstanding, i.e. PRP0001 as _HID() vs. Device
Properties as _DSD() method.

I think you quoted the thoughts WRT latter, and not related to the _HID() part!

> > [4] https://patchwork.kernel.org/comment/15339311/
> > 
> > > You always can ask ASWG for the clarification. Maybe I learn something
> > > new about PRP0001 :-)
> > 
> > I had no interaction with ASWG before so I'm not sure what the process is,
> > but I will look it up.
> > 
> > > > > > > If PRP0001 is only for debugging, then I must also have misunderstood the
> > > > > > > Linux "device-property" API (define in include/linux/property.h).
> > > > > >
> > > > > > Not exactly.
> > > > > >
> > > > > > > There are some presentations available on the internet, e.g. [1], that I
> > > > > > > understand like PRP0001 + "device-property" API provide a way do access data
> > > > > > > from either Devicetree or ACPI, depending on what kind of platform you are on.
> > > > > >
> > > > > > No, these are not hard linked to each other (the relation is that
> > > > > > PRP0001 is a way to enumerate devices, which don't have dedicated ACPI
> > > > > > _HID, by using compatible property [1]). The _DSD per se (i.o.w.
> > > > > > device properties implementation in ACPI) is a different story [2].
> > > > > >
> > > > > > And I put [3] here, interesting to read. However, at that time I was
> > > > > > quite far from this topic.
> > > > > >
> > > > > > [1]: https://www.kernel.org/doc/html/latest/firmware-guide/acpi/enumeration.html#device-tree-namespace-link-device-id
> > > > > > [2]: https://uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel-1_2-3.htm.
> > > > > > [3]: https://patchwork.kernel.org/patch/7004241/
> > > > > >
> > > > > > > [1] https://elinux.org/images/2/2d/Device_tree_acpi_compatibility-david_woodhouse-kernel_recipes_2015.pdf
> > 
> > regards, Wolfgang
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>
diff mbox series

Patch

diff --git a/doc/device-tree-bindings/input/hid-over-i2c.txt b/doc/device-tree-bindings/input/hid-over-i2c.txt
new file mode 100644
index 00000000000..c76bafaf98d
--- /dev/null
+++ b/doc/device-tree-bindings/input/hid-over-i2c.txt
@@ -0,0 +1,44 @@ 
+* HID over I2C Device-Tree bindings
+
+HID over I2C provides support for various Human Interface Devices over the
+I2C bus. These devices can be for example touchpads, keyboards, touch screens
+or sensors.
+
+The specification has been written by Microsoft and is currently available here:
+http://msdn.microsoft.com/en-us/library/windows/hardware/hh852380.aspx
+
+If this binding is used, the kernel module i2c-hid will handle the communication
+with the device and the generic hid core layer will handle the protocol.
+
+Required properties:
+- compatible: must be "hid-over-i2c"
+- reg: i2c slave address
+- hid-descr-addr: HID descriptor address
+- interrupts: interrupt line
+
+Additional optional properties:
+
+Some devices may support additional optional properties to help with, e.g.,
+power sequencing. The following properties can be supported by one or more
+device-specific compatible properties, which should be used in addition to the
+"hid-over-i2c" string.
+
+- compatible:
+  * "wacom,w9013" (Wacom W9013 digitizer). Supports:
+    - vdd-supply (3.3V)
+    - vddl-supply (1.8V)
+    - post-power-on-delay-ms
+
+- vdd-supply: phandle of the regulator that provides the supply voltage.
+- post-power-on-delay-ms: time required by the device after enabling its regulators
+  or powering it on, before it is ready for communication.
+
+Example:
+
+	i2c-hid-dev@2c {
+		compatible = "hid-over-i2c";
+		reg = <0x2c>;
+		hid-descr-addr = <0x0020>;
+		interrupt-parent = <&gpx3>;
+		interrupts = <3 2>;
+	};