On Thu, Sep 22, 2022 at 09:39:30AM +0800, 朱银波 wrote: > > > > > -----原始邮件----- > > 发件人: "Krzysztof Kozlowski" <krzysztof.kozlowski@xxxxxxxxxx> > > 发送时间:2022-09-21 17:31:11 (星期三) > > 收件人: "朱银波" <zhuyinbo@xxxxxxxxxxx> > > 抄送: "Rafael J . Wysocki" <rafael@xxxxxxxxxx>, "Daniel Lezcano" <daniel.lezcano@xxxxxxxxxx>, "Amit Kucheria" <amitk@xxxxxxxxxx>, "Zhang Rui" <rui.zhang@xxxxxxxxx>, linux-pm@xxxxxxxxxxxxxxx, devicetree@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, zhanghongchen <zhanghongchen@xxxxxxxxxxx> > > 主题: Re: [PATCH v2 2/3] dt-bindings: thermal: Convert loongson2 to json-schema > > > > On 21/09/2022 11:22, 朱银波 wrote: > > >> -----原始邮件----- > > >> 发件人: "Krzysztof Kozlowski" <krzysztof.kozlowski@xxxxxxxxxx> > > >> 发送时间:2022-09-21 15:05:00 (星期三) > > >> 收件人: "Yinbo Zhu" <zhuyinbo@xxxxxxxxxxx>, "Rafael J . Wysocki" <rafael@xxxxxxxxxx>, "Daniel Lezcano" <daniel.lezcano@xxxxxxxxxx>, "Amit Kucheria" <amitk@xxxxxxxxxx>, "Zhang Rui" <rui.zhang@xxxxxxxxx>, "Rob Herring" <robh+dt@xxxxxxxxxx>, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@xxxxxxxxxx>, linux-pm@xxxxxxxxxxxxxxx, devicetree@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx > > >> 抄送: zhanghongchen <zhanghongchen@xxxxxxxxxxx> > > >> 主题: Re: [PATCH v2 2/3] dt-bindings: thermal: Convert loongson2 to json-schema > > >> > > >> On 21/09/2022 03:56, Yinbo Zhu wrote: > > >>> Convert the loongson2 thermal binding to DT schema format using > > >>> json-schema. > > >> > > >> Incorrect subject and incorrect commit msg. There is no conversion here. > > > Our soc architecture is the loongson2 series, so we will modify it accordingly. > > > > How the soc architecture is related to my comment that you do not > > perform conversion? > I got it, and I will aad a conversion. > > > > > > > >> > > >>> > > >>> Signed-off-by: Yinbo Zhu <c> > > >>> --- > > >>> Change in v2: > > >>> 1. Add description and type about the "id". > > >>> 2. Make the filename was based on compatible. > > >>> > > >>> .../bindings/thermal/loongson2-thermal.yaml | 52 +++++++++++++++++++ > > >>> 1 file changed, 52 insertions(+) > > >>> create mode 100644 Documentation/devicetree/bindings/thermal/loongson2-thermal.yaml > > >>> > > >>> diff --git a/Documentation/devicetree/bindings/thermal/loongson2-thermal.yaml b/Documentation/devicetree/bindings/thermal/loongson2-thermal.yaml > > >>> new file mode 100644 > > >>> index 000000000000..2994ae3a56aa > > >>> --- /dev/null > > >>> +++ b/Documentation/devicetree/bindings/thermal/loongson2-thermal.yaml > > >> > > >> > > >> No improvements here. You ignore my comments, so I am going to NAK it. > > > I don't get your point, that dts compatible is "loongson,loongson2-thermal", so this driver file name is named > > > loongson2-thermal that according what you said about "Filename based on compatible." > > > If what I understand is not what you expect, please tell me how to modify it. > > > > > > Filename must match the compatible, so: loongson,loongson2-thermal.yaml > I got it, and I will add a conversion. > > > > >> > > >> > > >>> @@ -0,0 +1,52 @@ > > >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > >>> +%YAML 1.2 > > >>> +--- > > >>> +$id: http://devicetree.org/schemas/thermal/loongson2-thermal.yaml# > > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > > >>> + > > >>> +title: Thermal sensors on loongson2 SoCs > > >>> + > > >>> +maintainers: > > >>> + - zhanghongchen <zhanghongchen@xxxxxxxxxxx> > > >>> + - Yinbo Zhu <zhuyinbo@xxxxxxxxxxx> > > >>> + > > >>> +properties: > > >>> + compatible: > > >>> + const: loongson,loongson2-thermal > > >>> + > > >>> + reg: > > >>> + maxItems: 1 > > >>> + > > >>> + id: > > >>> + $ref: '//schemas/types.yaml#/definitions/uint32' > > >> > > >> No improvements here, so let me be specific - you need to really justify > > >> such property or it cannot go to schema. > > > The loongson2_thermal.c driver need parse this "id" property. > > > > This is not reason to add properties to DT. DT describes the hardware, > > not driver behavior. > > > > Why hardware needs arbitrary, additional addressing number instead of > > standard unit address? > The loongson2 series soc supports up to four sensors, but the 2K1000 has only one sensor, so the ID must be 0. > For the 2K1000, in order to distinguish the differences between different hardware in the Loongson2 SoC series, > the ID is added to the dts Differences in SoCs is what 'compatible' is for. If 'loongson2' is not a specific SoC, then your compatible string is not specific enough. > > > > >> > > >>> + description: | > > >>> + Specify the thermal sensor id. > > >>> + minimum: 0 > > >>> + maximum: 3 > > >>> + > > >>> + interrupts: > > >>> + maxItems: 1 > > >>> + > > >>> + "#thermal-sensor-cells": > > >>> + const: 1 If one SoC only has 1 sensor, then this could be 0. However, you don't have to do that, but it's another way to distinguish differences. > > >>> + > > >>> +required: > > >>> + - compatible > > >>> + - reg > > >>> + - id > > >>> + - interrupt-parent > > >> > > >> Why? > > > The interrupts of our dts do not specify an interrupt parent, > > > eg. interrupts = <7 IRQ_TYPE_LEVEL_LOW> > > > so we need to add an interrupt parent property. > > > > You can add but I am asking why is it required? > Since there is more than one interrupt controller in the Loongson2 series soc, that need to specify the interrupt > controller in the dts, that is, the interrupt parent. If different interrupt parents are used in dts, the interrupt > numbers are different. It is perfectly valid for the 'interrupt-parent' to be in *any* parent node. So it is never required by any binding. Rob