[013/108] acpi: Add a binding for ACPI settings in the device tree

Message ID 20200126220508.13.I7842b2dd0d6b475301fc044c6640d8089873053f@changeid
State New
Headers show
Series
  • RFC: dm: Add programatic generation of ACPI tables
Related show

Commit Message

Simon Glass Jan. 27, 2020, 5:05 a.m.
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>
---

 doc/device-tree-bindings/device.txt | 37 +++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 doc/device-tree-bindings/device.txt

Comments

Wolfgang Wallner Jan. 31, 2020, 8:16 a.m. | #1
Hi Simon,

> +Devices
> +=======
> +
> +Device bindings are described by their own individual binding files.
> +
> +U-Boot provides for some optional properties which are documented here.
> +
> + - 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,cid : Contains the string to use as the compatible ID (_CID)
> + - 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 (Human Interface Device)
> +    identifier _HID

Nit: "_HID" in ACPI stands for "Hardware ID"

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

Would it make sense to automatically infer ACPI properties from existing
device tree properties?

E.g. if the compatible string contains "hid-over-i2c", then _CID could be set
to "PNP0C50".
And a "hid-over-i2c" device would also have a "hid-descr-addr" device tree
property for the offset of the HID descriptor register which could be passed
on in ACPI as part of the _DSM method.

> +
> +
> +Example
> +-------
> +
> +synaptics_touchpad: synaptics-touchpad at 2c {
> +	compatible = "i2c-chip";
> +	reg = <0x2c>;
> +	acpi,hid = "PNP0C50";
> +	acpi,cid = "PNP0C50";
> +	acpi,desc = "Synaptics Touchpad";
> +	interrupts-extended = <&acpi_gpe GPIO_18_IRQ
> +			IRQ_TYPE_EDGE_FALLING>;
> +	acpi,probed;
> +	acpi,hid-desc-reg-offset = <0x20>;

regards, Wolfgang
Simon Glass March 9, 2020, 3:26 a.m. | #2
Hi Wolfgang,

On Mon, 3 Feb 2020 at 02:52, Wolfgang Wallner
<wolfgang.wallner at br-automation.com> wrote:
>
> Hi Simon,
>
> > -----"Simon Glass" <sjg at chromium.org> schrieb: -----
> > Hi Wolfgang,
> >
> > On Fri, 31 Jan 2020 at 01:18, Wolfgang Wallner
> > <wolfgang.wallner at br-automation.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > > +Devices
> > > > +=======
> > > > +
> > > > +Device bindings are described by their own individual binding files.
> > > > +
> > > > +U-Boot provides for some optional properties which are documented here.
> > > > +
> > > > + - 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,cid : Contains the string to use as the compatible ID (_CID)
> > > > + - 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 (Human Interface Device)
> > > > +    identifier _HID
> > >
> > > Nit: "_HID" in ACPI stands for "Hardware ID"
> >
> > OK thank you, will fix. I am trying to write things out in full since
> > it is very hard to find out what all these different things mean.
>
> I appreciate that, it makes it easier for to follow the text.
>
> > > > + - acpi,hid-desc-reg-offset : 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
> > >
> > > Would it make sense to automatically infer ACPI properties from existing
> > > device tree properties?
> > >
> > > E.g. if the compatible string contains "hid-over-i2c", then _CID could be set
> > > to "PNP0C50".
> >
> > Yes we could do that. So is that true for all devices that use this i2c driver?
>
> As far as I understand the ids "PNP0C50" and "hid-over-i2c" serve the same
> purpose, just in two different description languages.
>
> Devicetree: If an I2C over HID device is described in Devicetree, it should
>             have a compatible string of "hid-over-i2c" (see [1] for details).
>
> ACPI:       If an I2C over HID device is described in ACPI, it should have a
>             _CID of "PNP0C50" (See [2] for details).
>
> [1] https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> [2] https://docs.microsoft.com/en-us/windows-hardware/drivers/hid/plug-and-play-support-and-power-management
>
> As both identifiers are standardized, I think we could infer one from the
> other.
>
> I'm not sure whether inferring ACPI-properties from Devicetree properties is
> worth it, but I wanted to bring up the idea for discussion as adding new
> properties for someting where we already have something similar feels
> redundant.
>
> > > And a "hid-over-i2c" device would also have a "hid-descr-addr" device tree
> > > property for the offset of the HID descriptor register which could be passed
> > > on in ACPI as part of the _DSM method.
> >
> > Are you suggesting renaming acpi,hid-desc-reg-offset to hid-descr-addr?
>
> An ACPI "PNP0C50"-device has a _DSM (Device Specific Method) which provides the
> HID descriptor offset (for UUID "3CDFF6F7-4267-4555-AD05-B30A3D8938DE").
>
> A Devicetree "hid-over-i2c"-device has a property "hid-descr-addr" which
> provides the HID descriptor offset.
>
> So instead of adding a new "acpi,hid-desc-reg-offset" I think we could also
> use the existing "hid-descr-addr" property.

OK I've updated that in v2.

>
> Interrupts are another topic that need to be described in both worlds:
>
>   Devicetree: "interrupts"-property
>   ACPI:       "_CRS"-method (Current Resource Settings)
>
> Maybe we could also infer that description too?

It might be possible, I think. If we do that we'll need to document it
somewhere. But CRS does have multiple GPIOs sometimes, so I'm not sure
if we would want to scan the DT node and add all of those to the CRS?
In any case this is something we can refine later since it doesn't
affect the binding.

Regards,
Simon

Patch

diff --git a/doc/device-tree-bindings/device.txt b/doc/device-tree-bindings/device.txt
new file mode 100644
index 0000000000..e625e67f43
--- /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.
+
+ - 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,cid : Contains the string to use as the compatible ID (_CID)
+ - 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 (Human Interface Device)
+    identifier _HID
+ - acpi,hid-desc-reg-offset : 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 = "i2c-chip";
+	reg = <0x2c>;
+	acpi,hid = "PNP0C50";
+	acpi,cid = "PNP0C50";
+	acpi,desc = "Synaptics Touchpad";
+	interrupts-extended = <&acpi_gpe GPIO_18_IRQ
+			IRQ_TYPE_EDGE_FALLING>;
+	acpi,probed;
+	acpi,hid-desc-reg-offset = <0x20>;
+};