On 22/03/2025 00:21, Caleb James DeLisle wrote: > Thank you for the review. > > On 21/03/2025 21:56, Krzysztof Kozlowski wrote: >> On 21/03/2025 14:46, Caleb James DeLisle wrote: >>> Add device tree binding documentation for the high-precision timer (HPT) >>> in the EcoNet EN751221 SoC. >>> >>> Signed-off-by: Caleb James DeLisle <cjd@xxxxxxxx> >> Previous patch was not tested, so was this one tested? > > Yes, all of this has been tested on multiple devices, I believe I was > unclear in the question I added in patch 3. Hm? How can you test a binding on a device? I meant here bindings - they were not tested. > >> >>> --- >>> .../bindings/timer/econet,timer-hpt.yaml | 58 +++++++++++++++++++ >>> 1 file changed, 58 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/timer/econet,timer-hpt.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/timer/econet,timer-hpt.yaml b/Documentation/devicetree/bindings/timer/econet,timer-hpt.yaml >>> new file mode 100644 >>> index 000000000000..8b7ff9bce947 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/timer/econet,timer-hpt.yaml >>> @@ -0,0 +1,58 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/timer/econet,timer-hpt.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: EcoNet High Precision Timer (HPT) >>> + >>> +maintainers: >>> + - Calev James DeLisle <cjd@xxxxxxxx> >>> + >>> +description: | >> Do not need '|' unless you need to preserve formatting. > Ok >> >>> + The EcoNet High Precision Timer (HPT) is a timer peripheral found in various >>> + EcoNet SoCs, including the EN751221 and EN751627 families. It provides per-VPE >>> + count/compare registers and a per-CPU control register, with a single interrupt >>> + line using a percpu-devid interrupt mechanism. >>> + >>> +properties: >>> + compatible: >>> + const: econet,timer-hpt >> Soc components must have soc-based compatible and then filename matching >> whatever you use as fallback. > > I have so far been unable to find good documentation on writing DT bindings > specifically for SoC devices. If you have anything to point me to, I will read it. > If not, even a good example of someone else doing it right is helpful. > > Currently, I see qcom,pdc.yaml appears to do what you say, so I in absence > of any other advice, I can try to do what they do. Just don't use generic fallback. > >> >>> + >>> + reg: >>> + minItems: 1 >>> + maxItems: 2 >> No, list items instead. > I see qcom,pdc.yaml using items: with per-item description so can follow that. >> >>> + description: | >>> + Physical base address and size of the timer's register space. On 34Kc >>> + processors, a single region is used. On 1004Kc processors, two regions are >>> + used, one for each core. >> So different hardware, different compatible. That's why you need >> soc-based compatibles. Follow standard SoC upstreaming rules and examples. > I presume this should ideally be with If: statements to further validate the DT (?) Yes >> >>> + >>> + interrupts: >>> + maxItems: 1 >>> + description: | >> Do not need '|' unless you need to preserve formatting. > Ok >> >>> + The interrupt number for the timer. >> Drop, redundant. > Ok >> >> >>> This is a percpu-devid interrupt shared >>> + across CPUs. >>> + >>> + clocks: >>> + maxItems: 1 >>> + description: | >>> + A clock to get the frequency of the timer. >> Drop description, redundant > Ok >> >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - interrupts >>> + - clocks >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + timer_hpt@1fbf0400 { >> No underscores > I knew that, my mistake. >> >> Node names should be generic. See also an explanation and list of >> examples (not exhaustive) in DT specification: >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > Thank you, this is useful. >> >> Look how other SoCs are calling this. > As said, any documentation link or example of someone who does this right > is much appreciated. In any case, thank you very much for your time and I > will address these points in v2. I gave one link above. Other could be one of my talks... or maybe what elinux.org has, but I did not verify it. Best regards, Krzysztof
