| Message ID | 20240806221237.1634126-3-yunhong.jiang@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | x86/acpi: Move ACPI MADT wakeup to generic code | expand |
On Tue, 06 Aug 2024 15:12:32 -0700, Yunhong Jiang wrote: > Add the binding to use the ACPI wakeup mailbox mechanism to bringup APs. > > Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com> > --- > .../devicetree/bindings/x86/wakeup.yaml | 41 +++++++++++++++++++ > 1 file changed, 41 insertions(+) > create mode 100644 Documentation/devicetree/bindings/x86/wakeup.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: ./Documentation/devicetree/bindings/x86/wakeup.yaml:4:1: [error] syntax error: expected '<document start>', but found '<block mapping start>' (syntax) ./Documentation/devicetree/bindings/x86/wakeup.yaml:41:111: [warning] line too long (153 > 110 characters) (line-length) dtschema/dtc warnings/errors: make[2]: *** Deleting file 'Documentation/devicetree/bindings/x86/wakeup.example.dts' Documentation/devicetree/bindings/x86/wakeup.yaml:4:1: did not find expected <document start> make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/x86/wakeup.example.dts] Error 1 make[2]: *** Waiting for unfinished jobs.... ./Documentation/devicetree/bindings/x86/wakeup.yaml:4:1: did not find expected <document start> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/x86/wakeup.yaml: ignoring, error parsing file make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1430: dt_binding_check] Error 2 make: *** [Makefile:240: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240806221237.1634126-3-yunhong.jiang@linux.intel.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On 07/08/2024 00:12, Yunhong Jiang wrote: > Add the binding to use the ACPI wakeup mailbox mechanism to bringup APs. We do not have bindings for ACPI. I think in the past it was mentioned pretty clear - we do not care what ACPI has in the wild. > > Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com> > --- > .../devicetree/bindings/x86/wakeup.yaml | 41 +++++++++++++++++++ > 1 file changed, 41 insertions(+) > create mode 100644 Documentation/devicetree/bindings/x86/wakeup.yaml > > diff --git a/Documentation/devicetree/bindings/x86/wakeup.yaml b/Documentation/devicetree/bindings/x86/wakeup.yaml > new file mode 100644 > index 000000000000..8af40dcdb592 > --- /dev/null > +++ b/Documentation/devicetree/bindings/x86/wakeup.yaml > @@ -0,0 +1,41 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > + > +$id: http://devicetree.org/schemas/x86/wakeup.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# This was absolutely never tested and does not look like proper bindings file. This just does not work. Go to example-schema and use it as template. NAK Best regards, Krzysztof
On Wed, Aug 07, 2024 at 07:57:43AM +0200, Krzysztof Kozlowski wrote: > On 07/08/2024 00:12, Yunhong Jiang wrote: > > Add the binding to use the ACPI wakeup mailbox mechanism to bringup APs. > > We do not have bindings for ACPI. I think in the past it was mentioned > pretty clear - we do not care what ACPI has in the wild. Thank you for review. Can you please give a bit more information on "do not have bindings for ACPI"? We don't put the ACPI table into the device tree, but reuse some existing ACPI mailbox mechanism. Is this acceptable for you? > > > > > Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com> > > --- > > .../devicetree/bindings/x86/wakeup.yaml | 41 +++++++++++++++++++ > > 1 file changed, 41 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/x86/wakeup.yaml > > > > diff --git a/Documentation/devicetree/bindings/x86/wakeup.yaml b/Documentation/devicetree/bindings/x86/wakeup.yaml > > new file mode 100644 > > index 000000000000..8af40dcdb592 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/x86/wakeup.yaml > > @@ -0,0 +1,41 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > + > > +$id: http://devicetree.org/schemas/x86/wakeup.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > This was absolutely never tested and does not look like proper bindings > file. This just does not work. Go to example-schema and use it as template. > > NAK > > Best regards, > Krzysztof Oops, I used the example-schema but apparently did something wrong. Will have a check. --jyh >
On Tue, Aug 06, 2024 at 05:38:25PM -0600, Rob Herring (Arm) wrote: > > On Tue, 06 Aug 2024 15:12:32 -0700, Yunhong Jiang wrote: > > Add the binding to use the ACPI wakeup mailbox mechanism to bringup APs. > > > > Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com> > > --- > > .../devicetree/bindings/x86/wakeup.yaml | 41 +++++++++++++++++++ > > 1 file changed, 41 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/x86/wakeup.yaml > > > > My bot found errors running 'make dt_binding_check' on your patch: > > yamllint warnings/errors: > ./Documentation/devicetree/bindings/x86/wakeup.yaml:4:1: [error] syntax error: expected '<document start>', but found '<block mapping start>' (syntax) > ./Documentation/devicetree/bindings/x86/wakeup.yaml:41:111: [warning] line too long (153 > 110 characters) (line-length) > > dtschema/dtc warnings/errors: > make[2]: *** Deleting file 'Documentation/devicetree/bindings/x86/wakeup.example.dts' > Documentation/devicetree/bindings/x86/wakeup.yaml:4:1: did not find expected <document start> > make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/x86/wakeup.example.dts] Error 1 > make[2]: *** Waiting for unfinished jobs.... > ./Documentation/devicetree/bindings/x86/wakeup.yaml:4:1: did not find expected <document start> > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/x86/wakeup.yaml: ignoring, error parsing file > make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1430: dt_binding_check] Error 2 > make: *** [Makefile:240: __sub-make] Error 2 > > doc reference errors (make refcheckdocs): > > See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240806221237.1634126-3-yunhong.jiang@linux.intel.com > > The base for the series is generally the latest rc1. A different dependency > should be noted in *this* patch. > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure 'yamllint' is installed and dt-schema is up to > date: > > pip3 install dtschema --upgrade > > Please check and re-submit after running the above command yourself. Note > that DT_SCHEMA_FILES can be set to your schema file to speed up checking > your schema. However, it must be unset to test all examples with your schema. > I remember I ran 'make dt_binding_check' in the early development stage, but possibly missed the error information. Will fix this. Thank you --jyh
On 07/08/2024 18:56, Yunhong Jiang wrote: > On Wed, Aug 07, 2024 at 07:57:43AM +0200, Krzysztof Kozlowski wrote: >> On 07/08/2024 00:12, Yunhong Jiang wrote: >>> Add the binding to use the ACPI wakeup mailbox mechanism to bringup APs. >> >> We do not have bindings for ACPI. I think in the past it was mentioned >> pretty clear - we do not care what ACPI has in the wild. > > Thank you for review. > Can you please give a bit more information on "do not have bindings for ACPI"? > We don't put the ACPI table into the device tree, but reuse some existing ACPI > mailbox mechanism. Is this acceptable for you? I understood that rationale behind this patch is "ACPI" thus that reply. This one sentence in commit msg is not helping. Entire binding description speaks about ACPI, so yeah - I don't care what ACPI does. Provide proper explanation/description of firmware or hardware, then sure. But the patch saying ACPI is doing something, so bindings will be doing the same is for me NAK. Whatever ACPI is doing is never a reason alone to do the same in Devicetree. Best regards, Krzysztof
On Thu, Aug 08, 2024 at 09:41:16AM +0200, Krzysztof Kozlowski wrote: > On 07/08/2024 18:56, Yunhong Jiang wrote: > > On Wed, Aug 07, 2024 at 07:57:43AM +0200, Krzysztof Kozlowski wrote: > >> On 07/08/2024 00:12, Yunhong Jiang wrote: > >>> Add the binding to use the ACPI wakeup mailbox mechanism to bringup APs. > >> > >> We do not have bindings for ACPI. I think in the past it was mentioned > >> pretty clear - we do not care what ACPI has in the wild. > > > > Thank you for review. > > Can you please give a bit more information on "do not have bindings for ACPI"? > > We don't put the ACPI table into the device tree, but reuse some existing ACPI > > mailbox mechanism. Is this acceptable for you? > > I understood that rationale behind this patch is "ACPI" thus that reply. > This one sentence in commit msg is not helping. Entire binding > description speaks about ACPI, so yeah - I don't care what ACPI does. > Provide proper explanation/description of firmware or hardware, then > sure. But the patch saying ACPI is doing something, so bindings will be > doing the same is for me NAK. Whatever ACPI is doing is never a reason > alone to do the same in Devicetree. Thank you for the explanation. I will make the description as ACPI independent. > > Best regards, > Krzysztof > >
diff --git a/Documentation/devicetree/bindings/x86/wakeup.yaml b/Documentation/devicetree/bindings/x86/wakeup.yaml new file mode 100644 index 000000000000..8af40dcdb592 --- /dev/null +++ b/Documentation/devicetree/bindings/x86/wakeup.yaml @@ -0,0 +1,41 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 + +$id: http://devicetree.org/schemas/x86/wakeup.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: x86 acpi wakeup mailbox + +description: | + +This document describes the "acpi-wakeup-mailbox" method for enabling secondary +CPUs. + +The ACPI spec defines a mechanism to let the bootstrap processor wake up +application processors with a mailbox. The "acpi-wakeup-mailbox" enable-method +follows the "Multiprocessor Wakeup Mailbox Structure" defined in the ACPI +spec[1]. + +Since the ACPI mailbox structure is shared by all the CPUs, this enable method +applies to all CPUs and should be defined in the "cpus" node and should not be +defined on each "cpu" node. + +select: false + +properties: + wakeup-mailbox-addr: + $ref: /schemas/types.yaml#/definitions/uint64 + description: | + The physical address of the wakeup mailbox data structure. The address must + meet the ACPI spec requirement, like be 4K bytes aligned and it should be in + the reserved memory. + + wakeup-mailbox-version: + $ref: /schemas/types.yaml#/definitions/uint64 + description: | + The MailBoxVersion defined in the ACPI spec that this binding follows. + +required: + - wakeup-mailbox-addr + +[1] https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/05_ACPI_Software_Programming_Model/ACPI_Software_Programming_Model.html#multiprocessor-wakeup-structure
Add the binding to use the ACPI wakeup mailbox mechanism to bringup APs. Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com> --- .../devicetree/bindings/x86/wakeup.yaml | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 Documentation/devicetree/bindings/x86/wakeup.yaml