Message-ID: <1f423ccd-9b2b-3416-c594-7ded6a3b32b6@loongson.cn>
Date:   Tue, 27 Sep 2022 14:39:34 +0800
From:   Yinbo Zhu <zhuyinbo@...ngson.cn>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     Rob Herring <robh@...nel.org>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Amit Kucheria <amitk@...nel.org>,
        Zhang Rui <rui.zhang@...el.com>, linux-pm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        zhanghongchen <zhanghongchen@...ngson.cn>,
        刘佩宝 <liupeibao@...ngson.cn>,
        Yinbo Zhu <zhuyinbo@...ngson.cn>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Amit Kucheria <amitk@...nel.org>,
        Zhang Rui <rui.zhang@...el.com>, linux-pm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        zhanghongchen <zhanghongchen@...ngson.cn>,
        刘佩宝 <liupeibao@...ngson.cn>
Subject: Re: [PATCH v2 2/3] dt-bindings: thermal: Convert loongson2 to
 json-schema


在 2022/9/26 下午4:38, Yinbo Zhu 写道:
>
>
>> -----原始邮件-----
>> 发件人: "Krzysztof Kozlowski" <krzysztof.kozlowski@...aro.org>
>> 发送时间:2022-09-26 14:21:41 (星期一)
>> 收件人: "Yinbo Zhu" <zhuyinbo@...ngson.cn>, "Rob Herring" <robh@...nel.org>
>> 抄送: "Rafael J . Wysocki" <rafael@...nel.org>, "Daniel Lezcano" <daniel.lezcano@...aro.org>, "Amit Kucheria" <amitk@...nel.org>, "Zhang Rui" <rui.zhang@...el.com>, linux-pm@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, zhanghongchen <zhanghongchen@...ngson.cn>, "刘佩宝" <liupeibao@...ngson.cn>
>> 主题: Re: [PATCH v2 2/3] dt-bindings: thermal: Convert loongson2 to json-schema
>>
>> On 26/09/2022 06:00, Yinbo Zhu wrote:
>>>
>>>
>>>> -----原始邮件-----
>>>> 发件人: "Rob Herring" <robh@...nel.org>
>>>> 发送时间:2022-09-25 01:42:58 (星期日)
>>>> 收件人: "朱银波" <zhuyinbo@...ngson.cn>
>>>> 抄送: "Krzysztof Kozlowski" <krzysztof.kozlowski@...aro.org>, "Rafael J . Wysocki" <rafael@...nel.org>, "Daniel Lezcano" <daniel.lezcano@...aro.org>, "Amit Kucheria" <amitk@...nel.org>, "Zhang Rui" <rui.zhang@...el.com>, linux-pm@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, zhanghongchen <zhanghongchen@...ngson.cn>
>>>> 主题: Re: Re: [PATCH v2 2/3] dt-bindings: thermal: Convert loongson2 to json-schema
>>>>
>>>> On Thu, Sep 22, 2022 at 09:39:30AM +0800, 朱银波 wrote:
>>>>>
>>>>>
>>>>>> -----原始邮件-----
>>>>>> 发件人: "Krzysztof Kozlowski" <krzysztof.kozlowski@...aro.org>
>>>>>> 发送时间:2022-09-21 17:31:11 (星期三)
>>>>>> 收件人: "朱银波" <zhuyinbo@...ngson.cn>
>>>>>> 抄送: "Rafael J . Wysocki" <rafael@...nel.org>, "Daniel Lezcano" <daniel.lezcano@...aro.org>, "Amit Kucheria" <amitk@...nel.org>, "Zhang Rui" <rui.zhang@...el.com>, linux-pm@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, zhanghongchen <zhanghongchen@...ngson.cn>
>>>>>> 主题: Re: [PATCH v2 2/3] dt-bindings: thermal: Convert loongson2 to json-schema
>>>>>>
>>>>>> On 21/09/2022 11:22, 朱银波 wrote:
>>>>>>>> -----原始邮件-----
>>>>>>>> 发件人: "Krzysztof Kozlowski" <krzysztof.kozlowski@...aro.org>
>>>>>>>> 发送时间:2022-09-21 15:05:00 (星期三)
>>>>>>>> 收件人: "Yinbo Zhu" <zhuyinbo@...ngson.cn>, "Rafael J . Wysocki" <rafael@...nel.org>, "Daniel Lezcano" <daniel.lezcano@...aro.org>, "Amit Kucheria" <amitk@...nel.org>, "Zhang Rui" <rui.zhang@...el.com>, "Rob Herring" <robh+dt@...nel.org>, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@...aro.org>, linux-pm@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
>>>>>>>> 抄送: zhanghongchen <zhanghongchen@...ngson.cn>
>>>>>>>> 主题: 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@...ngson.cn>
>>>>>>>>> +  - Yinbo Zhu <zhuyinbo@...ngson.cn>
>>>>>>>>> +
>>>>>>>>> +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.
>>> If other loongson2 platforms are different from the thermal sensor, I will add compatible
>>> "loongson,loongson2-thermal-xxx", please you note.
>> No, this compatible looks wrong then. What is your SoC model number? You
>> called loongson2 a "series", so that's not appropriate. Compatible
>> should be specific.
> Must it be strict here? I have a look about other platforms are similar to this one, eg.
> examples:
>    - |
>      tmu@...00 {
>          compatible = "fsl,qoriq-tmu";
> the qoriq series soc include ls1046a/ls1043/ls1088a/...
>
> and the compatible was "loongson,loongson2-xxx" is internal rules, we hope that code can as the rules
> to upstream.
>
>>>>>>>>> +    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.
>>> okay ,I got it.
>>>>>>>>> +
>>>>>>>>> +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.
>>> I don't get your meaning, You mean I can add it in the dts, but I don't need to add it in the binding file, right?
>> You should not add it to the binding because it is not related to the
>> device itself.
> okay, I got it.
>>>> Rob
>>>
>> Don't include such footers. We cannot talk over confidential emails and
>> you clearly state that here.
>>
>> Best regards,
>> Krzysztof

I'm sorry to add this footers,  this is a problem with my website 
mailbox. Later, I use my local mailbox to send emails,

there will be no footprints in my local mailbox.


TKs,

BRs

Yinbo Zhu.

>

Powered by blists - more mailing lists