On 05/07/2023 10:56, Pandey, Radhey Shyam wrote:
>> -----Original Message-----
>> From: Suzuki K Poulose <suzuki.poulose(a)arm.com>
>> Sent: Wednesday, July 5, 2023 1:57 PM
>> To: Pandey, Radhey Shyam <radhey.shyam.pandey(a)amd.com>;
>> mathieu.poirier(a)linaro.org; mike.leach(a)linaro.org; leo.yan(a)linaro.org;
>> alexander.shishkin(a)linux.intel.com
>> Cc: coresight(a)lists.linaro.org; linux-arm-kernel(a)lists.infradead.org
>> Subject: Re: Linux coresight arm64 : Incorrect data in cstrace.bin
>>
>> Hi Radhe Shyam
>>
>> On 05/07/2023 05:38, Pandey, Radhey Shyam wrote:
>>> Hi,
>>>
>>> I am using linux 6.1 kernel coresight framework to capture ETM trace.
>>> Enabled coresight driver and added coresight component device node in
>>> DTS. With that, we could probe all coresight components.
>>>
>>> But when we do capture using sysfs and then read the trace.bin using
>>> ptm2human we see invalid trace data. Trace content changes on each
>>> capture. Any pointers to help narrow down the issue?
>>>
>>> Tried: sysfs capture and perf (with OpenCSD enabled)
>>>
>>> Development board: VCK190 :
>>> https://docs.xilinx.com/r/en-US/ug1366-vck190-eval-bd
>>> https://www.xilinx.com/support/documents/architecture-
>> manuals/am011-ve
>>> rsal-acap-trm.pdf
>>>
>>> xilinx-vck190-20231:/home/petalinux# dmesg | grep -i amba
>>> [ 0.301054] Serial: AMBA PL011 UART driver
>>> [ 0.306226] amba f0c20000.funnel: Fixing up cyclic dependency with
>> f0d70000.etm
>>> [ 0.313624] amba f0c20000.funnel: Fixing up cyclic dependency with
>> f0d30000.etm
>>> [ 0.321110] amba f0c30000.etf: Fixing up cyclic dependency with
>> f0c20000.funnel
>>> [ 1.470247] Serial: AMBA driver
>>> xilinx-vck190-20231:/home/petalinux# dmesg | grep -i coresight
>>> [ 1.865006] cs_system_cfg: CoreSight Configuration manager initialised
>>> [ 1.876858] coresight etm0: CPU0: etm v4.0 initialized
>>> [ 1.885666] coresight etm1: CPU1: etm v4.0 initialized
>>> [ 1.894435] coresight-cpu-debug f0d00000.debug1: Coresight debug-CPU0
>> initialized
>>> [ 1.902072] coresight-cpu-debug f0d40000.debug1: Coresight debug-CPU1
>> initialized
>>>
>>> xilinx-vck190-20231:/home/petalinux# ls /sys/bus/coresight/devices/
>>> etm0 etm1 funnel0 tmc_etf0
>>>
>>> cd /sys/bus/coresight/devices/
>>> echo 1 > tmc_etf0/enable_sink
>>> echo 1 > etm0/enable_source
>>> echo 0 > etm0/enable_source
>>> echo 0 > tmc_etf0/enable_sink
>>> cd /root/ dd if=/dev/tmc_etf0 of=cstrace_28Jun.bin
>>>
>>> ./ptm2human/ptm2human -e -i cstrace.bin -d
>>
>> Please note that ptm2human is for PTM trace decoding and
>> ETMv4 uses a different format and thus is not compatible.
>>
>>>
>>> I also tried OpenCSD integration with PERF.
>>
>> Have you made sure the perf is "linked" to the opencsd ?
>
> Earlier I statically linked but after adding CORESIGHT=1.
>
>
> linux-xlnx$ make ARCH=arm64 NO_LIBELF=1 NO_JVMTI=1 VF=1 CORESIGHT=1 -C tools/perf/
> make: Entering directory 'linux-xlnx/tools/perf'
> BUILD: Doing 'make -j24' parallel build
>
> Makefile.config:520: *** Error: No libopencsd library found or the version is not up-to-date.
> Please install recent libopencsd to build with CORESIGHT=1. Stop.
>
>
> echo $CSINCLUDES
> /scratch/development/coresight/my-opencsd/decoder/include
> radheys@xhdradheys41:/scratch/development/linux-xlnx$ ls /scratch/development/coresight/my-opencsd/decoder/include
> common i_dec interfaces mem_acc opencsd opencsd.h pkt_printers
>
> echo $CSLIBS
> /scratch/development/coresight/my-opencsd/decoder/lib/builddir
> radheys@xhdradheys41:/scratch/development/linux-xlnx$ ls /scratch/development/coresight/my-opencsd/decoder/lib/builddir
> libopencsd.a libopencsd_c_api.so libopencsd_c_api.so.1.4.0 libopencsd.so.1
> libopencsd_c_api.a libopencsd_c_api.so.1 libopencsd.so libopencsd.so.1.4.0
>
> Anything I am missing to fix this opencsd lib not found?
There is a minimum version of 1.1.1 but it looks like you are using
higher than that so that part should be fine.
Personally I "make install" OpenCSD to the system path. I had a play
around with $CSLIBS it and maybe there is also some stickyness to the
feature detection so a make clean might help to make sure the errors you
are seeing are real.
Lastly you can print the output of why the feature test compilation
failed which might help. I see some linking issues in there if
I delete my system installed version of OpenCSD, so there might be a bug
with just using CSINCLUDES and CSLIBS:
$ cat tools/build/feature/test-libopencsd.make.output
/usr/bin/ld: warning: libopencsd.so.1, needed by
opencsd-local/decoder/lib/builddir//libopencsd_c_api.so, not found (try
using -rpath or -rpath-link)
/usr/bin/ld: opencsd-local/decoder/lib/builddir//libopencsd_c_api.so:
undefined reference to `vtable for StmTrcPacket'
/usr/bin/ld: opencsd-local/decoder/lib/builddir//libopencsd_c_api.so:
undefined reference to `DecodeTree::getDecoderStats(unsigned char,
_ocsd_decode_stats**)'
/usr/bin/ld: opencsd-local/decoder/lib/builddir//libopencsd_c_api.so:
undefined reference to
`DecodeTree::addRawFramePrinter(RawFramePrinter**, unsigned int)'
Can you try the make install and see if that works? If that works but
the other way doesn't I can try looking into why its not and make a fix.
Thanks
James
On 05/07/2023 10:56, Pandey, Radhey Shyam wrote:
>> -----Original Message-----
>> From: Suzuki K Poulose <suzuki.poulose(a)arm.com>
>> Sent: Wednesday, July 5, 2023 1:57 PM
>> To: Pandey, Radhey Shyam <radhey.shyam.pandey(a)amd.com>;
>> mathieu.poirier(a)linaro.org; mike.leach(a)linaro.org; leo.yan(a)linaro.org;
>> alexander.shishkin(a)linux.intel.com
>> Cc: coresight(a)lists.linaro.org; linux-arm-kernel(a)lists.infradead.org
>> Subject: Re: Linux coresight arm64 : Incorrect data in cstrace.bin
>>
>> Hi Radhe Shyam
>>
>> On 05/07/2023 05:38, Pandey, Radhey Shyam wrote:
>>> Hi,
>>>
>>> I am using linux 6.1 kernel coresight framework to capture ETM trace.
>>> Enabled coresight driver and added coresight component device node in
>>> DTS. With that, we could probe all coresight components.
>>>
>>> But when we do capture using sysfs and then read the trace.bin using
>>> ptm2human we see invalid trace data. Trace content changes on each
>>> capture. Any pointers to help narrow down the issue?
>>>
>>> Tried: sysfs capture and perf (with OpenCSD enabled)
>>>
>>> Development board: VCK190 :
>>> https://docs.xilinx.com/r/en-US/ug1366-vck190-eval-bd
>>> https://www.xilinx.com/support/documents/architecture-
>> manuals/am011-ve
>>> rsal-acap-trm.pdf
>>>
>>> xilinx-vck190-20231:/home/petalinux# dmesg | grep -i amba
>>> [ 0.301054] Serial: AMBA PL011 UART driver
>>> [ 0.306226] amba f0c20000.funnel: Fixing up cyclic dependency with
>> f0d70000.etm
>>> [ 0.313624] amba f0c20000.funnel: Fixing up cyclic dependency with
>> f0d30000.etm
>>> [ 0.321110] amba f0c30000.etf: Fixing up cyclic dependency with
>> f0c20000.funnel
>>> [ 1.470247] Serial: AMBA driver
>>> xilinx-vck190-20231:/home/petalinux# dmesg | grep -i coresight
>>> [ 1.865006] cs_system_cfg: CoreSight Configuration manager initialised
>>> [ 1.876858] coresight etm0: CPU0: etm v4.0 initialized
>>> [ 1.885666] coresight etm1: CPU1: etm v4.0 initialized
>>> [ 1.894435] coresight-cpu-debug f0d00000.debug1: Coresight debug-CPU0
>> initialized
>>> [ 1.902072] coresight-cpu-debug f0d40000.debug1: Coresight debug-CPU1
>> initialized
>>>
>>> xilinx-vck190-20231:/home/petalinux# ls /sys/bus/coresight/devices/
>>> etm0 etm1 funnel0 tmc_etf0
>>>
>>> cd /sys/bus/coresight/devices/
>>> echo 1 > tmc_etf0/enable_sink
>>> echo 1 > etm0/enable_source
>>> echo 0 > etm0/enable_source
>>> echo 0 > tmc_etf0/enable_sink
>>> cd /root/ dd if=/dev/tmc_etf0 of=cstrace_28Jun.bin
>>>
>>> ./ptm2human/ptm2human -e -i cstrace.bin -d
>>
>> Please note that ptm2human is for PTM trace decoding and
>> ETMv4 uses a different format and thus is not compatible.
>>
>>>
>>> I also tried OpenCSD integration with PERF.
>>
>> Have you made sure the perf is "linked" to the opencsd ?
>
> Earlier I statically linked but after adding CORESIGHT=1.
>
>
> linux-xlnx$ make ARCH=arm64 NO_LIBELF=1 NO_JVMTI=1 VF=1 CORESIGHT=1 -C tools/perf/
> make: Entering directory 'linux-xlnx/tools/perf'
> BUILD: Doing 'make -j24' parallel build
>
> Makefile.config:520: *** Error: No libopencsd library found or the version is not up-to-date.
> Please install recent libopencsd to build with CORESIGHT=1. Stop.
>
>
> echo $CSINCLUDES
> /scratch/development/coresight/my-opencsd/decoder/include
> radheys@xhdradheys41:/scratch/development/linux-xlnx$ ls /scratch/development/coresight/my-opencsd/decoder/include
> common i_dec interfaces mem_acc opencsd opencsd.h pkt_printers
>
> echo $CSLIBS
> /scratch/development/coresight/my-opencsd/decoder/lib/builddir
> radheys@xhdradheys41:/scratch/development/linux-xlnx$ ls /scratch/development/coresight/my-opencsd/decoder/lib/builddir
> libopencsd.a libopencsd_c_api.so libopencsd_c_api.so.1.4.0 libopencsd.so.1
> libopencsd_c_api.a libopencsd_c_api.so.1 libopencsd.so libopencsd.so.1.4.0
>
> Anything I am missing to fix this opencsd lib not found?
Have you checked the libopencsd version ? It looks it must be able to
pick up the path for lib/headers.
Suzuki
On 30/06/2023 22:02, Namhyung Kim wrote:
> On Wed, Jun 28, 2023 at 1:06 PM Namhyung Kim <namhyung(a)kernel.org> wrote:
>>
>> On Wed, Jun 28, 2023 at 3:34 AM James Clark <james.clark(a)arm.com> wrote:
>>>
>>>
>>>
>>> On 27/06/2023 18:19, Ian Rogers wrote:
>>>> On Tue, Jun 27, 2023 at 9:58 AM Namhyung Kim <namhyung(a)kernel.org> wrote:
>>>>>
>>>>> On Tue, Jun 27, 2023 at 9:43 AM Ian Rogers <irogers(a)google.com> wrote:
>>>>>>
>>>>>> On Mon, Jun 26, 2023 at 5:02 PM Namhyung Kim <namhyung(a)kernel.org> wrote:
>>>>>>>
>>>>>>> On Mon, Jun 26, 2023 at 05:10:58PM +0100, James Clark wrote:
>>>>>>>> thread__find_map() chooses to exit without assigning a thread to the
>>>>>>>> addr_location in some scenarios, for example when there are samples from
>>>>>>>> a guest and perf_guest == false. This results in a segfault when adding
>>>>>>>> to the histogram because it uses unguarded accesses to the thread member
>>>>>>>> of the addr_location.
>>>>>>>
>>>>>>> Looking at the commit 0dd5041c9a0ea ("perf addr_location: Add
>>>>>>> init/exit/copy functions") that introduced the change, I'm not sure if
>>>>>>> it's the intend behavior.
>>>>>>>
>>>>>>> It might change maps and map, but not thread. Then I think no reason
>>>>>>> to not set the al->thread at the beginning.
>>>>>>>
>>>>>>> How about this? Ian?
>>>>>>> (I guess we can get rid of the duplicate 'al->map = NULL' part)
>>>>>>
>>>>>> It seemed strange that we were failing to find a map (the function's
>>>>>> purpose) but then populating the address_location. The change below
>>>>>> brings back that somewhat odd behavior. I'm okay with reverting to the
>>>>>> old behavior, clearly there were users relying on it. We should
>>>>>> probably also copy maps and not just thread, as that was the previous
>>>>>> behavior.
>>>>>
>>>>> Probably. But it used to support samples without maps and I think
>>>>> that's why it ignores the return value of thread__find_map(). So
>>>>> we can expect al.map is NULL and maybe fine to leave it for now.
>>>>>
>>>>> As machine__resolve() returns -1 if it gets no thread, we should set
>>>>> al.thread when it returns 0.
>>>>>
>>>>> Can I get your Acked-by?
>>>>
>>>> Yep:
>>>> Acked-by: Ian Rogers <irogers(a)google.com>
>>>
>>> Looks good to me too. Should I resend the set with this change instead
>>> of my one?
>>
>> No, I can take care of that. I'll take this as your Acked-by. :)
>
> This part is applied to perf-tools-next, thanks!
Thanks Namhyung
On 23/06/2023 19:22, Tanmay Jagdale wrote:
> The existing method of synthesizing instruction samples has the
> following issues:
> 1. Non-branch instructions have mnemonics of branch instructions.
> 2. Branch target address is missing.
>
> Set the sample flags only when we reach the last instruction in
> the tidq (which would be a branch instruction) to solve issue 1).
>
> To fix issue 2), start synthesizing the instructions from the
> previous packet (tidq->prev_packet) instead of current packet
> (tidq->packet). This way, it is easy to figure out the target
> address of the branch instruction in tidq->prev_packet which
> is the current packet's (tidq->packet) first executed instruction.
>
> After the switch to processing the previous packet first, we no
> longer need to swap the packets during cs_etm__flush().
Hi Tanmay,
I think the fix for setting the right flags and instruction type makes
sense, but is it possible to do it without the change to swapping in
cs_etm__flush() or some of the other changes to
cs_etm__synth_instruction_sample()?
I'm seeing some differences in the output related to the PID that's
assigned to a sample and some of the addresses that aren't explained by
the commit message. Also there is no corresponding change to
cs_etm__synth_branch_sample(), which is also using prev_packet etc so
I'm wondering if that's correct now without the swap? That function gets
used with the default itrace options or itrace=b
For example if I run 'perf script --itrace=i100ns' and diff the output
before and after your change I see a difference even though branch and
instruction info isn't printed, so I wouldn't expect to see any changes.
This is on a systemwide recording of a system under load.
Thanks
James
>
> Signed-off-by: Tanmay Jagdale <tanmay(a)marvell.com>
> ---
> tools/perf/util/cs-etm.c | 32 +++++++++++++++++++++++++-------
> 1 file changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 91299cc56bf7..446e00d98fd5 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1418,10 +1418,26 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
> sample.stream_id = etmq->etm->instructions_id;
> sample.period = period;
> sample.cpu = tidq->packet->cpu;
> - sample.flags = tidq->prev_packet->flags;
> sample.cpumode = event->sample.header.misc;
>
> - cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->packet, &sample);
> + cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->prev_packet, &sample);
> +
> + /* Populate branch target information only when we encounter
> + * branch instruction, which is at the end of tidq->prev_packet.
> + */
> + if (addr == (tidq->prev_packet->end_addr - 4)) {
> + /* Update the perf_sample flags using the prev_packet
> + * since that is the queue we are synthesizing.
> + */
> + sample.flags = tidq->prev_packet->flags;
> +
> + /* The last instruction of the previous queue would be a
> + * branch operation. Get the target of that branch by looking
> + * into the first executed instruction of the current packet
> + * queue.
> + */
> + sample.addr = cs_etm__first_executed_instr(tidq->packet);
> + }
>
> if (etm->synth_opts.last_branch)
> sample.branch_stack = tidq->last_branch;
> @@ -1641,7 +1657,7 @@ static int cs_etm__sample(struct cs_etm_queue *etmq,
> /* Get instructions remainder from previous packet */
> instrs_prev = tidq->period_instructions;
>
> - tidq->period_instructions += tidq->packet->instr_count;
> + tidq->period_instructions += tidq->prev_packet->instr_count;
>
> /*
> * Record a branch when the last instruction in
> @@ -1721,8 +1737,11 @@ static int cs_etm__sample(struct cs_etm_queue *etmq,
> * been executed, but PC has not advanced to next
> * instruction)
> */
> + /* Get address from prev_packet since we are synthesizing
> + * that in cs_etm__synth_instruction_sample()
> + */
> addr = cs_etm__instr_addr(etmq, trace_chan_id,
> - tidq->packet, offset - 1);
> + tidq->prev_packet, offset - 1);
> ret = cs_etm__synth_instruction_sample(
> etmq, tidq, addr,
> etm->instructions_sample_period);
> @@ -1786,7 +1805,7 @@ static int cs_etm__flush(struct cs_etm_queue *etmq,
>
> /* Handle start tracing packet */
> if (tidq->prev_packet->sample_type == CS_ETM_EMPTY)
> - goto swap_packet;
> + goto reset_last_br;
>
> if (etmq->etm->synth_opts.last_branch &&
> etmq->etm->synth_opts.instructions &&
> @@ -1822,8 +1841,7 @@ static int cs_etm__flush(struct cs_etm_queue *etmq,
> return err;
> }
>
> -swap_packet:
> - cs_etm__packet_swap(etm, tidq);
> +reset_last_br:
>
> /* Reset last branches after flush the trace */
> if (etm->synth_opts.last_branch)
On 27/06/2023 18:19, Ian Rogers wrote:
> On Tue, Jun 27, 2023 at 9:58 AM Namhyung Kim <namhyung(a)kernel.org> wrote:
>>
>> On Tue, Jun 27, 2023 at 9:43 AM Ian Rogers <irogers(a)google.com> wrote:
>>>
>>> On Mon, Jun 26, 2023 at 5:02 PM Namhyung Kim <namhyung(a)kernel.org> wrote:
>>>>
>>>> On Mon, Jun 26, 2023 at 05:10:58PM +0100, James Clark wrote:
>>>>> thread__find_map() chooses to exit without assigning a thread to the
>>>>> addr_location in some scenarios, for example when there are samples from
>>>>> a guest and perf_guest == false. This results in a segfault when adding
>>>>> to the histogram because it uses unguarded accesses to the thread member
>>>>> of the addr_location.
>>>>
>>>> Looking at the commit 0dd5041c9a0ea ("perf addr_location: Add
>>>> init/exit/copy functions") that introduced the change, I'm not sure if
>>>> it's the intend behavior.
>>>>
>>>> It might change maps and map, but not thread. Then I think no reason
>>>> to not set the al->thread at the beginning.
>>>>
>>>> How about this? Ian?
>>>> (I guess we can get rid of the duplicate 'al->map = NULL' part)
>>>
>>> It seemed strange that we were failing to find a map (the function's
>>> purpose) but then populating the address_location. The change below
>>> brings back that somewhat odd behavior. I'm okay with reverting to the
>>> old behavior, clearly there were users relying on it. We should
>>> probably also copy maps and not just thread, as that was the previous
>>> behavior.
>>
>> Probably. But it used to support samples without maps and I think
>> that's why it ignores the return value of thread__find_map(). So
>> we can expect al.map is NULL and maybe fine to leave it for now.
>>
>> As machine__resolve() returns -1 if it gets no thread, we should set
>> al.thread when it returns 0.
>>
>> Can I get your Acked-by?
>
> Yep:
> Acked-by: Ian Rogers <irogers(a)google.com>
Looks good to me too. Should I resend the set with this change instead
of my one?
>
> Thanks,
> Ian
>
>> Thanks,
>> Namhyung
On 6/20/2023 4:49 PM, Greg Kroah-Hartman wrote:
> On Tue, Jun 20, 2023 at 04:31:59PM +0800, Tao Zhang wrote:
>> 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"?
> Yes.
Hi Greg,
I‘d like to clarify the usage of this sysfs file again.
In the current design, three integers will be written to "dsb_edge_ctrl"
to configure DSB edge detection.
Integer #1: The start number of edge detection which needs to be configured.
Integer #2: The end number of edge detection which needs to be configured.
Integer #3: The type of the edge detection needs to be configured.
Below is an example.
echo 0x3 0x25 0x1 > dsb_edge_ctrl
It will configure edge detection #3 to #37 as "falling edge detection".
Since these three integers are interrelated and written to achieve the
same function, can we use these three integers as "one tuple" here?
Best,
Tao
>
>> I see that more than one value are written to the sysfs file
>> "trigout_attach".
> Then someone missed that and it needs to be fixed.
>
>>>> +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.
> Again, they are broken, please send patches to fix them.
>
>> Is this "one value" limitation the usage rule of Linux sysfs system?
> Yes.
>
> thanks,
>
> greg k-h
On 27/06/2023 10:20, Greg Kroah-Hartman wrote:
> On Tue, Jun 27, 2023 at 10:06:11AM +0100, Suzuki K Poulose wrote:
>> Hi Greg,
>>
>> On 29/05/2023 07:25, Anshuman Khandual wrote:
>>> From: Suzuki K Poulose <suzuki.poulose(a)arm.com>
>>>
>>> Drop ETM4X ACPI ID from the AMBA ACPI device list, and instead just move it
>>> inside the new ACPI devices list detected and used via platform driver.
>>>
>>> Cc: "Rafael J. Wysocki" <rafael(a)kernel.org>
>>> Cc: Len Brown <lenb(a)kernel.org>
>>> Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org>
>>> Cc: Suzuki K Poulose <suzuki.poulose(a)arm.com>
>>> Cc: Mike Leach <mike.leach(a)linaro.org>
>>> Cc: Leo Yan <leo.yan(a)linaro.org>
>>> Cc: Sudeep Holla <sudeep.holla(a)arm.com>
>>> Cc: Lorenzo Pieralisi <lpieralisi(a)kernel.org>
>>> Cc: linux-acpi(a)vger.kernel.org
>>> Cc: coresight(a)lists.linaro.org
>>> Cc: linux-arm-kernel(a)lists.infradead.org
>>> Cc: linux-kernel(a)vger.kernel.org
>>> Reviewed-by: Sudeep Holla <sudeep.holla(a)arm.com> (for ACPI specific changes)
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com>
>>> Signed-off-by: Anshuman Khandual <anshuman.khandual(a)arm.com>
>>> ---
>>> drivers/acpi/acpi_amba.c | 1 -
>>> drivers/hwtracing/coresight/coresight-etm4x-core.c | 10 ++++++++++
>>> 2 files changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/acpi/acpi_amba.c b/drivers/acpi/acpi_amba.c
>>> index f5b443ab01c2..099966cbac5a 100644
>>> --- a/drivers/acpi/acpi_amba.c
>>> +++ b/drivers/acpi/acpi_amba.c
>>> @@ -22,7 +22,6 @@
>>> static const struct acpi_device_id amba_id_list[] = {
>>> {"ARMH0061", 0}, /* PL061 GPIO Device */
>>> {"ARMH0330", 0}, /* ARM DMA Controller DMA-330 */
>>> - {"ARMHC500", 0}, /* ARM CoreSight ETM4x */
>>> {"ARMHC501", 0}, /* ARM CoreSight ETR */
>>> {"ARMHC502", 0}, /* ARM CoreSight STM */
>>> {"ARMHC503", 0}, /* ARM CoreSight Debug */
>>
>> This is a bit awkward request.
>>
>> I would like to get your opinion on merging this to coresight tree.
>> This change is removing the coresight ETMv4 from the ACPI AMBA
>> scan list and moving it to the coresight driver. This change is
>> essential for
>> 1) Adding ACPI support for later versions of ETMv4 that are not AMBA
>> devices.
>> 2) Adding power management support for AMBA ETMv4 with ACPI.
>>
>> The above change has been reviewed by Sudeep (Arm64 ACPI reviewer), but
>> hasn't been Acked by the ACPI maintainer (Rafael) even after a month of
>> follow up with at least 4 reminders [0].
>>
>> Are you happy with the Reviews from Sudeep and given the minimal
>> change to the drivers/acpi/acpi_amba.c file ?
>
> As we can't do anything now with the merge window open, please resend
> after 6.5-rc1 is out and ask for the ACPI developers to ack this.
Sure, will do that.
Thanks
Greg