Re: [PATCH v2 1/3] dt-bindings: i3c: Add Qualcomm I3C master controller

From: Mukesh Kumar Savaliya
Date: Sat Mar 29 2025 - 05:09:10 EST




On 3/28/2025 8:03 PM, Krzysztof Kozlowski wrote:
On 28/03/2025 15:28, Mukesh Kumar Savaliya wrote:
Thanks Krzysztof for immediate review !

On 3/26/2025 7:53 PM, Krzysztof Kozlowski wrote:
On 26/03/2025 15:16, Mukesh Kumar Savaliya wrote:
Add device tree bindings for the Qualcomm I3C controller. This includes
the necessary documentation and properties required to describe the
hardware in the device tree.

Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@xxxxxxxxxxx>
---
.../bindings/i3c/qcom,i3c-master.yaml | 60 +++++++++++++++++++
1 file changed, 60 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i3c/qcom,i3c-master.yaml

diff --git a/Documentation/devicetree/bindings/i3c/qcom,i3c-master.yaml b/Documentation/devicetree/bindings/i3c/qcom,i3c-master.yaml
new file mode 100644
index 000000000000..af6b393f2327
--- /dev/null
+++ b/Documentation/devicetree/bindings/i3c/qcom,i3c-master.yaml

Naming: nothing improved.

Sorry, i didn't get exactly. To your comment "Filename matching
compatible." i have Changed the compatible string to "qcom,i3c-master"
and now it's matching to file name.

shall i make filename as qcom,i3c-master-qcom.yaml ? If this is wrong,
please suggest.

Removed "bindings" from the subject line too. I have removed "master"
from the description, shall i remove from subject also and keep only
controller ? I kept master controller thinking it may be good as "master
controller" instead "controller".

I still see the "master" in the compatible and filename.

File Name : qcom,i3c-geni-qcom.yaml
Compatible name : "qcom,i3c-geni"
+
Remove master from subject too "dt-bindings: i3c: Add Qualcomm I3C controller"

will this be fine ?


@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i3c/qcom,i3c-master.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Geni based QUP I3C Controller
+
+maintainers:
+ - Mukesh Kumar Savaliya <quic_msavaliy@xxxxxxxxxxx>
+
+description:
+ I3C in master mode supports up to 12.5MHz, SDR mode data transfer in mixed
+ bus mode (I2C and I3C target devices on same i3c bus). It also supports
+ hotjoin, IBI mechanism.
+
+ I3C Controller nodes must be child of GENI based Qualcomm Universal
+ Peripharal. Please refer GENI based QUP wrapper controller node bindings
+ described in Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml.
+
+allOf:
+ - $ref: i3c.yaml#
+
+properties:
+ compatible:
+ const: qcom,i3c-master

And this got worse. It makes also no sense either: how can you claim
that this covers all possible future I3C masters from Qualcomm?

What was the resolution of previous discussion?

Below was my understanding and reply.
"
I think i should remove const. kept it for now as no other compatible to
be added as of now.
let me remove const.
SoC name is not required, as this compatible is generic to all the SOCs.

I don't see any talks about const, what are you referring to?

+properties:
+ compatible:
+ : qcom,i3c-geni
will this be fine ?


"

But i realized i missed to remove const, sorry for that.


I have also checked for qcom,spi-geni-qcom.yaml, it has below :
properties:
compatible:
const: qcom,geni-spi

Let me know if i can correct for SPI in separate new patch ?

I love such arguments - let's find whatever old bindings and whatever
antipattern.
Sure, once i know what should be the ideal pattern which is unique/should be unique, will share you other files.

What was the outcome of the discussion? I think was about geni, no? What
did Rob write? So how did it became i3c-master?

yes, so "qcom,i3c-geni" is fine ?

Best regards,
Krzysztof