[V1,0/1] soc: qcom: dcc: Add QAD, Cti-trigger and Bootconfig support for Data Capture and Compare(DCC)

Message ID cover.1673270769.git.quic_schowdhu@quicinc.com
Headers
Series soc: qcom: dcc: Add QAD, Cti-trigger and Bootconfig support for Data Capture and Compare(DCC) |

Message

Souradeep Chowdhury Jan. 9, 2023, 2:02 p.m. UTC
  This patch adds the Bootconfig, QAD and CTI-Trigger support for DCC.

1.Bootconfig

Bootconfig parser has been added to DCC driver so that the register addresses
can be configured during boot-time. This is used to debug crashes that can happen
during boot-time. The expected format of a bootconfig is as follows:-

dcc_config {
	link_list_0 {
		qcom-curr-link-list = <The list number to configure>
		qcom-link-list =  <Address as same format as dcc separated by '_'>,
	}
}

Example:

dcc_config {
	link_list_0 {
		qcom-curr-link-list = 6
		qcom-link-list = R_0x1781005c_1_apb,
				 R_0x1782005c_1_apb
	}
	link_list_1 {
		qcom-curr-link-list = 5
		qcom-link-list = R_0x1784005c_1_apb
	}
}

2.QAD

QAD can be enabled as a part of debugfs file under each individual list folder.
QAD is used to specify the access control for DCC configurations, on enabling
it the access control to dcc configuration space is restricted.

3.CTI-trigger

CTI trigger is used to enable the Cross trigger interface for DCC. On enabling
CTI trigger the dcc software trigger can be done by writing to CTI trig-out.
Also the hwtrigger debugfs file is created which needs to be disabled for enabling
CTI-trigger.

Changes in V1

*Fixed the W=1 warnings in V0 of the patch

Souradeep Chowdhury (1):
  soc: qcom: dcc: Add QAD, Ctitrigger and Bootconfig support for DCC

 Documentation/ABI/testing/debugfs-driver-dcc |  24 +++
 drivers/soc/qcom/dcc.c                       | 284 ++++++++++++++++++++++++++-
 2 files changed, 304 insertions(+), 4 deletions(-)

--
2.7.4
  

Comments

Bjorn Andersson Jan. 9, 2023, 3:18 p.m. UTC | #1
On Mon, Jan 09, 2023 at 07:32:25PM +0530, Souradeep Chowdhury wrote:
> This patch adds the Bootconfig, QAD and CTI-Trigger support for DCC.
> 

As with the other patch, please move your motivation into the commit
message of the patch.

That said, this seems to be 3 different topics, and hence should be
three different patches.

> 1.Bootconfig
> 
> Bootconfig parser has been added to DCC driver so that the register addresses
> can be configured during boot-time. This is used to debug crashes that can happen
> during boot-time. The expected format of a bootconfig is as follows:-
> 
> dcc_config {
> 	link_list_0 {
> 		qcom-curr-link-list = <The list number to configure>
> 		qcom-link-list =  <Address as same format as dcc separated by '_'>,
> 	}
> }
> 
> Example:
> 
> dcc_config {
> 	link_list_0 {

The name of the node does not seem to have any significance; this could
be nice to mention. I also think it would set a good precedence if you
used the number of the qcom-curr-link-list in the node name, rather than
just an iterator...

> 		qcom-curr-link-list = 6
> 		qcom-link-list = R_0x1781005c_1_apb,
> 				 R_0x1782005c_1_apb
> 	}
> 	link_list_1 {
> 		qcom-curr-link-list = 5
> 		qcom-link-list = R_0x1784005c_1_apb
> 	}
> }
> 
> 2.QAD
> 
> QAD can be enabled as a part of debugfs file under each individual list folder.
> QAD is used to specify the access control for DCC configurations, on enabling
> it the access control to dcc configuration space is restricted.
> 

Who is locked out from this restricted access? Please mention why this
is a good thing.

> 3.CTI-trigger
> 
> CTI trigger is used to enable the Cross trigger interface for DCC. On enabling
> CTI trigger the dcc software trigger can be done by writing to CTI trig-out.
> Also the hwtrigger debugfs file is created which needs to be disabled for enabling
> CTI-trigger.
> 

Please mention why hwtrigger needs to be disabled, and why does it need
to be disabled?

> Changes in V1
> 
> *Fixed the W=1 warnings in V0 of the patch

Please follow the standard practice of giving your first version of a
patch version 1.

Thanks,
Bjorn

> 
> Souradeep Chowdhury (1):
>   soc: qcom: dcc: Add QAD, Ctitrigger and Bootconfig support for DCC
> 
>  Documentation/ABI/testing/debugfs-driver-dcc |  24 +++
>  drivers/soc/qcom/dcc.c                       | 284 ++++++++++++++++++++++++++-
>  2 files changed, 304 insertions(+), 4 deletions(-)
> 
> --
> 2.7.4
>
  
Souradeep Chowdhury Jan. 10, 2023, 10:59 a.m. UTC | #2
On 1/9/2023 8:48 PM, Bjorn Andersson wrote:
> On Mon, Jan 09, 2023 at 07:32:25PM +0530, Souradeep Chowdhury wrote:
>> This patch adds the Bootconfig, QAD and CTI-Trigger support for DCC.
>>
> 
> As with the other patch, please move your motivation into the commit
> message of the patch.
> 
> That said, this seems to be 3 different topics, and hence should be
> three different patches.

Ack

> 
>> 1.Bootconfig
>>
>> Bootconfig parser has been added to DCC driver so that the register addresses
>> can be configured during boot-time. This is used to debug crashes that can happen
>> during boot-time. The expected format of a bootconfig is as follows:-
>>
>> dcc_config {
>> 	link_list_0 {
>> 		qcom-curr-link-list = <The list number to configure>
>> 		qcom-link-list =  <Address as same format as dcc separated by '_'>,
>> 	}
>> }
>>
>> Example:
>>
>> dcc_config {
>> 	link_list_0 {
> 
> The name of the node does not seem to have any significance; this could
> be nice to mention. I also think it would set a good precedence if you
> used the number of the qcom-curr-link-list in the node name, rather than
> just an iterator...

Ack

> 
>> 		qcom-curr-link-list = 6
>> 		qcom-link-list = R_0x1781005c_1_apb,
>> 				 R_0x1782005c_1_apb
>> 	}
>> 	link_list_1 {
>> 		qcom-curr-link-list = 5
>> 		qcom-link-list = R_0x1784005c_1_apb
>> 	}
>> }
>>
>> 2.QAD
>>
>> QAD can be enabled as a part of debugfs file under each individual list folder.
>> QAD is used to specify the access control for DCC configurations, on enabling
>> it the access control to dcc configuration space is restricted.
>>
> 
> Who is locked out from this restricted access? Please mention why this
> is a good thing.

Ack

> 
>> 3.CTI-trigger
>>
>> CTI trigger is used to enable the Cross trigger interface for DCC. On enabling
>> CTI trigger the dcc software trigger can be done by writing to CTI trig-out.
>> Also the hwtrigger debugfs file is created which needs to be disabled for enabling
>> CTI-trigger.
>>
> 
> Please mention why hwtrigger needs to be disabled, and why does it need
> to be disabled?

Ack

> 
>> Changes in V1
>>
>> *Fixed the W=1 warnings in V0 of the patch
> 
> Please follow the standard practice of giving your first version of a
> patch version 1.

Ack

> 
> Thanks,
> Bjorn
> 
>>
>> Souradeep Chowdhury (1):
>>    soc: qcom: dcc: Add QAD, Ctitrigger and Bootconfig support for DCC
>>
>>   Documentation/ABI/testing/debugfs-driver-dcc |  24 +++
>>   drivers/soc/qcom/dcc.c                       | 284 ++++++++++++++++++++++++++-
>>   2 files changed, 304 insertions(+), 4 deletions(-)
>>
>> --
>> 2.7.4
>>
  
Krzysztof Kozlowski Jan. 10, 2023, 1:29 p.m. UTC | #3
On 09/01/2023 15:02, Souradeep Chowdhury wrote:
> This patch adds the Bootconfig, QAD and CTI-Trigger support for DCC.
> 


(...)

> Souradeep Chowdhury (1):
>   soc: qcom: dcc: Add QAD, Ctitrigger and Bootconfig support for DCC
> 
>  Documentation/ABI/testing/debugfs-driver-dcc |  24 +++
>  drivers/soc/qcom/dcc.c                       | 284 ++++++++++++++++++++++++++-

No need to send the patch to unrelated people. Be sure to rebase it on
latest kernel and then use scripts/get_maintainers.pl to get folks and
lists necessary to CC.

Unless your code adds here DT properties, but then you miss bindings, so
would have to be fixed.

Best regards,
Krzysztof