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

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

Thanks Bjorn !

On 9/30/2024 8:50 AM, Bjorn Andersson wrote:
> On Fri, Sep 27, 2024 at 12:01:05PM GMT, Mukesh Kumar Savaliya wrote:
>> Adds qcom,shared-se flag usage. Use this when particular I2C serial
>> controller needs to be shared between two subsystems.
>>
> 
> https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> 
>> SE = Serial Engine, meant for I2C controller here.
>> TRE = Transfer Ring Element, refers to Queued Descriptor.
>> SS = Subsystems (APPS processor, Modem, TZ, ADSP etc).
> 
> Expressing yourself in terms of abbreviations just makes the text harder
> to read. The dictionary is nice, but I don't see that it adds value to
> introduce these abbreviations with the reader.
Sure, thought it will ease the explanations of abbreviations. I learnt 
that not to use it and directly use complete name in opensource way.
> 
>>
>> 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.
>>
> 
> Don't describe your problem using a bullet-point list. You should be
> able to express it in a flowing English sentence/paragraph.
Sure Bjorn.
> 
>> 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).
> 
> I see no value in establishing the "SS" abbreviation here, what would be
> useful is to write this sentence such that it establishes the "SE"
> abbreviation (to avoid having to expand the property).
> 
> On the other hand, it's a boolean property in a serial-engine node, so
> I don't know if it's worth repeating "se" here. "qcom,is-shared" sounds
> like a better boolean in a se-node.
> 
Sure, Done. I thought shared-se will make it understand that SE is 
shared. I will modify to qcom,is-shared as recommended.
Also i am removing SS and make it multiprocessor as asked by Bryan.
> Regards,
> Bjorn
> 
>> +    type: boolean
>> +
>>     reg:
>>       maxItems: 1
>>   
>> -- 
>> 2.25.1
>>