linux-kernel - Re: [PATCH v1 1/7] dt-bindings: i2c: qcom,i2c-geni: Document DT properties for QUP firmware loading

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: <dacfdaf0-329f-4580-94e0-7c3e26b52776@kernel.org>
Date: Wed, 4 Dec 2024 16:06:47 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Viken Dadhaniya <quic_vdadhani@...cinc.com>, andi.shyti@...nel.org,
 robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 gregkh@...uxfoundation.org, jirislaby@...nel.org, broonie@...nel.or,
 andersson@...nel.org, konradybcio@...nel.org, johan+linaro@...nel.org,
 dianders@...omium.org, agross@...nel.org, linux-arm-msm@...r.kernel.org,
 linux-i2c@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
 linux-spi@...r.kernel.org
Cc: =quic_msavaliy@...cinc.com, quic_anupkulk@...cinc.com,
 Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
Subject: Re: [PATCH v1 1/7] dt-bindings: i2c: qcom,i2c-geni: Document DT
 properties for QUP firmware loading

On 04/12/2024 16:03, Viken Dadhaniya wrote:
> Document the 'qcom,load-firmware' and 'qcom,xfer-mode' properties to
> support SE(Serial Engine) firmware loading from the protocol driver and to
> select the data transfer mode, either GPI DMA (Generic Packet Interface)
> or non-GPI mode (PIO/CPU DMA).


You described the desired Linux feature or behavior, not the actual
hardware. The bindings are about the latter, so instead you need to
rephrase the property and its description to match actual hardware
capabilities/features/configuration etc.

I don't quite get why firmware-name is not suitable here, what is
"protocol driver" in this context and how firmware is loaded from it?

> 
> I2C controller can operate in one of two modes based on the
> 'qcom,xfer-mode' property, and the firmware is loaded accordingly.
> 
> Co-developed-by: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
> Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
> Signed-off-by: Viken Dadhaniya <quic_vdadhani@...cinc.com>
> ---
>  .../devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml   | 11 +++++++++++
>  1 file changed, 11 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..a26f34fce1bb 100644
> --- a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
> +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
> @@ -66,6 +66,15 @@ properties:
>    required-opps:
>      maxItems: 1
>  
> +  qcom,load-firmware:
> +    type: boolean
> +    description: Optional property to load SE (serial engine) Firmware from protocol driver.


Please wrap code according to coding style (checkpatch is not a coding
style description, but only a tool).


> +
> +  qcom,xfer-mode:
> +    description: Value 1,2 and 3 represents FIFO, CPU DMA and GSI DMA mode respectively.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2, 3]


Use string but anyway this would need some changes and explanation why
lack of DMA cannot be used to determine that. CPU DMA and GSI DMA also
need some background.



Best regards,
Krzysztof