Message ID | 20220412125035.40312-1-quic_jinlmao@quicinc.com |
---|---|
Headers | show |
Series | Coresight: Add support for TPDM and TPDA | expand |
Hi, On Tue, 12 Apr 2022 at 13:51, Mao Jinlong <quic_jinlmao@quicinc.com> wrote: > > Adds new coresight-tpdm.yaml file describing the bindings required > to define tpdm in the device trees. > > Signed-off-by: Tao Zhang <quic_taozha@quicinc.com> > Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com> > --- > .../bindings/arm/coresight-tpdm.yaml | 99 +++++++++++++++++++ > .../devicetree/bindings/arm/coresight.txt | 7 ++ > MAINTAINERS | 1 + > 3 files changed, 107 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/coresight-tpdm.yaml > > diff --git a/Documentation/devicetree/bindings/arm/coresight-tpdm.yaml b/Documentation/devicetree/bindings/arm/coresight-tpdm.yaml > new file mode 100644 > index 000000000000..05210e0fc262 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/coresight-tpdm.yaml > @@ -0,0 +1,99 @@ > +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause > +# Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/arm/coresight-tpdm.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Trace, Profiling and Diagnostics Monitor - TPDM > + > +description: | > + The TPDM or Monitor serves as data collection component for various dataset > + types specified in the QPMDA spec. It covers Implementation defined ((ImplDef), > + Basic Counts (BC), Tenure Counts (TC), Continuous Multi-Bit (CMB), and Discrete > + Single Bit (DSB). It performs data collection in the data producing clock > + domain and transfers it to the data collection time domain, generally ATB > + clock domain. > + > + The primary use case of the TPDM is to collect data from different data > + sources and send it to a TPDA for packetization, timestamping, and funneling. > + > +maintainers: > + - Suzuki K Poulose <suzuki.poulose@arm.com> > + - Mathieu Poirier <mathieu.poirier@linaro.org> > + These should be e-mail addresses of maintainers for this binding, not the coresight sub-system. See writing-schema.rst > +properties: > + $nodename: > + pattern: "^tpdm(@[0-9a-f]+)$" > + compatible: > + items: > + - const: qcom,coresight-tpdm > + - const: arm,primecell > + > + reg: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + clock-names: > + items: > + - const: apb_pclk > + > + out-ports: > + description: | > + Output connections from the TPDM to coresight funnle/tpda. > + $ref: /schemas/graph.yaml#/properties/ports > + properties: > + port: > + description: Output connection from the TPDM to coresight > + funnel/tpda. > + $ref: /schemas/graph.yaml#/properties/port > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + > +additionalProperties: false > + > +examples: > + # minimum TPDM definition. TPDM connect to coresight funnel. > + - | > + tpdm@6980000 { > + compatible = "qcom,coresight-tpdm", "arm,primecell"; > + reg = <0x6980000 0x1000>; > + > + clocks = <&aoss_qmp>; > + clock-names = "apb_pclk"; > + > + out-ports { > + port { > + tpdm_turing_out_funnel_turing: endpoint { > + remote-endpoint = > + <&funnel_turing_in_tpdm_turing>; > + }; > + }; > + }; > + }; > + # minimum TPDM definition. TPDM connect to coresight TPDA. > + - | > + tpdm@684c000 { > + compatible = "qcom,coresight-tpdm", "arm,primecell"; > + reg = <0x684c000 0x1000>; > + > + clocks = <&aoss_qmp>; > + clock-names = "apb_pclk"; > + > + out-ports { > + port { > + tpdm_prng_out_tpda_qdss: endpoint { > + remote-endpoint = > + <&tpda_qdss_in_tpdm_prng>; > + }; > + }; > + }; > + }; > + > +... > diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt > index c68d93a35b6c..f7ce8af48574 100644 > --- a/Documentation/devicetree/bindings/arm/coresight.txt > +++ b/Documentation/devicetree/bindings/arm/coresight.txt > @@ -52,6 +52,10 @@ its hardware characteristcs. > "arm,coresight-cti", "arm,primecell"; > See coresight-cti.yaml for full CTI definitions. > > + - Trace, Profiling and Diagnostics Monitor (TPDM): > + "qcom,coresight-tpdm", "arm,primecell"; > + See coresight-tpdm.yaml for full TPDM definitions. > + > * reg: physical base address and length of the register > set(s) of the component. > > @@ -82,6 +86,9 @@ its hardware characteristcs. > * Required properties for Coresight Cross Trigger Interface (CTI) > See coresight-cti.yaml for full CTI definitions. > > +* Required properties for Trace, Profiling and Diagnostics Monitor (TPDM) > + See coresight-tpdm.yaml for full TPDM definitions. > + > * Required properties for devices that don't show up on the AMBA bus, such as > non-configurable replicators and non-configurable funnels: > > diff --git a/MAINTAINERS b/MAINTAINERS > index 61d9f114c37f..0d39bb37935d 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1977,6 +1977,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git > F: Documentation/ABI/testing/sysfs-bus-coresight-devices-* > F: Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt > F: Documentation/devicetree/bindings/arm/coresight-cti.yaml > +F: Documentation/devicetree/bindings/arm/coresight-tpdm.yaml > F: Documentation/devicetree/bindings/arm/coresight.txt > F: Documentation/devicetree/bindings/arm/ete.yaml > F: Documentation/devicetree/bindings/arm/trbe.yaml > -- > 2.17.1 > With the above: Reviewed by: Mike Leach <mike.leach@linaro.org>
Hi Mike, On 4/14/2022 11:22 PM, Mike Leach wrote: > Hi, > > On Tue, 12 Apr 2022 at 13:51, Mao Jinlong <quic_jinlmao@quicinc.com> wrote: >> Adds new coresight-tpdm.yaml file describing the bindings required >> to define tpdm in the device trees. >> >> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com> >> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com> >> --- >> .../bindings/arm/coresight-tpdm.yaml | 99 +++++++++++++++++++ >> .../devicetree/bindings/arm/coresight.txt | 7 ++ >> MAINTAINERS | 1 + >> 3 files changed, 107 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/arm/coresight-tpdm.yaml >> >> diff --git a/Documentation/devicetree/bindings/arm/coresight-tpdm.yaml b/Documentation/devicetree/bindings/arm/coresight-tpdm.yaml >> new file mode 100644 >> index 000000000000..05210e0fc262 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/arm/coresight-tpdm.yaml >> @@ -0,0 +1,99 @@ >> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause >> +# Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/arm/coresight-tpdm.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Trace, Profiling and Diagnostics Monitor - TPDM >> + >> +description: | >> + The TPDM or Monitor serves as data collection component for various dataset >> + types specified in the QPMDA spec. It covers Implementation defined ((ImplDef), >> + Basic Counts (BC), Tenure Counts (TC), Continuous Multi-Bit (CMB), and Discrete >> + Single Bit (DSB). It performs data collection in the data producing clock >> + domain and transfers it to the data collection time domain, generally ATB >> + clock domain. >> + >> + The primary use case of the TPDM is to collect data from different data >> + sources and send it to a TPDA for packetization, timestamping, and funneling. >> + >> +maintainers: >> + - Suzuki K Poulose <suzuki.poulose@arm.com> >> + - Mathieu Poirier <mathieu.poirier@linaro.org> >> + > These should be e-mail addresses of maintainers for this binding, not > the coresight sub-system. > See writing-schema.rst I will update. > >> +properties: >> + $nodename: >> + pattern: "^tpdm(@[0-9a-f]+)$" >> + compatible: >> + items: >> + - const: qcom,coresight-tpdm >> + - const: arm,primecell >> + >> + reg: >> + maxItems: 1 >> + >> + clocks: >> + maxItems: 1 >> + >> + clock-names: >> + items: >> + - const: apb_pclk >> + >> + out-ports: >> + description: | >> + Output connections from the TPDM to coresight funnle/tpda. >> + $ref: /schemas/graph.yaml#/properties/ports >> + properties: >> + port: >> + description: Output connection from the TPDM to coresight >> + funnel/tpda. >> + $ref: /schemas/graph.yaml#/properties/port >> + >> +required: >> + - compatible >> + - reg >> + - clocks >> + - clock-names >> + >> +additionalProperties: false >> + >> +examples: >> + # minimum TPDM definition. TPDM connect to coresight funnel. >> + - | >> + tpdm@6980000 { >> + compatible = "qcom,coresight-tpdm", "arm,primecell"; >> + reg = <0x6980000 0x1000>; >> + >> + clocks = <&aoss_qmp>; >> + clock-names = "apb_pclk"; >> + >> + out-ports { >> + port { >> + tpdm_turing_out_funnel_turing: endpoint { >> + remote-endpoint = >> + <&funnel_turing_in_tpdm_turing>; >> + }; >> + }; >> + }; >> + }; >> + # minimum TPDM definition. TPDM connect to coresight TPDA. >> + - | >> + tpdm@684c000 { >> + compatible = "qcom,coresight-tpdm", "arm,primecell"; >> + reg = <0x684c000 0x1000>; >> + >> + clocks = <&aoss_qmp>; >> + clock-names = "apb_pclk"; >> + >> + out-ports { >> + port { >> + tpdm_prng_out_tpda_qdss: endpoint { >> + remote-endpoint = >> + <&tpda_qdss_in_tpdm_prng>; >> + }; >> + }; >> + }; >> + }; >> + >> +... >> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt >> index c68d93a35b6c..f7ce8af48574 100644 >> --- a/Documentation/devicetree/bindings/arm/coresight.txt >> +++ b/Documentation/devicetree/bindings/arm/coresight.txt >> @@ -52,6 +52,10 @@ its hardware characteristcs. >> "arm,coresight-cti", "arm,primecell"; >> See coresight-cti.yaml for full CTI definitions. >> >> + - Trace, Profiling and Diagnostics Monitor (TPDM): >> + "qcom,coresight-tpdm", "arm,primecell"; >> + See coresight-tpdm.yaml for full TPDM definitions. >> + >> * reg: physical base address and length of the register >> set(s) of the component. >> >> @@ -82,6 +86,9 @@ its hardware characteristcs. >> * Required properties for Coresight Cross Trigger Interface (CTI) >> See coresight-cti.yaml for full CTI definitions. >> >> +* Required properties for Trace, Profiling and Diagnostics Monitor (TPDM) >> + See coresight-tpdm.yaml for full TPDM definitions. >> + >> * Required properties for devices that don't show up on the AMBA bus, such as >> non-configurable replicators and non-configurable funnels: >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 61d9f114c37f..0d39bb37935d 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -1977,6 +1977,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git >> F: Documentation/ABI/testing/sysfs-bus-coresight-devices-* >> F: Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt >> F: Documentation/devicetree/bindings/arm/coresight-cti.yaml >> +F: Documentation/devicetree/bindings/arm/coresight-tpdm.yaml >> F: Documentation/devicetree/bindings/arm/coresight.txt >> F: Documentation/devicetree/bindings/arm/ete.yaml >> F: Documentation/devicetree/bindings/arm/trbe.yaml >> -- >> 2.17.1 >> > With the above: > Reviewed by: Mike Leach <mike.leach@linaro.org>
Hi, On Tue, 12 Apr 2022 at 13:51, Mao Jinlong <quic_jinlmao@quicinc.com> wrote: > > TPDM serves as data collection component for various dataset types. > DSB(Discrete Single Bit) is one of the dataset types. DSB subunit > can be enabled for data collection by writing 1 to the first bit of > DSB_CR register. This change is to add enable/disable function for > DSB dataset by writing DSB_CR register. > > Signed-off-by: Tao Zhang <quic_taozha@quicinc.com> > Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com> > --- > drivers/hwtracing/coresight/coresight-tpdm.c | 61 ++++++++++++++++++++ > drivers/hwtracing/coresight/coresight-tpdm.h | 21 +++++++ > 2 files changed, 82 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c > index 3900ae50670a..d7b970cdcf51 100644 > --- a/drivers/hwtracing/coresight/coresight-tpdm.c > +++ b/drivers/hwtracing/coresight/coresight-tpdm.c > @@ -20,7 +20,28 @@ > > DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm"); > > +static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata) > +{ > + u32 val; > + > + /* Set the enable bit of DSB control register to 1 */ > + val = readl_relaxed(drvdata->base + TPDM_DSB_CR); > + val = val | BIT(0); Please use #defined values, declared in the header file rather than BIT(x) here. (the etm4x drivers have been recently updated to use the same pattern). e.g. val |= TPDM_DSB_CR_ENA; > + writel_relaxed(val, drvdata->base + TPDM_DSB_CR); > +} > + > /* TPDM enable operations */ > +static void _tpdm_enable(struct tpdm_drvdata *drvdata) > +{ > + CS_UNLOCK(drvdata->base); > + > + /* Check if DSB datasets is present for TPDM. */ > + if (test_bit(TPDM_DS_DSB, drvdata->datasets)) > + tpdm_enable_dsb(drvdata); > + > + CS_LOCK(drvdata->base); > +} > + > static int tpdm_enable(struct coresight_device *csdev, > struct perf_event *event, u32 mode) > { > @@ -32,6 +53,7 @@ static int tpdm_enable(struct coresight_device *csdev, > return -EBUSY; > } > > + _tpdm_enable(drvdata); > drvdata->enable = true; > mutex_unlock(&drvdata->lock); > > @@ -39,7 +61,29 @@ static int tpdm_enable(struct coresight_device *csdev, > return 0; > } > > +static void tpdm_disable_dsb(struct tpdm_drvdata *drvdata) > +{ > + u32 val; > + > + /* Set the enable bit of DSB control register to 0 */ > + val = readl_relaxed(drvdata->base + TPDM_DSB_CR); > + val = val & ~BIT(0); val &= ~TPDM_DSB_CR_ENA; > + writel_relaxed(val, drvdata->base + TPDM_DSB_CR); > +} > + > /* TPDM disable operations */ > +static void _tpdm_disable(struct tpdm_drvdata *drvdata) > +{ > + CS_UNLOCK(drvdata->base); > + > + /* Check if DSB datasets is present for TPDM. */ > + if (test_bit(TPDM_DS_DSB, drvdata->datasets)) > + tpdm_disable_dsb(drvdata); > + > + CS_LOCK(drvdata->base); > + > +} > + > static void tpdm_disable(struct coresight_device *csdev, > struct perf_event *event) > { > @@ -51,6 +95,7 @@ static void tpdm_disable(struct coresight_device *csdev, > return; > } > > + _tpdm_disable(drvdata); > drvdata->enable = false; > mutex_unlock(&drvdata->lock); > > @@ -66,6 +111,21 @@ static const struct coresight_ops tpdm_cs_ops = { > .source_ops = &tpdm_source_ops, > }; > > +static void tpdm_init_default_data(struct tpdm_drvdata *drvdata) > +{ > + int i; > + u32 pidr; > + > + CS_UNLOCK(drvdata->base); > + /* Get the datasets present on the TPDM. */ > + pidr = readl_relaxed(drvdata->base + CORESIGHT_PERIPHIDR0); > + for (i = 0; i < TPDM_DATASETS; i++) { > + if (pidr & BIT(i)) > + __set_bit(i, drvdata->datasets); Could this be considerably simpified? - there are a maximum of 7 bits for datasets - and you are setting the same bit in drvdata->datasets for each bit in pidr e.g. if the datasets declaration is a simple unsigned long, then the following is easier to read and understand: drvdata->datasets |= pidr & GENMASK(TPDM_DATASETS - 1, 0); The tests above could then become: if (drvdata->datasets & BIT(TPDM_DS_DSB)) ... > + } > + CS_LOCK(drvdata->base); > +} > + > static int tpdm_probe(struct amba_device *adev, const struct amba_id *id) > { > struct device *dev = &adev->dev; > @@ -104,6 +164,7 @@ static int tpdm_probe(struct amba_device *adev, const struct amba_id *id) > if (IS_ERR(drvdata->csdev)) > return PTR_ERR(drvdata->csdev); > > + tpdm_init_default_data(drvdata); > /* Decrease pm refcount when probe is done.*/ > pm_runtime_put(&adev->dev); > > diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h > index 94a7748a5426..8f05070879c4 100644 > --- a/drivers/hwtracing/coresight/coresight-tpdm.h > +++ b/drivers/hwtracing/coresight/coresight-tpdm.h > @@ -6,6 +6,25 @@ > #ifndef _CORESIGHT_CORESIGHT_TPDM_H > #define _CORESIGHT_CORESIGHT_TPDM_H > > +/* The max number of the datasets that TPDM supports */ > +#define TPDM_DATASETS 7 > + > +/* DSB Subunit Registers */ > +#define TPDM_DSB_CR (0x780) > + declare the enable bit here with comment documenting usage - see above. > +/** > + * This enum is for PERIPHIDR0 register of TPDM. > + * The fields [6:0] of PERIPHIDR0 are used to determine what > + * interfaces and subunits are present on a given TPDM. > + * > + * PERIPHIDR0[0] : Fix to 1 if ImplDef subunit present, else 0 > + * PERIPHIDR0[1] : Fix to 1 if DSB subunit present, else 0 > + */ > +enum tpdm_dataset { > + TPDM_DS_IMPLDEF, > + TPDM_DS_DSB, > +}; > + > /** > * struct tpdm_drvdata - specifics associated to an TPDM component > * @base: memory mapped base address for this component. > @@ -13,6 +32,7 @@ > * @csdev: component vitals needed by the framework. > * @lock: lock for the enable value. > * @enable: enable status of the component. > + * @datasets: The datasets types present of the TPDM. > */ > > struct tpdm_drvdata { > @@ -21,6 +41,7 @@ struct tpdm_drvdata { > struct coresight_device *csdev; > struct mutex lock; > bool enable; > + DECLARE_BITMAP(datasets, TPDM_DATASETS); Could this simply be declared as an unsigned long? - then simplify the code in the .c file. Regards Mike > }; > > #endif /* _CORESIGHT_CORESIGHT_TPDM_H */ > -- > 2.17.1 >
Hi On Tue, 12 Apr 2022 at 13:51, Mao Jinlong <quic_jinlmao@quicinc.com> wrote: > > Add API usage document for sysfs API in TPDM driver. > > Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com> > --- > .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm > > diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm > new file mode 100644 > index 000000000000..d70ba429f38d > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm > @@ -0,0 +1,13 @@ > +What: /sys/bus/coresight/devices/<tpdm-name>/integration_test > +Date: April 2022 > +KernelVersion 5.18 > +Contact: Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com> > +Description: > + (Write) Run integration test for tpdm. Integration test > + will generate test data for tpdm. It can help to make > + sure that the trace path is enabled and the link configurations > + are fine. > + > + value to this sysfs node: > + 1 : Genreate 64 bits data s/Genreate/Generate > + 2 : Generate 32 bits data > -- > 2.17.1 > Reviewed by: Mike Leach <mike.leach@linaro.org>
Hi On Tue, 12 Apr 2022 at 13:51, Mao Jinlong <quic_jinlmao@quicinc.com> wrote: > > TPDA(Trace, Profiling and Diagnostics Aggregator) is > to provide packetization, funneling and timestamping of > TPDM data. Multiple monitors are connected to different > input ports of TPDA.This change is to add tpda > enable/disable/probe functions for coresight tpda driver. > > - - - - - - - - - - - - > | TPDM 0| | TPDM 1 | | TPDM 2| > - - - - - - - - - - - - > | | | > |_ _ _ _ _ _ | _ _ _ _ | > | | | > | | | > ------------------ > | TPDA | > ------------------ > > Signed-off-by: Tao Zhang <quic_taozha@quicinc.com> > Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com> > --- > drivers/hwtracing/coresight/Kconfig | 11 ++ > drivers/hwtracing/coresight/Makefile | 1 + > drivers/hwtracing/coresight/coresight-tpda.c | 192 +++++++++++++++++++ > drivers/hwtracing/coresight/coresight-tpda.h | 32 ++++ > 4 files changed, 236 insertions(+) > create mode 100644 drivers/hwtracing/coresight/coresight-tpda.c > create mode 100644 drivers/hwtracing/coresight/coresight-tpda.h > > diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig > index 60248fef4089..317c5e7f4819 100644 > --- a/drivers/hwtracing/coresight/Kconfig > +++ b/drivers/hwtracing/coresight/Kconfig > @@ -223,4 +223,15 @@ config CORESIGHT_TPDM_INTEGRATION_TEST > operation to facilitate integration testing and software bringup > and/or to instrument topology discovery. The TPDM utilizes integration > mode to accomplish integration testing and software bringup. > + > +config CORESIGHT_TPDA > + tristate "CoreSight Trace, Profiling & Diagnostics Aggregator driver" > + help > + This driver provides support for configuring aggregator. This is > + primarily useful for pulling the data sets from one or more > + attached monitors and pushing the resultant data out. Multiple > + monitors are connected on different input ports of TPDA. > + > + To compile this driver as a module, choose M here: the module will be > + called coresight-tpda. > endif TDPA / TDPM are functionally linked - it does not make sense to configure one without the other. Kernel configuration should reflect this - as a minimum TDPM configuration should select TDPA. > diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile > index 6bb9b1746bc7..1712d82e7260 100644 > --- a/drivers/hwtracing/coresight/Makefile > +++ b/drivers/hwtracing/coresight/Makefile > @@ -26,5 +26,6 @@ obj-$(CONFIG_CORESIGHT_CATU) += coresight-catu.o > obj-$(CONFIG_CORESIGHT_CTI) += coresight-cti.o > obj-$(CONFIG_CORESIGHT_TRBE) += coresight-trbe.o > obj-$(CONFIG_CORESIGHT_TPDM) += coresight-tpdm.o > +obj-$(CONFIG_CORESIGHT_TPDA) += coresight-tpda.o > coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \ > coresight-cti-sysfs.o > diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c > new file mode 100644 > index 000000000000..9519990c68e2 > --- /dev/null > +++ b/drivers/hwtracing/coresight/coresight-tpda.c > @@ -0,0 +1,192 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include <linux/amba/bus.h> > +#include <linux/bitmap.h> > +#include <linux/coresight.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/fs.h> > +#include <linux/io.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > + > +#include "coresight-priv.h" > +#include "coresight-tpda.h" > +#include "coresight-trace-id.h" > + > +DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda"); > + > +/* Settings pre enabling port control register */ > +static void tpda_enable_pre_port(struct tpda_drvdata *drvdata) > +{ > + u32 val; > + > + val = readl_relaxed(drvdata->base + TPDA_CR); > + val |= (drvdata->atid << 6); > + writel_relaxed(val, drvdata->base + TPDA_CR); > +} > + > +static void tpda_enable_port(struct tpda_drvdata *drvdata, int port) > +{ > + u32 val; > + > + val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port)); > + /* Enable the port */ > + val = val | BIT(0); #define a constant in the header to use here. > + writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port)); > +} > + > +static void _tpda_enable(struct tpda_drvdata *drvdata, int port) > +{ > + CS_UNLOCK(drvdata->base); > + > + if (!drvdata->enable) > + tpda_enable_pre_port(drvdata); > + > + tpda_enable_port(drvdata, port); > + > + CS_LOCK(drvdata->base); > +} > + > +static int tpda_enable(struct coresight_device *csdev, int inport, int outport) > +{ > + struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + > + mutex_lock(&drvdata->lock); > + _tpda_enable(drvdata, inport); > + drvdata->enable = true; > + mutex_unlock(&drvdata->lock); > + > + dev_info(drvdata->dev, "TPDA inport %d enabled\n", inport); > + return 0; > +} > + > +static void _tpda_disable(struct tpda_drvdata *drvdata, int port) > +{ > + u32 val; > + > + CS_UNLOCK(drvdata->base); > + > + val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port)); > + val = val & ~BIT(0); use a #defined constant. > + writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port)); > + > + CS_LOCK(drvdata->base); > +} > + > +static void tpda_disable(struct coresight_device *csdev, int inport, > + int outport) > +{ > + struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + > + mutex_lock(&drvdata->lock); > + _tpda_disable(drvdata, inport); > + drvdata->enable = false; > + mutex_unlock(&drvdata->lock); > + > + dev_info(drvdata->dev, "TPDA inport %d disabled\n", inport); > +} > + > +static const struct coresight_ops_link tpda_link_ops = { > + .enable = tpda_enable, > + .disable = tpda_disable, > +}; > + > +static const struct coresight_ops tpda_cs_ops = { > + .link_ops = &tpda_link_ops, > +}; > + > +static int tpda_init_default_data(struct tpda_drvdata *drvdata) > +{ > + int atid; > + /* > + * TPDA must has a unique atid. This atid can uniquely > + * identify the TPDM trace source connect to the TPDA. > + */ > + atid = coresight_trace_id_get_system_id(coresight_get_trace_id_map()); > + if (atid < 0) > + return atid; > + > + drvdata->atid = atid; > + return 0; > +} > + > +static int tpda_probe(struct amba_device *adev, const struct amba_id *id) > +{ > + int ret; > + struct device *dev = &adev->dev; > + struct coresight_platform_data *pdata; > + struct tpda_drvdata *drvdata; > + struct coresight_desc desc = { 0 }; > + > + pdata = coresight_get_platform_data(dev); > + if (IS_ERR(pdata)) > + return PTR_ERR(pdata); > + adev->dev.platform_data = pdata; > + > + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > + if (!drvdata) > + return -ENOMEM; > + > + drvdata->dev = &adev->dev; > + dev_set_drvdata(dev, drvdata); > + > + drvdata->base = devm_ioremap_resource(dev, &adev->res); > + if (!drvdata->base) > + return -ENOMEM; > + > + mutex_init(&drvdata->lock); > + > + ret = tpda_init_default_data(drvdata); > + if (ret) > + return ret; > + > + desc.name = coresight_alloc_device_name(&tpda_devs, dev); > + if (!desc.name) > + return -ENOMEM; > + desc.type = CORESIGHT_DEV_TYPE_LINK; > + desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_MERG; > + desc.ops = &tpda_cs_ops; > + desc.pdata = adev->dev.platform_data; > + desc.dev = &adev->dev; > + drvdata->csdev = coresight_register(&desc); > + if (IS_ERR(drvdata->csdev)) > + return PTR_ERR(drvdata->csdev); > + > + pm_runtime_put(&adev->dev); > + > + dev_dbg(drvdata->dev, "TPDA initialized\n"); > + return 0; > +} > + > +/* > + * Different TPDA has different periph id. > + * The difference is 0-7 bits' value. So ignore 0-7 bits. > + */ > +static struct amba_id tpda_ids[] = { > + { > + .id = 0x000f0f00, > + .mask = 0x000fff00, > + }, > + { 0, 0}, > +}; > + > +static struct amba_driver tpda_driver = { > + .drv = { > + .name = "coresight-tpda", > + .owner = THIS_MODULE, > + .suppress_bind_attrs = true, > + }, > + .probe = tpda_probe, > + .id_table = tpda_ids, > +}; > + There is no code to release resources when this module is unloaded. You need a .remove function, that as a minimum calls coresight_unregister(). Regards Mike > +module_amba_driver(tpda_driver); > + > +MODULE_LICENSE("GPL v2"); > +MODULE_DESCRIPTION("Trace, Profiling & Diagnostic Aggregator driver"); > diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/hwtracing/coresight/coresight-tpda.h > new file mode 100644 > index 000000000000..6ac33b9c1ea4 > --- /dev/null > +++ b/drivers/hwtracing/coresight/coresight-tpda.h > @@ -0,0 +1,32 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#ifndef _CORESIGHT_CORESIGHT_TPDA_H > +#define _CORESIGHT_CORESIGHT_TPDA_H > + > +#define TPDA_CR (0x000) > +#define TPDA_Pn_CR(n) (0x004 + (n * 4)) > + > +#define TPDA_MAX_INPORTS 32 > + > +/** > + * struct tpda_drvdata - specifics associated to an TPDA component > + * @base: memory mapped base address for this component. > + * @dev: The device entity associated to this component. > + * @csdev: component vitals needed by the framework. > + * @lock: lock for the enable value. > + * @enable: enable status of the component. > + * @traceid: trace source identification for the data packet by TPDA. > + */ > +struct tpda_drvdata { > + void __iomem *base; > + struct device *dev; > + struct coresight_device *csdev; > + struct mutex lock; > + bool enable; > + u32 atid; > +}; > + > +#endif /* _CORESIGHT_CORESIGHT_TPDA_H */ > -- > 2.17.1 > -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
Thank you Mike for the review. On 4/19/2022 5:23 PM, Mike Leach wrote: > Hi, > > On Tue, 12 Apr 2022 at 13:51, Mao Jinlong <quic_jinlmao@quicinc.com> wrote: >> TPDM serves as data collection component for various dataset types. >> DSB(Discrete Single Bit) is one of the dataset types. DSB subunit >> can be enabled for data collection by writing 1 to the first bit of >> DSB_CR register. This change is to add enable/disable function for >> DSB dataset by writing DSB_CR register. >> >> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com> >> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com> >> --- >> drivers/hwtracing/coresight/coresight-tpdm.c | 61 ++++++++++++++++++++ >> drivers/hwtracing/coresight/coresight-tpdm.h | 21 +++++++ >> 2 files changed, 82 insertions(+) >> >> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c >> index 3900ae50670a..d7b970cdcf51 100644 >> --- a/drivers/hwtracing/coresight/coresight-tpdm.c >> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c >> @@ -20,7 +20,28 @@ >> >> DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm"); >> >> +static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata) >> +{ >> + u32 val; >> + >> + /* Set the enable bit of DSB control register to 1 */ >> + val = readl_relaxed(drvdata->base + TPDM_DSB_CR); >> + val = val | BIT(0); > Please use #defined values, declared in the header file rather than > BIT(x) here. (the etm4x drivers have been recently updated to use the > same pattern). > e.g. > val |= TPDM_DSB_CR_ENA; I will update in next version. >> + writel_relaxed(val, drvdata->base + TPDM_DSB_CR); >> +} >> + >> /* TPDM enable operations */ >> +static void _tpdm_enable(struct tpdm_drvdata *drvdata) >> +{ >> + CS_UNLOCK(drvdata->base); >> + >> + /* Check if DSB datasets is present for TPDM. */ >> + if (test_bit(TPDM_DS_DSB, drvdata->datasets)) >> + tpdm_enable_dsb(drvdata); >> + >> + CS_LOCK(drvdata->base); >> +} >> + >> static int tpdm_enable(struct coresight_device *csdev, >> struct perf_event *event, u32 mode) >> { >> @@ -32,6 +53,7 @@ static int tpdm_enable(struct coresight_device *csdev, >> return -EBUSY; >> } >> >> + _tpdm_enable(drvdata); >> drvdata->enable = true; >> mutex_unlock(&drvdata->lock); >> >> @@ -39,7 +61,29 @@ static int tpdm_enable(struct coresight_device *csdev, >> return 0; >> } >> >> +static void tpdm_disable_dsb(struct tpdm_drvdata *drvdata) >> +{ >> + u32 val; >> + >> + /* Set the enable bit of DSB control register to 0 */ >> + val = readl_relaxed(drvdata->base + TPDM_DSB_CR); >> + val = val & ~BIT(0); > val &= ~TPDM_DSB_CR_ENA; I will update in next version. > >> + writel_relaxed(val, drvdata->base + TPDM_DSB_CR); >> +} >> + >> /* TPDM disable operations */ >> +static void _tpdm_disable(struct tpdm_drvdata *drvdata) >> +{ >> + CS_UNLOCK(drvdata->base); >> + >> + /* Check if DSB datasets is present for TPDM. */ >> + if (test_bit(TPDM_DS_DSB, drvdata->datasets)) >> + tpdm_disable_dsb(drvdata); >> + >> + CS_LOCK(drvdata->base); >> + >> +} >> + >> static void tpdm_disable(struct coresight_device *csdev, >> struct perf_event *event) >> { >> @@ -51,6 +95,7 @@ static void tpdm_disable(struct coresight_device *csdev, >> return; >> } >> >> + _tpdm_disable(drvdata); >> drvdata->enable = false; >> mutex_unlock(&drvdata->lock); >> >> @@ -66,6 +111,21 @@ static const struct coresight_ops tpdm_cs_ops = { >> .source_ops = &tpdm_source_ops, >> }; >> >> +static void tpdm_init_default_data(struct tpdm_drvdata *drvdata) >> +{ >> + int i; >> + u32 pidr; >> + >> + CS_UNLOCK(drvdata->base); >> + /* Get the datasets present on the TPDM. */ >> + pidr = readl_relaxed(drvdata->base + CORESIGHT_PERIPHIDR0); >> + for (i = 0; i < TPDM_DATASETS; i++) { >> + if (pidr & BIT(i)) >> + __set_bit(i, drvdata->datasets); > Could this be considerably simpified? - there are a maximum of 7 bits > for datasets - and you are setting the same bit in drvdata->datasets > for each bit in pidr > > e.g. if the datasets declaration is a simple unsigned long, then the > following is easier to read and understand: > > drvdata->datasets |= pidr & GENMASK(TPDM_DATASETS - 1, 0); > > The tests above could then become: > if (drvdata->datasets & BIT(TPDM_DS_DSB)) ... > I will check your suggestion and update in next version. >> + } >> + CS_LOCK(drvdata->base); >> +} >> + >> static int tpdm_probe(struct amba_device *adev, const struct amba_id *id) >> { >> struct device *dev = &adev->dev; >> @@ -104,6 +164,7 @@ static int tpdm_probe(struct amba_device *adev, const struct amba_id *id) >> if (IS_ERR(drvdata->csdev)) >> return PTR_ERR(drvdata->csdev); >> >> + tpdm_init_default_data(drvdata); >> /* Decrease pm refcount when probe is done.*/ >> pm_runtime_put(&adev->dev); >> >> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h >> index 94a7748a5426..8f05070879c4 100644 >> --- a/drivers/hwtracing/coresight/coresight-tpdm.h >> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h >> @@ -6,6 +6,25 @@ >> #ifndef _CORESIGHT_CORESIGHT_TPDM_H >> #define _CORESIGHT_CORESIGHT_TPDM_H >> >> +/* The max number of the datasets that TPDM supports */ >> +#define TPDM_DATASETS 7 >> + >> +/* DSB Subunit Registers */ >> +#define TPDM_DSB_CR (0x780) >> + > declare the enable bit here with comment documenting usage - see above. > >> +/** >> + * This enum is for PERIPHIDR0 register of TPDM. >> + * The fields [6:0] of PERIPHIDR0 are used to determine what >> + * interfaces and subunits are present on a given TPDM. >> + * >> + * PERIPHIDR0[0] : Fix to 1 if ImplDef subunit present, else 0 >> + * PERIPHIDR0[1] : Fix to 1 if DSB subunit present, else 0 >> + */ >> +enum tpdm_dataset { >> + TPDM_DS_IMPLDEF, >> + TPDM_DS_DSB, >> +}; >> + >> /** >> * struct tpdm_drvdata - specifics associated to an TPDM component >> * @base: memory mapped base address for this component. >> @@ -13,6 +32,7 @@ >> * @csdev: component vitals needed by the framework. >> * @lock: lock for the enable value. >> * @enable: enable status of the component. >> + * @datasets: The datasets types present of the TPDM. >> */ >> >> struct tpdm_drvdata { >> @@ -21,6 +41,7 @@ struct tpdm_drvdata { >> struct coresight_device *csdev; >> struct mutex lock; >> bool enable; >> + DECLARE_BITMAP(datasets, TPDM_DATASETS); > Could this simply be declared as an unsigned long? - then simplify the > code in the .c file. > > Regards > > Mike > >> }; >> >> #endif /* _CORESIGHT_CORESIGHT_TPDM_H */ >> -- >> 2.17.1 >> >
On 4/19/2022 5:25 PM, Mike Leach wrote: > Hi > > On Tue, 12 Apr 2022 at 13:51, Mao Jinlong <quic_jinlmao@quicinc.com> wrote: >> Add API usage document for sysfs API in TPDM driver. >> >> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com> >> --- >> .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm >> >> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm >> new file mode 100644 >> index 000000000000..d70ba429f38d >> --- /dev/null >> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm >> @@ -0,0 +1,13 @@ >> +What: /sys/bus/coresight/devices/<tpdm-name>/integration_test >> +Date: April 2022 >> +KernelVersion 5.18 >> +Contact: Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com> >> +Description: >> + (Write) Run integration test for tpdm. Integration test >> + will generate test data for tpdm. It can help to make >> + sure that the trace path is enabled and the link configurations >> + are fine. >> + >> + value to this sysfs node: >> + 1 : Genreate 64 bits data > s/Genreate/Generate I will update this. > >> + 2 : Generate 32 bits data >> -- >> 2.17.1 >> > Reviewed by: Mike Leach <mike.leach@linaro.org> > >
Hi Mike, On 4/19/2022 5:51 PM, Mike Leach wrote: > Hi > > On Tue, 12 Apr 2022 at 13:51, Mao Jinlong <quic_jinlmao@quicinc.com> wrote: >> TPDA(Trace, Profiling and Diagnostics Aggregator) is >> to provide packetization, funneling and timestamping of >> TPDM data. Multiple monitors are connected to different >> input ports of TPDA.This change is to add tpda >> enable/disable/probe functions for coresight tpda driver. >> >> - - - - - - - - - - - - >> | TPDM 0| | TPDM 1 | | TPDM 2| >> - - - - - - - - - - - - >> | | | >> |_ _ _ _ _ _ | _ _ _ _ | >> | | | >> | | | >> ------------------ >> | TPDA | >> ------------------ >> >> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com> >> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com> >> --- >> drivers/hwtracing/coresight/Kconfig | 11 ++ >> drivers/hwtracing/coresight/Makefile | 1 + >> drivers/hwtracing/coresight/coresight-tpda.c | 192 +++++++++++++++++++ >> drivers/hwtracing/coresight/coresight-tpda.h | 32 ++++ >> 4 files changed, 236 insertions(+) >> create mode 100644 drivers/hwtracing/coresight/coresight-tpda.c >> create mode 100644 drivers/hwtracing/coresight/coresight-tpda.h >> >> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig >> index 60248fef4089..317c5e7f4819 100644 >> --- a/drivers/hwtracing/coresight/Kconfig >> +++ b/drivers/hwtracing/coresight/Kconfig >> @@ -223,4 +223,15 @@ config CORESIGHT_TPDM_INTEGRATION_TEST >> operation to facilitate integration testing and software bringup >> and/or to instrument topology discovery. The TPDM utilizes integration >> mode to accomplish integration testing and software bringup. >> + >> +config CORESIGHT_TPDA >> + tristate "CoreSight Trace, Profiling & Diagnostics Aggregator driver" >> + help >> + This driver provides support for configuring aggregator. This is >> + primarily useful for pulling the data sets from one or more >> + attached monitors and pushing the resultant data out. Multiple >> + monitors are connected on different input ports of TPDA. >> + >> + To compile this driver as a module, choose M here: the module will be >> + called coresight-tpda. >> endif > TDPA / TDPM are functionally linked - it does not make sense to > configure one without the other. > Kernel configuration should reflect this - as a minimum TDPM > configuration should select TDPA. > > > >> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile >> index 6bb9b1746bc7..1712d82e7260 100644 >> --- a/drivers/hwtracing/coresight/Makefile >> +++ b/drivers/hwtracing/coresight/Makefile >> @@ -26,5 +26,6 @@ obj-$(CONFIG_CORESIGHT_CATU) += coresight-catu.o >> obj-$(CONFIG_CORESIGHT_CTI) += coresight-cti.o >> obj-$(CONFIG_CORESIGHT_TRBE) += coresight-trbe.o >> obj-$(CONFIG_CORESIGHT_TPDM) += coresight-tpdm.o >> +obj-$(CONFIG_CORESIGHT_TPDA) += coresight-tpda.o >> coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \ >> coresight-cti-sysfs.o >> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c >> new file mode 100644 >> index 000000000000..9519990c68e2 >> --- /dev/null >> +++ b/drivers/hwtracing/coresight/coresight-tpda.c >> @@ -0,0 +1,192 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. >> + */ >> + >> +#include <linux/amba/bus.h> >> +#include <linux/bitmap.h> >> +#include <linux/coresight.h> >> +#include <linux/device.h> >> +#include <linux/err.h> >> +#include <linux/fs.h> >> +#include <linux/io.h> >> +#include <linux/kernel.h> >> +#include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/platform_device.h> >> + >> +#include "coresight-priv.h" >> +#include "coresight-tpda.h" >> +#include "coresight-trace-id.h" >> + >> +DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda"); >> + >> +/* Settings pre enabling port control register */ >> +static void tpda_enable_pre_port(struct tpda_drvdata *drvdata) >> +{ >> + u32 val; >> + >> + val = readl_relaxed(drvdata->base + TPDA_CR); >> + val |= (drvdata->atid << 6); >> + writel_relaxed(val, drvdata->base + TPDA_CR); >> +} >> + >> +static void tpda_enable_port(struct tpda_drvdata *drvdata, int port) >> +{ >> + u32 val; >> + >> + val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port)); >> + /* Enable the port */ >> + val = val | BIT(0); > #define a constant in the header to use here. I will update it. > >> + writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port)); >> +} >> + >> +static void _tpda_enable(struct tpda_drvdata *drvdata, int port) >> +{ >> + CS_UNLOCK(drvdata->base); >> + >> + if (!drvdata->enable) >> + tpda_enable_pre_port(drvdata); >> + >> + tpda_enable_port(drvdata, port); >> + >> + CS_LOCK(drvdata->base); >> +} >> + >> +static int tpda_enable(struct coresight_device *csdev, int inport, int outport) >> +{ >> + struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); >> + >> + mutex_lock(&drvdata->lock); >> + _tpda_enable(drvdata, inport); >> + drvdata->enable = true; >> + mutex_unlock(&drvdata->lock); >> + >> + dev_info(drvdata->dev, "TPDA inport %d enabled\n", inport); >> + return 0; >> +} >> + >> +static void _tpda_disable(struct tpda_drvdata *drvdata, int port) >> +{ >> + u32 val; >> + >> + CS_UNLOCK(drvdata->base); >> + >> + val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port)); >> + val = val & ~BIT(0); > use a #defined constant. > >> + writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port)); >> + >> + CS_LOCK(drvdata->base); >> +} >> + >> +static void tpda_disable(struct coresight_device *csdev, int inport, >> + int outport) >> +{ >> + struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); >> + >> + mutex_lock(&drvdata->lock); >> + _tpda_disable(drvdata, inport); >> + drvdata->enable = false; >> + mutex_unlock(&drvdata->lock); >> + >> + dev_info(drvdata->dev, "TPDA inport %d disabled\n", inport); >> +} >> + >> +static const struct coresight_ops_link tpda_link_ops = { >> + .enable = tpda_enable, >> + .disable = tpda_disable, >> +}; >> + >> +static const struct coresight_ops tpda_cs_ops = { >> + .link_ops = &tpda_link_ops, >> +}; >> + >> +static int tpda_init_default_data(struct tpda_drvdata *drvdata) >> +{ >> + int atid; >> + /* >> + * TPDA must has a unique atid. This atid can uniquely >> + * identify the TPDM trace source connect to the TPDA. >> + */ >> + atid = coresight_trace_id_get_system_id(coresight_get_trace_id_map()); >> + if (atid < 0) >> + return atid; >> + >> + drvdata->atid = atid; >> + return 0; >> +} >> + >> +static int tpda_probe(struct amba_device *adev, const struct amba_id *id) >> +{ >> + int ret; >> + struct device *dev = &adev->dev; >> + struct coresight_platform_data *pdata; >> + struct tpda_drvdata *drvdata; >> + struct coresight_desc desc = { 0 }; >> + >> + pdata = coresight_get_platform_data(dev); >> + if (IS_ERR(pdata)) >> + return PTR_ERR(pdata); >> + adev->dev.platform_data = pdata; >> + >> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); >> + if (!drvdata) >> + return -ENOMEM; >> + >> + drvdata->dev = &adev->dev; >> + dev_set_drvdata(dev, drvdata); >> + >> + drvdata->base = devm_ioremap_resource(dev, &adev->res); >> + if (!drvdata->base) >> + return -ENOMEM; >> + >> + mutex_init(&drvdata->lock); >> + >> + ret = tpda_init_default_data(drvdata); >> + if (ret) >> + return ret; >> + >> + desc.name = coresight_alloc_device_name(&tpda_devs, dev); >> + if (!desc.name) >> + return -ENOMEM; >> + desc.type = CORESIGHT_DEV_TYPE_LINK; >> + desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_MERG; >> + desc.ops = &tpda_cs_ops; >> + desc.pdata = adev->dev.platform_data; >> + desc.dev = &adev->dev; >> + drvdata->csdev = coresight_register(&desc); >> + if (IS_ERR(drvdata->csdev)) >> + return PTR_ERR(drvdata->csdev); >> + >> + pm_runtime_put(&adev->dev); >> + >> + dev_dbg(drvdata->dev, "TPDA initialized\n"); >> + return 0; >> +} >> + >> +/* >> + * Different TPDA has different periph id. >> + * The difference is 0-7 bits' value. So ignore 0-7 bits. >> + */ >> +static struct amba_id tpda_ids[] = { >> + { >> + .id = 0x000f0f00, >> + .mask = 0x000fff00, >> + }, >> + { 0, 0}, >> +}; >> + >> +static struct amba_driver tpda_driver = { >> + .drv = { >> + .name = "coresight-tpda", >> + .owner = THIS_MODULE, >> + .suppress_bind_attrs = true, >> + }, >> + .probe = tpda_probe, >> + .id_table = tpda_ids, >> +}; >> + > There is no code to release resources when this module is unloaded. > You need a .remove function, that as a minimum calls > coresight_unregister(). > > Regards > > Mike Yes. Need add remove function for tpda driver. > >> +module_amba_driver(tpda_driver); >> + >> +MODULE_LICENSE("GPL v2"); >> +MODULE_DESCRIPTION("Trace, Profiling & Diagnostic Aggregator driver"); >> diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/hwtracing/coresight/coresight-tpda.h >> new file mode 100644 >> index 000000000000..6ac33b9c1ea4 >> --- /dev/null >> +++ b/drivers/hwtracing/coresight/coresight-tpda.h >> @@ -0,0 +1,32 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* >> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. >> + */ >> + >> +#ifndef _CORESIGHT_CORESIGHT_TPDA_H >> +#define _CORESIGHT_CORESIGHT_TPDA_H >> + >> +#define TPDA_CR (0x000) >> +#define TPDA_Pn_CR(n) (0x004 + (n * 4)) >> + >> +#define TPDA_MAX_INPORTS 32 >> + >> +/** >> + * struct tpda_drvdata - specifics associated to an TPDA component >> + * @base: memory mapped base address for this component. >> + * @dev: The device entity associated to this component. >> + * @csdev: component vitals needed by the framework. >> + * @lock: lock for the enable value. >> + * @enable: enable status of the component. >> + * @traceid: trace source identification for the data packet by TPDA. >> + */ >> +struct tpda_drvdata { >> + void __iomem *base; >> + struct device *dev; >> + struct coresight_device *csdev; >> + struct mutex lock; >> + bool enable; >> + u32 atid; >> +}; >> + >> +#endif /* _CORESIGHT_CORESIGHT_TPDA_H */ >> -- >> 2.17.1 >> > > -- > Mike Leach > Principal Engineer, ARM Ltd. > Manchester Design Centre. UK