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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 27/09/2024 13:20, Konrad Dybcio wrote:
> 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.

Yeah, but I also think about other things like clock controllers, TLMMs,
MMUs and so on. Each of them will get a new "qcom,shared-xxx" property?

I expect Mukesh to solve it in qcom-wide way, not only his one problem.

Best regards,
Krzysztof