linux-kernel - Re: [PATCH v2 1/3] dt-bindings: serial: Add Loongson UART controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <778e50bc-58db-42e6-aee3-3b1e01ca227d@loongson.cn>
Date: Sun, 25 Aug 2024 11:34:17 +0800
From: 郑豪威 <zhenghaowei@...ngson.cn>
To: Krzysztof Kozlowski <krzk@...nel.org>, gregkh@...uxfoundation.org,
 jirislaby@...nel.org, robh@...nel.org, krzk+dt@...nel.org,
 conor+dt@...nel.org, chenhuacai@...nel.org, kernel@...0n.name,
 p.zabel@...gutronix.de, zhuyinbo <zhuyinbo@...ngson.cn>,
 Jianmin Lv <lvjianmin@...ngson.cn>, wanghongliang <wanghongliang@...ngson.cn>
Cc: linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org, loongarch@...ts.linux.dev
Subject: Re: [PATCH v2 1/3] dt-bindings: serial: Add Loongson UART controller


在 2024/8/12 16:25, Krzysztof Kozlowski 写道:
> On 12/08/2024 10:09, 郑豪威 wrote:
>> 在 2024/8/9 18:05, Krzysztof Kozlowski 写道:
>>> On 09/08/2024 11:55, 郑豪威 wrote:
>>>>>>>> +    description: Enables fractional-N division. Currently,
>>>>>>>> +      only LS2K1500 and LS2K2000 support this feature.
>>>>>>>> +
>>>>>>>> +  rts-invert:
>>>>>>>> +    description: Inverts the RTS value in the MCR register.
>>>>>>>> +      This should be used on Loongson-3 series CPUs, Loongson-2K
>>>>>>>> +      series CPUs, and Loongson LS7A bridge chips.
>>>>>>>> +
>>>>>>>> +  dtr-invert:
>>>>>>>> +    description: Inverts the DTR value in the MCR register.
>>>>>>>> +      This should be used on Loongson-3 series CPUs, Loongson-2K
>>>>>>>> +      series CPUs, and Loongson LS7A bridge chips.
>>>>>>>> +
>>>>>>>> +  cts-invert:
>>>>>>>> +    description: Inverts the CTS value in the MSR register.
>>>>>>>> +      This should be used on Loongson-2K0500, Loongson-2K1000,
>>>>>>>> +      and Loongson LS7A bridge chips.
>>>>>>>> +
>>>>>>>> +  dsr-invert:
>>>>>>>> +    description: Inverts the DSR value in the MSR register.
>>>>>>>> +      This should be used on Loongson-2K0500, Loongson-2K1000,
>>>>>>>> +      and Loongson LS7A bridge chips.
>>>>> Same questions for all these. Why choosing invert is a board level
>>>>> decision? If it "should be used" then why it is not used always?
>>>>>
>>>> Because these features are not applicable to all chips, such as
>>>> 'fractional-division',
>>> Hm?
>>>
>>>> which is currently supported only by 2K1500 and 2K2000, and for
>>>> Loongson-3 series
>>> These are SoCs. Compatible defines that. Please align with your
>>> colleagues, because *we talked about this* already.
>>>
>>> Best regards,
>>> Krzysztof
>> I consulted with my colleagues and would like to confirm with you. For
>> the five
>>
>> properties provided, fractional-division is offered as a new feature,
>> supported by
>>
>> 2K1500 and 2K2000. As for the invert property, it is due to a bug in our
>> controller,
>>
>> and its usage may vary across different chips. Should we add different
>> compatible
>>
>> values to address these issues for different chips, whether they are new
>> features or
>>
>> controller bugs?
> How did you align? We had already talks with you about this problem -
> you need specific compatibles. How you explain above properties, all of
> them are deducible from the compatible, so drop them.
>
> Your entire argument above does not address at all my concerns, so
> before you respond repeating the same, really talk with your colleagues.
>
> One of many previous discussions:
> https://lore.kernel.org/linux-devicetree/25c30964-6bd3-c7eb-640a-ba1f513b7675@linaro.org/
>
> https://lore.kernel.org/linux-devicetree/20230526-dolly-reheat-06c4d5658415@wendy/
>
> I wish we do not have to keep repeating the same to Loongson. Please
> STORE the feedback for any future submissions, so you will not repeat
> the same mistakes over and over.
>
> Best regards,
> Krzysztof

Hi:

I have been aligning with my colleagues over the past few days and

reviewing previous discussions. Based on these, I have made the

following modifications according to the differences in the controller:

+properties:
+  compatible:
+    oneOf:
+      - enum:
+          - loongson,ls7a-uart
+          - loongson,ls3a5000-uart
+          - loongson,ls2k2000-uart
+      - items:
+          - enum:
+              - loongson,ls2k1000-uart
+              - loongson,ls2k0500-uart
+          - const: loongson,ls7a-uart
+      - items:
+          - enum:
+              - loongson,ls2k1500-uart
+          - const: loongson,ls2k2000-uart
+      - items:
+          - enum:
+              - loongson,ls3a6000-uart
+          - const: loongson,ls3a5000-uart
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clock-frequency: true
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clock-frequency
+
+allOf:
+  - $ref: serial.yaml
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/clock/loongson,ls2k-clk.h>
+
+    serial@...20000 {
+        compatible = "loongson,ls2k1000-uart", "loongson,ls7a-uart";
+        reg = <0x1fe20000 0x10>;
+        clock-frequency = <125000000>;
+        interrupt-parent = <&liointc0>;
+        interrupts = <0x0 IRQ_TYPE_LEVEL_HIGH>;
+    };

Does this modification meet the expectation?

Best regards,

Haowei Zheng