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

Konrad Dybcio
2024-09-27T11:20:47Z

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