[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <efb7bba5-b19c-49f5-8ff5-214ce070015d@kernel.org>
Date: Sun, 25 Aug 2024 08:55:57 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: 郑豪威 <zhenghaowei@...ngson.cn>,
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
On 25/08/2024 05:34, 郑豪威 wrote:
>
> 在 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?
Yes, assuming ls7a is a specific SoC, not a family of SoC.
Best regards,
Krzysztof
Powered by blists - more mailing lists