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

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] [day] [month] [year] [list]

Message-ID: <IA1PR20MB495391BFC32D280993485C04BBB2A@IA1PR20MB4953.namprd20.prod.outlook.com>
Date:   Wed, 15 Nov 2023 07:13:33 +0800
From:   Inochi Amaoto <inochiama@...look.com>
To:     Conor Dooley <conor@...nel.org>
Cc:     Inochi Amaoto <inochiama@...look.com>,
        Chen Wang <unicorn_wang@...look.com>,
        Chao Wei <chao.wei@...hgo.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Xiaoguang Xing <xiaoguang.xing@...hgo.com>,
        Guo Ren <guoren@...nel.org>,
        Jisheng Zhang <jszhang@...nel.org>,
        linux-riscv@...ts.infradead.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] riscv: dts: sophgo: separate sg2042 mtime and mtimecmp to fit aclint format

>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@...look.com>
>>>> 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@...4000000 {
>>>>                             <&cpu63_intc 3>;
>>>>           };
>>>>
>>>> -        clint_mtimer0: timer@...c000000 {
>>>> +        clint_mtimer0: timer@...c004000 {
>>>
>>> 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.