[v2,1/9] dt-bindings: arm: Add support for DSB element

  Add property "qcom,dsb-elem-size" to support DSB(Discrete Single
Bit) element for TPDA. Specifies the DSB element size supported
by each monitor connected to the aggregator on each port. Should
be specified in pairs (port, dsb element size).

Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
Signed-off-by: Tao Zhang <taozha@qti.qualcomm.com>
---
 .../bindings/arm/qcom,coresight-tpda.yaml          | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
  

On 19/01/2023 08:41, Tao Zhang wrote:
> Add property "qcom,dsb-elem-size" to support DSB(Discrete Single
> Bit) element for TPDA. Specifies the DSB element size supported
> by each monitor connected to the aggregator on each port. Should
> be specified in pairs (port, dsb element size).
> 
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> Signed-off-by: Tao Zhang <taozha@qti.qualcomm.com>

You are the same person and it is still the same organization
(Qualcomm), right? Only one SoB.

> ---
>  .../bindings/arm/qcom,coresight-tpda.yaml          | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
> index 2ec9b5b..298db7f 100644
> --- a/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
> @@ -58,6 +58,26 @@ properties:
>      minItems: 1
>      maxItems: 2
>  
> +  qcom,dsb-element-size:
> +    description: |
> +      Specifies the DSB(Discrete Single Bit) element size supported by
> +      each monitor connected to the aggregator on each port. Should be
> +      specified in pairs <port, dsb element size>.

s/port/port number/

> +
> +      Note: The maximum value of the port number depends on how many
> +      input ports the current TPDA has. DSB element size currently only
> +      supports 32-bit and 64-bit.
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    items:

Are some reasonable maxItems known?

> +      items:
> +        - description: |
> +            "port" indicates TPDA input port number

What is "port"? You quoted it like it was some name of variable or
property. Where is then?

> +          minimum: 0
> +        - description: |
> +            "dsb element size" indicates dsb element size

"A" indicates A. This sentence does not make sense.

Also missing units.

s/dsb/DSB/

> +          minimum: 0
> +          maximum: 64
> +
>    clocks:
>      maxItems: 1
>  
> @@ -100,6 +120,8 @@ examples:
>         compatible = "qcom,coresight-tpda", "arm,primecell";
>         reg = <0x6004000 0x1000>;
>  
> +       qcom,dsb-element-size = <0 32>;
> +
>         clocks = <&aoss_qmp>;
>         clock-names = "apb_pclk";
>  

Best regards,
Krzysztof
  

Hi Krzysztof,

On 1/19/2023 6:44 PM, Krzysztof Kozlowski wrote:
> On 19/01/2023 08:41, Tao Zhang wrote:
>> Add property "qcom,dsb-elem-size" to support DSB(Discrete Single
>> Bit) element for TPDA. Specifies the DSB element size supported
>> by each monitor connected to the aggregator on each port. Should
>> be specified in pairs (port, dsb element size).
>>
>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>> Signed-off-by: Tao Zhang <taozha@qti.qualcomm.com>
> You are the same person and it is still the same organization
> (Qualcomm), right? Only one SoB.
I will change and update this in the next patch series.
>
>> ---
>>   .../bindings/arm/qcom,coresight-tpda.yaml          | 22 ++++++++++++++++++++++
>>   1 file changed, 22 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
>> index 2ec9b5b..298db7f 100644
>> --- a/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
>> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
>> @@ -58,6 +58,26 @@ properties:
>>       minItems: 1
>>       maxItems: 2
>>   
>> +  qcom,dsb-element-size:
>> +    description: |
>> +      Specifies the DSB(Discrete Single Bit) element size supported by
>> +      each monitor connected to the aggregator on each port. Should be
>> +      specified in pairs <port, dsb element size>.
> s/port/port number/

It should be "port number" here.

I will change "<port, dsb element size>" to "<port number, DSB element 
size>" in the next patch series.

>> +
>> +      Note: The maximum value of the port number depends on how many
>> +      input ports the current TPDA has. DSB element size currently only
>> +      supports 32-bit and 64-bit.
>> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
>> +    items:
> Are some reasonable maxItems known?

This is related to hardware design, depending on how many input ports 
the TPDA has.

We cannot limit it to a reasonable maximum value from the software.

According to the existing hardware design, TPDA with the most input 
ports has about 30 input ports.

But there may be TPDA with more input ports.

>
>> +      items:
>> +        - description: |
>> +            "port" indicates TPDA input port number
> What is "port"? You quoted it like it was some name of variable or
> property. Where is then?

The "port" here refers to the port number of other Coresight devices 
connected to the TPDA input port.

I will change and update it in the next patch series.

>> +          minimum: 0
>> +        - description: |
>> +            "dsb element size" indicates dsb element size
> "A" indicates A. This sentence does not make sense.
>
> Also missing units.
>
> s/dsb/DSB/
"DSB element size" indicate the size of the element in DSB. DSB(Discrete 
Single

Bit) is a data collection unit.

I will change and update it in the next patch series.

>
>> +          minimum: 0
>> +          maximum: 64
>> +
>>     clocks:
>>       maxItems: 1
>>   
>> @@ -100,6 +120,8 @@ examples:
>>          compatible = "qcom,coresight-tpda", "arm,primecell";
>>          reg = <0x6004000 0x1000>;
>>   
>> +       qcom,dsb-element-size = <0 32>;
>> +
>>          clocks = <&aoss_qmp>;
>>          clock-names = "apb_pclk";
>>   
> Best regards,
> Krzysztof

Best,

Tao
  

On 19/01/2023 07:41, Tao Zhang wrote:
> Add property "qcom,dsb-elem-size" to support DSB(Discrete Single
> Bit) element for TPDA. Specifies the DSB element size supported
> by each monitor connected to the aggregator on each port. Should
> be specified in pairs (port, dsb element size).
> 
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> Signed-off-by: Tao Zhang <taozha@qti.qualcomm.com>
> ---
>   .../bindings/arm/qcom,coresight-tpda.yaml          | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
> index 2ec9b5b..298db7f 100644
> --- a/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
> @@ -58,6 +58,26 @@ properties:
>       minItems: 1
>       maxItems: 2
>   
> +  qcom,dsb-element-size:
> +    description: |
> +      Specifies the DSB(Discrete Single Bit) element size supported by
> +      each monitor connected to the aggregator on each port. Should be
> +      specified in pairs <port, dsb element size>.

Isn't this a property of the TPDM connected to the port ? i.e. the DSB 
size ? Thus shouldn't this be part of the TPDM device (and the TPDA will 
be able to find it from the TPDM device) ?


Suzuki
  

On 19/01/2023 07:41, Tao Zhang wrote:
> Add property "qcom,dsb-elem-size" to support DSB(Discrete Single
> Bit) element for TPDA. Specifies the DSB element size supported
> by each monitor connected to the aggregator on each port. Should
> be specified in pairs (port, dsb element size).
> 
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> Signed-off-by: Tao Zhang <taozha@qti.qualcomm.com>
> ---
>   .../bindings/arm/qcom,coresight-tpda.yaml          | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
> index 2ec9b5b..298db7f 100644
> --- a/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
> @@ -58,6 +58,26 @@ properties:
>       minItems: 1
>       maxItems: 2
>   
> +  qcom,dsb-element-size:
> +    description: |
> +      Specifies the DSB(Discrete Single Bit) element size supported by
> +      each monitor connected to the aggregator on each port. Should be
> +      specified in pairs <port, dsb element size>.
> +
> +      Note: The maximum value of the port number depends on how many
> +      input ports the current TPDA has. DSB element size currently only
> +      supports 32-bit and 64-bit.
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    items:
> +      items:
> +        - description: |
> +            "port" indicates TPDA input port number
> +          minimum: 0
> +        - description: |
> +            "dsb element size" indicates dsb element size
> +          minimum: 0
> +          maximum: 64
> +
>     clocks:
>       maxItems: 1
>   
> @@ -100,6 +120,8 @@ examples:
>          compatible = "qcom,coresight-tpda", "arm,primecell";
>          reg = <0x6004000 0x1000>;
>   
> +       qcom,dsb-element-size = <0 32>;
> +

If we go down this route,

nit: please could you provide a bit more complex example, e.g. with 2 
entries ?

Suzuki

>          clocks = <&aoss_qmp>;
>          clock-names = "apb_pclk";
>
  

Hi Suzuki,

在 2/22/2023 2:11 AM, Suzuki K Poulose 写道:
> On 19/01/2023 07:41, Tao Zhang wrote:
>> Add property "qcom,dsb-elem-size" to support DSB(Discrete Single
>> Bit) element for TPDA. Specifies the DSB element size supported
>> by each monitor connected to the aggregator on each port. Should
>> be specified in pairs (port, dsb element size).
>>
>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>> Signed-off-by: Tao Zhang <taozha@qti.qualcomm.com>
>> ---
>>   .../bindings/arm/qcom,coresight-tpda.yaml          | 22 
>> ++++++++++++++++++++++
>>   1 file changed, 22 insertions(+)
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml 
>> b/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
>> index 2ec9b5b..298db7f 100644
>> --- a/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
>> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
>> @@ -58,6 +58,26 @@ properties:
>>       minItems: 1
>>       maxItems: 2
>>   +  qcom,dsb-element-size:
>> +    description: |
>> +      Specifies the DSB(Discrete Single Bit) element size supported by
>> +      each monitor connected to the aggregator on each port. Should be
>> +      specified in pairs <port, dsb element size>.
>
> Isn't this a property of the TPDM connected to the port ? i.e. the DSB 
> size ? Thus shouldn't this be part of the TPDM device (and the TPDA 
> will be able to find it from the TPDM device) ?
>
Since  the port number is about the input port of TPDA, this property 
needs to be configured in the TPDA-related settings.

In our current design, TPDM doesn't have a register to record the DSB 
size, and TPDA cannot actively know the TPDM DSB size from the TPDM device.

>
> Suzuki

Best,

Tao
  

On 27/02/2023 03:07, Tao Zhang wrote:
> Hi Suzuki,
> 
> 在 2/22/2023 2:11 AM, Suzuki K Poulose 写道:
>> On 19/01/2023 07:41, Tao Zhang wrote:
>>> Add property "qcom,dsb-elem-size" to support DSB(Discrete Single
>>> Bit) element for TPDA. Specifies the DSB element size supported
>>> by each monitor connected to the aggregator on each port. Should
>>> be specified in pairs (port, dsb element size).
>>>
>>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>>> Signed-off-by: Tao Zhang <taozha@qti.qualcomm.com>
>>> ---
>>>   .../bindings/arm/qcom,coresight-tpda.yaml          | 22 
>>> ++++++++++++++++++++++
>>>   1 file changed, 22 insertions(+)
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml 
>>> b/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
>>> index 2ec9b5b..298db7f 100644
>>> --- a/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
>>> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
>>> @@ -58,6 +58,26 @@ properties:
>>>       minItems: 1
>>>       maxItems: 2
>>>   +  qcom,dsb-element-size:
>>> +    description: |
>>> +      Specifies the DSB(Discrete Single Bit) element size supported by
>>> +      each monitor connected to the aggregator on each port. Should be
>>> +      specified in pairs <port, dsb element size>.
>>
>> Isn't this a property of the TPDM connected to the port ? i.e. the DSB 
>> size ? Thus shouldn't this be part of the TPDM device (and the TPDA 
>> will be able to find it from the TPDM device) ?
>>
> Since  the port number is about the input port of TPDA, this property 
> needs to be configured in the TPDA-related settings.

That is because, you chose to describe the property of TPDM in TPDA ?
Instead if you do it as follows :

  tpdm {
        qcom,tpdm-dsb-elemenet-size = <32>
        out_ports {

	port {
              remote-endpoint=<&tpda_port_number>;
	}
  }

  tpda {
        in_ports {
            port {
                  remote-endpoint=<&tpdm0_port0>;
            }
        }
  }


The TPDA driver can figure out the "port" that a given TPDM is connected
to and thus find out the DSB size. For the tpda driver, pdata->conns
could hold the reference to the TPDM device and thus fetch the DSB size.
(Note: James is working on a patch to add input port connections to the
platform data).

Suzuki