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

[Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>]

Thanks Krzysztof for giving clarity on ask and reviewing this change.

On 12/2/2024 12:49 PM, Krzysztof Kozlowski wrote:
> On 02/12/2024 05:00, Mukesh Kumar Savaliya wrote:
>> Hi Krzysztof,
>>
>> On 11/29/2024 8:44 PM, Krzysztof Kozlowski wrote:
>>> On 29/11/2024 15:43, Mukesh Kumar Savaliya wrote:
>>>> Adds qcom,shared-se flag usage. Use this flag when I2C serial controller
>>>> needs to be shared in multiprocessor system(APPS,Modem,ADSP) environment.
>>>>
>>>> SE(Serial Engine HW controller acting as protocol master controller) is an
>>>> I2C controller. Basically a programmable SERDES(serializer/deserializer)
>>>> coupled with data DMA entity, capable in handling a bus protocol, and data
>>>> moves to/from system memory.
>>>>
>>>> Two clients from different processors can share an I2C controller 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.
>>>>
>>>> Transfer gets serialized by Lock TRE + DMA xfer + Unlock TRE at HW level.
>>>>
>>>> Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>
>>>> ---
>>>>    .../devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml       | 8 ++++++++
>>>>    1 file changed, 8 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..88682a333399 100644
>>>> --- a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
>>>> +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
>>>> @@ -60,6 +60,14 @@ properties:
>>>>      power-domains:
>>>>        maxItems: 1
>>>>    
>>>> +  qcom,shared-se:
>>>> +    description: True if I2C controller is shared between two or more system processors.
>>>> +        SE(Serial Engine HW controller working as protocol master controller) is an
>>>> +        I2C controller. Basically, a programmable SERDES(serializer/deserializer)
>>>> +        coupled with data DMA entity, capable in handling a bus protocol, and data
>>>> +        moves to/from system memory.
>>> I replied why I NAK it. You did not really address my concerns, but
>>> replied with some generic statement. After that generic statement you
>>> gave me exactly 0 seconds to react and you sent v5.
>>>
>> Sorry for 0 seconds, i thought of addressing comment and uploading it
>> new patch as i wanted to explain SE. whatever i have added for SE
>> explanation is in qualcomm hardware programming guide document.
>>> Really 0 seconds to respond to your comment, while you give yourself
>>> days to respond to my comments.
>>>
>>> This is not how it works.
>>>
>> Sure, let me first conclude here what exactly should be done.
>>> NAK
>>>
>>> Implement previous feedback. Don't send any new versions before you
>>> understand what you have to do and get some agreement with reviewers.
>>>
>> Sure, this is definitely a good way. what did i do for previous comment ?
>> I have opened SE and expanded, explained.
>>
>> which statement or explanation should i rephrase ? Is it description
>> statement from this yaml file ? Could you please suggested better word
>> instead of shared-se if this flag name is not suitable ?
>>
>> I could not get this ask -
>> "There are few of such flags already and there are some patches adding
>> it in different flavors."
> 
> Come with one flag or enum, if needed, covering all your cases like this.
> 
Let me explain, this feature is one of the additional software case 
adding on base protocol support. if we dont have more than one usecase 
or repurposing this feature, why do we need to add enums ? I see one 
flag gpi_mode but it's internal to driver not exposed to user or expose 
any usecase/feature.

Below was our earlier context, just wanted to add for clarity.
--
 > 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.
 >
IP blocks like SE can be shared. Here we are talking about I2C sharing.
In future it can be SPI sharing. But design wise it fits better to add
flag per SE node. Same we shall be adding for SPI too in future.

Please let me know your further suggestions.
--

As we want to finalize agreement on this dt-bindings patch, will wait 
for agreement and confirmation before V6.

> Best regards,
> Krzysztof