mbox series

[V7,0/7] Add driver support for Data Capture and Compare Engine(DCC) for SM8150,SC7280,SC7180,SDM845

Message ID cover.1646285069.git.quic_schowdhu@quicinc.com
Headers show
Series Add driver support for Data Capture and Compare Engine(DCC) for SM8150,SC7280,SC7180,SDM845 | expand

Message

Souradeep Chowdhury March 3, 2022, 6:27 a.m. UTC
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 debugfs 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.

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 debugfs files once the driver
is probed. The details and usage of this debugfs files are documented in
Documentation/ABI/testing/debugfs-driver-dcc.

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.

We get all the clk register details from IP documentation and configure it
via DCC config_read debugfs node. Before that we set the current linked list.

/* Set the current linked list */
echo 3 > /sys/kernel/debug/dcc/../curr_list

/* Program the linked list with the addresses */
echo 0x10c004 > /sys/kernel/debug/dcc/../config_read
echo 0x10c008 > /sys/kernel/debug/dcc/../config_read
echo 0x10c00c > /sys/kernel/debug/dcc/../config_read
echo 0x10c010 > /sys/kernel/debug/dcc/../config_read
..... 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/kernel/debug/dcc/../config_read

/* Enable DCC */
echo 1 > /sys/kernel/debug/dcc/../enable

/* Run the timestamp test for working case */

/* Send SW trigger */
echo 1 > /sys/kernel/debug/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/kernel/debug/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 V7

*The DCC interface has been shifted from sysfs to debugfs. The new interface
 details are documented in Documentation/ABI/testing/debugfs-driver-dcc.

*All the rest of the  comments from previous versions have been implemented.


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/debugfs-driver-dcc       |  124 ++
 .../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                             | 1465 ++++++++++++++++++++
 10 files changed, 1673 insertions(+)
 create mode 100644 Documentation/ABI/testing/debugfs-driver-dcc
 create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,dcc.yaml
 create mode 100644 drivers/soc/qcom/dcc.c

--
2.7.4

Comments

Souradeep Chowdhury April 19, 2022, 11:58 a.m. UTC | #1
On 3/3/2022 11:57 AM, 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 debugfs 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.
>
> 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 debugfs files once the driver
> is probed. The details and usage of this debugfs files are documented in
> Documentation/ABI/testing/debugfs-driver-dcc.
>
> 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.
>
> We get all the clk register details from IP documentation and configure it
> via DCC config_read debugfs node. Before that we set the current linked list.
>
> /* Set the current linked list */
> echo 3 > /sys/kernel/debug/dcc/../curr_list
>
> /* Program the linked list with the addresses */
> echo 0x10c004 > /sys/kernel/debug/dcc/../config_read
> echo 0x10c008 > /sys/kernel/debug/dcc/../config_read
> echo 0x10c00c > /sys/kernel/debug/dcc/../config_read
> echo 0x10c010 > /sys/kernel/debug/dcc/../config_read
> ..... 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/kernel/debug/dcc/../config_read
>
> /* Enable DCC */
> echo 1 > /sys/kernel/debug/dcc/../enable
>
> /* Run the timestamp test for working case */
>
> /* Send SW trigger */
> echo 1 > /sys/kernel/debug/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/kernel/debug/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 V7
>
> *The DCC interface has been shifted from sysfs to debugfs. The new interface
>   details are documented in Documentation/ABI/testing/debugfs-driver-dcc.
>
> *All the rest of the  comments from previous versions have been implemented.
>
>
> 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/debugfs-driver-dcc       |  124 ++
>   .../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                             | 1465 ++++++++++++++++++++
>   10 files changed, 1673 insertions(+)
>   create mode 100644 Documentation/ABI/testing/debugfs-driver-dcc
>   create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,dcc.yaml
>   create mode 100644 drivers/soc/qcom/dcc.c
>
> --
> 2.7.4

Hi,

Gentle Ping

  Let me know if there is any feedback regarding this patch

Thanks,

Souradeep
Trilok Soni May 2, 2022, 11:01 p.m. UTC | #2
On 3/2/2022 10:27 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 debugfs 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.
> 

Bjorn, can you please take a look at this series, it is pending review 
from last two months now.

---Trilok Soni
Bjorn Andersson May 3, 2022, 6:03 p.m. UTC | #3
On Thu 03 Mar 00:27 CST 2022, 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 user inputs via the sysfs interface. The user gives
> addresses as inputs and these addresses are stored in the
> dcc sram. In case of a system crash or a manual software
> trigger by the user through the debugfs interface,
> the dcc captures and stores the values at these addresses.
> This patch contains the driver which has all the methods
> pertaining to the debugfs interface, auxiliary functions to
> support all the four fundamental operations of dcc namely
> read, write, read/modify/write and loop. The probe method
> here instantiates all the resources necessary for dcc to
> operate mainly the dedicated dcc sram where it stores the
> values. The DCC driver can be used for debugging purposes
> without going for a reboot since it can perform software
> triggers as well based on user inputs.
> 
> Also added the documentation for debugfs entries and explained
> the functionalities of each debugfs file that has been created
> for dcc.
> 
> The following is the justification of using debugfs interface
> over the other alternatives like sysfs/ioctls
> 
> i) As can be seen from the debugfs attribute descriptions,
> some of the debugfs attribute files here contains multiple
> arguments which needs to be accepted from the user. This goes
> against the design style of sysfs.
> 
> ii) The user input patterns have been made simple and convenient
> in this case with the use of debugfs interface as user doesn't
> need to shuffle between different files to execute one instruction
> as was the case on using other alternatives.
> 
> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> ---
>  Documentation/ABI/testing/debugfs-driver-dcc |  124 +++
>  drivers/soc/qcom/Kconfig                     |    8 +
>  drivers/soc/qcom/Makefile                    |    1 +
>  drivers/soc/qcom/dcc.c                       | 1465 ++++++++++++++++++++++++++
>  4 files changed, 1598 insertions(+)
>  create mode 100644 Documentation/ABI/testing/debugfs-driver-dcc
>  create mode 100644 drivers/soc/qcom/dcc.c
> 
> diff --git a/Documentation/ABI/testing/debugfs-driver-dcc b/Documentation/ABI/testing/debugfs-driver-dcc
> new file mode 100644
> index 0000000..70029ab
> --- /dev/null
> +++ b/Documentation/ABI/testing/debugfs-driver-dcc
> @@ -0,0 +1,124 @@
> +What:          /sys/kernel/debug/dcc/.../trigger
> +Date:          March 2022
> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> +Description:
> +		This is the debugfs interface for manual software
> +		triggers. The user can simply enter a 1 against
> +		the debugfs file and enable a manual trigger.
> +		Example:
> +		echo  1 > /sys/kernel/debug/dcc/.../trigger
> +
> +What:          /sys/kernel/debug/dcc/.../enable
> +Date:          March 2022
> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> +Description:
> +		This debugfs interface is used for enabling the
> +		the dcc hardware. On enabling the dcc, all the
> +		addresses entered by the user is written into
> +		dcc sram which is read by the dcc hardware on
> +		manual or crash induced triggers.
> +		Example:
> +		echo  0 > /sys/bus/platform/devices/.../enable
> +		(disable dcc)
> +		echo  1 > /sys/bus/platform/devices/.../enable
> +		(enable dcc)
> +
> +What:          /sys/kernel/debug/dcc/.../config_read

As mentioned last time, I don't like this interface of having 6 files
that the user can write to in order to append items in the currently
selected linked list.

Why can't this be a single "config" which takes a multiline string of
operations? (Bonus point for supporting appending to the list).


This would also serve as a natural place to dump the linked list back to
the user for inspection.

> +Date:          March 2022
> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> +Description:
> +		This stores the addresses of the registers which
> +		needs to be read in case of a hardware crash or
> +		manual software triggers. The address entered here
> +		are considered under read type instruction.
> +		Example:
> +		echo <1> <2> <3> >/sys/kernel/debug/dcc/../config_read
> +		1->Address to be considered for reading the value.
> +		2->The word count of the addresses, read n words
> +		   starting from address <1>.
> +		3->Can be a 1 or 0 which indicates if it is apb or ahb
> +		bus respectively.
> +
> +What:          /sys/kernel/debug/dcc/.../config_write
> +Date:          March 2022
> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> +Description:
> +		This file allows user to write a value to the register
> +		address given as argument. The reason for this feature
> +		of dcc is that for accessing certain registers it is
> +		necessary to set some bits of some other register.
> +		Example:
> +		echo <1> <2> <3> > /sys/bus/platform/devices/.../config_write
> +		1->Address to be considered for writing the value.
> +		2->The value that needs to be written at the location.
> +		3->Can be a 1 or 0 which indicates if it is apb or ahb
> +		bus respectively.
> +
> +What:          /sys/kernel/debug/dcc/.../config_reset
> +Date:          March 2022
> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> +Description:
> +		This file is used to reset the configuration of
> +		a dcc driver to the default configuration. This
> +		means that all the previous addresses stored in
> +		the driver gets removed and user needs to enter
> +		the address values from the start.
> +		Example:
> +		echo  1 > /sys/bus/platform/devices/.../config_reset
> +
> +What:          /sys/kernel/debug/dcc/.../config_loop
> +Date:		March 2022
> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> +Description:
> +		This file is used to enter the loop type addresses for
> +		dcc. DCC hardware provides feature to loop among multiple
> +		addresses. For debugging purposes register values need to
> +		be captured repeatedly in a loop. On giving the loop count
> +		as n, the value at address will be captured n times in a
> +		loop. At most 8 loop addresses can be configured at once.
> +		Example:
> +		echo <1> <2> <3> > /sys/kernel/debug/dcc/../config_loop
> +		1->The loop count, the number of times the value of the
> +		   addresses will be captured.
> +		2->The address count, total number of addresses to be
> +		   entered in this instruction.
> +		3->The series of addresses to be entered separated by a
> +		   space like <addr1> <addr2>... and so on.
> +
> +What:          /sys/kernel/debug/dcc/.../config_read_write
> +Date:          March 2022
> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> +Description:
> +		This file is used to read the value of the register
> +		and then write the value given as an argument to the
> +		register address. The address argument should be given
> +		of the form <addr> <mask> <value>.For debugging purposes
> +		sometimes we need to first read from a register and then
> +		set some values to the register.
> +		Example:
> +		echo <1> <2> <3> > /sys/kernel/debug/dcc/.../config_read_write
> +		1->The address which needs to be considered for read then write.
> +		2->The value that needs to be written on the address.
> +		3->The mask of the value to be written.
> +
> +What:		/sys/kernel/debug/dcc/.../ready
> +Date:		March 2022
> +Contact	Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> +Description:
> +		This file is used to check the status of the dcc
> +		hardware if it's ready to take the inputs. A 0
> +		here indicates dcc is in a ready condition.
> +		Example:
> +		cat /sys/kernel/debug/dcc/.../ready
> +
> +What:		/sys/kernel/debug/dcc/.../curr_list

I still don't like the idea of having a single set of files to interface
with all N lists. I think you should discover how many lists you have
and create N directories of files, each on operating on a given list.

> +Date:		March 2022
> +Contact:	Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> +Description:
> +		This attribute is used to enter the linklist to be
> +		used while appending addresses. The range of values
> +		for this is advertised either by a register or is
> +		predefined. Max value for this can be till 8.
> +		Example:
> +		echo 0 > /sys/kernel/debug/dcc/...curr_list
> +

Regards,
Bjorn
Bjorn Andersson May 3, 2022, 6:04 p.m. UTC | #4
On Thu 03 Mar 00:27 CST 2022, Souradeep Chowdhury wrote:

> Added the entries for all the files added as a part of driver support for
> DCC(Data Capture and Compare).
> 
> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> ---
>  MAINTAINERS | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a61f4f3..e57d927 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5146,6 +5146,14 @@ F:	include/linux/tfrc.h
>  F:	include/uapi/linux/dccp.h
>  F:	net/dccp/
>  
> +DCC QTI DRIVER
> +M:	Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> +L:	linux-arm-msm@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/ABI/testing/debugfs-driver-dcc
> +F:	Documentation/devicetree/bindings/arm/msm/qcom,dcc.yaml
> +F:	drivers/soc/qcom/dcc.c
> +
>  DECnet NETWORK LAYER
>  L:	linux-decnet-user@lists.sourceforge.net
>  S:	Orphan
> -- 
> 2.7.4
>
Souradeep Chowdhury May 5, 2022, 12:53 p.m. UTC | #5
On 5/3/2022 11:33 PM, Bjorn Andersson wrote:
> On Thu 03 Mar 00:27 CST 2022, 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 user inputs via the sysfs interface. The user gives
>> addresses as inputs and these addresses are stored in the
>> dcc sram. In case of a system crash or a manual software
>> trigger by the user through the debugfs interface,
>> the dcc captures and stores the values at these addresses.
>> This patch contains the driver which has all the methods
>> pertaining to the debugfs interface, auxiliary functions to
>> support all the four fundamental operations of dcc namely
>> read, write, read/modify/write and loop. The probe method
>> here instantiates all the resources necessary for dcc to
>> operate mainly the dedicated dcc sram where it stores the
>> values. The DCC driver can be used for debugging purposes
>> without going for a reboot since it can perform software
>> triggers as well based on user inputs.
>>
>> Also added the documentation for debugfs entries and explained
>> the functionalities of each debugfs file that has been created
>> for dcc.
>>
>> The following is the justification of using debugfs interface
>> over the other alternatives like sysfs/ioctls
>>
>> i) As can be seen from the debugfs attribute descriptions,
>> some of the debugfs attribute files here contains multiple
>> arguments which needs to be accepted from the user. This goes
>> against the design style of sysfs.
>>
>> ii) The user input patterns have been made simple and convenient
>> in this case with the use of debugfs interface as user doesn't
>> need to shuffle between different files to execute one instruction
>> as was the case on using other alternatives.
>>
>> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>> ---
>>   Documentation/ABI/testing/debugfs-driver-dcc |  124 +++
>>   drivers/soc/qcom/Kconfig                     |    8 +
>>   drivers/soc/qcom/Makefile                    |    1 +
>>   drivers/soc/qcom/dcc.c                       | 1465 ++++++++++++++++++++++++++
>>   4 files changed, 1598 insertions(+)
>>   create mode 100644 Documentation/ABI/testing/debugfs-driver-dcc
>>   create mode 100644 drivers/soc/qcom/dcc.c
>>
>> diff --git a/Documentation/ABI/testing/debugfs-driver-dcc b/Documentation/ABI/testing/debugfs-driver-dcc
>> new file mode 100644
>> index 0000000..70029ab
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/debugfs-driver-dcc
>> @@ -0,0 +1,124 @@
>> +What:          /sys/kernel/debug/dcc/.../trigger
>> +Date:          March 2022
>> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>> +Description:
>> +		This is the debugfs interface for manual software
>> +		triggers. The user can simply enter a 1 against
>> +		the debugfs file and enable a manual trigger.
>> +		Example:
>> +		echo  1 > /sys/kernel/debug/dcc/.../trigger
>> +
>> +What:          /sys/kernel/debug/dcc/.../enable
>> +Date:          March 2022
>> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>> +Description:
>> +		This debugfs interface is used for enabling the
>> +		the dcc hardware. On enabling the dcc, all the
>> +		addresses entered by the user is written into
>> +		dcc sram which is read by the dcc hardware on
>> +		manual or crash induced triggers.
>> +		Example:
>> +		echo  0 > /sys/bus/platform/devices/.../enable
>> +		(disable dcc)
>> +		echo  1 > /sys/bus/platform/devices/.../enable
>> +		(enable dcc)
>> +
>> +What:          /sys/kernel/debug/dcc/.../config_read
> As mentioned last time, I don't like this interface of having 6 files
> that the user can write to in order to append items in the currently
> selected linked list.
>
> Why can't this be a single "config" which takes a multiline string of
> operations? (Bonus point for supporting appending to the list).
>
>
> This would also serve as a natural place to dump the linked list back to
> the user for inspection.

Following is the justification of having multiple files in debugfs

1-> Since there are fundamentally 4 instructions for DCC, 
Read,Write,Read and then Write and Loop,having separate debugfs files 
for the same makes it

        convenient for the user to use this tool and also to 
document.This also is consistent with the design principles of debugfs 
as it supports logical segregation

        of Debugfs files based on the user instructions.

2-> We are maintaining a common linkedlist inside the driver and it can 
be viewed by the user through the "config_read" debugfs file. Will be 
adding this to the

        documentation as well.

Let me know your thoughts regarding the above.

>> +Date:          March 2022
>> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>> +Description:
>> +		This stores the addresses of the registers which
>> +		needs to be read in case of a hardware crash or
>> +		manual software triggers. The address entered here
>> +		are considered under read type instruction.
>> +		Example:
>> +		echo <1> <2> <3> >/sys/kernel/debug/dcc/../config_read
>> +		1->Address to be considered for reading the value.
>> +		2->The word count of the addresses, read n words
>> +		   starting from address <1>.
>> +		3->Can be a 1 or 0 which indicates if it is apb or ahb
>> +		bus respectively.
>> +
>> +What:          /sys/kernel/debug/dcc/.../config_write
>> +Date:          March 2022
>> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>> +Description:
>> +		This file allows user to write a value to the register
>> +		address given as argument. The reason for this feature
>> +		of dcc is that for accessing certain registers it is
>> +		necessary to set some bits of some other register.
>> +		Example:
>> +		echo <1> <2> <3> > /sys/bus/platform/devices/.../config_write
>> +		1->Address to be considered for writing the value.
>> +		2->The value that needs to be written at the location.
>> +		3->Can be a 1 or 0 which indicates if it is apb or ahb
>> +		bus respectively.
>> +
>> +What:          /sys/kernel/debug/dcc/.../config_reset
>> +Date:          March 2022
>> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>> +Description:
>> +		This file is used to reset the configuration of
>> +		a dcc driver to the default configuration. This
>> +		means that all the previous addresses stored in
>> +		the driver gets removed and user needs to enter
>> +		the address values from the start.
>> +		Example:
>> +		echo  1 > /sys/bus/platform/devices/.../config_reset
>> +
>> +What:          /sys/kernel/debug/dcc/.../config_loop
>> +Date:		March 2022
>> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>> +Description:
>> +		This file is used to enter the loop type addresses for
>> +		dcc. DCC hardware provides feature to loop among multiple
>> +		addresses. For debugging purposes register values need to
>> +		be captured repeatedly in a loop. On giving the loop count
>> +		as n, the value at address will be captured n times in a
>> +		loop. At most 8 loop addresses can be configured at once.
>> +		Example:
>> +		echo <1> <2> <3> > /sys/kernel/debug/dcc/../config_loop
>> +		1->The loop count, the number of times the value of the
>> +		   addresses will be captured.
>> +		2->The address count, total number of addresses to be
>> +		   entered in this instruction.
>> +		3->The series of addresses to be entered separated by a
>> +		   space like <addr1> <addr2>... and so on.
>> +
>> +What:          /sys/kernel/debug/dcc/.../config_read_write
>> +Date:          March 2022
>> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>> +Description:
>> +		This file is used to read the value of the register
>> +		and then write the value given as an argument to the
>> +		register address. The address argument should be given
>> +		of the form <addr> <mask> <value>.For debugging purposes
>> +		sometimes we need to first read from a register and then
>> +		set some values to the register.
>> +		Example:
>> +		echo <1> <2> <3> > /sys/kernel/debug/dcc/.../config_read_write
>> +		1->The address which needs to be considered for read then write.
>> +		2->The value that needs to be written on the address.
>> +		3->The mask of the value to be written.
>> +
>> +What:		/sys/kernel/debug/dcc/.../ready
>> +Date:		March 2022
>> +Contact	Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>> +Description:
>> +		This file is used to check the status of the dcc
>> +		hardware if it's ready to take the inputs. A 0
>> +		here indicates dcc is in a ready condition.
>> +		Example:
>> +		cat /sys/kernel/debug/dcc/.../ready
>> +
>> +What:		/sys/kernel/debug/dcc/.../curr_list
> I still don't like the idea of having a single set of files to interface
> with all N lists. I think you should discover how many lists you have
> and create N directories of files, each on operating on a given list.

As explained before there cannot be different files based on lists as 
the number of lists to be used

varies across platforms where DCC is used. Also we are giving the user 
the flexibility to configure

multiple lists at one go whereas the dumps are collected in the form of 
separate lists that are

configured by the user.

>
>> +Date:		March 2022
>> +Contact:	Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>> +Description:
>> +		This attribute is used to enter the linklist to be
>> +		used while appending addresses. The range of values
>> +		for this is advertised either by a register or is
>> +		predefined. Max value for this can be till 8.
>> +		Example:
>> +		echo 0 > /sys/kernel/debug/dcc/...curr_list
>> +
> Regards,
> Bjorn
Bjorn Andersson May 6, 2022, 2:53 a.m. UTC | #6
On Thu 05 May 05:53 PDT 2022, Souradeep Chowdhury wrote:

> 
> On 5/3/2022 11:33 PM, Bjorn Andersson wrote:
> > On Thu 03 Mar 00:27 CST 2022, 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 user inputs via the sysfs interface. The user gives
> > > addresses as inputs and these addresses are stored in the
> > > dcc sram. In case of a system crash or a manual software
> > > trigger by the user through the debugfs interface,
> > > the dcc captures and stores the values at these addresses.
> > > This patch contains the driver which has all the methods
> > > pertaining to the debugfs interface, auxiliary functions to
> > > support all the four fundamental operations of dcc namely
> > > read, write, read/modify/write and loop. The probe method
> > > here instantiates all the resources necessary for dcc to
> > > operate mainly the dedicated dcc sram where it stores the
> > > values. The DCC driver can be used for debugging purposes
> > > without going for a reboot since it can perform software
> > > triggers as well based on user inputs.
> > > 
> > > Also added the documentation for debugfs entries and explained
> > > the functionalities of each debugfs file that has been created
> > > for dcc.
> > > 
> > > The following is the justification of using debugfs interface
> > > over the other alternatives like sysfs/ioctls
> > > 
> > > i) As can be seen from the debugfs attribute descriptions,
> > > some of the debugfs attribute files here contains multiple
> > > arguments which needs to be accepted from the user. This goes
> > > against the design style of sysfs.
> > > 
> > > ii) The user input patterns have been made simple and convenient
> > > in this case with the use of debugfs interface as user doesn't
> > > need to shuffle between different files to execute one instruction
> > > as was the case on using other alternatives.
> > > 
> > > Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> > > ---
> > >   Documentation/ABI/testing/debugfs-driver-dcc |  124 +++
> > >   drivers/soc/qcom/Kconfig                     |    8 +
> > >   drivers/soc/qcom/Makefile                    |    1 +
> > >   drivers/soc/qcom/dcc.c                       | 1465 ++++++++++++++++++++++++++
> > >   4 files changed, 1598 insertions(+)
> > >   create mode 100644 Documentation/ABI/testing/debugfs-driver-dcc
> > >   create mode 100644 drivers/soc/qcom/dcc.c
> > > 
> > > diff --git a/Documentation/ABI/testing/debugfs-driver-dcc b/Documentation/ABI/testing/debugfs-driver-dcc
> > > new file mode 100644
> > > index 0000000..70029ab
> > > --- /dev/null
> > > +++ b/Documentation/ABI/testing/debugfs-driver-dcc
> > > @@ -0,0 +1,124 @@
> > > +What:          /sys/kernel/debug/dcc/.../trigger
> > > +Date:          March 2022
> > > +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> > > +Description:
> > > +		This is the debugfs interface for manual software
> > > +		triggers. The user can simply enter a 1 against
> > > +		the debugfs file and enable a manual trigger.
> > > +		Example:
> > > +		echo  1 > /sys/kernel/debug/dcc/.../trigger
> > > +
> > > +What:          /sys/kernel/debug/dcc/.../enable
> > > +Date:          March 2022
> > > +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> > > +Description:
> > > +		This debugfs interface is used for enabling the
> > > +		the dcc hardware. On enabling the dcc, all the
> > > +		addresses entered by the user is written into
> > > +		dcc sram which is read by the dcc hardware on
> > > +		manual or crash induced triggers.
> > > +		Example:
> > > +		echo  0 > /sys/bus/platform/devices/.../enable
> > > +		(disable dcc)
> > > +		echo  1 > /sys/bus/platform/devices/.../enable
> > > +		(enable dcc)
> > > +
> > > +What:          /sys/kernel/debug/dcc/.../config_read
> > As mentioned last time, I don't like this interface of having 6 files
> > that the user can write to in order to append items in the currently
> > selected linked list.
> > 
> > Why can't this be a single "config" which takes a multiline string of
> > operations? (Bonus point for supporting appending to the list).
> > 
> > 
> > This would also serve as a natural place to dump the linked list back to
> > the user for inspection.
> 
> Following is the justification of having multiple files in debugfs
> 
> 1-> Since there are fundamentally 4 instructions for DCC, Read,Write,Read
> and then Write and Loop,having separate debugfs files for the same makes it
> 
> convenient for the user to use this tool and also to document.This
> also is consistent with the design principles of debugfs as it supports
> logical segregation of Debugfs files based on the user instructions.
> 

So say the user of DCC wants to read a register 10 times, they know that
the DCC operates on lists of operations, so they want to tell the
computer "read X, loop 10 times".

But the API is "write to loop file", "write to read file" and "write to
loop file".

You're achieving the same thing, but the user and the driver thinks in
terms of lists of operations and inbetween is a API which provides
"logical segregation" of the different parts of that list.

> 2-> We are maintaining a common linkedlist inside the driver and it can be
> viewed by the user through the "config_read" debugfs file. Will be adding
> this to the documentation as well.
> 

So I use a mix of config_* files to build the lists, and then
config_read is used to look at the list?

So some config_* files will when written append to the list, some
config_* files will perform some action (e.g. reset) and reading some
config_* files will return something useful.

> Let me know your thoughts regarding the above.
> 

I am not convinced that having multiple files provides a nice user
interface. But I certain that the use of the word "config" in various
different ways is wrong.

There are files in the interface which purpose is to append items to the
linked list, name them append_*. Reading the appended items on the list
should not be overloaded on one of the "append" files.


So perhaps:

append_read
append_write
append_rmw
append_loop
config (to dump the current config)
enable
ready
reset
trigger


But then looking at the append_* functions again and the examples below.
You could easily have a single append which takes read, write, rmw and
loop as a first keyword - and build a crude parser based on sscanf to
decode the strings.

Then all append_* becomes "append", which is a single file for adding to
the list and "config" is a single point to read the current list.
Perhaps you could name this file just "config", reading will dump the
list, writing will append to the list.

Here you would have a cleaner interface.


But as you write your own fops you could differentiate between write
and append, so you could make this slightly cleaner by manifesting the
"append" part by allowing the user to do:

  echo read 1, 2, 3 > config
  echo read 4, 5, 6 >> config

Which clearly shows that the first writes to the config and second
appends to the current config. With this interface reset would become:

  echo > config

You can still require that only single operation is written to or
appended to the list per write - to allow you to continue to rely on the
crude sscanf based parser.

With this your interface is reduced to:

config
enable
ready
trigger

> > > +Date:          March 2022
> > > +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> > > +Description:
> > > +		This stores the addresses of the registers which
> > > +		needs to be read in case of a hardware crash or
> > > +		manual software triggers. The address entered here
> > > +		are considered under read type instruction.
> > > +		Example:
> > > +		echo <1> <2> <3> >/sys/kernel/debug/dcc/../config_read
> > > +		1->Address to be considered for reading the value.
> > > +		2->The word count of the addresses, read n words
> > > +		   starting from address <1>.
> > > +		3->Can be a 1 or 0 which indicates if it is apb or ahb
> > > +		bus respectively.
> > > +
> > > +What:          /sys/kernel/debug/dcc/.../config_write
> > > +Date:          March 2022
> > > +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> > > +Description:
> > > +		This file allows user to write a value to the register
> > > +		address given as argument. The reason for this feature
> > > +		of dcc is that for accessing certain registers it is
> > > +		necessary to set some bits of some other register.
> > > +		Example:
> > > +		echo <1> <2> <3> > /sys/bus/platform/devices/.../config_write
> > > +		1->Address to be considered for writing the value.
> > > +		2->The value that needs to be written at the location.
> > > +		3->Can be a 1 or 0 which indicates if it is apb or ahb
> > > +		bus respectively.
> > > +
> > > +What:          /sys/kernel/debug/dcc/.../config_reset
> > > +Date:          March 2022
> > > +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> > > +Description:
> > > +		This file is used to reset the configuration of
> > > +		a dcc driver to the default configuration. This
> > > +		means that all the previous addresses stored in
> > > +		the driver gets removed and user needs to enter
> > > +		the address values from the start.
> > > +		Example:
> > > +		echo  1 > /sys/bus/platform/devices/.../config_reset
> > > +
> > > +What:          /sys/kernel/debug/dcc/.../config_loop
> > > +Date:		March 2022
> > > +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> > > +Description:
> > > +		This file is used to enter the loop type addresses for
> > > +		dcc. DCC hardware provides feature to loop among multiple
> > > +		addresses. For debugging purposes register values need to
> > > +		be captured repeatedly in a loop. On giving the loop count
> > > +		as n, the value at address will be captured n times in a
> > > +		loop. At most 8 loop addresses can be configured at once.
> > > +		Example:
> > > +		echo <1> <2> <3> > /sys/kernel/debug/dcc/../config_loop
> > > +		1->The loop count, the number of times the value of the
> > > +		   addresses will be captured.
> > > +		2->The address count, total number of addresses to be
> > > +		   entered in this instruction.
> > > +		3->The series of addresses to be entered separated by a
> > > +		   space like <addr1> <addr2>... and so on.
> > > +
> > > +What:          /sys/kernel/debug/dcc/.../config_read_write
> > > +Date:          March 2022
> > > +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> > > +Description:
> > > +		This file is used to read the value of the register
> > > +		and then write the value given as an argument to the
> > > +		register address. The address argument should be given
> > > +		of the form <addr> <mask> <value>.For debugging purposes
> > > +		sometimes we need to first read from a register and then
> > > +		set some values to the register.
> > > +		Example:
> > > +		echo <1> <2> <3> > /sys/kernel/debug/dcc/.../config_read_write
> > > +		1->The address which needs to be considered for read then write.
> > > +		2->The value that needs to be written on the address.
> > > +		3->The mask of the value to be written.
> > > +
> > > +What:		/sys/kernel/debug/dcc/.../ready
> > > +Date:		March 2022
> > > +Contact	Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> > > +Description:
> > > +		This file is used to check the status of the dcc
> > > +		hardware if it's ready to take the inputs. A 0
> > > +		here indicates dcc is in a ready condition.
> > > +		Example:
> > > +		cat /sys/kernel/debug/dcc/.../ready
> > > +
> > > +What:		/sys/kernel/debug/dcc/.../curr_list
> > I still don't like the idea of having a single set of files to interface
> > with all N lists. I think you should discover how many lists you have
> > and create N directories of files, each on operating on a given list.
> 
> As explained before there cannot be different files based on lists as
> the number of lists to be used varies across platforms where DCC is
> used.

Isn't this dcc->nr_link_list?

> Also we are giving the user the flexibility to configure
> multiple lists at one go whereas the dumps are collected in the form
> of separate lists that are configured by the user.
> 

I'm not sure I follow what you're trying to say here.

If we determine that nr_link_list is 2, you create a directory named 0
and one named 1, fill them with the interface files to operate on list 0
and list 1 respectively. Then you still allow the user to configure and
enable the 2 available lists?

The difference is that it's clear how many lists you have and it's clear
when you poke at 0/config_read that you're referring to the first list
and poking 0/enable will mean the same list - there's no curr_list to
mux between them in the interface.

Regards,
Bjorn

> > 
> > > +Date:		March 2022
> > > +Contact:	Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> > > +Description:
> > > +		This attribute is used to enter the linklist to be
> > > +		used while appending addresses. The range of values
> > > +		for this is advertised either by a register or is
> > > +		predefined. Max value for this can be till 8.
> > > +		Example:
> > > +		echo 0 > /sys/kernel/debug/dcc/...curr_list
> > > +
> > Regards,
> > Bjorn
Souradeep Chowdhury May 16, 2022, 10:32 a.m. UTC | #7
On 5/6/2022 8:23 AM, Bjorn Andersson wrote:
> On Thu 05 May 05:53 PDT 2022, Souradeep Chowdhury wrote:
>
>> On 5/3/2022 11:33 PM, Bjorn Andersson wrote:
>>> On Thu 03 Mar 00:27 CST 2022, 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 user inputs via the sysfs interface. The user gives
>>>> addresses as inputs and these addresses are stored in the
>>>> dcc sram. In case of a system crash or a manual software
>>>> trigger by the user through the debugfs interface,
>>>> the dcc captures and stores the values at these addresses.
>>>> This patch contains the driver which has all the methods
>>>> pertaining to the debugfs interface, auxiliary functions to
>>>> support all the four fundamental operations of dcc namely
>>>> read, write, read/modify/write and loop. The probe method
>>>> here instantiates all the resources necessary for dcc to
>>>> operate mainly the dedicated dcc sram where it stores the
>>>> values. The DCC driver can be used for debugging purposes
>>>> without going for a reboot since it can perform software
>>>> triggers as well based on user inputs.
>>>>
>>>> Also added the documentation for debugfs entries and explained
>>>> the functionalities of each debugfs file that has been created
>>>> for dcc.
>>>>
>>>> The following is the justification of using debugfs interface
>>>> over the other alternatives like sysfs/ioctls
>>>>
>>>> i) As can be seen from the debugfs attribute descriptions,
>>>> some of the debugfs attribute files here contains multiple
>>>> arguments which needs to be accepted from the user. This goes
>>>> against the design style of sysfs.
>>>>
>>>> ii) The user input patterns have been made simple and convenient
>>>> in this case with the use of debugfs interface as user doesn't
>>>> need to shuffle between different files to execute one instruction
>>>> as was the case on using other alternatives.
>>>>
>>>> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>>>> ---
>>>>    Documentation/ABI/testing/debugfs-driver-dcc |  124 +++
>>>>    drivers/soc/qcom/Kconfig                     |    8 +
>>>>    drivers/soc/qcom/Makefile                    |    1 +
>>>>    drivers/soc/qcom/dcc.c                       | 1465 ++++++++++++++++++++++++++
>>>>    4 files changed, 1598 insertions(+)
>>>>    create mode 100644 Documentation/ABI/testing/debugfs-driver-dcc
>>>>    create mode 100644 drivers/soc/qcom/dcc.c
>>>>
>>>> diff --git a/Documentation/ABI/testing/debugfs-driver-dcc b/Documentation/ABI/testing/debugfs-driver-dcc
>>>> new file mode 100644
>>>> index 0000000..70029ab
>>>> --- /dev/null
>>>> +++ b/Documentation/ABI/testing/debugfs-driver-dcc
>>>> @@ -0,0 +1,124 @@
>>>> +What:          /sys/kernel/debug/dcc/.../trigger
>>>> +Date:          March 2022
>>>> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>>>> +Description:
>>>> +		This is the debugfs interface for manual software
>>>> +		triggers. The user can simply enter a 1 against
>>>> +		the debugfs file and enable a manual trigger.
>>>> +		Example:
>>>> +		echo  1 > /sys/kernel/debug/dcc/.../trigger
>>>> +
>>>> +What:          /sys/kernel/debug/dcc/.../enable
>>>> +Date:          March 2022
>>>> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>>>> +Description:
>>>> +		This debugfs interface is used for enabling the
>>>> +		the dcc hardware. On enabling the dcc, all the
>>>> +		addresses entered by the user is written into
>>>> +		dcc sram which is read by the dcc hardware on
>>>> +		manual or crash induced triggers.
>>>> +		Example:
>>>> +		echo  0 > /sys/bus/platform/devices/.../enable
>>>> +		(disable dcc)
>>>> +		echo  1 > /sys/bus/platform/devices/.../enable
>>>> +		(enable dcc)
>>>> +
>>>> +What:          /sys/kernel/debug/dcc/.../config_read
>>> As mentioned last time, I don't like this interface of having 6 files
>>> that the user can write to in order to append items in the currently
>>> selected linked list.
>>>
>>> Why can't this be a single "config" which takes a multiline string of
>>> operations? (Bonus point for supporting appending to the list).
>>>
>>>
>>> This would also serve as a natural place to dump the linked list back to
>>> the user for inspection.
>> Following is the justification of having multiple files in debugfs
>>
>> 1-> Since there are fundamentally 4 instructions for DCC, Read,Write,Read
>> and then Write and Loop,having separate debugfs files for the same makes it
>>
>> convenient for the user to use this tool and also to document.This
>> also is consistent with the design principles of debugfs as it supports
>> logical segregation of Debugfs files based on the user instructions.
>>
> So say the user of DCC wants to read a register 10 times, they know that
> the DCC operates on lists of operations, so they want to tell the
> computer "read X, loop 10 times".
>
> But the API is "write to loop file", "write to read file" and "write to
> loop file".
>
> You're achieving the same thing, but the user and the driver thinks in
> terms of lists of operations and inbetween is a API which provides
> "logical segregation" of the different parts of that list.
>
>> 2-> We are maintaining a common linkedlist inside the driver and it can be
>> viewed by the user through the "config_read" debugfs file. Will be adding
>> this to the documentation as well.
>>
> So I use a mix of config_* files to build the lists, and then
> config_read is used to look at the list?
>
> So some config_* files will when written append to the list, some
> config_* files will perform some action (e.g. reset) and reading some
> config_* files will return something useful.
>
>> Let me know your thoughts regarding the above.
>>
> I am not convinced that having multiple files provides a nice user
> interface. But I certain that the use of the word "config" in various
> different ways is wrong.
>
> There are files in the interface which purpose is to append items to the
> linked list, name them append_*. Reading the appended items on the list
> should not be overloaded on one of the "append" files.
>
>
> So perhaps:
>
> append_read
> append_write
> append_rmw
> append_loop
> config (to dump the current config)
> enable
> ready
> reset
> trigger
>
>
> But then looking at the append_* functions again and the examples below.
> You could easily have a single append which takes read, write, rmw and
> loop as a first keyword - and build a crude parser based on sscanf to
> decode the strings.
>
> Then all append_* becomes "append", which is a single file for adding to
> the list and "config" is a single point to read the current list.
> Perhaps you could name this file just "config", reading will dump the
> list, writing will append to the list.
>
> Here you would have a cleaner interface.
>
>
> But as you write your own fops you could differentiate between write
> and append, so you could make this slightly cleaner by manifesting the
> "append" part by allowing the user to do:
>
>    echo read 1, 2, 3 > config
>    echo read 4, 5, 6 >> config
>
> Which clearly shows that the first writes to the config and second
> appends to the current config. With this interface reset would become:
>
>    echo > config
>
> You can still require that only single operation is written to or
> appended to the list per write - to allow you to continue to rely on the
> crude sscanf based parser.
>
> With this your interface is reduced to:
>
> config
> enable
> ready
> trigger

Ack. Will create a single config for this interface.

>
>>>> +Date:          March 2022
>>>> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>>>> +Description:
>>>> +		This stores the addresses of the registers which
>>>> +		needs to be read in case of a hardware crash or
>>>> +		manual software triggers. The address entered here
>>>> +		are considered under read type instruction.
>>>> +		Example:
>>>> +		echo <1> <2> <3> >/sys/kernel/debug/dcc/../config_read
>>>> +		1->Address to be considered for reading the value.
>>>> +		2->The word count of the addresses, read n words
>>>> +		   starting from address <1>.
>>>> +		3->Can be a 1 or 0 which indicates if it is apb or ahb
>>>> +		bus respectively.
>>>> +
>>>> +What:          /sys/kernel/debug/dcc/.../config_write
>>>> +Date:          March 2022
>>>> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>>>> +Description:
>>>> +		This file allows user to write a value to the register
>>>> +		address given as argument. The reason for this feature
>>>> +		of dcc is that for accessing certain registers it is
>>>> +		necessary to set some bits of some other register.
>>>> +		Example:
>>>> +		echo <1> <2> <3> > /sys/bus/platform/devices/.../config_write
>>>> +		1->Address to be considered for writing the value.
>>>> +		2->The value that needs to be written at the location.
>>>> +		3->Can be a 1 or 0 which indicates if it is apb or ahb
>>>> +		bus respectively.
>>>> +
>>>> +What:          /sys/kernel/debug/dcc/.../config_reset
>>>> +Date:          March 2022
>>>> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>>>> +Description:
>>>> +		This file is used to reset the configuration of
>>>> +		a dcc driver to the default configuration. This
>>>> +		means that all the previous addresses stored in
>>>> +		the driver gets removed and user needs to enter
>>>> +		the address values from the start.
>>>> +		Example:
>>>> +		echo  1 > /sys/bus/platform/devices/.../config_reset
>>>> +
>>>> +What:          /sys/kernel/debug/dcc/.../config_loop
>>>> +Date:		March 2022
>>>> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>>>> +Description:
>>>> +		This file is used to enter the loop type addresses for
>>>> +		dcc. DCC hardware provides feature to loop among multiple
>>>> +		addresses. For debugging purposes register values need to
>>>> +		be captured repeatedly in a loop. On giving the loop count
>>>> +		as n, the value at address will be captured n times in a
>>>> +		loop. At most 8 loop addresses can be configured at once.
>>>> +		Example:
>>>> +		echo <1> <2> <3> > /sys/kernel/debug/dcc/../config_loop
>>>> +		1->The loop count, the number of times the value of the
>>>> +		   addresses will be captured.
>>>> +		2->The address count, total number of addresses to be
>>>> +		   entered in this instruction.
>>>> +		3->The series of addresses to be entered separated by a
>>>> +		   space like <addr1> <addr2>... and so on.
>>>> +
>>>> +What:          /sys/kernel/debug/dcc/.../config_read_write
>>>> +Date:          March 2022
>>>> +Contact:       Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>>>> +Description:
>>>> +		This file is used to read the value of the register
>>>> +		and then write the value given as an argument to the
>>>> +		register address. The address argument should be given
>>>> +		of the form <addr> <mask> <value>.For debugging purposes
>>>> +		sometimes we need to first read from a register and then
>>>> +		set some values to the register.
>>>> +		Example:
>>>> +		echo <1> <2> <3> > /sys/kernel/debug/dcc/.../config_read_write
>>>> +		1->The address which needs to be considered for read then write.
>>>> +		2->The value that needs to be written on the address.
>>>> +		3->The mask of the value to be written.
>>>> +
>>>> +What:		/sys/kernel/debug/dcc/.../ready
>>>> +Date:		March 2022
>>>> +Contact	Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>>>> +Description:
>>>> +		This file is used to check the status of the dcc
>>>> +		hardware if it's ready to take the inputs. A 0
>>>> +		here indicates dcc is in a ready condition.
>>>> +		Example:
>>>> +		cat /sys/kernel/debug/dcc/.../ready
>>>> +
>>>> +What:		/sys/kernel/debug/dcc/.../curr_list
>>> I still don't like the idea of having a single set of files to interface
>>> with all N lists. I think you should discover how many lists you have
>>> and create N directories of files, each on operating on a given list.
>> As explained before there cannot be different files based on lists as
>> the number of lists to be used varies across platforms where DCC is
>> used.
> Isn't this dcc->nr_link_list?
Yes. This varies across different SoCs and also user may not have access 
to all the lists supported by DCC.
>
>> Also we are giving the user the flexibility to configure
>> multiple lists at one go whereas the dumps are collected in the form
>> of separate lists that are configured by the user.
>>
> I'm not sure I follow what you're trying to say here.
>
> If we determine that nr_link_list is 2, you create a directory named 0
> and one named 1, fill them with the interface files to operate on list 0
> and list 1 respectively. Then you still allow the user to configure and
> enable the 2 available lists?
>
> The difference is that it's clear how many lists you have and it's clear
> when you poke at 0/config_read that you're referring to the first list
> and poking 0/enable will mean the same list - there's no curr_list to
> mux between them in the interface.

So from DCC hardware perspective, it needs to be told that "list n 
begins at x and ends at y". That is why the limitation of DCC

hardware is that the lists can only be configured sequentially and not 
in a overlapping manner. So for example we need to enter

all the addresses to be stored for list n before jumping to any other 
lists. Seeing this limitation, will it be advantageous to have

different set of files for different lists as we will be configuring 
them sequentially anyway?

>
> Regards,
> Bjorn
>
>>>> +Date:		March 2022
>>>> +Contact:	Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>>>> +Description:
>>>> +		This attribute is used to enter the linklist to be
>>>> +		used while appending addresses. The range of values
>>>> +		for this is advertised either by a register or is
>>>> +		predefined. Max value for this can be till 8.
>>>> +		Example:
>>>> +		echo 0 > /sys/kernel/debug/dcc/...curr_list
>>>> +
>>> Regards,
>>> Bjorn