linux-kernel - Re: [PATCH v5 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] [day] [month] [year] [list]

Message-ID: <5gy5gldcybby53irzyu6lejbwb6wcorb2k4qpd4j7yrijemehu@wmj3tgvxvhdy>
Date: Fri, 29 Nov 2024 22:45:43 -0600
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, quic_vdadhani@...cinc.com
Subject: Re: [PATCH v5 1/4] dt-bindindgs: i2c: qcom,i2c-geni: Document shared
 flag

On Fri, Nov 29, 2024 at 08:13:54PM +0530, 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.
> 

Per https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
your commit message should start with a description of your problem.
"Add" isn't the right word to start a problem description with.

> SE(Serial Engine HW controller acting as protocol master controller) is an
> I2C controller. Basically a programmable SERDES(serializer/deserializer)

"Basically"?

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

The DeviceTree binding describes properties used to describe the
hardware; your commit message describes what a SE is and that it can
exist can exist in a configuration with multiple client etc etc.

> Transfer gets serialized by Lock TRE + DMA xfer + Unlock TRE at HW level.
> 

This isn't what this patch implements. It defines a property which when
specified means to the OS that any DMA transfers should be performed
using TRE lock/unlock operations.

> Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.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.

This attempts to describe the property.

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

But this is basically just 4 lines of text expanding the acronym "se",
but while it might give some insight into what this binding (the whole
binding) is about, I'm afraid it doesn't add value to the understanding
of the property...

Regards,
Bjorn

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