diff mbox series

[v2,13/39] acpi: Add a binding for ACPI settings in the device tree

Message ID 20200308214442.v2.13.I7842b2dd0d6b475301fc044c6640d8089873053f@changeid
State New
Headers show
Series dm: Add programmatic generation of ACPI tables (part A) | expand

Commit Message

Simon Glass March 9, 2020, 3:44 a.m. UTC
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 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 +++++++++++++++
 .../input/hid-over-i2c.txt                    | 44 +++++++++++++++++++
 2 files changed, 80 insertions(+)
 create mode 100644 doc/device-tree-bindings/device.txt
 create mode 100644 doc/device-tree-bindings/input/hid-over-i2c.txt

Comments

Wolfgang Wallner March 10, 2020, 9:15 a.m. UTC | #1
Hi Simon,

-----"Simon Glass" <sjg at chromium.org> schrieb: -----
> 
> 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 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 +++++++++++++++
>  .../input/hid-over-i2c.txt                    | 44 +++++++++++++++++++
>  2 files changed, 80 insertions(+)
>  create mode 100644 doc/device-tree-bindings/device.txt
>  create mode 100644 doc/device-tree-bindings/input/hid-over-i2c.txt
> 
> diff --git a/doc/device-tree-bindings/device.txt b/doc/device-tree-bindings/device.txt
> new file mode 100644
> index 0000000000..31ec2fa31b
> --- /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.
> +
> + - acpi,has-power-resource : (boolean) true if this device has a power resource.
> +    This causes a PRIC (ACPI PowerResource) to be written containing the

What is the meaning of PRIC? I can't find a defition for this term.

> +    properties provided by this binding, to describe how to handle powering the
> +    device up and down using GPIOs
> + - acpi,compatible : compatible string to report

What does "compatible string" mean in this context? Does it refer to the 
"Compatible ID" (_CID) of ACPI? As stated in the previous mail thread [1],
I think we could infer many of the ACPI properties from existing device tree
properties. Especially I suspect that ACPI's _CID could have a 1:1 mapping
to the compatible property in device tree. E.g. a driver which states that
it is compatible with "hid-over-i2c" in its device tree description would
know (implement internally) that it is also compatible with ACPI's _CID
"PNP0C50", thus we would not have to add this information in the device tree.

[1] https://lists.denx.de/pipermail/u-boot/2020-February/398856.html

> + - acpi,desc : Contains the string to use as the _DDN (DOS (Disk Operating
> +    System) Device Name)

Nit: I assume "desc" stands for "description". But strictly speaking it is
not a description, it is a device name. I would prefer something like
"acpi,devname" or even "acpi,ddn".

> + - acpi,hid : Contains the string to use as the HID (Hardware ID)
> +    identifier _HID
> + - hid-descr-addr : HID register offset (for Human Interface Devices)

This property is already described in the file hid-over-i2c.txt (with a similar
but slightly different text), thus I would not describe it here also.

> + - acpi,probed : Tells U-Boot to add 'linux,probed' to the ACPI tables so that
> +    Linux will not re-init the device

I can't find 'linux,probed' in the Linux kernel.
I only find it in patches specific for chromium-os[2], but the description
there does not match the description given here (there it is described as
being probed before insertion vs. here it is described as not being probed
any more).

[2] https://groups.google.com/a/chromium.org/forum/#!msg/chromium-os-reviews/4HTHl78IGHw/oz82uImnBgAJ

> + - acpi,uid : _UID value for device
> +
> +
> +Example
> +-------
> +
> +synaptics_touchpad: synaptics-touchpad at 2c {
> +	compatible = "hid-over-i2c";
> +	reg = <0x2c>;
> +	acpi,hid = "PNP0C50";
> +	acpi,desc = "Synaptics Touchpad";
> +	interrupts-extended = <&acpi_gpe GPIO_18_IRQ
> +			IRQ_TYPE_EDGE_FALLING>;
> +	acpi,probed;
> +	hid-descr-addr = <0x20>;
> +};
> 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

Adding the file hid-over-i2c.txt is straight forward, while getting the ACPI
bindings correct is probably more tricky. Would it make sense to add this file
in an individual patch?

> index 0000000000..c76bafaf98
> --- /dev/null
> +++ b/doc/device-tree-bindings/input/hid-over-i2c.txt

This file is taken from the Linux kernel, should this be stated at the
beginning? (I often see "Taken from ..." in source code files, I'm not sure
how this is handled for documentation).

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

This sentence is specific to the Linux kernel, and does not really apply to
U-Boot. I would propose one of the following:

* Point out specifically that the Linux kernel is talked about
* Drop the sentence
* State at the beginning of the file that the file is taken unmodified from the
  Linux kernel

> +
> +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 at 2c {
> +		compatible = "hid-over-i2c";
> +		reg = <0x2c>;
> +		hid-descr-addr = <0x0020>;
> +		interrupt-parent = <&gpx3>;
> +		interrupts = <3 2>;
> +	};
> -- 
> 2.25.1.481.gfbce0eb801-goog
> 

regards, Wolfgang
Andy Shevchenko March 10, 2020, 2:50 p.m. UTC | #2
On Sun, Mar 08, 2020 at 09:44:37PM -0600, Simon Glass wrote:
> 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.

...

> +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.
> +
> + - acpi,has-power-resource : (boolean) true if this device has a power resource.
> +    This causes a PRIC (ACPI PowerResource) to be written containing the

What is PRIC?

> +    properties provided by this binding, to describe how to handle powering the
> +    device up and down using GPIOs

> + - acpi,compatible : compatible string to report

Hmm... I didn't get this. Is it ACPI _CID?

> + - acpi,desc : 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

HID can be dropped to avoid confusion with below.

> + - hid-descr-addr : HID register offset (for Human Interface Devices)

> + - acpi,probed : Tells U-Boot to add 'linux,probed' to the ACPI tables so that
> +    Linux will not re-init the device

Why? How do we know that Linux will work correctly? Again, we must not depend
on the OS behaviour.

> + - acpi,uid : _UID value for device
> +
> +
> +Example
> +-------
> +
> +synaptics_touchpad: synaptics-touchpad at 2c {
> +	compatible = "hid-over-i2c";
> +	reg = <0x2c>;
> +	acpi,hid = "PNP0C50";
> +	acpi,desc = "Synaptics Touchpad";
> +	interrupts-extended = <&acpi_gpe GPIO_18_IRQ
> +			IRQ_TYPE_EDGE_FALLING>;
> +	acpi,probed;
> +	hid-descr-addr = <0x20>;
> +};
Simon Glass March 12, 2020, 3:22 a.m. UTC | #3
Hi Andy,

On Tue, 10 Mar 2020 at 08:51, Andy Shevchenko <
andriy.shevchenko at linux.intel.com> wrote:
>
> On Sun, Mar 08, 2020 at 09:44:37PM -0600, Simon Glass wrote:
> > 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.
>
> ...
>
> > +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.
> > +
> > + - acpi,has-power-resource : (boolean) true if this device has a power
resource.
> > +    This causes a PRIC (ACPI PowerResource) to be written containing
the
>
> What is PRIC?
>
> > +    properties provided by this binding, to describe how to handle
powering the
> > +    device up and down using GPIOs
>
> > + - acpi,compatible : compatible string to report
>
> Hmm... I didn't get this. Is it ACPI _CID?

Will add a pointer to the Linux ACPI docs for this.

>
> > + - acpi,desc : 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
>
> HID can be dropped to avoid confusion with below.

OK I am actually dropping the next line.

>
> > + - hid-descr-addr : HID register offset (for Human Interface Devices)
>
> > + - acpi,probed : Tells U-Boot to add 'linux,probed' to the ACPI tables
so that
> > +    Linux will not re-init the device
>
> Why? How do we know that Linux will work correctly? Again, we must not
depend
> on the OS behaviour.

The wording is incorrect, will update. This is effectively an 'optional'
device.

>
> > + - acpi,uid : _UID value for device
> > +
> > +
> > +Example
> > +-------
> > +
> > +synaptics_touchpad: synaptics-touchpad at 2c {
> > +     compatible = "hid-over-i2c";
> > +     reg = <0x2c>;
> > +     acpi,hid = "PNP0C50";
> > +     acpi,desc = "Synaptics Touchpad";
> > +     interrupts-extended = <&acpi_gpe GPIO_18_IRQ
> > +                     IRQ_TYPE_EDGE_FALLING>;
> > +     acpi,probed;
> > +     hid-descr-addr = <0x20>;
> > +};

Regards,
Simon
Simon Glass March 12, 2020, 3:24 a.m. UTC | #4
Hi Wolfgang,

On Tue, 10 Mar 2020 at 03:15, Wolfgang Wallner <
wolfgang.wallner at br-automation.com> wrote:
>
> Hi Simon,
>
> -----"Simon Glass" <sjg at chromium.org> schrieb: -----
> >
> > 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 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 +++++++++++++++
> >  .../input/hid-over-i2c.txt                    | 44 +++++++++++++++++++
> >  2 files changed, 80 insertions(+)
> >  create mode 100644 doc/device-tree-bindings/device.txt
> >  create mode 100644 doc/device-tree-bindings/input/hid-over-i2c.txt
> >
> > diff --git a/doc/device-tree-bindings/device.txt
b/doc/device-tree-bindings/device.txt
> > new file mode 100644
> > index 0000000000..31ec2fa31b
> > --- /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.
> > +
> > + - acpi,has-power-resource : (boolean) true if this device has a power
resource.
> > +    This causes a PRIC (ACPI PowerResource) to be written containing
the
>
> What is the meaning of PRIC? I can't find a defition for this term.

I'll drop that.

>
> > +    properties provided by this binding, to describe how to handle
powering the
> > +    device up and down using GPIOs
> > + - acpi,compatible : compatible string to report
>
> What does "compatible string" mean in this context? Does it refer to the
> "Compatible ID" (_CID) of ACPI? As stated in the previous mail thread [1],
> I think we could infer many of the ACPI properties from existing device
tree
> properties. Especially I suspect that ACPI's _CID could have a 1:1 mapping
> to the compatible property in device tree. E.g. a driver which states that
> it is compatible with "hid-over-i2c" in its device tree description would
> know (implement internally) that it is also compatible with ACPI's _CID
> "PNP0C50", thus we would not have to add this information in the device
tree.

This is described to in the Linux binding file:

Documentation/acpi/enumeration.txt

The special DT namespace link device ID, PRP0001, provides a means to use
the
existing DT-compatible device identification in ACPI and to satisfy the
above
requirements following from the ACPI specification at the same time.
Namely,
if PRP0001 is returned by _HID, the ACPI subsystem will look for the
"compatible" property in the device object's _DSD and will use the value of
that
property to identify the corresponding device in analogy with the original
DT
device identification algorithm.  If the "compatible" property is not
present
or its value is not valid, the device will not be enumerated by the ACPI
subsystem.  Otherwise, it will be enumerated automatically as a platform
device
(except when an I2C or SPI link from the device to its parent is present, in
which case the ACPI core will leave the device enumeration to the parent's
driver) and the identification strings from the "compatible" property value
will
be used to find a driver for the device along with the device IDs listed by
_CID
(if present).


So we set acpi,hid to "PRP0001" and acpi,compatible to the device-tree
compatible string.

>
> [1] https://lists.denx.de/pipermail/u-boot/2020-February/398856.html
>
> > + - acpi,desc : Contains the string to use as the _DDN (DOS (Disk
Operating
> > +    System) Device Name)
>
> Nit: I assume "desc" stands for "description". But strictly speaking it is
> not a description, it is a device name. I would prefer something like
> "acpi,devname" or even "acpi,ddn".

OK we can go with acpi,ddn. I have been thinking about that as it is one
less mapping to know.

>
> > + - acpi,hid : Contains the string to use as the HID (Hardware ID)
> > +    identifier _HID
> > + - hid-descr-addr : HID register offset (for Human Interface Devices)
>
> This property is already described in the file hid-over-i2c.txt (with a
similar
> but slightly different text), thus I would not describe it here also.

Will drop.

>
> > + - acpi,probed : Tells U-Boot to add 'linux,probed' to the ACPI tables
so that
> > +    Linux will not re-init the device
>
> I can't find 'linux,probed' in the Linux kernel.
> I only find it in patches specific for chromium-os[2], but the description
> there does not match the description given here (there it is described as
> being probed before insertion vs. here it is described as not being probed
> any more).
>
> [2]
https://groups.google.com/a/chromium.org/forum/#!msg/chromium-os-reviews/4HTHl78IGHw/oz82uImnBgAJ

Yes it seems that this has not yet made it upstream to Linux.

I will fix the description.

>
> > + - acpi,uid : _UID value for device
> > +
> > +
> > +Example
> > +-------
> > +
> > +synaptics_touchpad: synaptics-touchpad at 2c {
> > +     compatible = "hid-over-i2c";
> > +     reg = <0x2c>;
> > +     acpi,hid = "PNP0C50";
> > +     acpi,desc = "Synaptics Touchpad";
> > +     interrupts-extended = <&acpi_gpe GPIO_18_IRQ
> > +                     IRQ_TYPE_EDGE_FALLING>;
> > +     acpi,probed;
> > +     hid-descr-addr = <0x20>;
> > +};
> > 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
>
> Adding the file hid-over-i2c.txt is straight forward, while getting the
ACPI
> bindings correct is probably more tricky. Would it make sense to add this
file
> in an individual patch?

OK I'll split it.

>
> > index 0000000000..c76bafaf98
> > --- /dev/null
> > +++ b/doc/device-tree-bindings/input/hid-over-i2c.txt
>
> This file is taken from the Linux kernel, should this be stated at the
> beginning? (I often see "Taken from ..." in source code files, I'm not
sure
> how this is handled for documentation).

Yes we normally mention that.

>
> > @@ -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.
>
> This sentence is specific to the Linux kernel, and does not really apply
to
> U-Boot. I would propose one of the following:
>
> * Point out specifically that the Linux kernel is talked about
> * Drop the sentence
> * State at the beginning of the file that the file is taken unmodified
from the
>   Linux kernel

Yes it's funny that the bindings are supposed to describe hardware but
sometimes the software creeps in.

In fact binding files are always supposed to be unmodified, so I don't
think we can/should change it.

Regards,
Simon
Wolfgang Wallner March 12, 2020, 12:44 p.m. UTC | #5
Hi Simon,

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

[snip]

>  > > +    properties provided by this binding, to describe how to handle powering the
>  > > +    device up and down using GPIOs
>  > > + - acpi,compatible : compatible string to report
>  >
>  > What does "compatible string" mean in this context? Does it refer to the
>  > "Compatible ID" (_CID) of ACPI? As stated in the previous mail thread [1],
>  > I think we could infer many of the ACPI properties from existing device tree
>  > properties. Especially I suspect that ACPI's _CID could have a 1:1 mapping
>  > to the compatible property in device tree. E.g. a driver which states that
>  > it is compatible with "hid-over-i2c" in its device tree description would
>  > know (implement internally) that it is also compatible with ACPI's _CID
>  > "PNP0C50", thus we would not have to add this information in the device tree.
>  
>  This is described to in the Linux binding file:
>  
>  Documentation/acpi/enumeration.txt
>  
>  The special DT namespace link device ID, PRP0001, provides a means to use the
>  existing DT-compatible device identification in ACPI and to satisfy the above
>  requirements following from the ACPI specification at the same time.  Namely,
>  if PRP0001 is returned by _HID, the ACPI subsystem will look for the
>  "compatible" property in the device object's _DSD and will use the value of that
>  property to identify the corresponding device in analogy with the original DT
>  device identification algorithm.  If the "compatible" property is not present
>  or its value is not valid, the device will not be enumerated by the ACPI
>  subsystem.  Otherwise, it will be enumerated automatically as a platform device
>  (except when an I2C or SPI link from the device to its parent is present, in
>  which case the ACPI core will leave the device enumeration to the parent's
>  driver) and the identification strings from the "compatible" property value will
>  be used to find a driver for the device along with the device IDs listed by _CID
>  (if present).
>  
>  
>  So we set acpi,hid to "PRP0001" and acpi,compatible to the device-tree compatible string.
  
I think there are different cases that can be distinguished in which we need to
translate between ACPI and device tree.

1) The OS has an existing ACPI binding for the device. U-Boot uses device tree
   for device description, but the OS uses ACPI (e.g. Linux on x86).
   We need a way to tell U-Boot how to create the appropriate ACPI descriptons.

2) There is no existing ACPI binding for the device in the OS, but there is
   already an existing device tree binding. As stated in the Linux documentation
   you have referred to, it would not make sense to introduce a new ACPI binding
   that would just be the same as the existing device tree one. As I understand
   this is what the "PRP0001" tag was introduced for (to reuse existing device
   tree descriptions and save redundant work).

Handling case 2:
PRP0001 allows to wire existing device tree properties into ACPI descriptions,
and I think it provides a solution for most of case 2. So for case 2 I think
most of the ACPI description can be autogenerated:
  * Set _HID to "PRP0001"
  * Pack all existing device tree properties into the ACPI _DSD method
    (this also implies that no additional "acpi,compatible" property is
    required, as we can reuse the existing "compatible" property)

    I say "most of" because I don't know how to map certain parts between
    device tree and ACPI, e.g. interrupt descriptions.    
    
Handling case 1:
Case 1 is more tricky, and up to now I was assuming you are targeting case 1
with the new additional properties. It might not be as straigt forward as in
case 2, but I think also in this case we can infer most of the ACPI properties
from the existing device tree properties without adding new ones. This is the
example from the previous mail discussions where e.g. from a device tree
"compatible" property "hid-over-i2c" we could infer the ACPI _HID "PNP0C50"
without adding new properties in the device tree.

Again, I have left out more complex things like interrupts in this description.

Anyway, my experience with both device tree as well as ACPI is limited, so
please take all my statements with a grain of salt.

regards, Wolfgang
Simon Glass March 13, 2020, 12:36 a.m. UTC | #6
Hi Wolfgang,

On Thu, 12 Mar 2020 at 06:44, Wolfgang Wallner
<wolfgang.wallner at br-automation.com> wrote:
>
> Hi Simon,
>
> -----"Simon Glass" <sjg at chromium.org> schrieb: -----
>
> [snip]
>
> >  > > +    properties provided by this binding, to describe how to handle powering the
> >  > > +    device up and down using GPIOs
> >  > > + - acpi,compatible : compatible string to report
> >  >
> >  > What does "compatible string" mean in this context? Does it refer to the
> >  > "Compatible ID" (_CID) of ACPI? As stated in the previous mail thread [1],
> >  > I think we could infer many of the ACPI properties from existing device tree
> >  > properties. Especially I suspect that ACPI's _CID could have a 1:1 mapping
> >  > to the compatible property in device tree. E.g. a driver which states that
> >  > it is compatible with "hid-over-i2c" in its device tree description would
> >  > know (implement internally) that it is also compatible with ACPI's _CID
> >  > "PNP0C50", thus we would not have to add this information in the device tree.
> >
> >  This is described to in the Linux binding file:
> >
> >  Documentation/acpi/enumeration.txt
> >
> >  The special DT namespace link device ID, PRP0001, provides a means to use the
> >  existing DT-compatible device identification in ACPI and to satisfy the above
> >  requirements following from the ACPI specification at the same time.  Namely,
> >  if PRP0001 is returned by _HID, the ACPI subsystem will look for the
> >  "compatible" property in the device object's _DSD and will use the value of that
> >  property to identify the corresponding device in analogy with the original DT
> >  device identification algorithm.  If the "compatible" property is not present
> >  or its value is not valid, the device will not be enumerated by the ACPI
> >  subsystem.  Otherwise, it will be enumerated automatically as a platform device
> >  (except when an I2C or SPI link from the device to its parent is present, in
> >  which case the ACPI core will leave the device enumeration to the parent's
> >  driver) and the identification strings from the "compatible" property value will
> >  be used to find a driver for the device along with the device IDs listed by _CID
> >  (if present).
> >
> >
> >  So we set acpi,hid to "PRP0001" and acpi,compatible to the device-tree compatible string.
>
> I think there are different cases that can be distinguished in which we need to
> translate between ACPI and device tree.
>
> 1) The OS has an existing ACPI binding for the device. U-Boot uses device tree
>    for device description, but the OS uses ACPI (e.g. Linux on x86).
>    We need a way to tell U-Boot how to create the appropriate ACPI descriptons.

For now I'd like to do this in the U-Boot driver. We have so few
drivers doing it that I don't think we can generalise this yet. If we
start to see a pattern then we might start with some common code...

>
> 2) There is no existing ACPI binding for the device in the OS, but there is
>    already an existing device tree binding. As stated in the Linux documentation
>    you have referred to, it would not make sense to introduce a new ACPI binding
>    that would just be the same as the existing device tree one. As I understand
>    this is what the "PRP0001" tag was introduced for (to reuse existing device
>    tree descriptions and save redundant work).
>
> Handling case 2:
> PRP0001 allows to wire existing device tree properties into ACPI descriptions,
> and I think it provides a solution for most of case 2. So for case 2 I think
> most of the ACPI description can be autogenerated:
>   * Set _HID to "PRP0001"
>   * Pack all existing device tree properties into the ACPI _DSD method
>     (this also implies that no additional "acpi,compatible" property is
>     required, as we can reuse the existing "compatible" property)
>
>     I say "most of" because I don't know how to map certain parts between
>     device tree and ACPI, e.g. interrupt descriptions.

...and perhaps end up with something like this. But I really don't
feel comfortable trying to build out something like this with so few
use cases. We don't even have one* in coral yet.

>
> Handling case 1:
> Case 1 is more tricky, and up to now I was assuming you are targeting case 1
> with the new additional properties. It might not be as straigt forward as in
> case 2, but I think also in this case we can infer most of the ACPI properties
> from the existing device tree properties without adding new ones. This is the
> example from the previous mail discussions where e.g. from a device tree
> "compatible" property "hid-over-i2c" we could infer the ACPI _HID "PNP0C50"
> without adding new properties in the device tree.
>
> Again, I have left out more complex things like interrupts in this description.
>
> Anyway, my experience with both device tree as well as ACPI is limited, so
> please take all my statements with a grain of salt.

I suspect they may be some things we can do here to automate this. But
again, this is for the future when we actually start to see these
cases.

I have done the hid-over-i2c compatible string (automatically setting
CID to PNP0C50) as we have two coral drivers that do that.

Regards,
Simon
diff mbox series

Patch

diff --git a/doc/device-tree-bindings/device.txt b/doc/device-tree-bindings/device.txt
new file mode 100644
index 0000000000..31ec2fa31b
--- /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.
+
+ - acpi,has-power-resource : (boolean) true if this device has a power resource.
+    This causes a PRIC (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,desc : 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
+ - hid-descr-addr : HID register offset (for Human Interface Devices)
+ - acpi,probed : Tells U-Boot to add 'linux,probed' to the ACPI tables so that
+    Linux will not re-init the device
+ - acpi,uid : _UID value for device
+
+
+Example
+-------
+
+synaptics_touchpad: synaptics-touchpad at 2c {
+	compatible = "hid-over-i2c";
+	reg = <0x2c>;
+	acpi,hid = "PNP0C50";
+	acpi,desc = "Synaptics Touchpad";
+	interrupts-extended = <&acpi_gpe GPIO_18_IRQ
+			IRQ_TYPE_EDGE_FALLING>;
+	acpi,probed;
+	hid-descr-addr = <0x20>;
+};
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 0000000000..c76bafaf98
--- /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 at 2c {
+		compatible = "hid-over-i2c";
+		reg = <0x2c>;
+		hid-descr-addr = <0x0020>;
+		interrupt-parent = <&gpx3>;
+		interrupts = <3 2>;
+	};