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

Bjorn Andersson
2024-09-30T03:20:50Z

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.

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

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

Regards,
Bjorn

> + type: boolean
> +
> reg:
> maxItems: 1
>
> --
> 2.25.1
>