diff mbox series

[v3,1/4] dt-bindindgs: i2c: qcom,i2c-geni: Document shared flag

Message ID 20240927063108.2773304-2-quic_msavaliy@quicinc.com
State New
Headers show
Series Enable shared SE support over I2C | expand

Commit Message

Mukesh Kumar Savaliya Sept. 27, 2024, 6:31 a.m. UTC
Adds qcom,shared-se flag usage. Use this when particular I2C serial
controller needs to be shared between two subsystems.

SE = Serial Engine, meant for I2C controller here.
TRE = Transfer Ring Element, refers to Queued Descriptor.
SS = Subsystems (APPS processor, Modem, TZ, ADSP etc).

Example :
Two clients from different SS can share an I2C SE for same slave device
OR their owned slave devices.
Assume I2C Slave EEPROM device connected with I2C controller.
Each client from ADSP SS and APPS Linux SS can perform i2c transactions.
This gets serialized by lock TRE + DMA Transfers + Unlock TRE at HW level.

Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>
---
 Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Krzysztof Kozlowski Sept. 27, 2024, 9:24 a.m. UTC | #1
On Fri, Sep 27, 2024 at 12:01:05PM +0530, Mukesh Kumar Savaliya wrote:
> Adds qcom,shared-se flag usage. Use this when particular I2C serial
> controller needs to be shared between two subsystems.
> 
> SE = Serial Engine, meant for I2C controller here.
> TRE = Transfer Ring Element, refers to Queued Descriptor.
> SS = Subsystems (APPS processor, Modem, TZ, ADSP etc).
> 
> Example :
> Two clients from different SS can share an I2C SE for same slave device
> OR their owned slave devices.
> Assume I2C Slave EEPROM device connected with I2C controller.
> Each client from ADSP SS and APPS Linux SS can perform i2c transactions.
> This gets serialized by lock TRE + DMA Transfers + Unlock TRE at HW level.
> 
> Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>
> ---
>  Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
> index 9f66a3bb1f80..3b9b20a0edff 100644
> --- a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
> +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
> @@ -60,6 +60,10 @@ properties:
>    power-domains:
>      maxItems: 1
>  
> +  qcom,shared-se:
> +    description: True if I2C needs to be shared between two or more subsystems(SS).

The "SS" and subsystem should be explained in the binding. Please do not
use some qcom-specific abbreviations here, but explain exactly, e.g.
processors like application processor and DSP.

"se" is also not explained in the binding - please open it and look for
such explanation.

This all should be rephrased to make it clear... We talked about this
and I do not see much of improvements except commit msg, so we are
making circles. I don't know, get someone internally to help you in
upstreaming this.

Is sharing of IP blocks going to be also for other devices? If yes, then
this should be one property for all Qualcomm devices. If not, then be
sure that this is the case because I will bring it up if you come with
one more solution for something else.

Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 27, 2024, 10:01 a.m. UTC | #2
On 27/09/2024 08:31, Mukesh Kumar Savaliya wrote:
> Adds qcom,shared-se flag usage. Use this when particular I2C serial
> controller needs to be shared between two subsystems.
> 

Also, fix the typo in subject prefix. It is dt-bindings.

Best regards,
Krzysztof
Konrad Dybcio Sept. 27, 2024, 11:20 a.m. UTC | #3
On 27.09.2024 11:24 AM, Krzysztof Kozlowski wrote:
> On Fri, Sep 27, 2024 at 12:01:05PM +0530, Mukesh Kumar Savaliya wrote:
>> Adds qcom,shared-se flag usage. Use this when particular I2C serial
>> controller needs to be shared between two subsystems.
>>
>> SE = Serial Engine, meant for I2C controller here.
>> TRE = Transfer Ring Element, refers to Queued Descriptor.
>> SS = Subsystems (APPS processor, Modem, TZ, ADSP etc).
>>
>> Example :
>> Two clients from different SS can share an I2C SE for same slave device
>> OR their owned slave devices.
>> Assume I2C Slave EEPROM device connected with I2C controller.
>> Each client from ADSP SS and APPS Linux SS can perform i2c transactions.
>> This gets serialized by lock TRE + DMA Transfers + Unlock TRE at HW level.
>>
>> Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>
>> ---
>>  Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
>> index 9f66a3bb1f80..3b9b20a0edff 100644
>> --- a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
>> +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
>> @@ -60,6 +60,10 @@ properties:
>>    power-domains:
>>      maxItems: 1
>>  
>> +  qcom,shared-se:
>> +    description: True if I2C needs to be shared between two or more subsystems(SS).
> 
> The "SS" and subsystem should be explained in the binding. Please do not
> use some qcom-specific abbreviations here, but explain exactly, e.g.
> processors like application processor and DSP.
> 
> "se" is also not explained in the binding - please open it and look for
> such explanation.
> 
> This all should be rephrased to make it clear... We talked about this
> and I do not see much of improvements except commit msg, so we are
> making circles. I don't know, get someone internally to help you in
> upstreaming this.
> 
> Is sharing of IP blocks going to be also for other devices? If yes, then
> this should be one property for all Qualcomm devices. If not, then be
> sure that this is the case because I will bring it up if you come with
> one more solution for something else.

As far as I understand, everything that's not protocol-specific (in
this case it would be I2C tunables etc.) is common across all
protocols supported by the serial engine.

Konrad
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
index 9f66a3bb1f80..3b9b20a0edff 100644
--- a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
+++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
@@ -60,6 +60,10 @@  properties:
   power-domains:
     maxItems: 1
 
+  qcom,shared-se:
+    description: True if I2C needs to be shared between two or more subsystems(SS).
+    type: boolean
+
   reg:
     maxItems: 1