Re: [PATCH v2 2/2] riscv: dts: sophgo: separate sg2042 mtime and mtimecmp to fit aclint format

[Inochi Amaoto]

>On Tue, Nov 14, 2023 at 09:47:19AM +0800, Inochi Amaoto wrote:
>>> On 2023/11/14 8:45, Inochi Amaoto wrote:
>>>> Change the timer layout in the dtb to fit the format that needed by
>>>> the SBI.
>>>>
>>>> Signed-off-by: Inochi Amaoto <inochiama@xxxxxxxxxxx>
>>>> Fixes: 967a94a92aaa ("riscv: dts: add initial Sophgo SG2042 SoC device tree")
>>>> ---
>>>>   arch/riscv/boot/dts/sophgo/sg2042.dtsi | 80 +++++++++++++++-----------
>>>>   1 file changed, 48 insertions(+), 32 deletions(-)
>>>>
>>>> diff --git a/arch/riscv/boot/dts/sophgo/sg2042.dtsi b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
>>>> index 93256540d078..0b5d93b5c783 100644
>>>> --- a/arch/riscv/boot/dts/sophgo/sg2042.dtsi
>>>> +++ b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
>>>> @@ -93,144 +93,160 @@ clint_mswi: interrupt-controller@7094000000 {
>>>>                             <&cpu63_intc 3>;
>>>>           };
>>>>
>>>> -        clint_mtimer0: timer@70ac000000 {
>>>> +        clint_mtimer0: timer@70ac004000 {
>>>
>>> The address of timer register is changed,  and I guess it is another change not directly related to the topic of this patch.
>>>
>>> Can you please add some comments in the commit message?
>>>
>>
>> As it needs to follow aclint format, the timer offset is applied to
>> identify the actual timer. So there is a change.
>>
>>>>               compatible = "sophgo,sg2042-aclint-mtimer", "thead,c900-aclint-mtimer";
>>>> -            reg = <0x00000070 0xac000000 0x00000000 0x00007ff8>;
>>>> +            reg = <0x00000070 0xac004000 0x00000000 0x00000000>,
>>> Why the length of first item is zero? Can you please add some clarification in commit message?
>>
>> I uses length zero to address that the mtimer is not supported, so the
>> SBI can know there is no mtimer in the timer.
>
>No, that's unacceptably hacky. If there is only one of the two registers
>present, then you need to provide only one of them, not spoof the
>presence of two. I suppose that means you need to add reg-names to the
>binding & get your registers by name in the SBI implementation, not by
>index.
>

Yes, this is more clear. And it can fully utilize the change from DT
binding. I will figure out how to improve this with reg-names.