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

From: Conor Dooley <conor@kernel.org>
To: Inochi Amaoto <inochiama@outlook.com>
Cc: Chen Wang <unicorn_wang@outlook.com>,
	Chao Wei <chao.wei@sophgo.com>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Xiaoguang Xing <xiaoguang.xing@sophgo.com>,
	Guo Ren <guoren@kernel.org>, Jisheng Zhang <jszhang@kernel.org>,
	linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] riscv: dts: sophgo: separate sg2042 mtime and mtimecmp to fit aclint format
Date: Tue, 14 Nov 2023 15:24:56 +0000	[thread overview]
Message-ID: <20231114-skedaddle-precinct-66c8897227bb@squawk> (raw)
In-Reply-To: <IA1PR20MB49531C1FCBAB0E19CAFE19DFBBB2A@IA1PR20MB4953.namprd20.prod.outlook.com>

[-- Attachment #1.1: Type: text/plain, Size: 2087 bytes --]

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@outlook.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@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.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv