| Message ID | 20200408165737.v5.14.I7842b2dd0d6b475301fc044c6640d8089873053f@changeid |
|---|---|
| State | Superseded |
| Headers | show |
| Series | dm: Add programmatic generation of ACPI tables (part A) | expand |
Hi Simon, -----"Simon Glass" <sjg at chromium.org> schrieb: ----- >Betreff: [PATCH v5 14/29] acpi: Add a binding for ACPI settings in >the device tree > >Devices need to report various identifiers in the ACPI tables. Rather >than >hard-coding these in drivers it is typically better to put them in >the >device tree. > >Add a binding file to describe this. > >Signed-off-by: Simon Glass <sjg at chromium.org> >--- > >Changes in v5: None >Changes in v4: >- Put 'interrupts-extended' property on one line >- Rename acpi-probed to linux,probed >- Note that linux,probed is an out-of-tree feature > >Changes in v3: >- Drop mention of PRIC >- Rename acpi,desc to acpi,ddn >- Correct description of acpi,probed >- Drop hid-descr-addr >- Just add the device.txt binding file in this patch >- Change the example to ELAN >- Add a pointer to information about acpi,compatible > >Changes in v2: >- Fix definition of HID >- Infer hid-over-i2c CID value >- Add the hid-over-i2c binding document > > doc/device-tree-bindings/device.txt | 36 >+++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > create mode 100644 doc/device-tree-bindings/device.txt > >diff --git a/doc/device-tree-bindings/device.txt >b/doc/device-tree-bindings/device.txt >new file mode 100644 >index 00000000000..27bd3978d98 >--- /dev/null >+++ b/doc/device-tree-bindings/device.txt >@@ -0,0 +1,36 @@ >+Devices >+======= >+ >+Device bindings are described by their own individual binding files. >+ >+U-Boot provides for some optional properties which are documented >here. See >+also hid-over-i2c.txt which describes HID devices. See also >+Documentation/firmware-guide/acpi/enumeration.rst in the Linux >kernel for >+the acpi,compatible property. >+ >+ - acpi,has-power-resource : (boolean) true if this device has a >power resource. >+ This causes an ACPI PowerResource to be written containing the >properties >+ provided by this binding, to describe how to handle powering the >device up >+ and down using GPIOs >+ - acpi,compatible : compatible string to report >+ - acpi,ddn : Contains the string to use as the _DDN (DOS (Disk >Operating >+ System) Device Name) >+ - acpi,hid : Contains the string to use as the HID (Hardware ID) >+ identifier _HID >+ - acpi,uid : _UID value for device >+ - linux,probed : Tells U-Boot to add 'linux,probed' to the ACPI >tables so that >+ Linux will only load the driver if the device can be detected >(e.g. on I2C >+ bus). Note that this is an out-of-tree Linux feature. >+ >+ >+Example >+------- >+ >+elan_touchscreen: elan-touchscreen at 10 { >+ compatible = "i2c-chip"; >+ reg = <0x10>; >+ acpi,hid = "ELAN0001"; >+ acpi,ddn = "ELAN Touchscreen"; >+ interrupts-extended = <&acpi_gpe GPIO_21_IRQ >IRQ_TYPE_EDGE_FALLING>; >+ linux,probed; >+}; >-- >2.26.0.292.g33ef6b2f38-goog I have learned through the previous review discussions aspects about this binding which are not captured in the current patch. I tried to incorporate these findings, the modified text is proposed below. Additionally, I realized that I still don't understand how some parts of the proposed binding are intended to work. I have highlighted these aspects with a TODO note. Please have a look. regards, Wolfgang -------------------------------------------------------------------------- Devices ======= Device bindings are described by their own individual binding files. U-Boot provides for some optional properties which are documented here. ACPI-related properties: - acpi,compatible : ACPI devices may report a _HID or _CID of "PRP0001", in which case they are expected to provide a _DSD object with a "compatible" property. The value of this "compatible" property is specified by "acpi,compatible". See also section 6.2.5 of [1] as well as [2-3] for details. TODO: will PRP0001 be used as _HID or as _CID? Note: the value for "acpi,compatible" can be different from the "compatible" property used in Devicetree for certain devices, e.g. when a custom driver in U-Boot would not make sense. An example would be touch screen devices. - acpi,ddn : Contains the string to use as the _DDN (DOS (Disk Operating System) Device Name) - acpi,has-power-resource : (boolean) true if this device has a power resource. This causes an ACPI PowerResource to be written containing the properties provided by this binding, to describe how to handle powering the device up and down using GPIOs TODO: a list of the required properties for this to work would be helpful - acpi,hid : Contains the string to use as the HID (Hardware ID) identifier _HID TODO: How does this work togheter with "acpi, compatible"? (assuming "acpi,compatible" implies a _HID of "PRP0001") - acpi,uid : _UID value for the device [1] https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf [2] Documentation/firmware-guide/acpi/enumeration.rst in the Linux kernel [3] http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf Other properties: - linux,probed : With this property Linux can be told to only load the driver if the device can be detected (e.g. on I2C bus). When generating ACPI tables, this property is adopted into a device's _DSD object. Note that this is an out-of-tree Linux feature. Example ------- elan_touchscreen: elan-touchscreen at 10 { compatible = "i2c-chip"; reg = <0x10>; acpi,hid = "ELAN0001"; acpi,ddn = "ELAN Touchscreen"; acpi,uid = 1; interrupts-extended = <&acpi_gpe GPIO_21_IRQ IRQ_TYPE_EDGE_FALLING>; linux,probed; }; The example above would result in an ACPI entry similar to the following: Device(DEV1) // TODO: how is the name ("DEV1" in this example) selected? { Name (_HID, "ELAN0001") Name (_DDN, "ELAN Touchscreen") Name (_UID, 1) Name (_DSD, Package (0x02) { ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), Package (0x01) { Package (0x02) { "linux,probed", Package (0x01) {1} }, } }) } -------------------------------------------------------------------------- I have dropped the sentence "See also hid-over-i2c.txt which describes HID devices.", as I don't think it is still required. HID devices are not mentioned any more as the previous hid-over-i2c example was dropped.
On Wed, Apr 15, 2020 at 7:16 PM Wolfgang Wallner <wolfgang.wallner at br-automation.com> wrote: > I have learned through the previous review discussions aspects about this > binding which are not captured in the current patch. I tried to incorporate > these findings, the modified text is proposed below. > > Additionally, I realized that I still don't understand how some parts of the > proposed binding are intended to work. I have highlighted these aspects with > a TODO note. Please have a look. > > regards, Wolfgang > > -------------------------------------------------------------------------- > > Devices > ======= > > Device bindings are described by their own individual binding files. > U-Boot provides for some optional properties which are documented here. > > ACPI-related properties: > - acpi,compatible : ACPI devices may report a _HID or _CID of "PRP0001", in > which case they are expected to provide a _DSD object with a "compatible" > property. No, this is simple incorrect. PRP0001 should not be mentioned at all. _DSD() as I said in the other thread is orthogonal to the ACPI ID. > The value of this "compatible" property is specified by > "acpi,compatible". > See also section 6.2.5 of [1] as well as [2-3] for details. > > TODO: will PRP0001 be used as _HID or as _CID? None. Please, forget about PRP0001. > TODO: How does this work togheter with "acpi, compatible"? (assuming > "acpi,compatible" implies a _HID of "PRP0001") Ditto.
Hi Simon, On Thu, Apr 9, 2020 at 6:58 AM Simon Glass <sjg at chromium.org> 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. > > Signed-off-by: Simon Glass <sjg at chromium.org> > --- > > Changes in v5: None > Changes in v4: > - Put 'interrupts-extended' property on one line > - Rename acpi-probed to linux,probed > - Note that linux,probed is an out-of-tree feature > > Changes in v3: > - Drop mention of PRIC > - Rename acpi,desc to acpi,ddn > - Correct description of acpi,probed > - Drop hid-descr-addr > - Just add the device.txt binding file in this patch > - Change the example to ELAN > - Add a pointer to information about acpi,compatible > > Changes in v2: > - Fix definition of HID > - Infer hid-over-i2c CID value > - Add the hid-over-i2c binding document > > doc/device-tree-bindings/device.txt | 36 +++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > create mode 100644 doc/device-tree-bindings/device.txt > It looks that there is still unclosed issue about this patch. I will leave this patch un-applied. Regards, Bin
Hi Andy, -----"Andy Shevchenko" <andy.shevchenko at gmail.com> schrieb: ----- >Betreff: Re: [PATCH v5 14/29] acpi: Add a binding for ACPI settings >in the device tree [snip] >> - acpi,compatible : ACPI devices may report a _HID or _CID of >"PRP0001", in >> which case they are expected to provide a _DSD object with a >"compatible" >> property. > >No, this is simple incorrect. >PRP0001 should not be mentioned at all. _DSD() as I said in the other >thread is orthogonal to the ACPI ID. Can "acpi,compatible" work in another way (without PRP0001)? Or do you mean that "acpi,compatible" can't work at all? As far as I see PRP0001 would be the only way to pass a "compatible" property in ACPI. [snip] regards, Wolfgang
On Thu, Apr 16, 2020 at 11:27 AM Wolfgang Wallner <wolfgang.wallner at br-automation.com> wrote: > -----"Andy Shevchenko" <andy.shevchenko at gmail.com> schrieb: ----- [snip] > >> - acpi,compatible : ACPI devices may report a _HID or _CID of > >"PRP0001", in > >> which case they are expected to provide a _DSD object with a > >"compatible" > >> property. > > > >No, this is simple incorrect. > >PRP0001 should not be mentioned at all. _DSD() as I said in the other > >thread is orthogonal to the ACPI ID. > > Can "acpi,compatible" work in another way (without PRP0001)? > Or do you mean that "acpi,compatible" can't work at all? Oh yeah, it can't work, unfortunately. So, compatible enumeration depends on PRP0001, which may not be advertised for the products, while end users can do that on their own. So, we may only generate ACPI excerpts for the devices that have an official ID allocated. > As far as I see PRP0001 would be the only way to pass a "compatible" > property in ACPI.
diff --git a/doc/device-tree-bindings/device.txt b/doc/device-tree-bindings/device.txt new file mode 100644 index 00000000000..27bd3978d98 --- /dev/null +++ b/doc/device-tree-bindings/device.txt @@ -0,0 +1,36 @@ +Devices +======= + +Device bindings are described by their own individual binding files. + +U-Boot provides for some optional properties which are documented here. See +also hid-over-i2c.txt which describes HID devices. See also +Documentation/firmware-guide/acpi/enumeration.rst in the Linux kernel for +the acpi,compatible property. + + - acpi,has-power-resource : (boolean) true if this device has a power resource. + This causes an ACPI PowerResource to be written containing the properties + provided by this binding, to describe how to handle powering the device up + and down using GPIOs + - acpi,compatible : compatible string to report + - acpi,ddn : Contains the string to use as the _DDN (DOS (Disk Operating + System) Device Name) + - acpi,hid : Contains the string to use as the HID (Hardware ID) + identifier _HID + - acpi,uid : _UID value for device + - linux,probed : Tells U-Boot to add 'linux,probed' to the ACPI tables so that + Linux will only load the driver if the device can be detected (e.g. on I2C + bus). Note that this is an out-of-tree Linux feature. + + +Example +------- + +elan_touchscreen: elan-touchscreen at 10 { + compatible = "i2c-chip"; + reg = <0x10>; + acpi,hid = "ELAN0001"; + acpi,ddn = "ELAN Touchscreen"; + interrupts-extended = <&acpi_gpe GPIO_21_IRQ IRQ_TYPE_EDGE_FALLING>; + linux,probed; +};
Devices need to report various identifiers in the ACPI tables. Rather than hard-coding these in drivers it is typically better to put them in the device tree. Add a binding file to describe this. Signed-off-by: Simon Glass <sjg at chromium.org> --- Changes in v5: None Changes in v4: - Put 'interrupts-extended' property on one line - Rename acpi-probed to linux,probed - Note that linux,probed is an out-of-tree feature Changes in v3: - Drop mention of PRIC - Rename acpi,desc to acpi,ddn - Correct description of acpi,probed - Drop hid-descr-addr - Just add the device.txt binding file in this patch - Change the example to ELAN - Add a pointer to information about acpi,compatible Changes in v2: - Fix definition of HID - Infer hid-over-i2c CID value - Add the hid-over-i2c binding document doc/device-tree-bindings/device.txt | 36 +++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 doc/device-tree-bindings/device.txt