| Message ID | 20221121041757.418645-3-uwu@icenowy.me (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Palmer Dabbelt |
| Headers | show |
| Series | Some DT binding quirks for T-Head C9xx | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/patch_count | success | Link |
| conchuod/cover_letter | success | Series has a cover letter |
| conchuod/tree_selection | success | Guessed tree name to be for-next |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/build_warn_rv64 | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | warning | WARNING: 'thead' may be misspelled - perhaps 'thread'? |
| conchuod/source_inline | success | Was 0 now: 0 |
| 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 21/11/2022 05:17, Icenowy Zheng wrote: > T-Head OpenC906 is a open-source-licensed fixed-configuration of C906, > which is now public and able to be integrated. > > Add a compatible for the CLINT shipped as part of OpenC906, which should > just be ordinary C9xx CLINT. > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > --- > Documentation/devicetree/bindings/timer/sifive,clint.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.yaml b/Documentation/devicetree/bindings/timer/sifive,clint.yaml > index aada6957216c..86703e995e31 100644 > --- a/Documentation/devicetree/bindings/timer/sifive,clint.yaml > +++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml > @@ -35,6 +35,7 @@ properties: > - const: sifive,clint0 > - items: > - enum: > + - thead,openc906-clint > - allwinner,sun20i-d1-clint Add entries sorted alphabetically. This should be squashed with previous patch. Best regards, Krzysztof
在 2022-11-21星期一的 11:06 +0100，Krzysztof Kozlowski写道： > On 21/11/2022 05:17, Icenowy Zheng wrote: > > T-Head OpenC906 is a open-source-licensed fixed-configuration of > > C906, > > which is now public and able to be integrated. > > > > Add a compatible for the CLINT shipped as part of OpenC906, which > > should > > just be ordinary C9xx CLINT. > > > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > > --- > > Documentation/devicetree/bindings/timer/sifive,clint.yaml | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git > > a/Documentation/devicetree/bindings/timer/sifive,clint.yaml > > b/Documentation/devicetree/bindings/timer/sifive,clint.yaml > > index aada6957216c..86703e995e31 100644 > > --- a/Documentation/devicetree/bindings/timer/sifive,clint.yaml > > +++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml > > @@ -35,6 +35,7 @@ properties: > > - const: sifive,clint0 > > - items: > > - enum: > > + - thead,openc906-clint > > - allwinner,sun20i-d1-clint > > Add entries sorted alphabetically. This should be squashed with > previous > patch. I make it a seperated patch because I think it's a questionable approach. If you think it's okay, I will just squash it and put it as the second patch in the next iteration, with adding openc906-plic as the first one. > > Best regards, > Krzysztof >
On 22/11/2022 08:18, Icenowy Zheng wrote: > 在 2022-11-21星期一的 11:06 +0100，Krzysztof Kozlowski写道： >> On 21/11/2022 05:17, Icenowy Zheng wrote: >>> T-Head OpenC906 is a open-source-licensed fixed-configuration of >>> C906, >>> which is now public and able to be integrated. >>> >>> Add a compatible for the CLINT shipped as part of OpenC906, which >>> should >>> just be ordinary C9xx CLINT. >>> >>> Signed-off-by: Icenowy Zheng <uwu@icenowy.me> >>> --- >>> Documentation/devicetree/bindings/timer/sifive,clint.yaml | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/timer/sifive,clint.yaml >>> b/Documentation/devicetree/bindings/timer/sifive,clint.yaml >>> index aada6957216c..86703e995e31 100644 >>> --- a/Documentation/devicetree/bindings/timer/sifive,clint.yaml >>> +++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml >>> @@ -35,6 +35,7 @@ properties: >>> - const: sifive,clint0 >>> - items: >>> - enum: >>> + - thead,openc906-clint >>> - allwinner,sun20i-d1-clint >> >> Add entries sorted alphabetically. This should be squashed with >> previous >> patch. > > I make it a seperated patch because I think it's a questionable > approach. > > If you think it's okay, I will just squash it and put it as the second > patch in the next iteration, with adding openc906-plic as the first > one. What is a questionable approach? Why commit msg is not saying this? Best regards, Krzysztof
于 2022年11月22日 GMT+08:00 下午3:35:48, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 写到: >On 22/11/2022 08:18, Icenowy Zheng wrote: >> 在 2022-11-21星期一的 11:06 +0100，Krzysztof Kozlowski写道： >>> On 21/11/2022 05:17, Icenowy Zheng wrote: >>>> T-Head OpenC906 is a open-source-licensed fixed-configuration of >>>> C906, >>>> which is now public and able to be integrated. >>>> >>>> Add a compatible for the CLINT shipped as part of OpenC906, which >>>> should >>>> just be ordinary C9xx CLINT. >>>> >>>> Signed-off-by: Icenowy Zheng <uwu@icenowy.me> >>>> --- >>>> Documentation/devicetree/bindings/timer/sifive,clint.yaml | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git >>>> a/Documentation/devicetree/bindings/timer/sifive,clint.yaml >>>> b/Documentation/devicetree/bindings/timer/sifive,clint.yaml >>>> index aada6957216c..86703e995e31 100644 >>>> --- a/Documentation/devicetree/bindings/timer/sifive,clint.yaml >>>> +++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml >>>> @@ -35,6 +35,7 @@ properties: >>>> - const: sifive,clint0 >>>> - items: >>>> - enum: >>>> + - thead,openc906-clint >>>> - allwinner,sun20i-d1-clint >>> >>> Add entries sorted alphabetically. This should be squashed with >>> previous >>> patch. >> >> I make it a seperated patch because I think it's a questionable >> approach. >> >> If you think it's okay, I will just squash it and put it as the second >> patch in the next iteration, with adding openc906-plic as the first >> one. > >What is a questionable approach? Why commit msg is not saying this? Ah I mentioned it in the cover letter. The problem is just I doubt whether binding strings for single SoCs are necessary. > >Best regards, >Krzysztof >
On 22/11/2022 08:41, Icenowy Zheng wrote: > > > 于 2022年11月22日 GMT+08:00 下午3:35:48, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 写到: >> On 22/11/2022 08:18, Icenowy Zheng wrote: >>> 在 2022-11-21星期一的 11:06 +0100，Krzysztof Kozlowski写道： >>>> On 21/11/2022 05:17, Icenowy Zheng wrote: >>>>> T-Head OpenC906 is a open-source-licensed fixed-configuration of >>>>> C906, >>>>> which is now public and able to be integrated. >>>>> >>>>> Add a compatible for the CLINT shipped as part of OpenC906, which >>>>> should >>>>> just be ordinary C9xx CLINT. >>>>> >>>>> Signed-off-by: Icenowy Zheng <uwu@icenowy.me> >>>>> --- >>>>> Documentation/devicetree/bindings/timer/sifive,clint.yaml | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git >>>>> a/Documentation/devicetree/bindings/timer/sifive,clint.yaml >>>>> b/Documentation/devicetree/bindings/timer/sifive,clint.yaml >>>>> index aada6957216c..86703e995e31 100644 >>>>> --- a/Documentation/devicetree/bindings/timer/sifive,clint.yaml >>>>> +++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml >>>>> @@ -35,6 +35,7 @@ properties: >>>>> - const: sifive,clint0 >>>>> - items: >>>>> - enum: >>>>> + - thead,openc906-clint >>>>> - allwinner,sun20i-d1-clint >>>> >>>> Add entries sorted alphabetically. This should be squashed with >>>> previous >>>> patch. >>> >>> I make it a seperated patch because I think it's a questionable >>> approach. >>> >>> If you think it's okay, I will just squash it and put it as the second >>> patch in the next iteration, with adding openc906-plic as the first >>> one. >> >> What is a questionable approach? Why commit msg is not saying this? > > Ah I mentioned it in the cover letter. The problem is just I doubt whether > binding strings for single SoCs are necessary. > There is no question in cover letter. Just some minor remark *at the end* of it... If you have questions, be explicit, not force people to grep through several paragraphs and figure out your concerns. Best regards, Krzysztof
On Tue, Nov 22, 2022 at 03:41:27PM +0800, Icenowy Zheng wrote: > > > 于 2022年11月22日 GMT+08:00 下午3:35:48, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 写到: > >On 22/11/2022 08:18, Icenowy Zheng wrote: > >> 在 2022-11-21星期一的 11:06 +0100，Krzysztof Kozlowski写道： > >>> On 21/11/2022 05:17, Icenowy Zheng wrote: > >>>> T-Head OpenC906 is a open-source-licensed fixed-configuration of > >>>> C906, > >>>> which is now public and able to be integrated. > >>>> > >>>> Add a compatible for the CLINT shipped as part of OpenC906, which > >>>> should > >>>> just be ordinary C9xx CLINT. > >>>> > >>>> Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > >>>> --- > >>>> Documentation/devicetree/bindings/timer/sifive,clint.yaml | 1 + > >>>> 1 file changed, 1 insertion(+) > >>>> > >>>> diff --git > >>>> a/Documentation/devicetree/bindings/timer/sifive,clint.yaml > >>>> b/Documentation/devicetree/bindings/timer/sifive,clint.yaml > >>>> index aada6957216c..86703e995e31 100644 > >>>> --- a/Documentation/devicetree/bindings/timer/sifive,clint.yaml > >>>> +++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml > >>>> @@ -35,6 +35,7 @@ properties: > >>>> - const: sifive,clint0 > >>>> - items: > >>>> - enum: > >>>> + - thead,openc906-clint > >>>> - allwinner,sun20i-d1-clint > >>> > >>> Add entries sorted alphabetically. This should be squashed with > >>> previous > >>> patch. > >> > >> I make it a seperated patch because I think it's a questionable > >> approach. > >> > >> If you think it's okay, I will just squash it and put it as the second > >> patch in the next iteration, with adding openc906-plic as the first > >> one. > > > >What is a questionable approach? Why commit msg is not saying this? > > Ah I mentioned it in the cover letter. The problem is just I doubt whether > binding strings for single SoCs are necessary. They are. Unless all the quirks/bugs/features are somehow guaranteed to be exactly the same as other SoCs sharing the same compatible string, or there is another mechanism to identify the exact version (e.g. a version register). Rob
On Wed, Nov 30, 2022 at 12:13:30PM -0600, Rob Herring wrote: > On Tue, Nov 22, 2022 at 03:41:27PM +0800, Icenowy Zheng wrote: > > > > > > 于 2022年11月22日 GMT+08:00 下午3:35:48, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 写到: > > >On 22/11/2022 08:18, Icenowy Zheng wrote: > > >> 在 2022-11-21星期一的 11:06 +0100，Krzysztof Kozlowski写道： > > >>> On 21/11/2022 05:17, Icenowy Zheng wrote: > > >>>> T-Head OpenC906 is a open-source-licensed fixed-configuration of > > >>>> C906, > > >>>> which is now public and able to be integrated. > > >>>> > > >>>> Add a compatible for the CLINT shipped as part of OpenC906, which > > >>>> should > > >>>> just be ordinary C9xx CLINT. > > >>>> > > >>>> Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > > >>>> --- > > >>>> Documentation/devicetree/bindings/timer/sifive,clint.yaml | 1 + > > >>>> 1 file changed, 1 insertion(+) > > >>>> > > >>>> diff --git > > >>>> a/Documentation/devicetree/bindings/timer/sifive,clint.yaml > > >>>> b/Documentation/devicetree/bindings/timer/sifive,clint.yaml > > >>>> index aada6957216c..86703e995e31 100644 > > >>>> --- a/Documentation/devicetree/bindings/timer/sifive,clint.yaml > > >>>> +++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml > > >>>> @@ -35,6 +35,7 @@ properties: > > >>>> - const: sifive,clint0 > > >>>> - items: > > >>>> - enum: > > >>>> + - thead,openc906-clint > > >>>> - allwinner,sun20i-d1-clint > > >>> > > >>> Add entries sorted alphabetically. This should be squashed with > > >>> previous > > >>> patch. > > >> > > >> I make it a seperated patch because I think it's a questionable > > >> approach. > > >> > > >> If you think it's okay, I will just squash it and put it as the second > > >> patch in the next iteration, with adding openc906-plic as the first > > >> one. > > > > > >What is a questionable approach? Why commit msg is not saying this? > > > > Ah I mentioned it in the cover letter. The problem is just I doubt whether > > binding strings for single SoCs are necessary. > > They are. > > Unless all the quirks/bugs/features are somehow guaranteed to be exactly > the same as other SoCs sharing the same compatible string, or there is > another mechanism to identify the exact version (e.g. a version > register). Icenowy, Having thought about this a little - are we not *more* likely to see bug/quirk disparity between implementations of the OpenC906 stuff by the very nature of being an open-source IP? Thanks, Conor.
在 2022-12-01星期四的 19:18 +0000，Conor Dooley写道： > On Wed, Nov 30, 2022 at 12:13:30PM -0600, Rob Herring wrote: > > On Tue, Nov 22, 2022 at 03:41:27PM +0800, Icenowy Zheng wrote: > > > > > > > > > 于 2022年11月22日 GMT+08:00 下午3:35:48, Krzysztof Kozlowski > > > <krzysztof.kozlowski@linaro.org> 写到: > > > > On 22/11/2022 08:18, Icenowy Zheng wrote: > > > > > 在 2022-11-21星期一的 11:06 +0100，Krzysztof Kozlowski写道： > > > > > > On 21/11/2022 05:17, Icenowy Zheng wrote: > > > > > > > T-Head OpenC906 is a open-source-licensed fixed- > > > > > > > configuration of > > > > > > > C906, > > > > > > > which is now public and able to be integrated. > > > > > > > > > > > > > > Add a compatible for the CLINT shipped as part of > > > > > > > OpenC906, which > > > > > > > should > > > > > > > just be ordinary C9xx CLINT. > > > > > > > > > > > > > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > > > > > > > --- > > > > > > > Documentation/devicetree/bindings/timer/sifive,clint.yam > > > > > > > l | 1 + > > > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > > > > > diff --git > > > > > > > a/Documentation/devicetree/bindings/timer/sifive,clint.ya > > > > > > > ml > > > > > > > b/Documentation/devicetree/bindings/timer/sifive,clint.ya > > > > > > > ml > > > > > > > index aada6957216c..86703e995e31 100644 > > > > > > > --- > > > > > > > a/Documentation/devicetree/bindings/timer/sifive,clint.ya > > > > > > > ml > > > > > > > +++ > > > > > > > b/Documentation/devicetree/bindings/timer/sifive,clint.ya > > > > > > > ml > > > > > > > @@ -35,6 +35,7 @@ properties: > > > > > > > - const: sifive,clint0 > > > > > > > - items: > > > > > > > - enum: > > > > > > > + - thead,openc906-clint > > > > > > > - allwinner,sun20i-d1-clint > > > > > > > > > > > > Add entries sorted alphabetically. This should be squashed > > > > > > with > > > > > > previous > > > > > > patch. > > > > > > > > > > I make it a seperated patch because I think it's a > > > > > questionable > > > > > approach. > > > > > > > > > > If you think it's okay, I will just squash it and put it as > > > > > the second > > > > > patch in the next iteration, with adding openc906-plic as the > > > > > first > > > > > one. > > > > > > > > What is a questionable approach? Why commit msg is not saying > > > > this? > > > > > > Ah I mentioned it in the cover letter. The problem is just I > > > doubt whether > > > binding strings for single SoCs are necessary. > > > > They are. > > > > Unless all the quirks/bugs/features are somehow guaranteed to be > > exactly > > the same as other SoCs sharing the same compatible string, or there > > is > > another mechanism to identify the exact version (e.g. a version > > register). > > Icenowy, > > Having thought about this a little - are we not *more* likely to see > bug/quirk disparity between implementations of the OpenC906 stuff by > the very nature of being an open-source IP? It's an open-source edition of a specific version of the commercial IP, a fixed configuration. In addition, maybe we can just retrieve the version infomation via a T- Head custom CPU configuration register, mcpuid. Despite the implementation of this register is weird -- it contains 7 different read-only values, with the most significant nibble behaving as an index. > > Thanks, > Conor. >
On Fri, Dec 02, 2022 at 02:12:54PM +0800, Icenowy Zheng wrote: > 在 2022-12-01星期四的 19:18 +0000，Conor Dooley写道： > > On Wed, Nov 30, 2022 at 12:13:30PM -0600, Rob Herring wrote: > > > On Tue, Nov 22, 2022 at 03:41:27PM +0800, Icenowy Zheng wrote: > > > > > > > > > > > > 于 2022年11月22日 GMT+08:00 下午3:35:48, Krzysztof Kozlowski > > > > <krzysztof.kozlowski@linaro.org> 写到: > > > > > On 22/11/2022 08:18, Icenowy Zheng wrote: > > > > > > 在 2022-11-21星期一的 11:06 +0100，Krzysztof Kozlowski写道： > > > > > > > On 21/11/2022 05:17, Icenowy Zheng wrote: > > > > > > > > T-Head OpenC906 is a open-source-licensed fixed- > > > > > > > > configuration of > > > > > > > > C906, > > > > > > > > which is now public and able to be integrated. > > > > > > > > > > > > > > > > Add a compatible for the CLINT shipped as part of > > > > > > > > OpenC906, which > > > > > > > > should > > > > > > > > just be ordinary C9xx CLINT. > > > > > > > > > > > > > > > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > > > > > > > > --- > > > > > > > > Documentation/devicetree/bindings/timer/sifive,clint.yam > > > > > > > > l | 1 + > > > > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > > > > > > > diff --git > > > > > > > > a/Documentation/devicetree/bindings/timer/sifive,clint.ya > > > > > > > > ml > > > > > > > > b/Documentation/devicetree/bindings/timer/sifive,clint.ya > > > > > > > > ml > > > > > > > > index aada6957216c..86703e995e31 100644 > > > > > > > > --- > > > > > > > > a/Documentation/devicetree/bindings/timer/sifive,clint.ya > > > > > > > > ml > > > > > > > > +++ > > > > > > > > b/Documentation/devicetree/bindings/timer/sifive,clint.ya > > > > > > > > ml > > > > > > > > @@ -35,6 +35,7 @@ properties: > > > > > > > > - const: sifive,clint0 > > > > > > > > - items: > > > > > > > > - enum: > > > > > > > > + - thead,openc906-clint > > > > > > > > - allwinner,sun20i-d1-clint > > > > > > > > > > > > > > Add entries sorted alphabetically. This should be squashed > > > > > > > with > > > > > > > previous > > > > > > > patch. > > > > > > > > > > > > I make it a seperated patch because I think it's a > > > > > > questionable > > > > > > approach. > > > > > > > > > > > > If you think it's okay, I will just squash it and put it as > > > > > > the second > > > > > > patch in the next iteration, with adding openc906-plic as the > > > > > > first > > > > > > one. > > > > > > > > > > What is a questionable approach? Why commit msg is not saying > > > > > this? > > > > > > > > Ah I mentioned it in the cover letter. The problem is just I > > > > doubt whether > > > > binding strings for single SoCs are necessary. > > > > > > They are. > > > > > > Unless all the quirks/bugs/features are somehow guaranteed to be > > > exactly > > > the same as other SoCs sharing the same compatible string, or there > > > is > > > another mechanism to identify the exact version (e.g. a version > > > register). > > > > Icenowy, > > > > Having thought about this a little - are we not *more* likely to see > > bug/quirk disparity between implementations of the OpenC906 stuff by > > the very nature of being an open-source IP? > > It's an open-source edition of a specific version of the commercial IP, > a fixed configuration. > > In addition, maybe we can just retrieve the version infomation via a T- > Head custom CPU configuration register, mcpuid. Despite the > implementation of this register is weird -- it contains 7 different > read-only values, with the most significant nibble behaving as an > index. You lot all know the situation here a lot more than I do... I don't think "letting" people use the bare "thead,c900-foo" makes much sense as it gives us no chance to deal with quirks down the line. I don't think that using "thead,openc906-clint", "thead,c900-clint" makes all that much sense either, in case someone does something wacky with the open-source version of the core. That leaves us with either: "vendor,soc-clint", "thead,openc906-clint", "thead,c900-clint" or: "vendor,soc-clint", "thead,c900-clint" right? The first one seems like possibly the better option as you'd kinda expect that, in a perfect word, all of the open-source IP implementations would share quirks etc? Thanks, Conor.
在 2022-12-05星期一的 10:36 +0000，Conor Dooley写道： > On Fri, Dec 02, 2022 at 02:12:54PM +0800, Icenowy Zheng wrote: > > 在 2022-12-01星期四的 19:18 +0000，Conor Dooley写道： > > > On Wed, Nov 30, 2022 at 12:13:30PM -0600, Rob Herring wrote: > > > > On Tue, Nov 22, 2022 at 03:41:27PM +0800, Icenowy Zheng wrote: > > > > > > > > > > > > > > > 于 2022年11月22日 GMT+08:00 下午3:35:48, Krzysztof Kozlowski > > > > > <krzysztof.kozlowski@linaro.org> 写到: > > > > > > On 22/11/2022 08:18, Icenowy Zheng wrote: > > > > > > > 在 2022-11-21星期一的 11:06 +0100，Krzysztof Kozlowski写道： > > > > > > > > On 21/11/2022 05:17, Icenowy Zheng wrote: > > > > > > > > > T-Head OpenC906 is a open-source-licensed fixed- > > > > > > > > > configuration of > > > > > > > > > C906, > > > > > > > > > which is now public and able to be integrated. > > > > > > > > > > > > > > > > > > Add a compatible for the CLINT shipped as part of > > > > > > > > > OpenC906, which > > > > > > > > > should > > > > > > > > > just be ordinary C9xx CLINT. > > > > > > > > > > > > > > > > > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > > > > > > > > > --- > > > > > > > > > Documentation/devicetree/bindings/timer/sifive,clint > > > > > > > > > .yam > > > > > > > > > l | 1 + > > > > > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > > > > > > > > > diff --git > > > > > > > > > a/Documentation/devicetree/bindings/timer/sifive,clin > > > > > > > > > t.ya > > > > > > > > > ml > > > > > > > > > b/Documentation/devicetree/bindings/timer/sifive,clin > > > > > > > > > t.ya > > > > > > > > > ml > > > > > > > > > index aada6957216c..86703e995e31 100644 > > > > > > > > > --- > > > > > > > > > a/Documentation/devicetree/bindings/timer/sifive,clin > > > > > > > > > t.ya > > > > > > > > > ml > > > > > > > > > +++ > > > > > > > > > b/Documentation/devicetree/bindings/timer/sifive,clin > > > > > > > > > t.ya > > > > > > > > > ml > > > > > > > > > @@ -35,6 +35,7 @@ properties: > > > > > > > > > - const: sifive,clint0 > > > > > > > > > - items: > > > > > > > > > - enum: > > > > > > > > > + - thead,openc906-clint > > > > > > > > > - allwinner,sun20i-d1-clint > > > > > > > > > > > > > > > > Add entries sorted alphabetically. This should be > > > > > > > > squashed > > > > > > > > with > > > > > > > > previous > > > > > > > > patch. > > > > > > > > > > > > > > I make it a seperated patch because I think it's a > > > > > > > questionable > > > > > > > approach. > > > > > > > > > > > > > > If you think it's okay, I will just squash it and put it > > > > > > > as > > > > > > > the second > > > > > > > patch in the next iteration, with adding openc906-plic as > > > > > > > the > > > > > > > first > > > > > > > one. > > > > > > > > > > > > What is a questionable approach? Why commit msg is not > > > > > > saying > > > > > > this? > > > > > > > > > > Ah I mentioned it in the cover letter. The problem is just I > > > > > doubt whether > > > > > binding strings for single SoCs are necessary. > > > > > > > > They are. > > > > > > > > Unless all the quirks/bugs/features are somehow guaranteed to > > > > be > > > > exactly > > > > the same as other SoCs sharing the same compatible string, or > > > > there > > > > is > > > > another mechanism to identify the exact version (e.g. a version > > > > register). > > > > > > Icenowy, > > > > > > Having thought about this a little - are we not *more* likely to > > > see > > > bug/quirk disparity between implementations of the OpenC906 stuff > > > by > > > the very nature of being an open-source IP? > > > > It's an open-source edition of a specific version of the commercial > > IP, > > a fixed configuration. > > > > In addition, maybe we can just retrieve the version infomation via > > a T- > > Head custom CPU configuration register, mcpuid. Despite the > > implementation of this register is weird -- it contains 7 different > > read-only values, with the most significant nibble behaving as an > > index. > > You lot all know the situation here a lot more than I do... > I don't think "letting" people use the bare "thead,c900-foo" makes > much > sense as it gives us no chance to deal with quirks down the line. Well, after rechecking the manual, I found it possible to handle quirks -- T-Head has a custom "mcpuid" CSR (@ RISC-V CSR 0xFC0), which can be used to retrieve some identification info of the core, including its model ID, version, etc; and the T-Head PLIC/CLINT are part of their C906 SoC design that there's another "mapbaddr" CSR that could be used to retrieve the base address of them. So I think it okay to just use "thead,c900-clint" here, and when necessary, try to retrieve mcpuid for dealing with quirks. > I don't think that using "thead,openc906-clint", "thead,c900-clint" > makes all that much sense either, in case someone does something > wacky > with the open-source version of the core. > > That leaves us with either: > "vendor,soc-clint", "thead,openc906-clint", "thead,c900-clint" > or: > "vendor,soc-clint", "thead,c900-clint" > right? > > The first one seems like possibly the better option as you'd kinda > expect that, in a perfect word, all of the open-source IP > implementations would share quirks etc? > > Thanks, > Conor. >
On Mon, Dec 05, 2022 at 07:03:17PM +0800, Icenowy Zheng wrote: > 在 2022-12-05星期一的 10:36 +0000，Conor Dooley写道： > > You lot all know the situation here a lot more than I do... > > I don't think "letting" people use the bare "thead,c900-foo" makes > > much > > sense as it gives us no chance to deal with quirks down the line. > > Well, after rechecking the manual, I found it possible to handle quirks > -- T-Head has a custom "mcpuid" CSR (@ RISC-V CSR 0xFC0), which can be > used to retrieve some identification info of the core, including its > model ID, version, etc; and the T-Head PLIC/CLINT are part of their > C906 SoC design that there's another "mapbaddr" CSR that could be used > to retrieve the base address of them. > > So I think it okay to just use "thead,c900-clint" here, and when > necessary, try to retrieve mcpuid for dealing with quirks. I'm not super sure I follow. What's the relevance of "mapbaddr" here? We've got a reg property, so I don't think we need "mapbaddr"? For "mcpuid", can you be sure that implementers will not omit setting that value to something unique? I'd be happier if we were overly clear now rather than have some headaches later. Have I missed something? > > I don't think that using "thead,openc906-clint", "thead,c900-clint" > > makes all that much sense either, in case someone does something > > wacky > > with the open-source version of the core. > > > > That leaves us with either: > > "vendor,soc-clint", "thead,openc906-clint", "thead,c900-clint" > > or: > > "vendor,soc-clint", "thead,c900-clint" > > right? > > > > The first one seems like possibly the better option as you'd kinda > > expect that, in a perfect word, all of the open-source IP > > implementations would share quirks etc?
在 2022-12-05星期一的 15:05 +0000，Conor Dooley写道： > On Mon, Dec 05, 2022 at 07:03:17PM +0800, Icenowy Zheng wrote: > > 在 2022-12-05星期一的 10:36 +0000，Conor Dooley写道： > > > > You lot all know the situation here a lot more than I do... > > > I don't think "letting" people use the bare "thead,c900-foo" > > > makes > > > much > > > sense as it gives us no chance to deal with quirks down the line. > > > > Well, after rechecking the manual, I found it possible to handle > > quirks > > -- T-Head has a custom "mcpuid" CSR (@ RISC-V CSR 0xFC0), which can > > be > > used to retrieve some identification info of the core, including > > its > > model ID, version, etc; and the T-Head PLIC/CLINT are part of their > > C906 SoC design that there's another "mapbaddr" CSR that could be > > used > > to retrieve the base address of them. > > > > So I think it okay to just use "thead,c900-clint" here, and when > > necessary, try to retrieve mcpuid for dealing with quirks. > > I'm not super sure I follow. What's the relevance of "mapbaddr" here? > We've got a reg property, so I don't think we need "mapbaddr"? Yes, it's not relevant to us here, it's only to prove that PLIC/CLINT is part of C906 "Core Complex". > > For "mcpuid", can you be sure that implementers will not omit setting > that value to something unique? I'd be happier if we were overly > clear > now rather than have some headaches later. Have I missed something? These values are set by T-Head instead of individual SoC implementers as a CPU CSR, and it's not for uniqueness, but it's for identification of the CPU core revision (thus the PLIC/CLINT that come with it). > > > > I don't think that using "thead,openc906-clint", "thead,c900- > > > clint" > > > makes all that much sense either, in case someone does something > > > wacky > > > with the open-source version of the core. > > > > > > That leaves us with either: > > > "vendor,soc-clint", "thead,openc906-clint", "thead,c900-clint" > > > or: > > > "vendor,soc-clint", "thead,c900-clint" > > > right? > > > > > > The first one seems like possibly the better option as you'd > > > kinda > > > expect that, in a perfect word, all of the open-source IP > > > implementations would share quirks etc? >
On 5 December 2022 15:59:44 GMT, Icenowy Zheng <uwu@icenowy.me> wrote: >在 2022-12-05星期一的 15:05 +0000，Conor Dooley写道： >> On Mon, Dec 05, 2022 at 07:03:17PM +0800, Icenowy Zheng wrote: >> > 在 2022-12-05星期一的 10:36 +0000，Conor Dooley写道： >> >> > > You lot all know the situation here a lot more than I do... >> > > I don't think "letting" people use the bare "thead,c900-foo" >> > > makes >> > > much >> > > sense as it gives us no chance to deal with quirks down the line. >> > >> > Well, after rechecking the manual, I found it possible to handle >> > quirks >> > -- T-Head has a custom "mcpuid" CSR (@ RISC-V CSR 0xFC0), which can >> > be >> > used to retrieve some identification info of the core, including >> > its >> > model ID, version, etc; and the T-Head PLIC/CLINT are part of their >> > C906 SoC design that there's another "mapbaddr" CSR that could be >> > used >> > to retrieve the base address of them. >> > >> > So I think it okay to just use "thead,c900-clint" here, and when >> > necessary, try to retrieve mcpuid for dealing with quirks. >> >> I'm not super sure I follow. What's the relevance of "mapbaddr" here? >> We've got a reg property, so I don't think we need "mapbaddr"? > >Yes, it's not relevant to us here, it's only to prove that PLIC/CLINT >is part of C906 "Core Complex". > >> >> For "mcpuid", can you be sure that implementers will not omit setting >> that value to something unique? I'd be happier if we were overly >> clear >> now rather than have some headaches later. Have I missed something? > >These values are set by T-Head instead of individual SoC implementers >as a CPU CSR, and it's not for uniqueness, but it's for identification >of the CPU core revision (thus the PLIC/CLINT that come with it). I really am missing something here that must be obvious to you. Let me try and explain where my gap in understanding is. If someone takes the open cores & makes a minor tweak in the plic how does knowing mcpuid help us identify that that plic is marginally different? I must have missed something that should be apparent and look like an eejit right now! > >> >> > > I don't think that using "thead,openc906-clint", "thead,c900- >> > > clint" >> > > makes all that much sense either, in case someone does something >> > > wacky >> > > with the open-source version of the core. >> > > >> > > That leaves us with either: >> > > "vendor,soc-clint", "thead,openc906-clint", "thead,c900-clint" >> > > or: >> > > "vendor,soc-clint", "thead,c900-clint" >> > > right? >> > > >> > > The first one seems like possibly the better option as you'd >> > > kinda >> > > expect that, in a perfect word, all of the open-source IP >> > > implementations would share quirks etc? >> >
在 2022-12-05星期一的 16:54 +0000，Conor Dooley写道： > > > On 5 December 2022 15:59:44 GMT, Icenowy Zheng <uwu@icenowy.me> > wrote: > > 在 2022-12-05星期一的 15:05 +0000，Conor Dooley写道： > > > On Mon, Dec 05, 2022 at 07:03:17PM +0800, Icenowy Zheng wrote: > > > > 在 2022-12-05星期一的 10:36 +0000，Conor Dooley写道： > > > > > > > > You lot all know the situation here a lot more than I do... > > > > > I don't think "letting" people use the bare "thead,c900-foo" > > > > > makes > > > > > much > > > > > sense as it gives us no chance to deal with quirks down the > > > > > line. > > > > > > > > Well, after rechecking the manual, I found it possible to > > > > handle > > > > quirks > > > > -- T-Head has a custom "mcpuid" CSR (@ RISC-V CSR 0xFC0), which > > > > can > > > > be > > > > used to retrieve some identification info of the core, > > > > including > > > > its > > > > model ID, version, etc; and the T-Head PLIC/CLINT are part of > > > > their > > > > C906 SoC design that there's another "mapbaddr" CSR that could > > > > be > > > > used > > > > to retrieve the base address of them. > > > > > > > > So I think it okay to just use "thead,c900-clint" here, and > > > > when > > > > necessary, try to retrieve mcpuid for dealing with quirks. > > > > > > I'm not super sure I follow. What's the relevance of "mapbaddr" > > > here? > > > We've got a reg property, so I don't think we need "mapbaddr"? > > > > Yes, it's not relevant to us here, it's only to prove that > > PLIC/CLINT > > is part of C906 "Core Complex". > > > > > > > > For "mcpuid", can you be sure that implementers will not omit > > > setting > > > that value to something unique? I'd be happier if we were overly > > > clear > > > now rather than have some headaches later. Have I missed > > > something? > > > > These values are set by T-Head instead of individual SoC > > implementers > > as a CPU CSR, and it's not for uniqueness, but it's for > > identification > > of the CPU core revision (thus the PLIC/CLINT that come with it). > > I really am missing something here that must be obvious to you. > Let me try and explain where my gap in understanding is. > If someone takes the open cores & makes a minor tweak in the plic how > does knowing mcpuid help us identify that that plic is marginally > different? No, but my point is that in this situation we shouldn't use C900 compatible at all because it's no longer the vanilla C900 cores. My assumption is that the same IP cores are the same unless specially customized. > > I must have missed something that should be apparent and look like an > eejit right now! > > > > > > > > > > > I don't think that using "thead,openc906-clint", "thead,c900- > > > > > clint" > > > > > makes all that much sense either, in case someone does > > > > > something > > > > > wacky > > > > > with the open-source version of the core. > > > > > > > > > > That leaves us with either: > > > > > "vendor,soc-clint", "thead,openc906-clint", "thead,c900- > > > > > clint" > > > > > or: > > > > > "vendor,soc-clint", "thead,c900-clint" > > > > > right? > > > > > > > > > > The first one seems like possibly the better option as you'd > > > > > kinda > > > > > expect that, in a perfect word, all of the open-source IP > > > > > implementations would share quirks etc? > > > > >
On 6 December 2022 03:46:11 GMT, Icenowy Zheng <uwu@icenowy.me> wrote: >在 2022-12-05星期一的 16:54 +0000，Conor Dooley写道： >> >> >> On 5 December 2022 15:59:44 GMT, Icenowy Zheng <uwu@icenowy.me> >> wrote: >> > 在 2022-12-05星期一的 15:05 +0000，Conor Dooley写道： >> > > On Mon, Dec 05, 2022 at 07:03:17PM +0800, Icenowy Zheng wrote: >> > > > 在 2022-12-05星期一的 10:36 +0000，Conor Dooley写道： >> > > >> > > > > You lot all know the situation here a lot more than I do... >> > > > > I don't think "letting" people use the bare "thead,c900-foo" >> > > > > makes >> > > > > much >> > > > > sense as it gives us no chance to deal with quirks down the >> > > > > line. >> > > > >> > > > Well, after rechecking the manual, I found it possible to >> > > > handle >> > > > quirks >> > > > -- T-Head has a custom "mcpuid" CSR (@ RISC-V CSR 0xFC0), which >> > > > can >> > > > be >> > > > used to retrieve some identification info of the core, >> > > > including >> > > > its >> > > > model ID, version, etc; and the T-Head PLIC/CLINT are part of >> > > > their >> > > > C906 SoC design that there's another "mapbaddr" CSR that could >> > > > be >> > > > used >> > > > to retrieve the base address of them. >> > > > >> > > > So I think it okay to just use "thead,c900-clint" here, and >> > > > when >> > > > necessary, try to retrieve mcpuid for dealing with quirks. >> > > >> > > I'm not super sure I follow. What's the relevance of "mapbaddr" >> > > here? >> > > We've got a reg property, so I don't think we need "mapbaddr"? >> > >> > Yes, it's not relevant to us here, it's only to prove that >> > PLIC/CLINT >> > is part of C906 "Core Complex". >> > >> > > >> > > For "mcpuid", can you be sure that implementers will not omit >> > > setting >> > > that value to something unique? I'd be happier if we were overly >> > > clear >> > > now rather than have some headaches later. Have I missed >> > > something? >> > >> > These values are set by T-Head instead of individual SoC >> > implementers >> > as a CPU CSR, and it's not for uniqueness, but it's for >> > identification >> > of the CPU core revision (thus the PLIC/CLINT that come with it). >> >> I really am missing something here that must be obvious to you. >> Let me try and explain where my gap in understanding is. >> If someone takes the open cores & makes a minor tweak in the plic how >> does knowing mcpuid help us identify that that plic is marginally >> different? > >No, but my point is that in this situation we shouldn't use C900 >compatible at all because it's no longer the vanilla C900 cores. > >My assumption is that the same IP cores are the same unless specially >customized. Ah see that is assuming people get it right. I've been in the mindset of "what if the difference is only noticed after the DT has been shipped". I guess that's where we've been at odds. > >> >> I must have missed something that should be apparent and look like an >> eejit right now! >> >> > >> > > >> > > > > I don't think that using "thead,openc906-clint", "thead,c900- >> > > > > clint" >> > > > > makes all that much sense either, in case someone does >> > > > > something >> > > > > wacky >> > > > > with the open-source version of the core. >> > > > > >> > > > > That leaves us with either: >> > > > > "vendor,soc-clint", "thead,openc906-clint", "thead,c900- >> > > > > clint" >> > > > > or: >> > > > > "vendor,soc-clint", "thead,c900-clint" >> > > > > right? >> > > > > >> > > > > The first one seems like possibly the better option as you'd >> > > > > kinda >> > > > > expect that, in a perfect word, all of the open-source IP >> > > > > implementations would share quirks etc? >> > > >> > >
diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.yaml b/Documentation/devicetree/bindings/timer/sifive,clint.yaml index aada6957216c..86703e995e31 100644 --- a/Documentation/devicetree/bindings/timer/sifive,clint.yaml +++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml @@ -35,6 +35,7 @@ properties: - const: sifive,clint0 - items: - enum: + - thead,openc906-clint - allwinner,sun20i-d1-clint - const: thead,c900-clint - items:
T-Head OpenC906 is a open-source-licensed fixed-configuration of C906, which is now public and able to be integrated. Add a compatible for the CLINT shipped as part of OpenC906, which should just be ordinary C9xx CLINT. Signed-off-by: Icenowy Zheng <uwu@icenowy.me> --- Documentation/devicetree/bindings/timer/sifive,clint.yaml | 1 + 1 file changed, 1 insertion(+)