Re: [PATCH V1 2/6] soc: qcom: dcc: Add driver support for Data Capture and Compare unit(DCC)

From: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
To: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Souradeep Chowdhury <schowdhu@codeaurora.org>,
	Rob Herring <robh+dt@kernel.org>, Andy Gross <agross@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	devicetree@vger.kernel.org, sibis@codeaurora.org,
	Rajendra Nayak <rnayak@codeaurora.org>,
	vkoul@kernel.org
Subject: Re: [PATCH V1 2/6] soc: qcom: dcc: Add driver support for Data Capture and Compare unit(DCC)
Date: Thu, 11 Mar 2021 11:49:36 +0530	[thread overview]
Message-ID: <ab30490c016f906fd9bc5d789198530b@codeaurora.org> (raw)
In-Reply-To: <YElUCaBUOx7hEuIh@builder.lan>

Hi Bjorn,

On 2021-03-11 04:49, Bjorn Andersson wrote:
> On Wed 10 Mar 10:46 CST 2021, Souradeep Chowdhury wrote:
> 
>> The DCC is a DMA Engine designed to capture and store data
>> during system crash or software triggers. The DCC operates
>> based on link list entries which provides it with data and
>> addresses and the function it needs to perform. These
>> functions are read, write and loop. Added the basic driver
>> in this patch which contains a probe method which instantiates
>> the resources needed by the driver. DCC has it's own SRAM which
>> needs to be instantiated at probe time as well.
>> 
> 
> So to summarize, the DCC will upon a crash copy the configured region
> into the dcc-ram, where it can be retrieved either by dumping the 
> memory
> over USB or from sysfs on the next boot?
> 

Not just the next boot, but also for the current boot via /dev/dcc_sram,
more below.

>> Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
>> ---
>>  drivers/soc/qcom/Kconfig  |   8 +
>>  drivers/soc/qcom/Makefile |   1 +
>>  drivers/soc/qcom/dcc.c    | 388 
>> ++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 397 insertions(+)
>>  create mode 100644 drivers/soc/qcom/dcc.c
>> 

<snip>...

> 
> How about implementing this using pstore instead of exposing it through
> a custom /dev/dcc_sram (if I read the code correclty)
> 

Using pstore is definitely a good suggestion, we have been thinking of
adding it as a separate change once the basic support for DCC gets in.
But pstore ram backend again depends on warm reboot which is present 
only
in chrome compute platforms but android platforms do not officially 
support
warm reboot. Pstore with block devices as a backend would be ideal but 
it
is still a work in progress to use mmc as the backend [1].

Now the other reason as to why we need this dev interface in addition to
pstore,

  * Pstore contents are retrieved on the next boot, but DCC SRAM contents
    can be collected via dev interface for the current boot via SW 
trigger.
    Lets say we have some non-fatal errors in one of the subsystems and 
we
    want to analyze the register values, it becomes as simple as 
configuring
    that region, trigger dcc and collect the sram contents and parse it.

    echo "addr" > /sys/bus/platform/devices/***.dcc/config
    echo  1 > /sys/bus/platform/devices/***.dcc/trigger
    cat /dev/dcc_sram > dcc_sram.bin
    python dcc_parser.py -s dcc_sram.bin --v2 -o output/

We don't have to reboot at all for SW triggers. This is very useful and
widely used internally.

Is the custom /dev/dcc_sram not recommended because of the dependency on
the userspace component being not available openly? If so, we already 
have
the dcc parser upstream which we use to extract the sram contents [2].

[1] 
https://lore.kernel.org/lkml/20210120121047.2601-1-bbudiredla@marvell.com/
[2] 
https://source.codeaurora.org/quic/la/platform/vendor/qcom-opensource/tools/tree/dcc_parser

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation