On 17/06/2023 08:31, Binbin Zhou wrote: > On Fri, Jun 16, 2023 at 5:51 PM Krzysztof Kozlowski > <krzysztof.kozlowski@xxxxxxxxxx> wrote: >> >> On 16/06/2023 08:10, Binbin Zhou wrote: >>> Add the available CPUs in LoongArch binding with DT schema format using >>> json-schema. >>> >>> Signed-off-by: Binbin Zhou <zhoubinbin@xxxxxxxxxxx> >>> --- >>> .../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@xxxxxxxxxxx> >>> + >>> +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. > >> >>> + >>> +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