Re: [PATCH v2 06/11] dt-bindings: timer: Add Sophgo sg2042 clint

[Inochi Amaoto]

>On Wed, Sep 20, 2023 at 02:39:39PM +0800, Chen Wang wrote:
>> From: Inochi Amaoto <inochiama@xxxxxxxxxxx>
>>
>> Add two new compatible string formatted like `C9xx-clint-xxx` to identify
>> the timer and ipi device separately, and do not allow c900-clint as the
>> fallback to avoid conflict.
>>
>> Signed-off-by: Inochi Amaoto <inochiama@xxxxxxxxxxx>
>> Signed-off-by: Chen Wang <wangchen20@xxxxxxxxxxx>
>
>Have you ignored Krzysztof's comments on this? I don't see a response or
>a reaction to his comments about the compatibles on the last version.
>Additionally, where is the user for these? I don't see any drivers that
>actually make use of these.
>

Sorry for late reply.

The clint is parsed by sbi. As use the same compatible, the opensbi will
parse the device twice. This will cause a fault.

>Why do you need to have 2 compatibles (and therefore 2 devices) for the
>clint? I thought the clint was a single device, of which the mtimer and
>mswi bits were just "features"? Having split register ranges isn't a
>reason to have two compatibles, so I must be missing something here...
>
>Thanks,
>Conor.
>

Sorry for late reply, The clint consists of mtimer and ipi devices, which
is defined in [1]. This standard shows clint(or the aclint) has two device,
but not one. In another word, there is no need to defined mtimer and ipi
device on the same base address. So we need two compatibles to allow sbi
to identify them correctly.

[1] https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc

>> ---
>>  Documentation/devicetree/bindings/timer/sifive,clint.yaml | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.yaml b/Documentation/devicetree/bindings/timer/sifive,clint.yaml
>> index a0185e15a42f..ae69696c5c75 100644
>> --- a/Documentation/devicetree/bindings/timer/sifive,clint.yaml
>> +++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml
>> @@ -39,6 +39,14 @@ properties:
>>                - allwinner,sun20i-d1-clint
>>                - thead,th1520-clint
>>            - const: thead,c900-clint
>> +      - items:
>> +          - enum:
>> +              - sophgo,sg2042-clint-mtimer
>> +          - const: thead,c900-clint-mtimer
>> +      - items:
>> +          - enum:
>> +              - sophgo,sg2042-clint-mswi
>> +          - const: thead,c900-clint-mswi
>>        - items:
>>            - const: sifive,clint0
>>            - const: riscv,clint0
>> --
>> 2.25.1
>>
>