On 06/02/2025 14:43, Mukesh Kumar Savaliya wrote: > Hi Krzysztof, Thanks ! > > On 2/5/2025 8:12 PM, Krzysztof Kozlowski wrote: >> On 05/02/2025 15:31, Mukesh Kumar Savaliya wrote: >>> Add device tree bindings for the Qualcomm I3C master controller. This >>> includes the necessary documentation and properties required to describe >>> the hardware in the device tree. >> >> A nit, subject: drop second/last, redundant "bindings". The >> "dt-bindings" prefix is already stating that these are bindings. > Sure >> See also: >> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 >> >> Use modern terminology, which means: >> s/master/whatever else or even nothing/ >> See other recent bindings and discussions. >> > Sure >> >>> >>> Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@xxxxxxxxxxx> >>> --- >>> .../bindings/i3c/qcom,i3c-master.yaml | 57 +++++++++++++++++++ >>> 1 file changed, 57 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..ad63ea779fd6 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/i3c/qcom,i3c-master.yaml >> >> Filename matching compatible. >> > Changed compatible to "qcom,i3c-master" >>> @@ -0,0 +1,57 @@ >>> +# 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 I3C master controller >>> + >>> +maintainers: >>> + - Mukesh Kumar Savaliya <quic_msavaliy@xxxxxxxxxxx> >>> + >>> +allOf: >>> + - $ref: i3c.yaml# >>> + >>> +properties: >>> + compatible: >>> + const: qcom,geni-i3c >> >> No SoC? So to be sure: you claim all future SoCs will be using exactly >> the same interface. No new compatibles, no new properties will be added. >> > I think i should remove const. kept it for now as no other compatible to > be added as of now. > > let me remove const. No, it does not matter. Keep const. > > SoC name is not required, as this compatible is generic to all the SOCs. That's the statement you make. I accept it. I will bookmark this thread and use it whenever you try to add any future property here (to be clear: you agree you will not add new properties to fulfill *FUTURE* SoC differences). >>> + >>> + reg: >>> + minItems: 1 >> >> Drop >> > Not required ? I see other bindings are using it, so please confirm if i > can remove this. >>> + maxItems: 2 >> >> Drop and instead list and describe items >> > Okay, i can remove maxItems if not mandatory. Taken cdns,i3c-master.yaml > and added these. > >> >>> + >>> + clocks: >>> + minItems: 1 >> >> Look at other bindings. There is never code like this. >> > cdns,i3c-master.yaml taken as reference. >>> + >>> + clock-names: >>> + items: >>> + - const: se-clk >> >> Drop clock-names > Sure, took reference from cdns,i3c-master.yaml. >> >>> + >>> + interrupts-extended: >>> + minItems: 1 >>> + maxItems: 3 >> >> As well - there is never an interrupts-extended property. Just interrupts. >> > No, i see this property many places. Do you mean to say > interrupts-extended can be there in examples but not only add I already wrote what you should do: "Just interrupts." > "interrupts" property here ? > e.g timer/riscv,timer.yaml +41 lists it in yaml also. Please do not take one file and create coding style out of it, but instead look what ~2300 other YAML files do. Best regards, Krzysztof
