Re: [PATCH v3 2/2] dt-bindings: perf: starfive: Add StarLink PMU

On Thu, 16 Nov 2023 14:34:00 +0000
Conor Dooley [off-list ref] wrote:

On Thu, Nov 16, 2023 at 10:10:35AM +0800, Ji Sheng Teoh wrote:

quoted

On Wed, 15 Nov 2023 20:03:53 +0000
Conor Dooley [off-list ref] wrote:  

quoted

On Wed, Nov 15, 2023 at 11:36:08AM +0800, Ji Sheng Teoh wrote:  

quoted

Add device tree binding for StarFive's StarLink PMU (Performance
Monitor Unit).

Signed-off-by: Ji Sheng Teoh <jisheng.teoh@starfivetech.com>
---
 .../bindings/perf/starfive,starlink-pmu.yaml  | 46
+++++++++++++++++++ 1 file changed, 46 insertions(+)
 create mode 100644
Documentation/devicetree/bindings/perf/starfive,starlink-pmu.yaml

diff --git
a/Documentation/devicetree/bindings/perf/starfive,starlink-pmu.yaml
b/Documentation/devicetree/bindings/perf/starfive,starlink-pmu.yaml
new file mode 100644 index 000000000000..a9426a7faeae ---
/dev/null +++
b/Documentation/devicetree/bindings/perf/starfive,starlink-pmu.yaml
   

btw, since you changed the compatible, the filename should have
been changed to match it.  

The intention to keep the filename generic is to allow addition of
new version of StarLink PMU in future if any, similar to what
arm,cmn.yaml is doing. Hope that makes sense.  

No, please keep the filename matching the compatible. Even if the
filename contains "500", there's nothing stopping you from then adding
other pmu variants. There are many many examples of this in the tree.

Sure, will do that in v4. 

quoted

quoted

quoted

@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id:

http://devicetree.org/schemas/perf/starfive,starlink-pmu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml# +
+title: StarFive StarLink PMU
+
+maintainers:
+  - Ji Sheng Teoh [off-list ref]
+
+description:
+  StarFive's StarLink PMU integrates one or more CPU cores
with a shared L3
+  memory system. The PMU support overflow interrupt, up to 16
programmable
+  64bit event counters, and an independent 64bit cycle counter.
+  StarLink PMU is accessed via MMIO.
+
+properties:
+  compatible:
+    const: starfive,starlink-500-pmu    

So this is not what I had in mind by a "device". I was looking
for a compatible representing an soc in which this IP had been
integrated. A soc-specific compatible, rather than something
generic, is requirement for devicetree - we don't want various
integrations of this IP to all be using a generic compatible when
there may be subtle (or less subtle) differences between
integrations.

I'm trying to come up with the syntax for enforcing having two
compatibles with your current one as the fallback, but I have yet
to come up with the correct syntax for that that works correctly.

Hopefully by the time you get some feedback on the driver side of
this submission I will have a concrete suggestion for what to do
here.  

Thanks Conor for the enlightenment. In the meantime, to fit the
requirement I would suggest going for
"starfive,jh8100-starlink-pmu", making it JH8100 SOC specific if
that makes sense.  

Okay, you could definitely do that!

Cheers,
Conor.

Ok, will use that in v4. Thanks!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel