On 07/08/2024 10:23, 郑豪威 wrote:
>
> 在 2024/8/4 16:43, Krzysztof Kozlowski 写道:
>> On 04/08/2024 08:38,zhenghaowei@xxxxxxxxxxx wrote:
>>
>> Due to lack of changelog, I assume you send the same patch, so:
>>
>> <form letter>
>> This is a friendly reminder during the review process.
>>
>> It seems my or other reviewer's previous comments were not fully
>> addressed. Maybe the feedback got lost between the quotes, maybe you
>> just forgot to apply it. Please go back to the previous discussion and
>> either implement all requested changes or keep discussing them.
>>
>> Thank you.
>> </form letter>
>>
>> Also:
>>
>>> +
>>> + clocks:
>>> + maxItems: 1
>>> +
>>> + fractional-division:
>> Where are this and following defined? In which schema?
>>
> These and the ones below are new definitions, can I use them like this?
>
> + fractional-division:
> + description: Enables fractional-N division. Currently,
> + only LS2K1500 and LS2K2000 support this feature.
> + type: boolean
>
Missing vendor prefix, but what's more important, why would this be
property of DT? Just enable it always...
>>> + 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?
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - interrupts
>>> + - clocks
>>> +
>>> +allOf:
>>> + - $ref: serial.yaml
>>> +
>>> +unevaluatedProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + #include <dt-bindings/interrupt-controller/irq.h>
>>> + #include <dt-bindings/clock/loongson,ls2k-clk.h>
>>> +
>>> + serial@1fe001e0 {
>>> + compatible = "loongson,ls7a-uart";
>>> + reg = <0x0 0x1fe001e0 0x0 0x10>;
>>> + clock-frequency = <100000000>;
>>> + interrupt-parent = <&liointc>;
>>> + interrupts = <10 IRQ_TYPE_LEVEL_HIGH>;
>>> + fractional-division;
>>> + rts-invert;
>>> + dtr-invert;
>>> + };
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 8766f3e5e87e..a6306327dba5 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -13189,6 +13189,13 @@ S: Maintained
>>> F: Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
>>> F: drivers/i2c/busses/i2c-ls2x.c
>>>
>>> +LOONGSON UART DRIVER
>>> +M: Haowei Zheng<zhenghaowei@xxxxxxxxxxx>
>>> +L: linux-serial@xxxxxxxxxxxxxxx
>>> +S: Maintained
>>> +F: Documentation/devicetree/bindings/serial/loongson,ls7a-uart.yaml
>>> +F: drivers/tty/serial/8250/8250_loongson.c
>> There is no such file.
>>
>> Best regards,
>> Krzysztof
>
> The file "drivers/tty/serial/8250/8250_loongson.c" will be created in
> the patch
>
> "tty: serial: 8250: Add loongson uart driver support". Is it
> inappropriate to reference it here?
Apply this patch and run get_maintainers self tests. What do you see?
Of course it is not appropriate here. The file does not exist.
Best regards,
Krzysztof
