| Message ID | 20181203213223.16986-9-robh@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Devicetree schema | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
| snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 2 warnings, 0 checks, 91 lines checked |
Hi Rob, On Mon, Dec 03, 2018 at 03:31:57PM -0600, Rob Herring wrote: > Convert ARM PMU binding to DT schema format using json-schema. Just a couple of things below. > diff --git a/Documentation/devicetree/bindings/arm/pmu.yaml b/Documentation/devicetree/bindings/arm/pmu.yaml > new file mode 100644 > index 000000000000..3ea4abfbf276 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/pmu.yaml > @@ -0,0 +1,91 @@ [...] > +properties: > + compatible: > + oneOf: > + - items: > + - enum: > + - apm,potenza-pmu > + - arm,armv8-pmuv3 > + - arm,cortex-a73-pmu > + - arm,cortex-a72-pmu > + - arm,cortex-a57-pmu > + - arm,cortex-a53-pmu > + - arm,cortex-a35-pmu > + - arm,cortex-a17-pmu > + - arm,cortex-a15-pmu > + - arm,cortex-a12-pmu > + - arm,cortex-a9-pmu > + - arm,cortex-a8-pmu > + - arm,cortex-a7-pmu > + - arm,cortex-a5-pmu > + - arm,arm11mpcore-pmu > + - arm,arm1176-pmu > + - arm,arm1136-pmu > + - brcm,vulcan-pmu > + - cavium,thunder-pmu > + - qcom,scorpion-pmu > + - qcom,scorpion-mp-pmu > + - qcom,krait-pmu > + - items: > + - const: arm,cortex-a7-pmu > + - const: arm,cortex-a15-pmu What do these last two mean? > + > + interrupts: > + # Don't know how many CPUs, so no constraints to specify > + description: 1 per-cpu interrupt (PPI) or 1 interrupt per core. > + > + interrupt-affinity: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + description: > + When using SPIs, specifies a list of phandles to CPU > + nodes corresponding directly to the affinity of > + the SPIs listed in the interrupts property. > + > + When using a PPI, specifies a list of phandles to CPU > + nodes corresponding to the set of CPUs which have > + a PMU of this type signalling the PPI listed in the > + interrupts property, unless this is already specified > + by the PPI interrupt specifier itself (in which case > + the interrupt-affinity property shouldn't be present). > + > + This property should be present when there is more than > + a single SPI. > + > + qcom,no-pc-write: > + type: boolean > + description: > + Indicates that this PMU doesn't support the 0xc and 0xd events. > + > + secure-reg-access: > + type: boolean > + description: > + Indicates that the ARMv7 Secure Debug Enable Register > + (SDER) is accessible. This will cause the driver to do > + any setup required that is only possible in ARMv7 secure > + state. If not present the ARMv7 SDER will not be touched, > + which means the PMU may fail to operate unless external > + code (bootloader or security monitor) has performed the > + appropriate initialisation. Note that this property is > + not valid for non-ARMv7 CPUs or ARMv7 CPUs booting Linux > + in Non-secure state. > + > +required: > + - compatible I probably said this before, but I do think that it's a shame to lose the example binding, especially for something like the PMU where you can pretty much take an example and bang in your own IRQ numbers to get it up and running. Will
On Wed, Dec 5, 2018 at 4:08 AM Will Deacon <will.deacon@arm.com> wrote: > > Hi Rob, > > On Mon, Dec 03, 2018 at 03:31:57PM -0600, Rob Herring wrote: > > Convert ARM PMU binding to DT schema format using json-schema. > > Just a couple of things below. > > > diff --git a/Documentation/devicetree/bindings/arm/pmu.yaml b/Documentation/devicetree/bindings/arm/pmu.yaml > > new file mode 100644 > > index 000000000000..3ea4abfbf276 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/arm/pmu.yaml > > @@ -0,0 +1,91 @@ > > [...] > > > +properties: > > + compatible: > > + oneOf: > > + - items: > > + - enum: > > + - apm,potenza-pmu > > + - arm,armv8-pmuv3 > > + - arm,cortex-a73-pmu > > + - arm,cortex-a72-pmu > > + - arm,cortex-a57-pmu > > + - arm,cortex-a53-pmu > > + - arm,cortex-a35-pmu > > + - arm,cortex-a17-pmu > > + - arm,cortex-a15-pmu > > + - arm,cortex-a12-pmu > > + - arm,cortex-a9-pmu > > + - arm,cortex-a8-pmu > > + - arm,cortex-a7-pmu > > + - arm,cortex-a5-pmu > > + - arm,arm11mpcore-pmu > > + - arm,arm1176-pmu > > + - arm,arm1136-pmu > > + - brcm,vulcan-pmu > > + - cavium,thunder-pmu > > + - qcom,scorpion-pmu > > + - qcom,scorpion-mp-pmu > > + - qcom,krait-pmu > > + - items: > > + - const: arm,cortex-a7-pmu > > + - const: arm,cortex-a15-pmu > > What do these last two mean? The first list only allows a single compatible string. This list says there are 2 and that the compatible property should be: compatible = "arm,cortex-a7-pmu", "arm,cortex-a15-pmu"; Which shows up here: arch/arm/boot/dts/sun6i-a31.dtsi: compatible = "arm,cortex-a7-pmu", "arm,cortex-a15-pmu"; arch/arm/boot/dts/sun7i-a20.dtsi: compatible = "arm,cortex-a7-pmu", "arm,cortex-a15-pmu"; Maybe the dts is wrong and should be fixed? > > + interrupts: > > + # Don't know how many CPUs, so no constraints to specify > > + description: 1 per-cpu interrupt (PPI) or 1 interrupt per core. > > + > > + interrupt-affinity: > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > + description: > > + When using SPIs, specifies a list of phandles to CPU > > + nodes corresponding directly to the affinity of > > + the SPIs listed in the interrupts property. > > + > > + When using a PPI, specifies a list of phandles to CPU > > + nodes corresponding to the set of CPUs which have > > + a PMU of this type signalling the PPI listed in the > > + interrupts property, unless this is already specified > > + by the PPI interrupt specifier itself (in which case > > + the interrupt-affinity property shouldn't be present). > > + > > + This property should be present when there is more than > > + a single SPI. > > + > > + qcom,no-pc-write: > > + type: boolean > > + description: > > + Indicates that this PMU doesn't support the 0xc and 0xd events. > > + > > + secure-reg-access: > > + type: boolean > > + description: > > + Indicates that the ARMv7 Secure Debug Enable Register > > + (SDER) is accessible. This will cause the driver to do > > + any setup required that is only possible in ARMv7 secure > > + state. If not present the ARMv7 SDER will not be touched, > > + which means the PMU may fail to operate unless external > > + code (bootloader or security monitor) has performed the > > + appropriate initialisation. Note that this property is > > + not valid for non-ARMv7 CPUs or ARMv7 CPUs booting Linux > > + in Non-secure state. > > + > > +required: > > + - compatible > > I probably said this before, but I do think that it's a shame to lose the > example binding, especially for something like the PMU where you can > pretty much take an example and bang in your own IRQ numbers to get it > up and running. I just thought it is so trivial that it didn't add much. I think most folks would copy-n-paste from an actual dts file which has all the other nodes you just need to update addresses and irq nums. Rob
On Wed, Dec 05, 2018 at 09:42:04AM -0600, Rob Herring wrote: > On Wed, Dec 5, 2018 at 4:08 AM Will Deacon <will.deacon@arm.com> wrote: > > On Mon, Dec 03, 2018 at 03:31:57PM -0600, Rob Herring wrote: > > > +properties: > > > + compatible: > > > + oneOf: > > > + - items: > > > + - enum: > > > + - apm,potenza-pmu > > > + - arm,armv8-pmuv3 > > > + - arm,cortex-a73-pmu > > > + - arm,cortex-a72-pmu > > > + - arm,cortex-a57-pmu > > > + - arm,cortex-a53-pmu > > > + - arm,cortex-a35-pmu > > > + - arm,cortex-a17-pmu > > > + - arm,cortex-a15-pmu > > > + - arm,cortex-a12-pmu > > > + - arm,cortex-a9-pmu > > > + - arm,cortex-a8-pmu > > > + - arm,cortex-a7-pmu > > > + - arm,cortex-a5-pmu > > > + - arm,arm11mpcore-pmu > > > + - arm,arm1176-pmu > > > + - arm,arm1136-pmu > > > + - brcm,vulcan-pmu > > > + - cavium,thunder-pmu > > > + - qcom,scorpion-pmu > > > + - qcom,scorpion-mp-pmu > > > + - qcom,krait-pmu > > > + - items: > > > + - const: arm,cortex-a7-pmu > > > + - const: arm,cortex-a15-pmu > > > > What do these last two mean? > > The first list only allows a single compatible string. This list says > there are 2 and that the compatible property should be: > compatible = "arm,cortex-a7-pmu", "arm,cortex-a15-pmu"; > > Which shows up here: > arch/arm/boot/dts/sun6i-a31.dtsi: compatible = > "arm,cortex-a7-pmu", "arm,cortex-a15-pmu"; > arch/arm/boot/dts/sun7i-a20.dtsi: compatible = > "arm,cortex-a7-pmu", "arm,cortex-a15-pmu"; > > Maybe the dts is wrong and should be fixed? Yes, that's wrong and you could end up with the kernel exposing the wrong events to userspace. So I think we can fix the .dts and leave the binding without this. > > > +required: > > > + - compatible > > > > I probably said this before, but I do think that it's a shame to lose the > > example binding, especially for something like the PMU where you can > > pretty much take an example and bang in your own IRQ numbers to get it > > up and running. > > I just thought it is so trivial that it didn't add much. I think most > folks would copy-n-paste from an actual dts file which has all the > other nodes you just need to update addresses and irq nums. True... even more of a reason to fix thise sun* .dts files then! Will
diff --git a/Documentation/devicetree/bindings/arm/pmu.txt b/Documentation/devicetree/bindings/arm/pmu.txt deleted file mode 100644 index 13611a8199bb..000000000000 --- a/Documentation/devicetree/bindings/arm/pmu.txt +++ /dev/null @@ -1,70 +0,0 @@ -* ARM Performance Monitor Units - -ARM cores often have a PMU for counting cpu and cache events like cache misses -and hits. The interface to the PMU is part of the ARM ARM. The ARM PMU -representation in the device tree should be done as under:- - -Required properties: - -- compatible : should be one of - "apm,potenza-pmu" - "arm,armv8-pmuv3" - "arm,cortex-a73-pmu" - "arm,cortex-a72-pmu" - "arm,cortex-a57-pmu" - "arm,cortex-a53-pmu" - "arm,cortex-a35-pmu" - "arm,cortex-a17-pmu" - "arm,cortex-a15-pmu" - "arm,cortex-a12-pmu" - "arm,cortex-a9-pmu" - "arm,cortex-a8-pmu" - "arm,cortex-a7-pmu" - "arm,cortex-a5-pmu" - "arm,arm11mpcore-pmu" - "arm,arm1176-pmu" - "arm,arm1136-pmu" - "brcm,vulcan-pmu" - "cavium,thunder-pmu" - "qcom,scorpion-pmu" - "qcom,scorpion-mp-pmu" - "qcom,krait-pmu" -- interrupts : 1 combined interrupt or 1 per core. If the interrupt is a per-cpu - interrupt (PPI) then 1 interrupt should be specified. - -Optional properties: - -- interrupt-affinity : When using SPIs, specifies a list of phandles to CPU - nodes corresponding directly to the affinity of - the SPIs listed in the interrupts property. - - When using a PPI, specifies a list of phandles to CPU - nodes corresponding to the set of CPUs which have - a PMU of this type signalling the PPI listed in the - interrupts property, unless this is already specified - by the PPI interrupt specifier itself (in which case - the interrupt-affinity property shouldn't be present). - - This property should be present when there is more than - a single SPI. - - -- qcom,no-pc-write : Indicates that this PMU doesn't support the 0xc and 0xd - events. - -- secure-reg-access : Indicates that the ARMv7 Secure Debug Enable Register - (SDER) is accessible. This will cause the driver to do - any setup required that is only possible in ARMv7 secure - state. If not present the ARMv7 SDER will not be touched, - which means the PMU may fail to operate unless external - code (bootloader or security monitor) has performed the - appropriate initialisation. Note that this property is - not valid for non-ARMv7 CPUs or ARMv7 CPUs booting Linux - in Non-secure state. - -Example: - -pmu { - compatible = "arm,cortex-a9-pmu"; - interrupts = <100 101>; -}; diff --git a/Documentation/devicetree/bindings/arm/pmu.yaml b/Documentation/devicetree/bindings/arm/pmu.yaml new file mode 100644 index 000000000000..3ea4abfbf276 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/pmu.yaml @@ -0,0 +1,91 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/arm/pmu.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ARM Performance Monitor Units + +maintainers: + - Mark Rutland <mark.rutland@arm.com> + - Will Deacon <will.deacon@arm.com> + +description: |+ + ARM cores often have a PMU for counting cpu and cache events like cache misses + and hits. The interface to the PMU is part of the ARM ARM. The ARM PMU + representation in the device tree should be done as under:- + +properties: + compatible: + oneOf: + - items: + - enum: + - apm,potenza-pmu + - arm,armv8-pmuv3 + - arm,cortex-a73-pmu + - arm,cortex-a72-pmu + - arm,cortex-a57-pmu + - arm,cortex-a53-pmu + - arm,cortex-a35-pmu + - arm,cortex-a17-pmu + - arm,cortex-a15-pmu + - arm,cortex-a12-pmu + - arm,cortex-a9-pmu + - arm,cortex-a8-pmu + - arm,cortex-a7-pmu + - arm,cortex-a5-pmu + - arm,arm11mpcore-pmu + - arm,arm1176-pmu + - arm,arm1136-pmu + - brcm,vulcan-pmu + - cavium,thunder-pmu + - qcom,scorpion-pmu + - qcom,scorpion-mp-pmu + - qcom,krait-pmu + - items: + - const: arm,cortex-a7-pmu + - const: arm,cortex-a15-pmu + + interrupts: + # Don't know how many CPUs, so no constraints to specify + description: 1 per-cpu interrupt (PPI) or 1 interrupt per core. + + interrupt-affinity: + $ref: /schemas/types.yaml#/definitions/phandle-array + description: + When using SPIs, specifies a list of phandles to CPU + nodes corresponding directly to the affinity of + the SPIs listed in the interrupts property. + + When using a PPI, specifies a list of phandles to CPU + nodes corresponding to the set of CPUs which have + a PMU of this type signalling the PPI listed in the + interrupts property, unless this is already specified + by the PPI interrupt specifier itself (in which case + the interrupt-affinity property shouldn't be present). + + This property should be present when there is more than + a single SPI. + + qcom,no-pc-write: + type: boolean + description: + Indicates that this PMU doesn't support the 0xc and 0xd events. + + secure-reg-access: + type: boolean + description: + Indicates that the ARMv7 Secure Debug Enable Register + (SDER) is accessible. This will cause the driver to do + any setup required that is only possible in ARMv7 secure + state. If not present the ARMv7 SDER will not be touched, + which means the PMU may fail to operate unless external + code (bootloader or security monitor) has performed the + appropriate initialisation. Note that this property is + not valid for non-ARMv7 CPUs or ARMv7 CPUs booting Linux + in Non-secure state. + +required: + - compatible + +...
Convert ARM PMU binding to DT schema format using json-schema. Cc: Will Deacon <will.deacon@arm.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: linux-arm-kernel@lists.infradead.org Cc: devicetree@vger.kernel.org Signed-off-by: Rob Herring <robh@kernel.org> --- Documentation/devicetree/bindings/arm/pmu.txt | 70 -------------- .../devicetree/bindings/arm/pmu.yaml | 91 +++++++++++++++++++ 2 files changed, 91 insertions(+), 70 deletions(-) delete mode 100644 Documentation/devicetree/bindings/arm/pmu.txt create mode 100644 Documentation/devicetree/bindings/arm/pmu.yaml