[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9012a424-ae61-e7e1-0aca-4558f847ff99@quicinc.com>
Date: Wed, 15 Dec 2021 19:26:38 +0530
From: Souradeep Chowdhury <quic_schowdhu@...cinc.com>
To: Alex Elder <elder@...e.org>,
Souradeep Chowdhury <schowdhu@...eaurora.org>,
Andy Gross <agross@...nel.org>,
Bjorn Andersson <bjorn.andersson@...aro.org>,
Rob Herring <robh+dt@...nel.org>
CC: <linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
<devicetree@...r.kernel.org>,
"Sai Prakash Ranjan" <saiprakash.ranjan@...eaurora.org>,
Sibi Sankar <sibis@...eaurora.org>,
Rajendra Nayak <rnayak@...eaurora.org>, <vkoul@...nel.org>
Subject: Re: [PATCH V6 0/7] Add driver support for Data Capture and Compare
Engine(DCC) for SM8150,SC7280,SC7180,SDM845
On 12/14/2021 4:05 AM, Alex Elder wrote:
> On 8/10/21 12:54 PM, Souradeep Chowdhury wrote:
>> DCC(Data Capture and Compare) is a DMA engine designed for debugging
>> purposes.In case of a system
>> crash or manual software triggers by the user the DCC hardware stores
>> the value at the register
>> addresses which can be used for debugging purposes.The DCC driver
>> provides the user with sysfs
>> interface to configure the register addresses.The options that the
>> DCC hardware provides include
>> reading from registers,writing to registers,first reading and then
>> writing to registers and looping
>> through the values of the same register.
>
> I realize this was posted a long time ago but I spent a little
> time on it today, and I have some comments for you to consider.
> You'll need to post another version of this series if you're
> going to address some of my comments.
>
> Most of the comments are in patch 2, which contains all the code
> and the sysfs documentation. I have no comments on patches 3
> (MAINTAINERS update) or 4 through 7 (DTS updates for specific
> platforms).
>
> First, a few comments on this cover page. The most trivial
> comment is: Please make your lines narrower than 80 columns,
> like the rest of the patches.
>
> I appreciate that this goes into some detail about how this
> feature has been used. But I think it could benefit from
> a little better high-level overview of what it *does*.
> Your first paragraph is a concise summary, but I find it
> doesn't evoke a model in my mind of what exactly is going
> on, or what the hardware is doing. In fact, if you can
> provide a good high-level overview it might belong at the
> top of "dcc.c" in comments.
>
> Looking at the code (but not in any great depth), I see
> that there are "linked lists" of what appear to be things
> for the hardware to do with memory when this hardware is
> "triggered." If I understand it right, there can be up
> to 8 of these lists (though some versions of hardware
> might advertise the number supported via a register).
>
> If the following is wrong, I hope you'll offer a comparable
> explanation and will correct my misunderstanding.
>
> Each list consists of a set of actions to take. The actions
> available include: reading a register (possibly <count> times
> in succession); writing a register; and read/modify/writing
> a register (affecting only bits in a given mask). Actually,
> the way looping works is a little confusing to me.
>
> Each list can be enabled and disabled separately. When
> triggered, all lists are executed, and (somehow) the result
> is saved into a buffer that can be read via /dev/dcc_sram.
>
> So you use these sysfs files to configure the actions you'd
> like to take when a "trigger" is signaled. The content of
> /dev/dcc_sram can then be read to see what output your
> lists produced.
>
> Is that close to correct? If it is, great; I want to be
> sure I understand what the hardware is supposed to do
> before I comment much more on the way you represent it
> in the driver and in sysfs.
>
>> In certain cases a register write needs to be executed for accessing
>> the rest of the registers,
>> also the user might want to record the changing values of a register
>> with time for which he has the
>> option to use the loop feature.
>>
>> The options mentioned above are exposed to the user by sysfs files
>> once the driver is probed.The
>> details and usage of this sysfs files are documented in
>> Documentation/ABI/testing/sysfs-driver-dcc.
>
> Once you've confirmed I understand what's supposed to happen
> when the trigger fires, I think I'll have some comments on
> the way you represent the actions in these lists. But
> for now, maybe keep things as you have them, but address
> some of the comments I'm giving you today. Copy me on
> future revisions and I'll plan to review again.
>
> OK, that's enough on this file for now. Onto the binding and
> the code...
>
> -Alex
>
Hi Alex.
Thanks for your feedback. The understanding is correct regarding DCC
hardware.
Will address all the comments and post the next version copying you.
Thanks,
Souradeep
>> As an example let us consider a couple of debug scenarios where DCC
>> has been proved to be effective
>> for debugging purposes:-
>>
>> i)TimeStamp Related Issue
>>
>> On SC7180, there was a coresight timestamp issue where it would
>> occasionally be all 0 instead of proper
>> timestamp values.
>>
>> Proper timestamp:
>> Idx:3373; ID:10; I_TIMESTAMP : Timestamp.; Updated val =
>> 0x13004d8f5b7aa; CC=0x9e
>>
>> Zero timestamp:
>> Idx:3387; ID:10; I_TIMESTAMP : Timestamp.; Updated val = 0x0; CC=0xa2
>>
>> Now this is a non-fatal issue and doesn't need a system reset, but
>> still needs
>> to be rootcaused and fixed for those who do care about coresight etm
>> traces.
>> Since this is a timestamp issue, we would be looking for any
>> timestamp related
>> clocks and such.
>>
>> o we get all the clk register details from IP documentation and
>> configure it
>> via DCC config syfs node. Before that we set the current linked list.
>>
>> /* Set the current linked list */
>> echo 3 > /sys/bus/platform/devices/10a2000.dcc/curr_list
>>
>> /* Program the linked list with the addresses */
>> echo 0x10c004 > /sys/bus/platform/devices/10a2000.dcc/config
>> echo 0x10c008 > /sys/bus/platform/devices/10a2000.dcc/config
>> echo 0x10c00c > /sys/bus/platform/devices/10a2000.dcc/config
>> echo 0x10c010 > /sys/bus/platform/devices/10a2000.dcc/config
>> ..... and so on for other timestamp related clk registers
>>
>> /* Other way of specifying is in "addr len" pair, in below case it
>> specifies to capture 4 words starting 0x10C004 */
>>
>> echo 0x10C004 4 > /sys/bus/platform/devices/10a2000.dcc/config
>>
>> /* Enable DCC */
>> echo 1 > /sys/bus/platform/devices/10a2000.dcc/enable
>>
>> /* Run the timestamp test for working case */
>>
>> /* Send SW trigger */
>> echo 1 > /sys/bus/platform/devices/10a2000.dcc/trigger
>>
>> /* Read SRAM */
>> cat /dev/dcc_sram > dcc_sram1.bin
>>
>> /* Run the timestamp test for non-working case */
>>
>> /* Send SW trigger */
>> echo 1 > /sys/bus/platform/devices/10a2000.dcc/trigger
>>
>> /* Read SRAM */
>> cat /dev/dcc_sram > dcc_sram2.bin
>>
>> Get the parser from [1] and checkout the latest branch.
>>
>> /* Parse the SRAM bin */
>> python dcc_parser.py -s dcc_sram1.bin --v2 -o output/
>> python dcc_parser.py -s dcc_sram2.bin --v2 -o output/
>>
>> Sample parsed output of dcc_sram1.bin:
>>
>> <hwioDump version="1">
>> <timestamp>03/14/21</timestamp>
>> <generator>Linux DCC Parser</generator>
>> <chip name="None" version="None">
>> <register address="0x0010c004" value="0x80000000" />
>> <register address="0x0010c008" value="0x00000008" />
>> <register address="0x0010c00c" value="0x80004220" />
>> <register address="0x0010c010" value="0x80000000" />
>> </chip>
>> <next_ll_offset>next_ll_offset : 0x1c </next_ll_offset>
>> </hwioDump>
>>
>> ii)NOC register errors
>>
>> A particular class of registers called NOC which are functional
>> registers was reporting
>> errors while logging the values.To trace these errors the DCC has
>> been used effectively.
>> The steps followed were similar to the ones mentioned above.
>> In addition to NOC registers a few other dependent registers were
>> configured in DCC to
>> monitor it's values during a crash. A look at the dependent register
>> values revealed that
>> the crash was happening due to a secured access to one of these
>> dependent registers.
>> All these debugging activity and finding the root cause was achieved
>> using DCC.
>>
>> DCC parser is available at the following open source location
>>
>> https://source.codeaurora.org/quic/la/platform/vendor/qcom-opensource/tools/tree/dcc_parser
>>
>>
>> Changes in v6:
>>
>> *Added support in the dcc driver to handle multiple Qualcomm SoCs
>> including SC7180,SC7280,SDM845
>> along with existing SM8150.
>> *Added the support node in the respective device tree files for
>> SC7180,SC7280,SDM845.
>>
>> Souradeep Chowdhury (7):
>> dt-bindings: Added the yaml bindings for DCC
>> soc: qcom: dcc:Add driver support for Data Capture and Compare
>> unit(DCC)
>> MAINTAINERS: Add the entry for DCC(Data Capture and Compare) driver
>> support
>> arm64: dts: qcom: sm8150: Add Data Capture and Compare(DCC) support
>> node
>> arm64: dts: qcom: sc7280: Add Data Capture and Compare(DCC) support
>> node
>> arm64: dts: qcom: sc7180: Add Data Capture and Compare(DCC) support
>> node
>> arm64: dts: qcom: sdm845: Add Data Capture and Compare(DCC) support
>> node
>>
>> Documentation/ABI/testing/sysfs-driver-dcc | 114 ++
>> .../devicetree/bindings/arm/msm/qcom,dcc.yaml | 43 +
>> MAINTAINERS | 8 +
>> arch/arm64/boot/dts/qcom/sc7180.dtsi | 6 +
>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 6 +
>> arch/arm64/boot/dts/qcom/sdm845.dtsi | 6 +
>> arch/arm64/boot/dts/qcom/sm8150.dtsi | 6 +
>> drivers/soc/qcom/Kconfig | 8 +
>> drivers/soc/qcom/Makefile | 1 +
>> drivers/soc/qcom/dcc.c | 1549
>> ++++++++++++++++++++
>> 10 files changed, 1747 insertions(+)
>> create mode 100644 Documentation/ABI/testing/sysfs-driver-dcc
>> create mode 100644
>> Documentation/devicetree/bindings/arm/msm/qcom,dcc.yaml
>> create mode 100644 drivers/soc/qcom/dcc.c
>>
>
Powered by blists - more mailing lists