| Message ID | 20240131065953.9634-1-jialong.yang@shingroup.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] dt-bingdings: perf: Support uncore ARM NI-700 PMU | expand |
On 31/01/2024 07:59, JiaLong.Yang wrote: > Add file corresponding to hx_arm_ni.c introducing ARM NI-700 PMU driver > for HX. > > Signed-off-by: JiaLong.Yang <jialong.yang@shingroup.cn> > --- > v1 --> v2: > 1. Submit dt-bindings file Seperately. Where is the driver? Please read: https://elixir.bootlin.com/linux/v6.8-rc2/source/Documentation/process/submitting-patches.rst before posting. > 2. Do some check: > ~ #: make dt_binding_check DT_SCHEMA_FILES=perf > LINT Documentation/devicetree/bindings > CHKDT Documentation/devicetree/bindings/processed-schema.json > SCHEMA Documentation/devicetree/bindings/processed-schema.json > DTEX Documentation/devicetree/bindings/perf/hx,c2000-arm-ni.example.dts > DTC_CHK Documentation/devicetree/bindings/perf/hx,c2000-arm-ni.example.dtb No other changes? Sorry, this is not acceptable. You must implement feedback and then describe what you changed in the changelog. > > .../bindings/perf/hx,c2000-arm-ni.yaml | 53 +++++++++++++++++++ > .../devicetree/bindings/vendor-prefixes.yaml | 2 + > MAINTAINERS | 6 +++ > 3 files changed, 61 insertions(+) > create mode 100644 Documentation/devicetree/bindings/perf/hx,c2000-arm-ni.yaml > > diff --git a/Documentation/devicetree/bindings/perf/hx,c2000-arm-ni.yaml b/Documentation/devicetree/bindings/perf/hx,c2000-arm-ni.yaml > new file mode 100644 > index 000000000000..de50c79a5f23 > --- /dev/null > +++ b/Documentation/devicetree/bindings/perf/hx,c2000-arm-ni.yaml > @@ -0,0 +1,53 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/perf/hx,c2000-arm-ni.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: HX-C2000 NI (Network-on-chip Interconnect) Performance Monitors > + > +maintainers: > + - Jialong Yang <jialong.yang@shingroup.cn> > + > +properties: > + compatible: > + enum: > + - hx,c2000-arm-ni > + > + reg: > + items: > + - description: Physical address of the base (PERIPHBASE) and > + size of the whole NI configuration address space. > + > + interrupts: > + minItems: 1 NAK, I wrote why. You just ignored my comments and did not even wait for my feedback. > + items: > + - description: Overflow interrupt for clock domain 0 > + - description: Overflow interrupt for clock domain 1 > + - description: Overflow interrupt for clock domain 2 > + - description: Generally, one interrupt line for one PMU. But this also > + support one interrupt line for a NI if merged. > + > + pccs-id: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: Used to identify NIs in system which has more than > + one NI. No, reg does it. Drop the property. Anyway you miss here vendor prefix and proper explanation. > + > +required: > + - compatible > + - reg > + - interrupts > + > +additionalProperties: true NAK, not explained in changelog, not correct. > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + > + pmu@23ff0000 { Use 4 spaces for example indentation. > + compatible = "hx,c2000-arm-ni"; > + reg = <0x2b420000 0x10000>; > + pccs-id = <0>; > + interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>; > + }; > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml > index 1a0dc04f1db4..f6b4103ef436 100644 > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml > @@ -617,6 +617,8 @@ patternProperties: > description: Shenzhen Hugsun Technology Co. Ltd. > "^hwacom,.*": > description: HwaCom Systems Inc. > + "^hx,.*": hexin, > + description: HEXIN Technologies Co., Ltd. > "^hxt,.*": > description: HXT Semiconductor > "^hycon,.*": > diff --git a/MAINTAINERS b/MAINTAINERS > index 8999497011a2..101974780710 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -18866,6 +18866,12 @@ L: linux-riscv@lists.infradead.org > S: Maintained > F: arch/riscv/boot/dts/thead/ > > +HX ARM-NI-700 PMU DRIVERS > +M: Jialong Yang <jialong.yang@shingroup.cn> > +S: Supported > +F: Documentation/devicetree/bindings/perf/hx,c2000-arm-ni.yaml > +F: drivers/perf/hx_arm_ni.c There is no such file. Best regards, Krzysztof
在 2024/1/31 17:30, Krzysztof Kozlowski 写道: > On 31/01/2024 10:26, Yang Jialong 杨佳龙 wrote: >> >> >> 在 2024/1/31 15:49, Krzysztof Kozlowski 写道: >>> On 31/01/2024 07:59, JiaLong.Yang wrote: >>>> Add file corresponding to hx_arm_ni.c introducing ARM NI-700 PMU driver >>>> for HX. >>>> >>>> Signed-off-by: JiaLong.Yang <jialong.yang@shingroup.cn> >>>> --- >>>> v1 --> v2: >>>> 1. Submit dt-bindings file Seperately. >>> >>> Where is the driver? >>> >>> Please read: >>> https://elixir.bootlin.com/linux/v6.8-rc2/source/Documentation/process/submitting-patches.rst >>> before posting. >>> > > Keep all discussions public. Get. > > >>>> + pccs-id: >>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>> + description: Used to identify NIs in system which has more than >>>> + one NI. >>> >>> No, reg does it. Drop the property. Anyway you miss here vendor prefix >>> and proper explanation. >>> >> >> reg will tell phy address. Phy address is too long. I just want a id. >> example: perf stat -a -e ni_pmu_${pccs-id}/cycles/ >> I will use it in user space. Not only in driver. > > Custom vendor property is not for that purpose. Use for example IDR, DT > aliases or something else. > > I have considered TD aliases. It's not very easy. Two places... IDR.. I have tested. But it chouldn't correspond to the HWs. I need it to identify NIs. DT aliases is reachable. But no very easy. >>> > > Best regards, > Krzysztof > >
On 31/01/2024 11:13, Yang Jialong 杨佳龙 wrote: > > > 在 2024/1/31 17:30, Krzysztof Kozlowski 写道: >> On 31/01/2024 10:26, Yang Jialong 杨佳龙 wrote: >>> >>> >>> 在 2024/1/31 15:49, Krzysztof Kozlowski 写道: >>>> On 31/01/2024 07:59, JiaLong.Yang wrote: >>>>> Add file corresponding to hx_arm_ni.c introducing ARM NI-700 PMU driver >>>>> for HX. >>>>> >>>>> Signed-off-by: JiaLong.Yang <jialong.yang@shingroup.cn> >>>>> --- >>>>> v1 --> v2: >>>>> 1. Submit dt-bindings file Seperately. >>>> >>>> Where is the driver? >>>> >>>> Please read: >>>> https://elixir.bootlin.com/linux/v6.8-rc2/source/Documentation/process/submitting-patches.rst >>>> before posting. >>>> >> >> Keep all discussions public. > > Get. > >> >> >>>>> + pccs-id: >>>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>>> + description: Used to identify NIs in system which has more than >>>>> + one NI. >>>> >>>> No, reg does it. Drop the property. Anyway you miss here vendor prefix >>>> and proper explanation. >>>> >>> >>> reg will tell phy address. Phy address is too long. I just want a id. >>> example: perf stat -a -e ni_pmu_${pccs-id}/cycles/ >>> I will use it in user space. Not only in driver. >> >> Custom vendor property is not for that purpose. Use for example IDR, DT >> aliases or something else. >> >> > > I have considered TD aliases. It's not very easy. Two places... > IDR.. I have tested. But it chouldn't correspond to the HWs. > I need it to identify NIs. > DT aliases is reachable. But no very easy. Except that "you want" I did not see any rationale, any argument explaining why this is needed and why this has to be present. Best regards, Krzysztof
在 2024/1/31 18:16, Krzysztof Kozlowski 写道: > On 31/01/2024 11:13, Yang Jialong 杨佳龙 wrote: >> >> >> 在 2024/1/31 17:30, Krzysztof Kozlowski 写道: >>> On 31/01/2024 10:26, Yang Jialong 杨佳龙 wrote: >>>> >>>> >>>> 在 2024/1/31 15:49, Krzysztof Kozlowski 写道: >>>>> On 31/01/2024 07:59, JiaLong.Yang wrote: >>>>>> Add file corresponding to hx_arm_ni.c introducing ARM NI-700 PMU driver >>>>>> for HX. >>>>>> >>>>>> Signed-off-by: JiaLong.Yang <jialong.yang@shingroup.cn> >>>>>> --- >>>>>> v1 --> v2: >>>>>> 1. Submit dt-bindings file Seperately. >>>>> >>>>> Where is the driver? >>>>> >>>>> Please read: >>>>> https://elixir.bootlin.com/linux/v6.8-rc2/source/Documentation/process/submitting-patches.rst >>>>> before posting. >>>>> >>> >>> Keep all discussions public. >> >> Get. >> >>> >>> >>>>>> + pccs-id: >>>>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>>>> + description: Used to identify NIs in system which has more than >>>>>> + one NI. >>>>> >>>>> No, reg does it. Drop the property. Anyway you miss here vendor prefix >>>>> and proper explanation. >>>>> >>>> >>>> reg will tell phy address. Phy address is too long. I just want a id. >>>> example: perf stat -a -e ni_pmu_${pccs-id}/cycles/ >>>> I will use it in user space. Not only in driver. >>> >>> Custom vendor property is not for that purpose. Use for example IDR, DT >>> aliases or something else. >>> >>> >> >> I have considered TD aliases. It's not very easy. Two places... >> IDR.. I have tested. But it chouldn't correspond to the HWs. >> I need it to identify NIs. >> DT aliases is reachable. But no very easy. > > Except that "you want" I did not see any rationale, any argument > explaining why this is needed and why this has to be present. OK. DT aliases it good. > > Best regards, > Krzysztof > >
On 31/01/2024 10:18 am, Yang Jialong 杨佳龙 wrote: > > > 在 2024/1/31 18:16, Krzysztof Kozlowski 写道: >> On 31/01/2024 11:13, Yang Jialong 杨佳龙 wrote: >>> >>> >>> 在 2024/1/31 17:30, Krzysztof Kozlowski 写道: >>>> On 31/01/2024 10:26, Yang Jialong 杨佳龙 wrote: >>>>> >>>>> >>>>> 在 2024/1/31 15:49, Krzysztof Kozlowski 写道: >>>>>> On 31/01/2024 07:59, JiaLong.Yang wrote: >>>>>>> Add file corresponding to hx_arm_ni.c introducing ARM NI-700 PMU >>>>>>> driver >>>>>>> for HX. >>>>>>> >>>>>>> Signed-off-by: JiaLong.Yang <jialong.yang@shingroup.cn> >>>>>>> --- >>>>>>> v1 --> v2: >>>>>>> 1. Submit dt-bindings file Seperately. >>>>>> >>>>>> Where is the driver? >>>>>> >>>>>> Please read: >>>>>> https://elixir.bootlin.com/linux/v6.8-rc2/source/Documentation/process/submitting-patches.rst >>>>>> before posting. >>>>>> >>>> >>>> Keep all discussions public. >>> >>> Get. >>> >>>> >>>> >>>>>>> + pccs-id: >>>>>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>>>>> + description: Used to identify NIs in system which has more than >>>>>>> + one NI. >>>>>> >>>>>> No, reg does it. Drop the property. Anyway you miss here vendor >>>>>> prefix >>>>>> and proper explanation. >>>>>> >>>>> >>>>> reg will tell phy address. Phy address is too long. I just want a id. >>>>> example: perf stat -a -e ni_pmu_${pccs-id}/cycles/ >>>>> I will use it in user space. Not only in driver. >>>> >>>> Custom vendor property is not for that purpose. Use for example IDR, DT >>>> aliases or something else. >>>> >>>> >>> >>> I have considered TD aliases. It's not very easy. Two places... >>> IDR.. I have tested. But it chouldn't correspond to the HWs. >>> I need it to identify NIs. >>> DT aliases is reachable. But no very easy. >> >> Except that "you want" I did not see any rationale, any argument >> explaining why this is needed and why this has to be present. > > OK. DT aliases it good. The real way to address that particular issue is to fix perf to properly associate the PMU device with the underlying hardware device. Jonathan had a series doing that[1], but I'm not sure what its status is now. Thanks, Robin. [1] https://lore.kernel.org/linux-arm-kernel/20230404134225.13408-1-Jonathan.Cameron@huawei.com/
在 2024/2/1 0:59, Robin Murphy 写道: > On 31/01/2024 10:18 am, Yang Jialong 杨佳龙 wrote: >> >> >> 在 2024/1/31 18:16, Krzysztof Kozlowski 写道: >>> On 31/01/2024 11:13, Yang Jialong 杨佳龙 wrote: >>>> >>>> >>>> 在 2024/1/31 17:30, Krzysztof Kozlowski 写道: >>>>> On 31/01/2024 10:26, Yang Jialong 杨佳龙 wrote: >>>>>> >>>>>> >>>>>> 在 2024/1/31 15:49, Krzysztof Kozlowski 写道: >>>>>>> On 31/01/2024 07:59, JiaLong.Yang wrote: >>>>>>>> Add file corresponding to hx_arm_ni.c introducing ARM NI-700 PMU >>>>>>>> driver >>>>>>>> for HX. >>>>>>>> >>>>>>>> Signed-off-by: JiaLong.Yang <jialong.yang@shingroup.cn> >>>>>>>> --- >>>>>>>> v1 --> v2: >>>>>>>> 1. Submit dt-bindings file Seperately. >>>>>>> >>>>>>> Where is the driver? >>>>>>> >>>>>>> Please read: >>>>>>> https://elixir.bootlin.com/linux/v6.8-rc2/source/Documentation/process/submitting-patches.rst >>>>>>> before posting. >>>>>>> >>>>> >>>>> Keep all discussions public. >>>> >>>> Get. >>>> >>>>> >>>>> >>>>>>>> + pccs-id: >>>>>>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>>>>>> + description: Used to identify NIs in system which has more >>>>>>>> than >>>>>>>> + one NI. >>>>>>> >>>>>>> No, reg does it. Drop the property. Anyway you miss here vendor >>>>>>> prefix >>>>>>> and proper explanation. >>>>>>> >>>>>> >>>>>> reg will tell phy address. Phy address is too long. I just want a id. >>>>>> example: perf stat -a -e ni_pmu_${pccs-id}/cycles/ >>>>>> I will use it in user space. Not only in driver. >>>>> >>>>> Custom vendor property is not for that purpose. Use for example >>>>> IDR, DT >>>>> aliases or something else. >>>>> >>>>> >>>> >>>> I have considered TD aliases. It's not very easy. Two places... >>>> IDR.. I have tested. But it chouldn't correspond to the HWs. >>>> I need it to identify NIs. >>>> DT aliases is reachable. But no very easy. >>> >>> Except that "you want" I did not see any rationale, any argument >>> explaining why this is needed and why this has to be present. >> >> OK. DT aliases it good. > > The real way to address that particular issue is to fix perf to properly > associate the PMU device with the underlying hardware device. Jonathan > had a series doing that[1], but I'm not sure what its status is now. > Good news. In NI, we have to consider NIs and PMUs in one NI. > Thanks, > Robin. > > [1] > https://lore.kernel.org/linux-arm-kernel/20230404134225.13408-1-Jonathan.Cameron@huawei.com/ >
diff --git a/Documentation/devicetree/bindings/perf/hx,c2000-arm-ni.yaml b/Documentation/devicetree/bindings/perf/hx,c2000-arm-ni.yaml new file mode 100644 index 000000000000..de50c79a5f23 --- /dev/null +++ b/Documentation/devicetree/bindings/perf/hx,c2000-arm-ni.yaml @@ -0,0 +1,53 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/perf/hx,c2000-arm-ni.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: HX-C2000 NI (Network-on-chip Interconnect) Performance Monitors + +maintainers: + - Jialong Yang <jialong.yang@shingroup.cn> + +properties: + compatible: + enum: + - hx,c2000-arm-ni + + reg: + items: + - description: Physical address of the base (PERIPHBASE) and + size of the whole NI configuration address space. + + interrupts: + minItems: 1 + items: + - description: Overflow interrupt for clock domain 0 + - description: Overflow interrupt for clock domain 1 + - description: Overflow interrupt for clock domain 2 + - description: Generally, one interrupt line for one PMU. But this also + support one interrupt line for a NI if merged. + + pccs-id: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Used to identify NIs in system which has more than + one NI. + +required: + - compatible + - reg + - interrupts + +additionalProperties: true + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/interrupt-controller/irq.h> + + pmu@23ff0000 { + compatible = "hx,c2000-arm-ni"; + reg = <0x2b420000 0x10000>; + pccs-id = <0>; + interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>; + }; diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 1a0dc04f1db4..f6b4103ef436 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -617,6 +617,8 @@ patternProperties: description: Shenzhen Hugsun Technology Co. Ltd. "^hwacom,.*": description: HwaCom Systems Inc. + "^hx,.*": + description: HEXIN Technologies Co., Ltd. "^hxt,.*": description: HXT Semiconductor "^hycon,.*": diff --git a/MAINTAINERS b/MAINTAINERS index 8999497011a2..101974780710 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -18866,6 +18866,12 @@ L: linux-riscv@lists.infradead.org S: Maintained F: arch/riscv/boot/dts/thead/ +HX ARM-NI-700 PMU DRIVERS +M: Jialong Yang <jialong.yang@shingroup.cn> +S: Supported +F: Documentation/devicetree/bindings/perf/hx,c2000-arm-ni.yaml +F: drivers/perf/hx_arm_ni.c + RNBD BLOCK DRIVERS M: Md. Haris Iqbal <haris.iqbal@ionos.com> M: Jack Wang <jinpu.wang@ionos.com>
Add file corresponding to hx_arm_ni.c introducing ARM NI-700 PMU driver for HX. Signed-off-by: JiaLong.Yang <jialong.yang@shingroup.cn> --- v1 --> v2: 1. Submit dt-bindings file Seperately. 2. Do some check: ~ #: make dt_binding_check DT_SCHEMA_FILES=perf LINT Documentation/devicetree/bindings CHKDT Documentation/devicetree/bindings/processed-schema.json SCHEMA Documentation/devicetree/bindings/processed-schema.json DTEX Documentation/devicetree/bindings/perf/hx,c2000-arm-ni.example.dts DTC_CHK Documentation/devicetree/bindings/perf/hx,c2000-arm-ni.example.dtb .../bindings/perf/hx,c2000-arm-ni.yaml | 53 +++++++++++++++++++ .../devicetree/bindings/vendor-prefixes.yaml | 2 + MAINTAINERS | 6 +++ 3 files changed, 61 insertions(+) create mode 100644 Documentation/devicetree/bindings/perf/hx,c2000-arm-ni.yaml