Thanks Krzysztof for the review and helpful comments.
On 12/4/2024 8:36 PM, Krzysztof Kozlowski wrote:
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.
Sure, IIUC, i should explain the need of FW loading. Agree that binding
is for the hardware. This feature needs to have some intelligence to
know that Software driver needs to load Firmware or not ?
Let me add description about the actual hardware capabilities, its
features. Hope this can be better from my side on V2.
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?
yes, as per Dmitry's comment, i should replace with
/soc/sc7180/firmware. This would be become "firmware-name" property
instead of qcom,load-firmware.
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@xxxxxxxxxxx>
Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@xxxxxxxxxxx>
Signed-off-by: Viken Dadhaniya <quic_vdadhani@xxxxxxxxxxx>
---
.../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).
Actually i have ran dt-schema for yaml validation. I couldn't get if you
have any comment for description statement OR it's related to code ?
Could you please be more descriptive so i can adopt the suggestions.
+
+ 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.
Sure, i got it.
I need to add enum strings and shall provide explanation abut modes in use.
As per Doug's comment, we plan to keep GSI and non-GSI mode. It would be
more clear in next patch.
Best regards,
Krzysztof
