Re: [PATCH 1/6] dt-bindings: loongarch: Add CPU bindings for LoongArch

On 17/06/2023 08:31, Binbin Zhou wrote:

On Fri, Jun 16, 2023 at 5:51 PM Krzysztof Kozlowski
[off-list ref] wrote:

quoted

On 16/06/2023 08:10, Binbin Zhou wrote:

quoted

Add the available CPUs in LoongArch binding with DT schema format using
json-schema.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 .../devicetree/bindings/loongarch/cpus.yaml   | 65 +++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/loongarch/cpus.yaml

diff --git a/Documentation/devicetree/bindings/loongarch/cpus.yaml b/Documentation/devicetree/bindings/loongarch/cpus.yaml
new file mode 100644
index 000000000000..c3e2dba42c81
--- /dev/null
+++ b/Documentation/devicetree/bindings/loongarch/cpus.yaml

@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/loongarch/cpus.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LoongArch CPUs
+
+maintainers:
+  - Binbin Zhou <zhoubinbin@loongson.cn>
+
+description:
+  The device tree allows to describe the layout of CPUs in a system through
+  the "cpus" node, which in turn contains a number of subnodes (ie "cpu")
+  defining properties for every CPU.

I understand you copied it from ARM, but I would prefer to have here
something meaningful. Bindings description does not explain what is DTS,
but explains what the hardware is.

Hi Krzysztof:

I am very sorry, this is my problem and I will rewrite this part.

quoted

quoted

+
+properties:
+  compatible:
+    enum:
+      - loongson,la264
+      - loongson,la364
+
+  reg:
+    maxItems: 1
+
+  device_type: true
+
+  clock-frequency:
+    description: The frequency of cpu in Hz.
+
+  model:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: User-visible cpu name in /proc/cpuinfo.

First, aren't you mixing nodes?
Second, it is derived from compatible, so no need for such property.

Well, this attribute is an attempt to tweak it.

As the description says, this attribute was added to show the model
name in /proc/cpuinfo. here, we will show the custom name instead of
using the cpu core name directly.

DTS is not some translation layer. You can decode compatible and put
into /proc/cpuinfo whatever you wish.

For example, on a Loongson-3A5000 machine, although its cpu core is
la464, we can see:
[root@fedora ~]# cat /proc/cpuinfo
system type : generic-loongson-machine
..............
Model Name : Loongson-3A5000-HV
............
CPU MHz : 2500.00
...........

Unfortunately, some Loongson-2K chips are not designed with
corresponding CPUNAME registers, so we expect to add them in the DTS.

At first, we considered writing it directly into cpu compatible, but
it seems that dts compatible is all lower case, while our desired
model name contains upper case letters.

What do you think if we repositioned this attribute under cpu?

You already did it, so about which solution we talk about?

Anyway, I am against store translation of compatibles in DTS.

Best regards,
Krzysztof