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

Mukesh Kumar Savaliya
2024-11-13T16:09:08Z

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
>>