diff mbox series

[v2,1/2] dt-bindings: timer: thead,c900-aclint-mtimer: separate mtime and mtimecmp regs

Message ID IA1PR20MB49533EEEB2F7E8D93762DC1ABBB2A@IA1PR20MB4953.namprd20.prod.outlook.com
State New
Headers
Series Change the sg2042 timer layout to fit aclint format |

Commit Message

Inochi Amaoto Nov. 14, 2023, 12:45 a.m. UTC 
  The timer registers of aclint don't follow the clint layout and can
be mapped on any different offset. As sg2042 uses separated timer
and mswi for its clint, it should follow the aclint spec and have
separated registers.

The previous patch introduces a new type of T-HEAD aclint timer which
has clint timer layout. Although the timer has the clint layout, it
should follow the aclint spec and uses the separated mtime and mtimecmp
regs. So a ABI change is needed to make the timer fit the aclint spec.

To make T-HEAD aclint timer more closer to the aclint spec, use two regs
to represent the mtime and mtimecmp.

Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
Fixes: 4734449f7311 ("dt-bindings: timer: Add Sophgo sg2042 CLINT timer")
Link: https://lists.infradead.org/pipermail/opensbi/2023-October/005693.html
Link: https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc
---
 .../devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml  | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

--
2.42.1

Comments

Chen Wang Nov. 14, 2023, 1:09 a.m. UTC | #1 
On 2023/11/14 8:45, Inochi Amaoto wrote:
> The timer registers of aclint don't follow the clint layout and can
> be mapped on any different offset. As sg2042 uses separated timer
> and mswi for its clint, it should follow the aclint spec and have
> separated registers.
>
> The previous patch introduces a new type of T-HEAD aclint timer which
> has clint timer layout. Although the timer has the clint layout, it
> should follow the aclint spec and uses the separated mtime and mtimecmp
> regs. So a ABI change is needed to make the timer fit the aclint spec.
>
> To make T-HEAD aclint timer more closer to the aclint spec, use two regs
> to represent the mtime and mtimecmp.
>
> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
> Fixes: 4734449f7311 ("dt-bindings: timer: Add Sophgo sg2042 CLINT timer")
> Link: https://lists.infradead.org/pipermail/opensbi/2023-October/005693.html
> Link: https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc
> ---
>   .../devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml  | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
> index fbd235650e52..c3080962d902 100644
> --- a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
> +++ b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
> @@ -17,7 +17,7 @@ properties:
>         - const: thead,c900-aclint-mtimer
>
>     reg:
> -    maxItems: 1
> +    maxItems: 2

The first one is for mtime and the second one is for mtimecmp, right? 
Recommend to add some comment in binding file to make it clear.

Chen

>
>     interrupts-extended:
>       minItems: 1
> @@ -38,6 +38,7 @@ examples:
>                               <&cpu2intc 7>,
>                               <&cpu3intc 7>,
>                               <&cpu4intc 7>;
> -      reg = <0xac000000 0x00010000>;
> +      reg = <0xac000000 0x00000000>,
> +            <0xac000000 0x0000c000>;
>       };
>   ...
> --
> 2.42.1
>
Inochi Amaoto Nov. 14, 2023, 1:45 a.m. UTC | #2 
>On 2023/11/14 8:45, Inochi Amaoto wrote:
>> The timer registers of aclint don't follow the clint layout and can
>> be mapped on any different offset. As sg2042 uses separated timer
>> and mswi for its clint, it should follow the aclint spec and have
>> separated registers.
>>
>> The previous patch introduces a new type of T-HEAD aclint timer which
>> has clint timer layout. Although the timer has the clint layout, it
>> should follow the aclint spec and uses the separated mtime and mtimecmp
>> regs. So a ABI change is needed to make the timer fit the aclint spec.
>>
>> To make T-HEAD aclint timer more closer to the aclint spec, use two regs
>> to represent the mtime and mtimecmp.
>>
>> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
>> Fixes: 4734449f7311 ("dt-bindings: timer: Add Sophgo sg2042 CLINT timer")
>> Link: https://lists.infradead.org/pipermail/opensbi/2023-October/005693.html
>> Link: https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc
>> ---
>>   .../devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml  | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
>> index fbd235650e52..c3080962d902 100644
>> --- a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
>> +++ b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
>> @@ -17,7 +17,7 @@ properties:
>>         - const: thead,c900-aclint-mtimer
>>
>>     reg:
>> -    maxItems: 1
>> +    maxItems: 2
>
>The first one is for mtime and the second one is for mtimecmp, right?

Yes, that is right.

>Recommend to add some comment in binding file to make it clear.
>

Thanks for your advice.

>Chen
>
>>
>>     interrupts-extended:
>>       minItems: 1
>> @@ -38,6 +38,7 @@ examples:
>>                               <&cpu2intc 7>,
>>                               <&cpu3intc 7>,
>>                               <&cpu4intc 7>;
>> -      reg = <0xac000000 0x00010000>;
>> +      reg = <0xac000000 0x00000000>,
>> +            <0xac000000 0x0000c000>;
>>       };
>>   ...
>> --
>> 2.42.1
>>
>
Conor Dooley Nov. 14, 2023, 3:21 p.m. UTC | #3 
On Tue, Nov 14, 2023 at 09:45:33AM +0800, Inochi Amaoto wrote:
> >On 2023/11/14 8:45, Inochi Amaoto wrote:
> >> The timer registers of aclint don't follow the clint layout and can
> >> be mapped on any different offset. As sg2042 uses separated timer
> >> and mswi for its clint, it should follow the aclint spec and have
> >> separated registers.
> >>
> >> The previous patch introduces a new type of T-HEAD aclint timer which
> >> has clint timer layout. Although the timer has the clint layout, it
> >> should follow the aclint spec and uses the separated mtime and mtimecmp
> >> regs. So a ABI change is needed to make the timer fit the aclint spec.
> >>
> >> To make T-HEAD aclint timer more closer to the aclint spec, use two regs
> >> to represent the mtime and mtimecmp.
> >>
> >> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
> >> Fixes: 4734449f7311 ("dt-bindings: timer: Add Sophgo sg2042 CLINT timer")
> >> Link: https://lists.infradead.org/pipermail/opensbi/2023-October/005693.html
> >> Link: https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc
> >> ---
> >>   .../devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml  | 5 +++--
> >>   1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
> >> index fbd235650e52..c3080962d902 100644
> >> --- a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
> >> +++ b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
> >> @@ -17,7 +17,7 @@ properties:
> >>         - const: thead,c900-aclint-mtimer
> >>
> >>     reg:
> >> -    maxItems: 1
> >> +    maxItems: 2
> >
> >The first one is for mtime and the second one is for mtimecmp, right?
> 
> Yes, that is right.
> 
> >Recommend to add some comment in binding file to make it clear.
> >
> 
> Thanks for your advice.

Sorry for not noticing that on v1 - you should indeed describe these in
the binding, by using the items property.
Inochi Amaoto Nov. 14, 2023, 10:45 p.m. UTC | #4 
>On Tue, Nov 14, 2023 at 09:45:33AM +0800, Inochi Amaoto wrote:
>>> On 2023/11/14 8:45, Inochi Amaoto wrote:
>>>> The timer registers of aclint don't follow the clint layout and can
>>>> be mapped on any different offset. As sg2042 uses separated timer
>>>> and mswi for its clint, it should follow the aclint spec and have
>>>> separated registers.
>>>>
>>>> The previous patch introduces a new type of T-HEAD aclint timer which
>>>> has clint timer layout. Although the timer has the clint layout, it
>>>> should follow the aclint spec and uses the separated mtime and mtimecmp
>>>> regs. So a ABI change is needed to make the timer fit the aclint spec.
>>>>
>>>> To make T-HEAD aclint timer more closer to the aclint spec, use two regs
>>>> to represent the mtime and mtimecmp.
>>>>
>>>> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
>>>> Fixes: 4734449f7311 ("dt-bindings: timer: Add Sophgo sg2042 CLINT timer")
>>>> Link: https://lists.infradead.org/pipermail/opensbi/2023-October/005693.html
>>>> Link: https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc
>>>> ---
>>>>   .../devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml  | 5 +++--
>>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
>>>> index fbd235650e52..c3080962d902 100644
>>>> --- a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
>>>> +++ b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
>>>> @@ -17,7 +17,7 @@ properties:
>>>>         - const: thead,c900-aclint-mtimer
>>>>
>>>>     reg:
>>>> -    maxItems: 1
>>>> +    maxItems: 2
>>>
>>> The first one is for mtime and the second one is for mtimecmp, right?
>>
>> Yes, that is right.
>>
>>> Recommend to add some comment in binding file to make it clear.
>>>
>>
>> Thanks for your advice.
>
>Sorry for not noticing that on v1 -

Sorry for this, I have seen the v1 and improve the comment of the v2. I
will give a feedback next time. Anyway, thank you for your advice in v1.

>you should indeed describe these in the binding, by using the items property.
>

Thanks, I will have a try.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
index fbd235650e52..c3080962d902 100644
--- a/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
+++ b/Documentation/devicetree/bindings/timer/thead,c900-aclint-mtimer.yaml
@@ -17,7 +17,7 @@  properties:
       - const: thead,c900-aclint-mtimer

   reg:
-    maxItems: 1
+    maxItems: 2

   interrupts-extended:
     minItems: 1
@@ -38,6 +38,7 @@  examples:
                             <&cpu2intc 7>,
                             <&cpu3intc 7>,
                             <&cpu4intc 7>;
-      reg = <0xac000000 0x00010000>;
+      reg = <0xac000000 0x00000000>,
+            <0xac000000 0x0000c000>;
     };
 ...