| Message ID | 20181005165848.3474-14-robh@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Devicetree schema | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | warning | next/apply_patch Patch failed to apply |
| snowpatch_ozlabs/apply_patch | fail | Failed to apply to any branch |
Hi Rob, On Fri, Oct 05, 2018 at 11:58:25AM -0500, Rob Herring wrote: > 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 | 96 +++++++++++++++++++ > 2 files changed, 96 insertions(+), 70 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/arm/pmu.txt > create mode 100644 Documentation/devicetree/bindings/arm/pmu.yaml [...] > -- interrupts : 1 combined interrupt or 1 per core. If the interrupt is a per-cpu > - interrupt (PPI) then 1 interrupt should be specified. [...] > + interrupts: > + oneOf: > + - maxItems: 1 > + - minItems: 2 > + maxItems: 8 > + description: 1 interrupt per core. > + > + interrupts-extended: > + $ref: '#/properties/interrupts' This seems like a semantic different between the two representations, or am I missing something here? Specifically, both the introduction of interrupts-extended and also dropping any mention of using a single per-cpu interrupt (the single combined case is no longer support by Linux; not sure if you want to keep it in the binding). Will
On Tue, Oct 9, 2018 at 6:57 AM Will Deacon <will.deacon@arm.com> wrote: > > Hi Rob, > > On Fri, Oct 05, 2018 at 11:58:25AM -0500, Rob Herring wrote: > > 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 | 96 +++++++++++++++++++ > > 2 files changed, 96 insertions(+), 70 deletions(-) > > delete mode 100644 Documentation/devicetree/bindings/arm/pmu.txt > > create mode 100644 Documentation/devicetree/bindings/arm/pmu.yaml > > [...] > > > -- interrupts : 1 combined interrupt or 1 per core. If the interrupt is a per-cpu > > - interrupt (PPI) then 1 interrupt should be specified. > > [...] > > > + interrupts: > > + oneOf: > > + - maxItems: 1 > > + - minItems: 2 > > + maxItems: 8 > > + description: 1 interrupt per core. > > + > > + interrupts-extended: > > + $ref: '#/properties/interrupts' > > This seems like a semantic different between the two representations, or am > I missing something here? Specifically, both the introduction of > interrupts-extended and also dropping any mention of using a single per-cpu > interrupt (the single combined case is no longer support by Linux; not sure > if you want to keep it in the binding). 'interrupts-extended' was implied before as it is always supported and outside the scope of the binding. But now it is needed to validate bindings. There must be some use of it and that's why I added it. However, thinking some more about this, I think it may be better to have the tools add this in automatically whenever we have an interrupts property. I guess the single interrupt case is less obvious now with no description (it's the first list item of 'oneOf'). The schema If the single interrupt is not supported, then we can drop it here. Rob
On Tue, Oct 09, 2018 at 01:14:02PM -0500, Rob Herring wrote: > On Tue, Oct 9, 2018 at 6:57 AM Will Deacon <will.deacon@arm.com> wrote: > > > > Hi Rob, > > > > On Fri, Oct 05, 2018 at 11:58:25AM -0500, Rob Herring wrote: > > > 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 | 96 +++++++++++++++++++ > > > 2 files changed, 96 insertions(+), 70 deletions(-) > > > delete mode 100644 Documentation/devicetree/bindings/arm/pmu.txt > > > create mode 100644 Documentation/devicetree/bindings/arm/pmu.yaml > > > > [...] > > > > > -- interrupts : 1 combined interrupt or 1 per core. If the interrupt is a per-cpu > > > - interrupt (PPI) then 1 interrupt should be specified. > > > > [...] > > > > > + interrupts: > > > + oneOf: > > > + - maxItems: 1 > > > + - minItems: 2 > > > + maxItems: 8 > > > + description: 1 interrupt per core. > > > + > > > + interrupts-extended: > > > + $ref: '#/properties/interrupts' > > > > This seems like a semantic different between the two representations, or am > > I missing something here? Specifically, both the introduction of > > interrupts-extended and also dropping any mention of using a single per-cpu > > interrupt (the single combined case is no longer support by Linux; not sure > > if you want to keep it in the binding). > > 'interrupts-extended' was implied before as it is always supported and > outside the scope of the binding. But now it is needed to validate > bindings. There must be some use of it and that's why I added it. > However, thinking some more about this, I think it may be better to > have the tools add this in automatically whenever we have an > interrupts property. To be honest, if you'd included that in the commit message I'd have been happy :) > I guess the single interrupt case is less obvious now with no > description (it's the first list item of 'oneOf'). The schema If the > single interrupt is not supported, then we can drop it here. Well the description says "1 interrupt per core" which is incorrect. I also don't understand why maxItems is 8. Will
On Wed, Oct 10, 2018 at 11:50 AM Will Deacon <will.deacon@arm.com> wrote: > > On Tue, Oct 09, 2018 at 01:14:02PM -0500, Rob Herring wrote: > > On Tue, Oct 9, 2018 at 6:57 AM Will Deacon <will.deacon@arm.com> wrote: > > > > > > Hi Rob, > > > > > > On Fri, Oct 05, 2018 at 11:58:25AM -0500, Rob Herring wrote: > > > > 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 | 96 +++++++++++++++++++ > > > > 2 files changed, 96 insertions(+), 70 deletions(-) > > > > delete mode 100644 Documentation/devicetree/bindings/arm/pmu.txt > > > > create mode 100644 Documentation/devicetree/bindings/arm/pmu.yaml > > > > > > [...] > > > > > > > -- interrupts : 1 combined interrupt or 1 per core. If the interrupt is a per-cpu > > > > - interrupt (PPI) then 1 interrupt should be specified. > > > > > > [...] > > > > > > > + interrupts: > > > > + oneOf: > > > > + - maxItems: 1 > > > > + - minItems: 2 > > > > + maxItems: 8 > > > > + description: 1 interrupt per core. > > > > + > > > > + interrupts-extended: > > > > + $ref: '#/properties/interrupts' > > > > > > This seems like a semantic different between the two representations, or am > > > I missing something here? Specifically, both the introduction of > > > interrupts-extended and also dropping any mention of using a single per-cpu > > > interrupt (the single combined case is no longer support by Linux; not sure > > > if you want to keep it in the binding). > > > > 'interrupts-extended' was implied before as it is always supported and > > outside the scope of the binding. But now it is needed to validate > > bindings. There must be some use of it and that's why I added it. > > However, thinking some more about this, I think it may be better to > > have the tools add this in automatically whenever we have an > > interrupts property. > > To be honest, if you'd included that in the commit message I'd have been > happy :) > > > I guess the single interrupt case is less obvious now with no > > description (it's the first list item of 'oneOf'). The schema If the > > single interrupt is not supported, then we can drop it here. > > Well the description says "1 interrupt per core" which is incorrect. You are reading the schema wrong. There are 2 cases supported as defined by each '-'. The 2nd case is all the keywords until the indentation decreases. So 'description' is just description of the 2nd case. The first case is just "maxItems: 1". I probably didn't put a description because why write in free form text what the schema says (other than of course no one knows json-schema...). YAML combines the best of Makefiles and python. You can't have tabs and Indentation is significant. :) > I also > don't understand why maxItems is 8. Humm, I probably just made that up based on GICv2 limitations. What should it be? If there's not any inherit maximum, can we put something reasonable? There's not really any way to express that it should match the number of cores in the system. Rob
Hi Rob, On Wed, Oct 10, 2018 at 01:51:24PM -0500, Rob Herring wrote: > On Wed, Oct 10, 2018 at 11:50 AM Will Deacon <will.deacon@arm.com> wrote: > > On Tue, Oct 09, 2018 at 01:14:02PM -0500, Rob Herring wrote: > > > I guess the single interrupt case is less obvious now with no > > > description (it's the first list item of 'oneOf'). The schema If the > > > single interrupt is not supported, then we can drop it here. > > > > Well the description says "1 interrupt per core" which is incorrect. > > You are reading the schema wrong. There are 2 cases supported as > defined by each '-'. The 2nd case is all the keywords until the > indentation decreases. So 'description' is just description of the 2nd > case. The first case is just "maxItems: 1". I probably didn't put a > description because why write in free form text what the schema says > (other than of course no one knows json-schema...). Apologies, I've not read one of these things before and looks like I completely misread it. > YAML combines the best of Makefiles and python. You can't have tabs > and Indentation is significant. :) Oh wow, I'm in way over my head here! > > I also > > don't understand why maxItems is 8. > > Humm, I probably just made that up based on GICv2 limitations. What > should it be? If there's not any inherit maximum, can we put something > reasonable? There's not really any way to express that it should match > the number of cores in the system. What's the largest number you can think of? Will
On Tue, Oct 9, 2018 at 6:57 AM Will Deacon <will.deacon@arm.com> wrote: > > Hi Rob, > > On Fri, Oct 05, 2018 at 11:58:25AM -0500, Rob Herring wrote: > > 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 | 96 +++++++++++++++++++ > > 2 files changed, 96 insertions(+), 70 deletions(-) > > delete mode 100644 Documentation/devicetree/bindings/arm/pmu.txt > > create mode 100644 Documentation/devicetree/bindings/arm/pmu.yaml > > [...] > > > -- interrupts : 1 combined interrupt or 1 per core. If the interrupt is a per-cpu > > - interrupt (PPI) then 1 interrupt should be specified. > > [...] > > > + interrupts: > > + oneOf: > > + - maxItems: 1 > > + - minItems: 2 > > + maxItems: 8 > > + description: 1 interrupt per core. > > + > > + interrupts-extended: > > + $ref: '#/properties/interrupts' > > This seems like a semantic different between the two representations, or am > I missing something here? Specifically, both the introduction of > interrupts-extended and also dropping any mention of using a single per-cpu > interrupt (the single combined case is no longer support by Linux; not sure > if you want to keep it in the binding). In regards to no support for the single combined interrupt, it looks like Marvell Armada SoCs at least (armada-375 is what I'm looking at) have only a single interrupt. Though the interrupt gets routed to MPIC which then has a GIC PPI. So it isn't supported or happens to work still since it is a PPI? Rob
On 01/11/2018 19:32, Rob Herring wrote: > On Tue, Oct 9, 2018 at 6:57 AM Will Deacon <will.deacon@arm.com> wrote: >> >> Hi Rob, >> >> On Fri, Oct 05, 2018 at 11:58:25AM -0500, Rob Herring wrote: >>> 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 | 96 +++++++++++++++++++ >>> 2 files changed, 96 insertions(+), 70 deletions(-) >>> delete mode 100644 Documentation/devicetree/bindings/arm/pmu.txt >>> create mode 100644 Documentation/devicetree/bindings/arm/pmu.yaml >> >> [...] >> >>> -- interrupts : 1 combined interrupt or 1 per core. If the interrupt is a per-cpu >>> - interrupt (PPI) then 1 interrupt should be specified. >> >> [...] >> >>> + interrupts: >>> + oneOf: >>> + - maxItems: 1 >>> + - minItems: 2 >>> + maxItems: 8 >>> + description: 1 interrupt per core. >>> + >>> + interrupts-extended: >>> + $ref: '#/properties/interrupts' >> >> This seems like a semantic different between the two representations, or am >> I missing something here? Specifically, both the introduction of >> interrupts-extended and also dropping any mention of using a single per-cpu >> interrupt (the single combined case is no longer support by Linux; not sure >> if you want to keep it in the binding). > > In regards to no support for the single combined interrupt, it looks > like Marvell Armada SoCs at least (armada-375 is what I'm looking at) > have only a single interrupt. Though the interrupt gets routed to MPIC > which then has a GIC PPI. So it isn't supported or happens to work > still since it is a PPI? Well, the description of the MPIC in the Armada XP functional spec says: "Interrupt sources ID0–ID28 are private events per CPU. Thus, each processor has a different set of events map interrupts ID0–ID28." Odd grammar aside, that would seem to imply that <&mpic 3> is a per-cpu interrupt itself, thus AFAICS so long as it's cascaded to a GIC PPI and not an SPI then there's no issue there. Robin.
Hello, I'm jumping into the discussion, but I clearly don't have all the context of the discussion. On Thu, 8 Nov 2018 15:54:31 +0000, Robin Murphy wrote: > >> This seems like a semantic different between the two representations, or am > >> I missing something here? Specifically, both the introduction of > >> interrupts-extended and also dropping any mention of using a single per-cpu > >> interrupt (the single combined case is no longer support by Linux; not sure > >> if you want to keep it in the binding). > > > > In regards to no support for the single combined interrupt, it looks > > like Marvell Armada SoCs at least (armada-375 is what I'm looking at) > > have only a single interrupt. Though the interrupt gets routed to MPIC > > which then has a GIC PPI. So it isn't supported or happens to work > > still since it is a PPI? > > Well, the description of the MPIC in the Armada XP functional spec says: > > "Interrupt sources ID0–ID28 are private events per CPU. Thus, each > processor has a different set of events map interrupts ID0–ID28." > > Odd grammar aside, that would seem to imply that <&mpic 3> is a per-cpu > interrupt itself, thus AFAICS so long as it's cascaded to a GIC PPI and > not an SPI then there's no issue there. The Armada XP does not have a GIC at all, but only a MPIC as the primary interrupt controller. However the Armada 38x has both a GIC and a MPIC, and indeed the parent interrupts of the MPIC towards the GIC is: interrupts = <GIC_PPI 15 IRQ_TYPE_LEVEL_HIGH>; Best regards, Thomas
On 08/11/2018 15:59, Thomas Petazzoni wrote: > Hello, > > I'm jumping into the discussion, but I clearly don't have all the > context of the discussion. > > On Thu, 8 Nov 2018 15:54:31 +0000, Robin Murphy wrote: > >>>> This seems like a semantic different between the two representations, or am >>>> I missing something here? Specifically, both the introduction of >>>> interrupts-extended and also dropping any mention of using a single per-cpu >>>> interrupt (the single combined case is no longer support by Linux; not sure >>>> if you want to keep it in the binding). >>> >>> In regards to no support for the single combined interrupt, it looks >>> like Marvell Armada SoCs at least (armada-375 is what I'm looking at) >>> have only a single interrupt. Though the interrupt gets routed to MPIC >>> which then has a GIC PPI. So it isn't supported or happens to work >>> still since it is a PPI? >> >> Well, the description of the MPIC in the Armada XP functional spec says: >> >> "Interrupt sources ID0–ID28 are private events per CPU. Thus, each >> processor has a different set of events map interrupts ID0–ID28." >> >> Odd grammar aside, that would seem to imply that <&mpic 3> is a per-cpu >> interrupt itself, thus AFAICS so long as it's cascaded to a GIC PPI and >> not an SPI then there's no issue there. > > The Armada XP does not have a GIC at all, but only a MPIC as the > primary interrupt controller. > > However the Armada 38x has both a GIC and a MPIC, and indeed the parent > interrupts of the MPIC towards the GIC is: > > interrupts = <GIC_PPI 15 IRQ_TYPE_LEVEL_HIGH>; Yeah, perhaps I should have clarified that the XP manual was the only publicly-available one I could find, but I'm inferring from the binding and driver that the MPIC in 375/38x still behaves the same. Robin.
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..0dbb9e0566af --- /dev/null +++ b/Documentation/devicetree/bindings/arm/pmu.yaml @@ -0,0 +1,96 @@ +# SPDX-License-Identifier: None +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/bindings/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: + 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 + interrupts: + oneOf: + - maxItems: 1 + - minItems: 2 + maxItems: 8 + description: 1 interrupt per core. + + interrupts-extended: + $ref: '#/properties/interrupts' + + interrupt-affinity: + 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 + +oneOf: + - required: + - interrupts + - required: + - interrupts-extended + +...
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 | 96 +++++++++++++++++++ 2 files changed, 96 insertions(+), 70 deletions(-) delete mode 100644 Documentation/devicetree/bindings/arm/pmu.txt create mode 100644 Documentation/devicetree/bindings/arm/pmu.yaml