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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <anfqjd5rcslplfqannqqli2k2gnq2p4qsnkyrfgmavdpll7p42@c7o4tdouiav4>
Date: Sun, 29 Sep 2024 22:20:46 -0500
From: Bjorn Andersson <andersson@...nel.org>
To: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
Cc: konrad.dybcio@...aro.org, andi.shyti@...nel.org, 
	linux-arm-msm@...r.kernel.org, dmaengine@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-i2c@...r.kernel.org, conor+dt@...nel.org, agross@...nel.org, 
	devicetree@...r.kernel.org, vkoul@...nel.org, linux@...blig.org, dan.carpenter@...aro.org, 
	Frank.Li@....com, konradybcio@...nel.org, bryan.odonoghue@...aro.org, 
	krzk+dt@...nel.org, robh@...nel.org
Subject: Re: [PATCH v3 1/4] dt-bindindgs: i2c: qcom,i2c-geni: Document shared
 flag

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