Re: [PATCH v4 09/13] dt-bindings: riscv: Add T-Head PMU extension description

[Inochi Amaoto]

>
>On Fri, Dec 01, 2023 at 07:11:31AM +0800, Inochi Amaoto wrote:
>>>
>>> On Thu, Nov 30, 2023 at 08:16:38PM +0800, Inochi Amaoto wrote:
>>>>>
>>>>> Hi Inochi,
>>>>>
>>>>> On Thu, Nov 30, 2023 at 04:29:22PM +0800, Inochi Amaoto wrote:
>>>>>>>
>>>>>>> Hi Guo Ren,
>>>>>>>
>>>>>>> On Thu, Nov 23, 2023 at 05:14:30AM +0800, Guo Ren wrote:
>>>>>>>> On Wed, Nov 22, 2023 at 8:17 PM Yu Chien Peter Lin
>>>>>>>> <peterlin@xxxxxxxxxxxxx> wrote:
>>>>>>>>>
>>>>>>>>> Document the ISA string for T-Head performance monitor extension
>>>>>>>>> which provides counter overflow interrupt mechanism.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Yu Chien Peter Lin <peterlin@xxxxxxxxxxxxx>
>>>>>>>>> ---
>>>>>>>>> Changes v2 -> v3:
>>>>>>>>>   - New patch
>>>>>>>>> Changes v3 -> v4:
>>>>>>>>>   - No change
>>>>>>>>> ---
>>>>>>>>>  Documentation/devicetree/bindings/riscv/extensions.yaml | 6 ++++++
>>>>>>>>>  1 file changed, 6 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
>>>>>>>>> index c91ab0e46648..694efaea8fce 100644
>>>>>>>>> --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
>>>>>>>>> +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
>>>>>>>>> @@ -258,5 +258,11 @@ properties:
>>>>>>>>>              in commit 2e5236 ("Ztso is now ratified.") of the
>>>>>>>>>              riscv-isa-manual.
>>>>>>>>>
>>>>>>>>> +        - const: xtheadpmu
>>>>>>>>> +          description:
>>>>>>>>> +            The T-Head performance monitor extension for counter overflow. For more
>>>>>>>>> +            details, see the chapter 12 in the Xuantie C906 user manual.
>>>>>>>>> +            https://github.com/T-head-Semi/openc906/tree/main/doc
>>>>>>>>> +
>>>>>>>>>  additionalProperties: true
>>>>>>>>>  ...
>>>>>>>>> --
>>>>>>>>> 2.34.1
>>>>>>>>>
>>>>>>>> Reviewed-by: Guo Ren <guoren@xxxxxxxxxx>
>>>>>>>
>>>>>>> Thanks for the review.
>>>>>>> Would you share document about T-Head PMU?
>>>>>>>
>>>>>>
>>>>>> Hi, Peter Lin:
>>>>>>
>>>>>> You can use the following two document to get all events:
>>>>>> https://github.com/T-head-Semi/openc906/tree/main/doc
>>>>>> https://github.com/T-head-Semi/openc910/tree/main/doc
>>>>>>
>>>>>> There are also some RTL code can describe these events:
>>>>>> https://github.com/T-head-Semi/openc910/blob/e0c4ad8ec7f8c70f649d826ebd6c949086453272/C910_RTL_FACTORY/gen_rtl/pmu/rtl/ct_hpcp_top.v#L1123
>>>>>> https://github.com/T-head-Semi/openc906/blob/af5614d72de7e5a4b8609c427d2e20af1deb21c4/C906_RTL_FACTORY/gen_rtl/pmu/rtl/aq_hpcp_top.v#L543
>>>>>>
>>>>>> The perf events json can also be used as document, this is already
>>>>>> applied (with more detailed explanation):
>>>>>> https://lore.kernel.org/all/IA1PR20MB495325FCF603BAA841E29281BBBAA@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
>>>>>
>>>>> Thanks for reaching out!
>>>>> The updated description will be:
>>>>>
>>>>> - const: xtheadpmu
>>>>>  description:
>>>>>    The T-Head performance monitor extension for counter overflow, as ratified
>>>>>    in commit bd9206 ("Initial commit") of Xuantie C906 user manual.
>>>>>    https://github.com/T-head-Semi/openc906/tree/main/doc
>>>>>
>>>>> Is it OK with you?
>>>>>
>>>>
>>>> I suggest using perf event json as event description. The jsons provide
>>>> more detailed explanation for these events than the user manual.
>>>
>>> Does the "perf event json" describe the registers and interrupt behaviour?
>>>
>>
>> It does not. IIRC, the linux just uses SBI as perf driver backend. So
>> the registers and interrupt behaviour is primarily for SBI developer.
>
>Interrupts and registers are the reason that this patch (and the rest of
>the patchset) exists :)
>

Thanks, it seems I misunderstood something.

>> For registers and interrup detail, just reference the openc910 doc url
>> (https://github.com/T-head-Semi/openc910/tree/main/doc) and the T-HEAD
>> PMU driver in OpenSBI.
>
>The former, sure. But I will not accept driver implementations as the
>reference in this context.
>

OK, Let's drop it and left the document only.

>Thanks,
>Conor.
>
>