diff mbox series

[RFC,1/7] dt: bindings: add qcom,qps615.yaml

Message ID 20240626-qps615-v1-1-2ade7bd91e02@quicinc.com (mailing list archive)
State Changes Requested
Headers show
Series PCI: enable Power and configure the QPS615 PCIe switch | expand

Commit Message

Krishna chaitanya chundru June 26, 2024, 12:37 p.m. UTC
qps615 is a driver for Qualcomm PCIe switch driver which controls
power & configuration of the hardware.
Add a bindings document for the driver.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 .../devicetree/bindings/pci/qcom,qps615.yaml       | 73 ++++++++++++++++++++++
 1 file changed, 73 insertions(+)

Comments

Rob Herring June 26, 2024, 1:34 p.m. UTC | #1
On Wed, 26 Jun 2024 18:07:49 +0530, Krishna chaitanya chundru wrote:
> qps615 is a driver for Qualcomm PCIe switch driver which controls
> power & configuration of the hardware.
> Add a bindings document for the driver.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  .../devicetree/bindings/pci/qcom,qps615.yaml       | 73 ++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/pci/qcom,qps615.yaml:70:1: [error] syntax error: found character '\t' that cannot start any token (syntax)

dtschema/dtc warnings/errors:
make[2]: *** Deleting file 'Documentation/devicetree/bindings/pci/qcom,qps615.example.dts'
Documentation/devicetree/bindings/pci/qcom,qps615.yaml:70:1: found a tab character where an indentation space is expected
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/pci/qcom,qps615.example.dts] Error 1
make[2]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/pci/qcom,qps615.yaml:70:1: found a tab character where an indentation space is expected
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/qcom,qps615.yaml: ignoring, error parsing file
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1430: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240626-qps615-v1-1-2ade7bd91e02@quicinc.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Bjorn Andersson June 26, 2024, 3:01 p.m. UTC | #2
On Wed, Jun 26, 2024 at 06:07:49PM GMT, Krishna chaitanya chundru wrote:
> qps615 is a driver for Qualcomm PCIe switch driver which controls
> power & configuration of the hardware.
> Add a bindings document for the driver.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  .../devicetree/bindings/pci/qcom,qps615.yaml       | 73 ++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,qps615.yaml b/Documentation/devicetree/bindings/pci/qcom,qps615.yaml
> new file mode 100644
> index 000000000000..f090683f9e2f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/qcom,qps615.yaml
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/qcom,pcie-qps615.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm QPS615 PCIe switch
> +
> +maintainers:
> +  - Krishna chaitanya chundru <quic_krichai@quicinc.com>
> +
> +description: |
> +  Qualcomm QPS615 PCIe switch has one upstream and three downstream
> +  ports. One of the downstream ports is used as endpoint device of
> +  Ethernet MAC. Other two downstream ports are supposed to connect
> +  to external device.

Hopefully this isn't the only possible use of QPS615, so I'd suggest
that you omit the rbg3gen2-specific integration details from this
binding. In other words, describe the QPS615, not the QPS615 in rb3gen2.

> +
> +  The power controlled by the GPIO's, if we enable the GPIO's the
> +  power to the switch will be on.
> +
> +  The QPS615 PCIe switch is configured through I2C interface before
> +  PCIe link is established.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - pci1179,0623
> +
> +  reg:
> +    maxItems: 1
> +
> +  vdd-supply:
> +    description: |

'|' means "preserve formatting", but there's nothing here to preserve,
please drop it.

> +      Phandle to the vdd input voltage which are fixed regulators which
> +      in are mapped to the GPIO's.

The binding for a regulator consumer should not document how the
providing regulator is implemented. Only thing to describe here it which
"supply pin" this refers to, which turns this into "vdd input" or "vdd
pin supply" (if that's what the datasheet call this pin on the QPS615)
and as such you can drop the description because the name of the
property already states that.

> +
> +  switch-i2c-cntrl:

I'd prefer you call it "i2c-adapter" or perhaps "i2c-bus", because it's
not "the switch controller".

> +    description: |
> +      phandle to i2c controller which is used to configure the PCIe
> +      switch.

"Reference to the i2c adapter/bus used to configure the QPS615"

But I wonder if this somehow should be described on the particular i2c
bus and be referenced from here instead.

It's not obvious how to describe these components that sits on two
different busses - although I believe this binding refers to the
"configuration interface" sitting on the i2c bus, abut it's described on
the PCIe bus as it relates to power sequencing.

> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +  - switch-i2c-cntrl
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    pcie {
> +        #address-cells = <3>;
> +        #size-cells = <2>;
> +
> +        pcie@0 {
> +            device_type = "pci";
> +            reg = <0x0 0x0 0x0 0x0 0x0>;
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            ranges;
> +
> +            bus-range = <0x01 0xff>;
> +
> +            qps615@0 {
> +                compatible = "pci1179,0623";
> +                reg = <0x10000 0x0 0x0 0x0 0x0>;
> +
> +                vdd-supply = <&vdd>;
> +		switch-i2c-cntrl = <&foo>;

Indentation looks off here.

Regards,
Bjorn

> +            };
> +        };
> +    };
> 
> -- 
> 2.42.0
>
Rob Herring June 27, 2024, 3:31 p.m. UTC | #3
On Wed, Jun 26, 2024 at 06:07:49PM +0530, Krishna chaitanya chundru wrote:
> qps615 is a driver for Qualcomm PCIe switch driver which controls
> power & configuration of the hardware.
> Add a bindings document for the driver.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  .../devicetree/bindings/pci/qcom,qps615.yaml       | 73 ++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,qps615.yaml b/Documentation/devicetree/bindings/pci/qcom,qps615.yaml
> new file mode 100644
> index 000000000000..f090683f9e2f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/qcom,qps615.yaml
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/qcom,pcie-qps615.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm QPS615 PCIe switch
> +
> +maintainers:
> +  - Krishna chaitanya chundru <quic_krichai@quicinc.com>
> +
> +description: |
> +  Qualcomm QPS615 PCIe switch has one upstream and three downstream
> +  ports. One of the downstream ports is used as endpoint device of
> +  Ethernet MAC. Other two downstream ports are supposed to connect
> +  to external device.
> +
> +  The power controlled by the GPIO's, if we enable the GPIO's the
> +  power to the switch will be on.
> +
> +  The QPS615 PCIe switch is configured through I2C interface before
> +  PCIe link is established.
> +

As a PCI device and implementing a PCI bus, you need to reference 
pci-pci-bridge.yaml. And you'll need to fix your example when you add 
that.

> +properties:
> +  compatible:
> +    enum:
> +      - pci1179,0623
> +
> +  reg:
> +    maxItems: 1
> +
> +  vdd-supply:
> +    description: |
> +      Phandle to the vdd input voltage which are fixed regulators which
> +      in are mapped to the GPIO's.
> +
> +  switch-i2c-cntrl:
> +    description: |
> +      phandle to i2c controller which is used to configure the PCIe
> +      switch.

There's a somewhat standard property for this purpose: i2c-bus

> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +  - switch-i2c-cntrl
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    pcie {
> +        #address-cells = <3>;
> +        #size-cells = <2>;
> +
> +        pcie@0 {
> +            device_type = "pci";
> +            reg = <0x0 0x0 0x0 0x0 0x0>;
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            ranges;
> +
> +            bus-range = <0x01 0xff>;

Unless there's a h/w limitation, you don't need this.

> +
> +            qps615@0 {
> +                compatible = "pci1179,0623";
> +                reg = <0x10000 0x0 0x0 0x0 0x0>;
> +
> +                vdd-supply = <&vdd>;
> +		switch-i2c-cntrl = <&foo>;
> +            };
> +        };
> +    };
> 
> -- 
> 2.42.0
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pci/qcom,qps615.yaml b/Documentation/devicetree/bindings/pci/qcom,qps615.yaml
new file mode 100644
index 000000000000..f090683f9e2f
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/qcom,qps615.yaml
@@ -0,0 +1,73 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/qcom,pcie-qps615.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm QPS615 PCIe switch
+
+maintainers:
+  - Krishna chaitanya chundru <quic_krichai@quicinc.com>
+
+description: |
+  Qualcomm QPS615 PCIe switch has one upstream and three downstream
+  ports. One of the downstream ports is used as endpoint device of
+  Ethernet MAC. Other two downstream ports are supposed to connect
+  to external device.
+
+  The power controlled by the GPIO's, if we enable the GPIO's the
+  power to the switch will be on.
+
+  The QPS615 PCIe switch is configured through I2C interface before
+  PCIe link is established.
+
+properties:
+  compatible:
+    enum:
+      - pci1179,0623
+
+  reg:
+    maxItems: 1
+
+  vdd-supply:
+    description: |
+      Phandle to the vdd input voltage which are fixed regulators which
+      in are mapped to the GPIO's.
+
+  switch-i2c-cntrl:
+    description: |
+      phandle to i2c controller which is used to configure the PCIe
+      switch.
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+  - switch-i2c-cntrl
+
+additionalProperties: false
+
+examples:
+  - |
+    pcie {
+        #address-cells = <3>;
+        #size-cells = <2>;
+
+        pcie@0 {
+            device_type = "pci";
+            reg = <0x0 0x0 0x0 0x0 0x0>;
+            #address-cells = <3>;
+            #size-cells = <2>;
+            ranges;
+
+            bus-range = <0x01 0xff>;
+
+            qps615@0 {
+                compatible = "pci1179,0623";
+                reg = <0x10000 0x0 0x0 0x0 0x0>;
+
+                vdd-supply = <&vdd>;
+		switch-i2c-cntrl = <&foo>;
+            };
+        };
+    };