On 18/07/2023 15:31, Rob Herring wrote:
> The DT of_device.h and of_platform.h date back to the separate
> of_platform_bus_type before it as merged into the regular platform bus.
> As part of that merge prepping Arm DT support 13 years ago, they
> "temporarily" include each other. They also include platform_device.h
> and of.h. As a result, there's a pretty much random mix of those include
> files used throughout the tree. In order to detangle these headers and
> replace the implicit includes with struct declarations, users need to
> explicitly include the correct includes.
>
> Signed-off-by: Rob Herring <robh(a)kernel.org>
> ---
> v2:
> - Fix double include of of.h
> ---
> drivers/hwtracing/coresight/coresight-core.c | 2 +-
> drivers/hwtracing/coresight/coresight-platform.c | 2 --
> 2 files changed, 1 insertion(+), 3 deletions(-)
Rob,
Thanks for the updated version, I will queue this.
Suzuki
Hi Rob
Thanks for the cleanup. Please find my comments below.
On 14/07/2023 18:46, Rob Herring wrote:
> The DT of_device.h and of_platform.h date back to the separate
> of_platform_bus_type before it as merged into the regular platform bus.
> As part of that merge prepping Arm DT support 13 years ago, they
> "temporarily" include each other. They also include platform_device.h
> and of.h. As a result, there's a pretty much random mix of those include
> files used throughout the tree. In order to detangle these headers and
> replace the implicit includes with struct declarations, users need to
> explicitly include the correct includes.
>
> Signed-off-by: Rob Herring <robh(a)kernel.org>
> ---
> drivers/hwtracing/coresight/coresight-core.c | 2 +-
> drivers/hwtracing/coresight/coresight-platform.c | 3 +--
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 118fcf27854d..9fabe00a40d6 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -17,7 +17,7 @@
> #include <linux/mutex.h>
> #include <linux/clk.h>
> #include <linux/coresight.h>
> -#include <linux/of_platform.h>
> +#include <linux/property.h>
> #include <linux/delay.h>
> #include <linux/pm_runtime.h>
This looks correct to me.
>
> diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c
> index 3e2e135cb8f6..d94b0e77bcfc 100644
> --- a/drivers/hwtracing/coresight/coresight-platform.c
> +++ b/drivers/hwtracing/coresight/coresight-platform.c
> @@ -9,9 +9,8 @@
> #include <linux/slab.h>
> #include <linux/clk.h>
> #include <linux/of.h>
> -#include <linux/of_address.h>
> +#include <linux/of.h>
We already include of.h two lines above, so we could skip this.
> #include <linux/of_graph.h>
> -#include <linux/of_platform.h>
> #include <linux/platform_device.h>
> #include <linux/amba/bus.h>
> #include <linux/coresight.h>
Suzuki
On 6/20/2023 3:37 PM, Greg Kroah-Hartman wrote:
> On Tue, Jun 20, 2023 at 03:32:37PM +0800, Tao Zhang wrote:
>> Add the nodes to set value for DSB edge control and DSB edge
>> control mask. Each DSB subunit TPDM has maximum of n(n<16) EDCR
>> resgisters to configure edge control. DSB edge detection control
>> 00: Rising edge detection
>> 01: Falling edge detection
>> 10: Rising and falling edge detection (toggle detection)
>> And each DSB subunit TPDM has maximum of m(m<8) ECDMR registers to
>> configure mask. Eight 32 bit registers providing DSB interface
>> edge detection mask control.
>>
>> Signed-off-by: Tao Zhang <quic_taozha(a)quicinc.com>
>> ---
>> .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 32 +++++
>> drivers/hwtracing/coresight/coresight-tpdm.c | 143 ++++++++++++++++++++-
>> drivers/hwtracing/coresight/coresight-tpdm.h | 22 ++++
>> 3 files changed, 196 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> index 2a82cd0..34189e4a 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> @@ -60,3 +60,35 @@ Description:
>> Bit[3] : Set to 0 for low performance mode.
>> Set to 1 for high performance mode.
>> Bit[4:8] : Select byte lane for high performance mode.
>> +
>> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_edge_ctrl
>> +Date: March 2023
>> +KernelVersion 6.5
>> +Contact: Jinlong Mao (QUIC) <quic_jinlmao(a)quicinc.com>, Tao Zhang (QUIC) <quic_taozha(a)quicinc.com>
>> +Description:
>> + Read/Write a set of the edge control registers of the DSB
>> + in TPDM.
>> +
>> + Expected format is the following:
>> + <integer1> <integer2> <integer3>
> sysfs is "one value", not 3. Please never have to parse a sysfs file.
Do you mean sysfs file can only accept "one value"?
I see that more than one value are written to the sysfs file
"trigout_attach".
>
>> +static ssize_t dsb_edge_ctrl_show(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> + ssize_t size = 0;
>> + unsigned long bytes;
>> + int i;
>> +
>> + spin_lock(&drvdata->spinlock);
>> + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) {
>> + bytes = sysfs_emit_at(buf, size,
>> + "Index:0x%x Val:0x%x\n", i,
> Again, no, one value, no "string" needed to parse anything.
I also see other sysfs files can be read more than one value in other
drivers.
Is this "one value" limitation the usage rule of Linux sysfs system?
Or am I misunderstanding what you mean?
Best,
Tao
>
> greg k-h
On 11/07/2023 15:05, Greg Kroah-Hartman wrote:
> On Tue, Jul 11, 2023 at 03:05:36PM +0800, quanyang.wang(a)windriver.com wrote:
>> From: Quanyang Wang <quanyang.wang(a)windriver.com>
>>
>> For PREEMPT_RT kernel, spinlock_t locks become sleepable. The functions
>> etm_dying_cpu and etm_starting_cpu which call spin_lock/unlock run in
>> an irq-disabled context, this will trigger the following calltrace:
>>
>> BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:46
>> in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 25, name: migration/1
>> preempt_count: 1, expected: 0
>> RCU nest depth: 0, expected: 0
>> 1 lock held by migration/1/25:
>> #0: 82a7587c (&drvdata->spinlock){....}-{2:2}, at: etm_dying_cpu+0x28/0x54
>> Preemption disabled at:
>> [<801ec760>] cpu_stopper_thread+0x94/0x120
>> CPU: 1 PID: 25 Comm: migration/1 Not tainted 6.1.35-rt10-yocto-preempt-rt #30
>> Hardware name: Xilinx Zynq Platform
>> Stopper: multi_cpu_stop+0x0/0x174 <- __stop_cpus.constprop.0+0x48/0x88
>> unwind_backtrace from show_stack+0x18/0x1c
>> show_stack from dump_stack_lvl+0x58/0x70
>> dump_stack_lvl from __might_resched+0x14c/0x1c0
>> __might_resched from rt_spin_lock+0x4c/0x84
>> rt_spin_lock from etm_dying_cpu+0x28/0x54
>> etm_dying_cpu from cpuhp_invoke_callback+0x140/0x33c
>> cpuhp_invoke_callback from __cpuhp_invoke_callback_range+0xa4/0x104
>> __cpuhp_invoke_callback_range from take_cpu_down+0x7c/0xa8
>> take_cpu_down from multi_cpu_stop+0x15c/0x174
>> multi_cpu_stop from cpu_stopper_thread+0x9c/0x120
>> cpu_stopper_thread from smpboot_thread_fn+0x31c/0x360
>> smpboot_thread_fn from kthread+0x100/0x124
>> kthread from ret_from_fork+0x14/0x2c
>>
>> Convert struct etm_drvdata's spinlock to raw_spinlock to fix it.
>
> wait, why will a raw_spinlock fix this? Why not fix the root problem
> here, that of calling these locks inproperly in irq context?
>
> How is changing to a raw_spinlock going to fix the above splat?
>
> thanks,
>
> greg k-h
>
If it's just etm_starting_cpu() and etm_dying_cpu() as mentioned in the
commit message then can those spinlocks be removed?
Surely there can't be any concurrent access to the per-cpu data when the
hotplug callbacks are called?
James
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel(a)lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Em Tue, Jul 11, 2023 at 08:28:31AM +0000, Pandey, Radhey Shyam escreveu:
> > -----Original Message-----
> > From: James Clark <james.clark(a)arm.com>
> > Sent: Friday, July 7, 2023 9:16 PM
> > To: linux-perf-users(a)vger.kernel.org; Pandey, Radhey Shyam
> > <radhey.shyam.pandey(a)amd.com>
> > Cc: coresight(a)lists.linaro.org; James Clark <james.clark(a)arm.com>; Peter
> > Zijlstra <peterz(a)infradead.org>; Ingo Molnar <mingo(a)redhat.com>;
> > Arnaldo Carvalho de Melo <acme(a)kernel.org>; Mark Rutland
> > <mark.rutland(a)arm.com>; Alexander Shishkin
> > <alexander.shishkin(a)linux.intel.com>; Jiri Olsa <jolsa(a)kernel.org>;
> > Namhyung Kim <namhyung(a)kernel.org>; Ian Rogers <irogers(a)google.com>;
> > Adrian Hunter <adrian.hunter(a)intel.com>; Uwe Kleine-König <uwe@kleine-
> > koenig.org>; linux-kernel(a)vger.kernel.org
> > Subject: [PATCH] perf build: Fix library not found error when using CSLIBS
> >
> > -L only specifies the search path for libraries directly provided in the link line
> > with -l. Because -lopencsd isn't specified, it's only linked because it's a
> > dependency of -lopencsd_c_api. Dependencies like this are resolved using
> > the default system search paths or -rpath-link=... rather than -L. This means
> > that compilation only works if OpenCSD is installed to the system rather than
> > provided with the CSLIBS (-L) option.
> >
> > This could be fixed by adding -Wl,-rpath-link=$(CSLIBS) but that is less
> > conventional than just adding -lopencsd to the link line so that it uses -L. -
> > lopencsd seems to have been removed in commit ed17b1914978 ("perf
> > tools: Drop requirement for libstdc++.so for libopencsd check") because it
> > was thought that there was a chance compilation would work even if it didn't
> > exist, but I think that only applies to libstdc++ so there is no harm to add it
> > back. libopencsd.so and libopencsd_c_api.so would always exist together.
> >
> > Testing
> > =======
> >
> > The following scenarios now all work:
> >
> > * Cross build with OpenCSD installed
> > * Cross build using CSLIBS=...
> > * Native build with OpenCSD installed
> > * Native build using CSLIBS=...
> > * Static cross build with OpenCSD installed
> > * Static cross build with CSLIBS=...
> >
> > Reported-by: Radhey Shyam Pandey <radhey.shyam.pandey(a)amd.com>
> > Closes: https://lore.kernel.org/linux-arm-kernel/56905d7a-a91e-883a-b707-
> > 9d5f686ba5f1(a)arm.com/
> > Link: https://lore.kernel.org/all/36cc4dc6-bf4b-1093-1c0a-
> > 876e368af183(a)kleine-koenig.org/
> > Fixes: ed17b1914978 ("perf tools: Drop requirement for libstdc++.so for
> > libopencsd check")
> > Signed-off-by: James Clark <james.clark(a)arm.com>
> > ---
> > tools/perf/Makefile.config | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config index
> > 0609c19caabd..c5db0de49868 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -155,9 +155,9 @@ FEATURE_CHECK_LDFLAGS-libcrypto = -lcrypto ifdef
> > CSINCLUDES
> > LIBOPENCSD_CFLAGS := -I$(CSINCLUDES)
> > endif
> > -OPENCSDLIBS := -lopencsd_c_api
> > +OPENCSDLIBS := -lopencsd_c_api -lopencsd
> > ifeq ($(findstring -static,${LDFLAGS}),-static)
> > - OPENCSDLIBS += -lopencsd -lstdc++
> > + OPENCSDLIBS += -lstdc++
> > endif
> > ifdef CSLIBS
> > LIBOPENCSD_LDFLAGS := -L$(CSLIBS)
> > --
> > 2.34.1
>
> Tested-by: Radhey Shyam Pandey <radhey.shyam.pandey(a)amd.com>
> Cross compiled for aarch64 on x86_64.
>
> make ARCH=arm64 NO_LIBELF=1 NO_JVMTI=1 VF=1 CORESIGHT=1 -C tools/perf
>
> file <snip>/linux-xlnx/tools/perf/perf
> perf: ELF 64-bit LSB shared object, ARM aarch64, version 1 (SYSV),
> dynamically linked, interpreter /lib/ld-linux-aarch64.so.1,
> BuildID[sha1]=ef7c524338577b14e7c0f914d882dec4d26e93a2,
> for GNU/Linux 3.14.0, with debug_info, not stripped
Thanks for reporting and testing, applied to perf-tools. I see no
problems in my common case which is:
⬢[acme@toolbox perf-tools]$ alias m='make -k BUILD_BPF_SKEL=1 CORESIGHT=1 O=/tmp/build/perf-tools -C tools/perf install-bin && git status && perf test python ; perf record -o /dev/null sleep 0.01 ; perf stat --null sleep 0.01'
⬢[acme@toolbox perf-tools]$
⬢[acme@toolbox perf-tools]$ ldd ~/bin/perf | grep csd
libopencsd_c_api.so.1 => /lib64/libopencsd_c_api.so.1 (0x00007f9c012ea000)
libopencsd.so.1 => /lib64/libopencsd.so.1 (0x00007f9c00556000)
⬢[acme@toolbox perf-tools]$
- Arnaldo
This patch series is to add support for a streaming interface for
TMC ETR to allow for continuous log collection to secondary storage.
An interrupt based mechanism is used to stream out the data from the device.
QDSS_CS_QDSSCSR_ETRIRQCTRL register is used to set the IRQ byte counter
value. The value of this registers defines the number of bytes that when moved by
the ETR AXI interface. It will casues an interrupt which can be used by an
userspace program to know how much data is present in memory requiring copy to some
other location. A zero setting disables the interrupt.A one setting
means 8 bytes, two 16 bytes, etc. In other words, the value in this
register is the interrupt threshold times 8 bytes. ETR must be enabled
when use this interrupt function.
Sample:
echo 4096 > /sys/bus/coresight/devices/tmc_etr0/block_size
echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
echo 1 > /sys/bus/coresight/devices/stm0/enabl_source
cat /dev/byte-cntr > /data/qdss_etr.bin &
The log collection will stop after disabling the ETR.
Commit link:
https://git.codelinaro.org/clo/linux-kernel/coresight/-/commits/coresight-b…
Mao Jinlong (3):
Coresight: Add driver to support for CSR
coresight-tmc: byte-cntr: Add support for streaming interface for ETR
dt-bindings: arm: Adds CoreSight CSR hardware definitions
.../testing/sysfs-bus-coresight-devices-tmc | 7 +
.../bindings/arm/qcom,coresight-csr.yaml | 62 ++++
drivers/hwtracing/coresight/Kconfig | 12 +
drivers/hwtracing/coresight/Makefile | 3 +-
.../hwtracing/coresight/coresight-byte-cntr.c | 304 ++++++++++++++++++
.../hwtracing/coresight/coresight-byte-cntr.h | 49 +++
drivers/hwtracing/coresight/coresight-csr.c | 168 ++++++++++
drivers/hwtracing/coresight/coresight-csr.h | 59 ++++
.../hwtracing/coresight/coresight-tmc-core.c | 66 ++++
.../hwtracing/coresight/coresight-tmc-etr.c | 8 +-
drivers/hwtracing/coresight/coresight-tmc.h | 12 +-
11 files changed, 745 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-csr.yaml
create mode 100644 drivers/hwtracing/coresight/coresight-byte-cntr.c
create mode 100644 drivers/hwtracing/coresight/coresight-byte-cntr.h
create mode 100644 drivers/hwtracing/coresight/coresight-csr.c
create mode 100644 drivers/hwtracing/coresight/coresight-csr.h
--
2.17.1
linux/bitfield.h can be included as long as linux/kernel.h is included
first, so change the order of the includes and drop the duplicate macro.
Signed-off-by: James Clark <james.clark(a)arm.com>
---
tools/perf/util/cs-etm.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 1419b40dfbe8..9729d006550d 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -6,10 +6,11 @@
* Author: Mathieu Poirier <mathieu.poirier(a)linaro.org>
*/
+#include <linux/kernel.h>
+#include <linux/bitfield.h>
#include <linux/bitops.h>
#include <linux/coresight-pmu.h>
#include <linux/err.h>
-#include <linux/kernel.h>
#include <linux/log2.h>
#include <linux/types.h>
#include <linux/zalloc.h>
@@ -281,17 +282,6 @@ static int cs_etm__metadata_set_trace_id(u8 trace_chan_id, u64 *cpu_metadata)
return 0;
}
-/*
- * FIELD_GET (linux/bitfield.h) not available outside kernel code,
- * and the header contains too many dependencies to just copy over,
- * so roll our own based on the original
- */
-#define __bf_shf(x) (__builtin_ffsll(x) - 1)
-#define FIELD_GET(_mask, _reg) \
- ({ \
- (typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask)); \
- })
-
/*
* Get a metadata for a specific cpu from an array.
*
--
2.34.1
On 10/07/2023 06:23, Randy Dunlap wrote:
> Correct spelling problems as identified by codespell.
> Correct one grammar error.
>
> Fixes: 7a25ec8e481e ("coresight: Adding ABI documentation")
> Fixes: 7253e4c95616 ("coresight: etm3x: breaking down sysFS status interface")
> Signed-off-by: Randy Dunlap <rdunlap(a)infradead.org>
> Cc: Suzuki K Poulose <suzuki.poulose(a)arm.com>
> Cc: Mike Leach <mike.leach(a)linaro.org>
> Cc: James Clark <james.clark(a)arm.com>
> Cc: Leo Yan <leo.yan(a)linaro.org>
> Cc: coresight(a)lists.linaro.org
> Cc: linux-arm-kernel(a)lists.infradead.org
> Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org>
> Cc: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
> ---
> Documentation/ABI/testing/sysfs-bus-coresight-devices-etm3x | 12 +++++-----
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
Thanks for the fixes.
Reviewed-by: James Clark <james.clark(a)arm.com>
> diff -- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm3x b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm3x
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm3x
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm3x
> @@ -20,9 +20,9 @@ Date: November 2014
> KernelVersion: 3.19
> Contact: Mathieu Poirier <mathieu.poirier(a)linaro.org>
> Description: (RW) Used in conjunction with @addr_idx. Specifies
> - characteristics about the address comparator being configure,
> + characteristics about the address comparator being configured,
> for example the access type, the kind of instruction to trace,
> - processor contect ID to trigger on, etc. Individual fields in
> + processor context ID to trigger on, etc. Individual fields in
> the access type register may vary on the version of the trace
> entity.
>
> @@ -31,7 +31,7 @@ Date: November 2014
> KernelVersion: 3.19
> Contact: Mathieu Poirier <mathieu.poirier(a)linaro.org>
> Description: (RW) Used in conjunction with @addr_idx. Specifies the range of
> - addresses to trigger on. Inclusion or exclusion is specificed
> + addresses to trigger on. Inclusion or exclusion is specified
> in the corresponding access type register.
>
> What: /sys/bus/coresight/devices/<memory_map>.[etm|ptm]/addr_single
> @@ -304,19 +304,19 @@ What: /sys/bus/coresight/devices/<memor
> Date: September 2015
> KernelVersion: 4.4
> Contact: Mathieu Poirier <mathieu.poirier(a)linaro.org>
> -Description: (RO) Print the content of the ETM Trace Start/Stop Conrol
> +Description: (RO) Print the content of the ETM Trace Start/Stop Control
> register (0x018). The value is read directly from the HW.
>
> What: /sys/bus/coresight/devices/<memory_map>.[etm|ptm]/mgmt/etmtecr1
> Date: September 2015
> KernelVersion: 4.4
> Contact: Mathieu Poirier <mathieu.poirier(a)linaro.org>
> -Description: (RO) Print the content of the ETM Enable Conrol #1
> +Description: (RO) Print the content of the ETM Enable Control #1
> register (0x024). The value is read directly from the HW.
>
> What: /sys/bus/coresight/devices/<memory_map>.[etm|ptm]/mgmt/etmtecr2
> Date: September 2015
> KernelVersion: 4.4
> Contact: Mathieu Poirier <mathieu.poirier(a)linaro.org>
> -Description: (RO) Print the content of the ETM Enable Conrol #2
> +Description: (RO) Print the content of the ETM Enable Control #2
> register (0x01c). The value is read directly from the HW.