| Message ID | 55865e1ce40d2017f047d3a9e1a9ee30043b271f.1695189879.git.wangchen20@iscas.ac.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add Milk-V Pioneer RISC-V board support | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Series has a cover letter |
| conchuod/tree_selection | success | Guessed tree name to be for-next at HEAD 0bb80ecc33a8 |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 6 and now 6 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 9 this patch: 9 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 9 this patch: 9 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 29 this patch: 29 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 14 lines checked |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
On Wed, Sep 20, 2023 at 2:39 PM Chen Wang <unicornxw@gmail.com> wrote: > > From: Inochi Amaoto <inochiama@outlook.com> > > 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. Please explain more about the c900-clint mtimer & mswi, why do we need to separate the c900-clint into two pieces? When could we use c900-clint which eases dts design? > > Signed-off-by: Inochi Amaoto <inochiama@outlook.com> > Signed-off-by: Chen Wang <wangchen20@iscas.ac.cn> > --- > 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 >
On Wed, Sep 20, 2023 at 02:39:39PM +0800, Chen Wang wrote: > From: Inochi Amaoto <inochiama@outlook.com> > > 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@outlook.com> > Signed-off-by: Chen Wang <wangchen20@iscas.ac.cn> 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. 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. > --- > 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 >
>On Wed, Sep 20, 2023 at 02:39:39PM +0800, Chen Wang wrote: >> From: Inochi Amaoto <inochiama@outlook.com> >> >> 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@outlook.com> >> Signed-off-by: Chen Wang <wangchen20@iscas.ac.cn> > >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 and wrong message-id. 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 >> >
Yo, On Wed, Sep 20, 2023 at 05:08:41PM +0800, Inochi Amaoto wrote: > >On Wed, Sep 20, 2023 at 02:39:39PM +0800, Chen Wang wrote: > >> From: Inochi Amaoto <inochiama@outlook.com> > >> > >> 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@outlook.com> > >> Signed-off-by: Chen Wang <wangchen20@iscas.ac.cn> > > > >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 and wrong message-id. > > The clint is parsed by sbi. That needs to go in the commit message. > As use the same compatible, the opensbi will > parse the device twice. This will cause a fault. Then only have one compatible with 2 register ranges? Then your SBI implementation can use those two register ranges to find out the base address for the mtimer bits and for the mswi bits. I don't understand why this cannot be done, could you please explain. I also don't see anything in the opensbi repo right now that is using these (nor could I easily see any patches for opensbi adding this). Is there another SBI implementation that you are using that I can take a look at to try and understand this better? > >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... > Sorry for late reply, The clint consists of mtimer and ipi devices, which > is defined in [1]. Yes, I have looked at the spec. I went to check it again before replying here in case there was something immediately obvious that I was missing. > This standard shows clint(or the aclint) has two device, The wording used here doesn't really matter. It's one interrupt controller that does mtimer and mswi. > but not one. In another word, there is no need to defined mtimer and ipi > device on the same base address. There's also no need to have two compatibles for the same interrupt controller, so I do not get this reasoning. What actually _requires_ them to be split? > So we need two compatibles to allow sbi to identify them correctly. Why is it not sufficient to identify the individual memory regions? Thanks, Conor.
> >Yo, > >On Wed, Sep 20, 2023 at 05:08:41PM +0800, Inochi Amaoto wrote: >>> On Wed, Sep 20, 2023 at 02:39:39PM +0800, Chen Wang wrote: >>>> From: Inochi Amaoto <inochiama@outlook.com> >>>> >>>> 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@outlook.com> >>>> Signed-off-by: Chen Wang <wangchen20@iscas.ac.cn> >>> >>> 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 and wrong message-id. >> >> The clint is parsed by sbi. > >That needs to go in the commit message. Yes, it will. > >> As use the same compatible, the opensbi will >> parse the device twice. This will cause a fault. > >Then only have one compatible with 2 register ranges? Then your SBI >implementation can use those two register ranges to find out the base >address for the mtimer bits and for the mswi bits. >I don't understand why this cannot be done, could you please explain. That is a good idea, but now SBI use the second register ranges as mtimecmp address for aclint. And there is a aclint-mswi in the SBI. Maybe a change is needed? >I also don't see anything in the opensbi repo right now that is using >these (nor could I easily see any patches for opensbi adding this). >Is there another SBI implementation that you are using that I can take >a look at to try and understand this better? > This will be sumbit in a short time. Now we only use it is sophgo vendor SBI, which url is [1]. [1] https://github.com/sophgo/opensbi >>> 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... > >> Sorry for late reply, The clint consists of mtimer and ipi devices, which >> is defined in [1]. > >Yes, I have looked at the spec. I went to check it again before replying >here in case there was something immediately obvious that I was missing. > I think nothing missed. >> This standard shows clint(or the aclint) has two device, > >The wording used here doesn't really matter. It's one interrupt >controller that does mtimer and mswi. > >> but not one. In another word, there is no need to defined mtimer and ipi >> device on the same base address. > >There's also no need to have two compatibles for the same interrupt >controller, so I do not get this reasoning. What actually _requires_ >them to be split? > Yes, it is one, but can be mapped into different address. So I think we need two. >> So we need two compatibles to allow sbi to identify them correctly. > >Why is it not sufficient to identify the individual memory regions? > FYI, Anup. As I have no idea for aclint implementation. >Thanks, >Conor. >
On 20/09/2023 08:39, Chen Wang wrote: > From: Inochi Amaoto <inochiama@outlook.com> > > 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 Why? You received comment about it, so please provide proper explanation in the commit msg. Same device does not get two different compatibles. You also did not respond to my comments, so you basically ignored it and send the same. NAK Best regards, Krzysztof
>On 20/09/2023 08:39, Chen Wang wrote: >> From: Inochi Amaoto <inochiama@outlook.com> >> >> 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 > >Why? > If use the same compatible, SBI will process this twice in both ipi and timer, use different compatible will allow SBI to treat these as different. AFAIK, the aclint in SBI use the same concepts, which make hard to use the second register range. I have explained in another response. https://lore.kernel.org/all/IA1PR20MB495313B7E9B2FC529BE0BB2ABBF9A@IA1PR20MB4953.namprd20.prod.outlook.com/ >You received comment about it, so please provide proper explanation in >the commit msg. > >Same device does not get two different compatibles. > >You also did not respond to my comments, so you basically ignored it and >send the same. > >NAK > >Best regards, >Krzysztof > Sorry for this, as I mistake the idea of the last message. All of this will be fixed in the next patch.
On 20/09/2023 14:15, Inochi Amaoto wrote: >> On 20/09/2023 08:39, Chen Wang wrote: >>> From: Inochi Amaoto <inochiama@outlook.com> >>> >>> 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 >> >> Why? >> > > If use the same compatible, SBI will process this twice in both ipi and > timer, use different compatible will allow SBI to treat these as different. > AFAIK, the aclint in SBI use the same concepts, which make hard to use the > second register range. I have explained in another response. What is a SBI? Linux driver? If so, why some intermediate Linux driver choice should affect bindings? Best regards, Krzysztof
>On 20/09/2023 14:15, Inochi Amaoto wrote: >>> On 20/09/2023 08:39, Chen Wang wrote: >>>> From: Inochi Amaoto <inochiama@outlook.com> >>>> >>>> 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 >>> >>> Why? >>> >> >> If use the same compatible, SBI will process this twice in both ipi and >> timer, use different compatible will allow SBI to treat these as different. >> AFAIK, the aclint in SBI use the same concepts, which make hard to use the >> second register range. I have explained in another response. > >What is a SBI? Linux driver? If so, why some intermediate Linux driver >choice should affect bindings? >Best regards, >Krzysztof > SBI (Supervisor Binary Interface) is defined by riscv, which is an interface between the Supervisor Execution Environment (SEE) and the supervisor. The detailed documentation can be found in [1]. The implement of SBI needs fdt info of the platform, which is provided by kernel. So we need a dt-bindings for these devices, and these will be processed by SBI. [1] https://github.com/riscv-non-isa/riscv-sbi-doc
On Wed, Sep 20, 2023 at 08:40:07PM +0800, Inochi Amaoto wrote: > >On 20/09/2023 14:15, Inochi Amaoto wrote: > >>> On 20/09/2023 08:39, Chen Wang wrote: > >>>> From: Inochi Amaoto <inochiama@outlook.com> > >>>> > >>>> 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 > >>> > >>> Why? > >>> > >> > >> If use the same compatible, SBI will process this twice in both ipi and > >> timer, use different compatible will allow SBI to treat these as different. > >> AFAIK, the aclint in SBI use the same concepts, which make hard to use the > >> second register range. I have explained in another response. > > > >What is a SBI? Linux driver? If so, why some intermediate Linux driver > >choice should affect bindings? > >Best regards, > >Krzysztof > > > > SBI (Supervisor Binary Interface) is defined by riscv, which is an interface > between the Supervisor Execution Environment (SEE) and the supervisor. The > detailed documentation can be found in [1]. > > The implement of SBI needs fdt info of the platform, which is provided by > kernel. So we need a dt-bindings for these devices, and these will be > processed by SBI. > > [1] https://github.com/riscv-non-isa/riscv-sbi-doc Yeah, this is the unfortunate problem of half-baked bindings (IMO) ending up in OpenSBI (which likely means they also ended up in QEMU). This T-Head stuff is coming across our (metaphorical) desks, so we are obviously going to try to do things correctly. I may end up speaking to Anup later today, if I do I will point him at this thread (if he hasn't seen it already).
On Wed, Sep 20, 2023 at 07:24:21PM +0800, Inochi Amaoto wrote: > > > >Yo, > > > >On Wed, Sep 20, 2023 at 05:08:41PM +0800, Inochi Amaoto wrote: > >>> On Wed, Sep 20, 2023 at 02:39:39PM +0800, Chen Wang wrote: > >>>> From: Inochi Amaoto <inochiama@outlook.com> > >>>> > >>>> 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@outlook.com> > >>>> Signed-off-by: Chen Wang <wangchen20@iscas.ac.cn> > >>> > >>> 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 and wrong message-id. > >> > >> The clint is parsed by sbi. > > > >That needs to go in the commit message. > > Yes, it will. Thanks. > >> As use the same compatible, the opensbi will > >> parse the device twice. This will cause a fault. > > > >Then only have one compatible with 2 register ranges? Then your SBI > >implementation can use those two register ranges to find out the base > >address for the mtimer bits and for the mswi bits. > >I don't understand why this cannot be done, could you please explain. > > That is a good idea, but now SBI use the second register ranges as > mtimecmp address for aclint. And there is a aclint-mswi in the SBI. > Maybe a change is needed? Yeah, I don't think the model for this in OpenSBI at the moment (and since I checked, in QEMU too) is correct. I think we should re-do things correctly and it'd be great if things didn't get merged to those projects that end up being objected to by dt-binding people. I've started keeping a closer eye on QEMU recently in that regard, but I am not super attentive. I'll try to be better at that going forward! > > >I also don't see anything in the opensbi repo right now that is using > >these (nor could I easily see any patches for opensbi adding this). > >Is there another SBI implementation that you are using that I can take > >a look at to try and understand this better? > > > > This will be sumbit in a short time. > Now we only use it is sophgo vendor SBI, which url is [1]. > > [1] https://github.com/sophgo/opensbi Thanks. > >>> 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... > > > >> Sorry for late reply, The clint consists of mtimer and ipi devices, which > >> is defined in [1]. > > > >Yes, I have looked at the spec. I went to check it again before replying > >here in case there was something immediately obvious that I was missing. > > > > I think nothing missed. > > >> This standard shows clint(or the aclint) has two device, > > > >The wording used here doesn't really matter. It's one interrupt > >controller that does mtimer and mswi. > > > >> but not one. In another word, there is no need to defined mtimer and ipi > >> device on the same base address. > > > >There's also no need to have two compatibles for the same interrupt > >controller, so I do not get this reasoning. What actually _requires_ > >them to be split? > > > > Yes, it is one, but can be mapped into different address. So I think we > need two. Not two compatibles though, just two memory addresses that you need to locate (or maybe even 3, for SSWI?) > > >> So we need two compatibles to allow sbi to identify them correctly. > > > >Why is it not sufficient to identify the individual memory regions? > > > > FYI, Anup. As I have no idea for aclint implementation. > > >Thanks, > >Conor. > >
On 20/09/2023 14:58, Conor Dooley wrote: > On Wed, Sep 20, 2023 at 08:40:07PM +0800, Inochi Amaoto wrote: >>> On 20/09/2023 14:15, Inochi Amaoto wrote: >>>>> On 20/09/2023 08:39, Chen Wang wrote: >>>>>> From: Inochi Amaoto <inochiama@outlook.com> >>>>>> >>>>>> 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 >>>>> >>>>> Why? >>>>> >>>> >>>> If use the same compatible, SBI will process this twice in both ipi and >>>> timer, use different compatible will allow SBI to treat these as different. >>>> AFAIK, the aclint in SBI use the same concepts, which make hard to use the >>>> second register range. I have explained in another response. >>> >>> What is a SBI? Linux driver? If so, why some intermediate Linux driver >>> choice should affect bindings? >>> Best regards, >>> Krzysztof >>> >> >> SBI (Supervisor Binary Interface) is defined by riscv, which is an interface >> between the Supervisor Execution Environment (SEE) and the supervisor. The >> detailed documentation can be found in [1]. >> >> The implement of SBI needs fdt info of the platform, which is provided by >> kernel. So we need a dt-bindings for these devices, and these will be >> processed by SBI. >> >> [1] https://github.com/riscv-non-isa/riscv-sbi-doc > > Yeah, this is the unfortunate problem of half-baked bindings (IMO) > ending up in OpenSBI (which likely means they also ended up in QEMU). > This T-Head stuff is coming across our (metaphorical) desks, so we are > obviously going to try to do things correctly. I may end up speaking to > Anup later today, if I do I will point him at this thread (if he hasn't > seen it already). If the OpenSBI started to work with some half-baked-bindings before proper review, it's their loss. If we do not push back on such stuff, how our review can ever matter? Anyway, firmware/SBI/whatever parsing compatible twice is not really a reason to model same thing with two different compatibles. Assuming of course this is the same thing. Best regards, Krzysztof
On Wed, Sep 20, 2023 at 6:28 PM Conor Dooley <conor@kernel.org> wrote: > > On Wed, Sep 20, 2023 at 08:40:07PM +0800, Inochi Amaoto wrote: > > >On 20/09/2023 14:15, Inochi Amaoto wrote: > > >>> On 20/09/2023 08:39, Chen Wang wrote: > > >>>> From: Inochi Amaoto <inochiama@outlook.com> > > >>>> > > >>>> 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 > > >>> > > >>> Why? > > >>> > > >> > > >> If use the same compatible, SBI will process this twice in both ipi and > > >> timer, use different compatible will allow SBI to treat these as different. > > >> AFAIK, the aclint in SBI use the same concepts, which make hard to use the > > >> second register range. I have explained in another response. > > > > > >What is a SBI? Linux driver? If so, why some intermediate Linux driver > > >choice should affect bindings? > > >Best regards, > > >Krzysztof > > > > > > > SBI (Supervisor Binary Interface) is defined by riscv, which is an interface > > between the Supervisor Execution Environment (SEE) and the supervisor. The > > detailed documentation can be found in [1]. > > > > The implement of SBI needs fdt info of the platform, which is provided by > > kernel. So we need a dt-bindings for these devices, and these will be > > processed by SBI. > > > > [1] https://github.com/riscv-non-isa/riscv-sbi-doc > > Yeah, this is the unfortunate problem of half-baked bindings (IMO) > ending up in OpenSBI (which likely means they also ended up in QEMU). > This T-Head stuff is coming across our (metaphorical) desks, so we are > obviously going to try to do things correctly. I may end up speaking to > Anup later today, if I do I will point him at this thread (if he hasn't > seen it already). RISC-V ACLINT is one of those unfortunate non-ISA specs (like SiFive PLIC) which is implemented by various organizations but not officially ratified by RVI. The SiFive CLINT has flexibility related limitations which makes it not useful for multi-socket and mult-die systems. The SiFive CLINT is also not useful for systems with AIA because with AIA M-mode has a new way of doing M-mode IPIs. Due to this reasons, the RISC-V ACLINT spec breaks down traditional SiFive CLINT into two separate devices namely mtimer and mswi. This allows platforms to implement only the required set of devices. The mtimer as defined by the ACLINT specifications also allows platforms to place mtime and mtimecmp registers at different locations. Refer, https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc We need a separate DT bindings document for ACLINT MTIMER and ACLINT MSWI because these are separate devices. The Sophgo sg2042 SoC should add their implementation specific compatible strings in this document. Regards, Anup
On Wed, Sep 20, 2023 at 08:08:49PM +0530, Anup Patel wrote: > On Wed, Sep 20, 2023 at 6:28 PM Conor Dooley <conor@kernel.org> wrote: > > > > On Wed, Sep 20, 2023 at 08:40:07PM +0800, Inochi Amaoto wrote: > > > >On 20/09/2023 14:15, Inochi Amaoto wrote: > > > >>> On 20/09/2023 08:39, Chen Wang wrote: > > > >>>> From: Inochi Amaoto <inochiama@outlook.com> > > > >>>> > > > >>>> 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 > > > >>> > > > >>> Why? > > > >>> > > > >> > > > >> If use the same compatible, SBI will process this twice in both ipi and > > > >> timer, use different compatible will allow SBI to treat these as different. > > > >> AFAIK, the aclint in SBI use the same concepts, which make hard to use the > > > >> second register range. I have explained in another response. > > > > > > > >What is a SBI? Linux driver? If so, why some intermediate Linux driver > > > >choice should affect bindings? > > > >Best regards, > > > >Krzysztof > > > > > > > > > > SBI (Supervisor Binary Interface) is defined by riscv, which is an interface > > > between the Supervisor Execution Environment (SEE) and the supervisor. The > > > detailed documentation can be found in [1]. > > > > > > The implement of SBI needs fdt info of the platform, which is provided by > > > kernel. So we need a dt-bindings for these devices, and these will be > > > processed by SBI. > > > > > > [1] https://github.com/riscv-non-isa/riscv-sbi-doc > > > > Yeah, this is the unfortunate problem of half-baked bindings (IMO) > > ending up in OpenSBI (which likely means they also ended up in QEMU). > > This T-Head stuff is coming across our (metaphorical) desks, so we are > > obviously going to try to do things correctly. I may end up speaking to > > Anup later today, if I do I will point him at this thread (if he hasn't > > seen it already). > > RISC-V ACLINT is one of those unfortunate non-ISA specs (like > SiFive PLIC) which is implemented by various organizations but > not officially ratified by RVI. Yeah, I brought this stuff up at the weekly pw sync call, and Paul pointed that out. > The SiFive CLINT has flexibility related limitations which makes it > not useful for multi-socket and mult-die systems. The SiFive CLINT > is also not useful for systems with AIA because with AIA M-mode has > a new way of doing M-mode IPIs. Due to this reasons, the RISC-V > ACLINT spec breaks down traditional SiFive CLINT into two separate > devices namely mtimer and mswi. This allows platforms to implement > only the required set of devices. The mtimer as defined by the ACLINT > specifications also allows platforms to place mtime and mtimecmp > registers at different locations. > > Refer, https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc > > We need a separate DT bindings document for ACLINT MTIMER > and ACLINT MSWI because these are separate devices. The > Sophgo sg2042 SoC should add their implementation specific > compatible strings in this document. If the spec isn't frozen, I'm not accepting a binding for the "generic" version of it. Bindings for this specific implemtnation are okay. For sure though, squeezing this into the sifive,plic binding isn't appropriate. What was pointed out, I think by Samuel, that the reason that this may need to be split is the fact that there are many possible MTIMER register ranges & possibly sswi stuff too that would need to be differentiated. > > Regards, > Anup
>On Wed, Sep 20, 2023 at 08:08:49PM +0530, Anup Patel wrote: >> On Wed, Sep 20, 2023 at 6:28 PM Conor Dooley <conor@kernel.org> wrote: >>> >>> On Wed, Sep 20, 2023 at 08:40:07PM +0800, Inochi Amaoto wrote: >>>>> On 20/09/2023 14:15, Inochi Amaoto wrote: >>>>>>> On 20/09/2023 08:39, Chen Wang wrote: >>>>>>>> From: Inochi Amaoto <inochiama@outlook.com> >>>>>>>> >>>>>>>> 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 >>>>>>> >>>>>>> Why? >>>>>>> >>>>>> >>>>>> If use the same compatible, SBI will process this twice in both ipi and >>>>>> timer, use different compatible will allow SBI to treat these as different. >>>>>> AFAIK, the aclint in SBI use the same concepts, which make hard to use the >>>>>> second register range. I have explained in another response. >>>>> >>>>> What is a SBI? Linux driver? If so, why some intermediate Linux driver >>>>> choice should affect bindings? >>>>> Best regards, >>>>> Krzysztof >>>>> >>>> >>>> SBI (Supervisor Binary Interface) is defined by riscv, which is an interface >>>> between the Supervisor Execution Environment (SEE) and the supervisor. The >>>> detailed documentation can be found in [1]. >>>> >>>> The implement of SBI needs fdt info of the platform, which is provided by >>>> kernel. So we need a dt-bindings for these devices, and these will be >>>> processed by SBI. >>>> >>>> [1] https://github.com/riscv-non-isa/riscv-sbi-doc >>> >>> Yeah, this is the unfortunate problem of half-baked bindings (IMO) >>> ending up in OpenSBI (which likely means they also ended up in QEMU). >>> This T-Head stuff is coming across our (metaphorical) desks, so we are >>> obviously going to try to do things correctly. I may end up speaking to >>> Anup later today, if I do I will point him at this thread (if he hasn't >>> seen it already). >> >> RISC-V ACLINT is one of those unfortunate non-ISA specs (like >> SiFive PLIC) which is implemented by various organizations but >> not officially ratified by RVI. > >Yeah, I brought this stuff up at the weekly pw sync call, and Paul >pointed that out. > >> The SiFive CLINT has flexibility related limitations which makes it >> not useful for multi-socket and mult-die systems. The SiFive CLINT >> is also not useful for systems with AIA because with AIA M-mode has >> a new way of doing M-mode IPIs. Due to this reasons, the RISC-V >> ACLINT spec breaks down traditional SiFive CLINT into two separate >> devices namely mtimer and mswi. This allows platforms to implement >> only the required set of devices. The mtimer as defined by the ACLINT >> specifications also allows platforms to place mtime and mtimecmp >> registers at different locations. >> >> Refer, https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc >> >> We need a separate DT bindings document for ACLINT MTIMER >> and ACLINT MSWI because these are separate devices. The >> Sophgo sg2042 SoC should add their implementation specific >> compatible strings in this document. > >If the spec isn't frozen, I'm not accepting a binding for the "generic" >version of it. Bindings for this specific implemtnation are okay. >For sure though, squeezing this into the sifive,plic binding isn't >appropriate. > A specific implemtnation sounds like a good idea, as the clint layout of sg2042 is wired and hard to merge (ipi is continous but the timer is per cluster). For a specific implemtnation, I wonder if it is better to use two files to identify these device each, instead of one for all? >What was pointed out, I think by Samuel, that the reason that this may >need to be split is the fact that there are many possible MTIMER >register ranges & possibly sswi stuff too that would need to be >differentiated. > >> >> Regards, >> Anup >
>On Wed, Sep 20, 2023 at 07:24:21PM +0800, Inochi Amaoto wrote: >>> >>> Yo, >>> >>> On Wed, Sep 20, 2023 at 05:08:41PM +0800, Inochi Amaoto wrote: >>>>> On Wed, Sep 20, 2023 at 02:39:39PM +0800, Chen Wang wrote: >>>>>> From: Inochi Amaoto <inochiama@outlook.com> >>>>>> >>>>>> 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@outlook.com> >>>>>> Signed-off-by: Chen Wang <wangchen20@iscas.ac.cn> >>>>> >>>>> 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 and wrong message-id. >>>> >>>> The clint is parsed by sbi. >>> >>> That needs to go in the commit message. >> >> Yes, it will. > >Thanks. > >>>> As use the same compatible, the opensbi will >>>> parse the device twice. This will cause a fault. >>> >>> Then only have one compatible with 2 register ranges? Then your SBI >>> implementation can use those two register ranges to find out the base >>> address for the mtimer bits and for the mswi bits. >>> I don't understand why this cannot be done, could you please explain. >> >> That is a good idea, but now SBI use the second register ranges as >> mtimecmp address for aclint. And there is a aclint-mswi in the SBI. >> Maybe a change is needed? > >Yeah, I don't think the model for this in OpenSBI at the moment (and >since I checked, in QEMU too) is correct. I think we should re-do things >correctly and it'd be great if things didn't get merged to those >projects that end up being objected to by dt-binding people. >I've started keeping a closer eye on QEMU recently in that regard, but I >am not super attentive. I'll try to be better at that going forward! > >> >>> I also don't see anything in the opensbi repo right now that is using >>> these (nor could I easily see any patches for opensbi adding this). >>> Is there another SBI implementation that you are using that I can take >>> a look at to try and understand this better? >>> >> >> This will be sumbit in a short time. >> Now we only use it is sophgo vendor SBI, which url is [1]. >> >> [1] https://github.com/sophgo/opensbi > >Thanks. > >>>>> 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... >>> >>>> Sorry for late reply, The clint consists of mtimer and ipi devices, which >>>> is defined in [1]. >>> >>> Yes, I have looked at the spec. I went to check it again before replying >>> here in case there was something immediately obvious that I was missing. >>> >> >> I think nothing missed. >> >>>> This standard shows clint(or the aclint) has two device, >>> >>> The wording used here doesn't really matter. It's one interrupt >>> controller that does mtimer and mswi. >>> >>>> but not one. In another word, there is no need to defined mtimer and ipi >>>> device on the same base address. >>> >>> There's also no need to have two compatibles for the same interrupt >>> controller, so I do not get this reasoning. What actually _requires_ >>> them to be split? >>> >> >> Yes, it is one, but can be mapped into different address. So I think we >> need two. > >Not two compatibles though, just two memory addresses that you need to >locate (or maybe even 3, for SSWI?) > We may need four (mtime, mtimecmp, mswi, sswi) if use register range. Anyway, I will use a vendor spec implementation as a temporary solution. I hope this will be corrected in a predictable future, and we can use a standard way to resolve this at that time. :) >> >>>> So we need two compatibles to allow sbi to identify them correctly. >>> >>> Why is it not sufficient to identify the individual memory regions? >>> >> >> FYI, Anup. As I have no idea for aclint implementation. >> >>> Thanks, >>> Conor. >>> >
On Thu, Sep 21, 2023 at 08:43:47AM +0800, Inochi Amaoto wrote: > >>>> but not one. In another word, there is no need to defined mtimer and ipi > >>>> device on the same base address. > >>> > >>> There's also no need to have two compatibles for the same interrupt > >>> controller, so I do not get this reasoning. What actually _requires_ > >>> them to be split? > >>> > >> > >> Yes, it is one, but can be mapped into different address. So I think we > >> need two. > > > >Not two compatibles though, just two memory addresses that you need to > >locate (or maybe even 3, for SSWI?) > > > > We may need four (mtime, mtimecmp, mswi, sswi) if use register range. Why would you need 4? The first two certainly could be individual reg entries, no? > Anyway, I will use a vendor spec implementation as a temporary solution. > I hope this will be corrected in a predictable future, and we can use a > standard way to resolve this at that time. :) If the spec doesn't get frozen, there'll not be a standard way merged. Hopefully not too many others go off an implement non-frozen specs, and we will not really need to worry all that much about it. Cheers, Conor.
> >On Thu, Sep 21, 2023 at 08:43:47AM +0800, Inochi Amaoto wrote: > >>>>>> but not one. In another word, there is no need to defined mtimer and ipi >>>>>> device on the same base address. >>>>> >>>>> There's also no need to have two compatibles for the same interrupt >>>>> controller, so I do not get this reasoning. What actually _requires_ >>>>> them to be split? >>>>> >>>> >>>> Yes, it is one, but can be mapped into different address. So I think we >>>> need two. >>> >>> Not two compatibles though, just two memory addresses that you need to >>> locate (or maybe even 3, for SSWI?) >>> >> >> We may need four (mtime, mtimecmp, mswi, sswi) if use register range. > >Why would you need 4? The first two certainly could be individual >reg entries, no? > After reading the aclint doc again, I found the all of them can be mapped on the different address. (See the section 2.1 in that doc). But for now, the mtime and mtimecmp have the same base address in any platform. Anyway, the frozen spec in future will decided how many ranges we need. >> Anyway, I will use a vendor spec implementation as a temporary solution. >> I hope this will be corrected in a predictable future, and we can use a >> standard way to resolve this at that time. :) > >If the spec doesn't get frozen, there'll not be a standard way merged. >Hopefully not too many others go off an implement non-frozen specs, and >we will not really need to worry all that much about it. > >Cheers, >Conor. >
Yo, On Thu, Sep 21, 2023 at 04:18:57PM +0800, Inochi Amaoto wrote: > >On Thu, Sep 21, 2023 at 08:43:47AM +0800, Inochi Amaoto wrote: > >>>>>> but not one. In another word, there is no need to defined mtimer and ipi > >>>>>> device on the same base address. > >>>>> > >>>>> There's also no need to have two compatibles for the same interrupt > >>>>> controller, so I do not get this reasoning. What actually _requires_ > >>>>> them to be split? > >>>>> > >>>> > >>>> Yes, it is one, but can be mapped into different address. So I think we > >>>> need two. > >>> > >>> Not two compatibles though, just two memory addresses that you need to > >>> locate (or maybe even 3, for SSWI?) > >>> > >> > >> We may need four (mtime, mtimecmp, mswi, sswi) if use register range. > > > >Why would you need 4? The first two certainly could be individual > >reg entries, no? > > > > After reading the aclint doc again, I found the all of them can be mapped > on the different address. (See the section 2.1 in that doc). Right, that's what I meant by individual reg entries. If there's some dynamic gap between them, then one reg entry would cover mtime and one would cover the base of the mtimecmp region. > But for now, > the mtime and mtimecmp have the same base address in any platform. How? The mtimecmp base address would have to be offset from the mtime base address. Is what you mean that, for example, mtime is at an offset of 0x0 from the base address & mtimecmp0 is at, for example, an offset of 0x8 so a single reg entry can cover both? Also, "any platform"? I figure you mean in this one specific platform? > Anyway, > the frozen spec in future will decided how many ranges we need. Isn't the spec abandoned? There may well be no frozen spec. > >> Anyway, I will use a vendor spec implementation as a temporary solution. > >> I hope this will be corrected in a predictable future, and we can use a > >> standard way to resolve this at that time. :) > > > >If the spec doesn't get frozen, there'll not be a standard way merged. > >Hopefully not too many others go off an implement non-frozen specs, and > >we will not really need to worry all that much about it. Cheers, Conor.
>Yo, > >On Thu, Sep 21, 2023 at 04:18:57PM +0800, Inochi Amaoto wrote: >>> On Thu, Sep 21, 2023 at 08:43:47AM +0800, Inochi Amaoto wrote: > >>>>>>>> but not one. In another word, there is no need to defined mtimer and ipi >>>>>>>> device on the same base address. >>>>>>> >>>>>>> There's also no need to have two compatibles for the same interrupt >>>>>>> controller, so I do not get this reasoning. What actually _requires_ >>>>>>> them to be split? >>>>>>> >>>>>> >>>>>> Yes, it is one, but can be mapped into different address. So I think we >>>>>> need two. >>>>> >>>>> Not two compatibles though, just two memory addresses that you need to >>>>> locate (or maybe even 3, for SSWI?) >>>>> >>>> >>>> We may need four (mtime, mtimecmp, mswi, sswi) if use register range. >>> >>> Why would you need 4? The first two certainly could be individual >>> reg entries, no? >>> >> >> After reading the aclint doc again, I found the all of them can be mapped >> on the different address. (See the section 2.1 in that doc). > >Right, that's what I meant by individual reg entries. If there's some >dynamic gap between them, then one reg entry would cover mtime and one >would cover the base of the mtimecmp region. > Thanks. >> But for now, >> the mtime and mtimecmp have the same base address in any platform. > >How? The mtimecmp base address would have to be offset from the mtime >base address. Is what you mean that, for example, mtime is at an offset >of 0x0 from the base address & mtimecmp0 is at, for example, an offset >of 0x8 so a single reg entry can cover both? > I mean "the same" is just what you said: use the offset to access mtime and mtimecmp, and left one register range in the DT. In your example, using a offset of 0x4 to mark the mtimecmp will allow us to use one register range like DTs already in the riscv arch. But this way does have problems when the timer is more complex. In fact, I think two register range could do better, and will give us more freedom to cover these regs. >Also, "any platform"? I figure you mean in this one specific platform? > I mean the platforms already upstreamed in both of kernel and OpenSBI. Not for this one. >> Anyway, >> the frozen spec in future will decided how many ranges we need. > >Isn't the spec abandoned? There may well be no frozen spec. > I guess it is. That is not a good thing. >>>> Anyway, I will use a vendor spec implementation as a temporary solution. >>>> I hope this will be corrected in a predictable future, and we can use a >>>> standard way to resolve this at that time. :) >>> >>> If the spec doesn't get frozen, there'll not be a standard way merged. >>> Hopefully not too many others go off an implement non-frozen specs, and >>> we will not really need to worry all that much about it. > >Cheers, >Conor. >
> >On Wed, Sep 20, 2023 at 08:08:49PM +0530, Anup Patel wrote: >> On Wed, Sep 20, 2023 at 6:28 PM Conor Dooley <conor@kernel.org> wrote: >>> >>> On Wed, Sep 20, 2023 at 08:40:07PM +0800, Inochi Amaoto wrote: >>>>> On 20/09/2023 14:15, Inochi Amaoto wrote: >>>>>>> On 20/09/2023 08:39, Chen Wang wrote: >>>>>>>> From: Inochi Amaoto <inochiama@outlook.com> >>>>>>>> >>>>>>>> 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 >>>>>>> >>>>>>> Why? >>>>>>> >>>>>> >>>>>> If use the same compatible, SBI will process this twice in both ipi and >>>>>> timer, use different compatible will allow SBI to treat these as different. >>>>>> AFAIK, the aclint in SBI use the same concepts, which make hard to use the >>>>>> second register range. I have explained in another response. >>>>> >>>>> What is a SBI? Linux driver? If so, why some intermediate Linux driver >>>>> choice should affect bindings? >>>>> Best regards, >>>>> Krzysztof >>>>> >>>> >>>> SBI (Supervisor Binary Interface) is defined by riscv, which is an interface >>>> between the Supervisor Execution Environment (SEE) and the supervisor. The >>>> detailed documentation can be found in [1]. >>>> >>>> The implement of SBI needs fdt info of the platform, which is provided by >>>> kernel. So we need a dt-bindings for these devices, and these will be >>>> processed by SBI. >>>> >>>> [1] https://github.com/riscv-non-isa/riscv-sbi-doc >>> >>> Yeah, this is the unfortunate problem of half-baked bindings (IMO) >>> ending up in OpenSBI (which likely means they also ended up in QEMU). >>> This T-Head stuff is coming across our (metaphorical) desks, so we are >>> obviously going to try to do things correctly. I may end up speaking to >>> Anup later today, if I do I will point him at this thread (if he hasn't >>> seen it already). >> >> RISC-V ACLINT is one of those unfortunate non-ISA specs (like >> SiFive PLIC) which is implemented by various organizations but >> not officially ratified by RVI. > >Yeah, I brought this stuff up at the weekly pw sync call, and Paul >pointed that out. > >> The SiFive CLINT has flexibility related limitations which makes it >> not useful for multi-socket and mult-die systems. The SiFive CLINT >> is also not useful for systems with AIA because with AIA M-mode has >> a new way of doing M-mode IPIs. Due to this reasons, the RISC-V >> ACLINT spec breaks down traditional SiFive CLINT into two separate >> devices namely mtimer and mswi. This allows platforms to implement >> only the required set of devices. The mtimer as defined by the ACLINT >> specifications also allows platforms to place mtime and mtimecmp >> registers at different locations. >> >> Refer, https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc >> >> We need a separate DT bindings document for ACLINT MTIMER >> and ACLINT MSWI because these are separate devices. The >> Sophgo sg2042 SoC should add their implementation specific >> compatible strings in this document. > >If the spec isn't frozen, I'm not accepting a binding for the "generic" >version of it. Bindings for this specific implemtnation are okay. >For sure though, squeezing this into the sifive,plic binding isn't >appropriate. > It seems I have missed a point. I wonder whether it is better to add a "aclint" binding firstly and then add sg2042 to it, or just use sg2042 specific binding? If use "aclint" binding, I wonder it is OK to add thead quirks as compatible specific properties, or left this to the SBI to handle? e.g. T-HEAD timer is not 64bit timer, and we should identify this. >What was pointed out, I think by Samuel, that the reason that this may >need to be split is the fact that there are many possible MTIMER >register ranges & possibly sswi stuff too that would need to be >differentiated. > >> >> Regards, >> Anup >
On Fri, Sep 22, 2023 at 01:16:35PM +0800, Inochi Amaoto wrote: > >> The SiFive CLINT has flexibility related limitations which makes it > >> not useful for multi-socket and mult-die systems. The SiFive CLINT > >> is also not useful for systems with AIA because with AIA M-mode has > >> a new way of doing M-mode IPIs. Due to this reasons, the RISC-V > >> ACLINT spec breaks down traditional SiFive CLINT into two separate > >> devices namely mtimer and mswi. This allows platforms to implement > >> only the required set of devices. The mtimer as defined by the ACLINT > >> specifications also allows platforms to place mtime and mtimecmp > >> registers at different locations. > >> > >> Refer, https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc > >> > >> We need a separate DT bindings document for ACLINT MTIMER > >> and ACLINT MSWI because these are separate devices. The > >> Sophgo sg2042 SoC should add their implementation specific > >> compatible strings in this document. > > > >If the spec isn't frozen, I'm not accepting a binding for the "generic" > >version of it. Bindings for this specific implemtnation are okay. > >For sure though, squeezing this into the sifive,plic binding isn't > >appropriate. > > > > It seems I have missed a point. I wonder whether it is better to add a > "aclint" binding firstly and then add sg2042 to it, or just use sg2042 > specific binding? sg2042 specific, being frozen is a requirement for merging patches related to RVI specifications. > If use "aclint" binding, I wonder it is OK to add > thead quirks as compatible specific properties, or left this to the SBI to > handle? e.g. T-HEAD timer is not 64bit timer, and we should identify this. The compatible string alone should be sufficient to identify the width of the timer etc. Thanks, Conor.
> >On Fri, Sep 22, 2023 at 01:16:35PM +0800, Inochi Amaoto wrote: > >>>> The SiFive CLINT has flexibility related limitations which makes it >>>> not useful for multi-socket and mult-die systems. The SiFive CLINT >>>> is also not useful for systems with AIA because with AIA M-mode has >>>> a new way of doing M-mode IPIs. Due to this reasons, the RISC-V >>>> ACLINT spec breaks down traditional SiFive CLINT into two separate >>>> devices namely mtimer and mswi. This allows platforms to implement >>>> only the required set of devices. The mtimer as defined by the ACLINT >>>> specifications also allows platforms to place mtime and mtimecmp >>>> registers at different locations. >>>> >>>> Refer, https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc >>>> >>>> We need a separate DT bindings document for ACLINT MTIMER >>>> and ACLINT MSWI because these are separate devices. The >>>> Sophgo sg2042 SoC should add their implementation specific >>>> compatible strings in this document. >>> >>> If the spec isn't frozen, I'm not accepting a binding for the "generic" >>> version of it. Bindings for this specific implemtnation are okay. >>> For sure though, squeezing this into the sifive,plic binding isn't >>> appropriate. >>> >> >> It seems I have missed a point. I wonder whether it is better to add a >> "aclint" binding firstly and then add sg2042 to it, or just use sg2042 >> specific binding? > >sg2042 specific, being frozen is a requirement for merging patches >related to RVI specifications. > Thanks >> If use "aclint" binding, I wonder it is OK to add >> thead quirks as compatible specific properties, or left this to the SBI to >> handle? e.g. T-HEAD timer is not 64bit timer, and we should identify this. > >The compatible string alone should be sufficient to identify the width >of the timer etc. > OK, I will take it >Thanks, >Conor. >
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