| Message ID | 20241112-qps615_pwr-v3-1-29a1e98aa2b0@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: Enable Power and configure the QPS615 PCIe switch | expand |
On Tue, Nov 12, 2024 at 08:31:33PM +0530, Krishna chaitanya chundru wrote: > Add binding describing the Qualcomm PCIe switch, QPS615, > which provides Ethernet MAC integrated to the 3rd downstream port > and two downstream PCIe ports. > Reviewed-by: Bjorn Andersson <andersson@kernel.org> Regards, Bjorn > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> > --- > .../devicetree/bindings/pci/qcom,qps615.yaml | 205 +++++++++++++++++++++ > 1 file changed, 205 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..e6a63a0bb0f3 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pci/qcom,qps615.yaml > @@ -0,0 +1,205 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pci/qcom,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. The 3rd downstream port has integrated endpoint device of > + Ethernet MAC. Other two downstream ports are supposed to connect > + to external device. > + > + The QPS615 PCIe switch can be configured through I2C interface before > + PCIe link is established to change FTS, ASPM related entry delays, > + tx amplitude etc for better power efficiency and functionality. > + > +properties: > + compatible: > + enum: > + - pci1179,0623 > + > + reg: > + maxItems: 1 > + > + i2c-parent: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + description: | > + A phandle to the parent I2C node and the slave address of the device > + used to do configure qps615 to change FTS, tx amplitude etc. > + items: > + - description: Phandle to the I2C controller node > + - description: I2C slave address > + > + vdd18-supply: true > + > + vdd09-supply: true > + > + vddc-supply: true > + > + vddio1-supply: true > + > + vddio2-supply: true > + > + vddio18-supply: true > + > + reset-gpios: > + maxItems: 1 > + description: > + GPIO controlling the RESX# pin. > + > + qps615,axi-clk-freq-hz: > + description: > + AXI clock rate which is internal bus of the switch > + The switch only runs in two frequencies i.e 250MHz and 125MHz. > + enum: [125000000, 250000000] > + > +allOf: > + - $ref: "#/$defs/qps615-node" > + > +patternProperties: > + "@1?[0-9a-f](,[0-7])?$": > + description: child nodes describing the internal downstream ports > + the qps615 switch. > + type: object > + $ref: "#/$defs/qps615-node" > + unevaluatedProperties: false > + > +$defs: > + qps615-node: > + type: object > + > + properties: > + qcom,l0s-entry-delay-ns: > + description: Aspm l0s entry delay. > + > + qcom,l1-entry-delay-ns: > + description: Aspm l1 entry delay. > + > + qcom,tx-amplitude-millivolt: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: Change Tx Margin setting for low power consumption. > + > + qcom,no-dfe-support: > + type: boolean > + description: Disable DFE (Decision Feedback Equalizer), which mitigates > + intersymbol interference and some reflections caused by impedance mismatches. > + > + qcom,nfts: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Number of Fast Training Sequence (FTS) used during L0s to L0 exit > + for bit and Symbol lock. > + > + allOf: > + - $ref: /schemas/pci/pci-bus.yaml# > + > +unevaluatedProperties: false > + > +required: > + - vdd18-supply > + - vdd09-supply > + - vddc-supply > + - vddio1-supply > + - vddio2-supply > + - vddio18-supply > + - i2c-parent > + - reset-gpios > + > +examples: > + - | > + > + #include <dt-bindings/gpio/gpio.h> > + > + 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>; > + > + pcie@0,0 { > + compatible = "pci1179,0623"; > + reg = <0x10000 0x0 0x0 0x0 0x0>; > + device_type = "pci"; > + #address-cells = <3>; > + #size-cells = <2>; > + ranges; > + bus-range = <0x02 0xff>; > + > + i2c-parent = <&qup_i2c 0x77>; > + > + vdd18-supply = <&vdd>; > + vdd09-supply = <&vdd>; > + vddc-supply = <&vdd>; > + vddio1-supply = <&vdd>; > + vddio2-supply = <&vdd>; > + vddio18-supply = <&vdd>; > + > + reset-gpios = <&gpio 1 GPIO_ACTIVE_LOW>; > + > + pcie@1,0 { > + reg = <0x20800 0x0 0x0 0x0 0x0>; > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + ranges; > + bus-range = <0x03 0xff>; > + > + qcom,no-dfe-support; > + }; > + > + pcie@2,0 { > + reg = <0x21000 0x0 0x0 0x0 0x0>; > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + ranges; > + bus-range = <0x04 0xff>; > + > + qcom,nfts = <10>; > + }; > + > + pcie@3,0 { > + reg = <0x21800 0x0 0x0 0x0 0x0>; > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + ranges; > + bus-range = <0x05 0xff>; > + > + qcom,tx-amplitude-millivolt = <10>; > + pcie@0,0 { > + reg = <0x50000 0x0 0x0 0x0 0x0>; > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + ranges; > + > + qcom,l1-entry-delay-ns = <10>; > + }; > + > + pcie@0,1 { > + reg = <0x50100 0x0 0x0 0x0 0x0>; > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + ranges; > + > + qcom,l0s-entry-delay-ns = <10>; > + }; > + }; > + }; > + }; > + }; > > -- > 2.34.1 >
On Tue, Nov 12, 2024 at 08:31:33PM +0530, Krishna chaitanya chundru wrote: > Add binding describing the Qualcomm PCIe switch, QPS615, > which provides Ethernet MAC integrated to the 3rd downstream port > and two downstream PCIe ports. > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> > --- > .../devicetree/bindings/pci/qcom,qps615.yaml | 205 +++++++++++++++++++++ > 1 file changed, 205 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..e6a63a0bb0f3 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pci/qcom,qps615.yaml > @@ -0,0 +1,205 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pci/qcom,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. The 3rd downstream port has integrated endpoint device of > + Ethernet MAC. Other two downstream ports are supposed to connect > + to external device. > + > + The QPS615 PCIe switch can be configured through I2C interface before > + PCIe link is established to change FTS, ASPM related entry delays, > + tx amplitude etc for better power efficiency and functionality. > + > +properties: > + compatible: > + enum: > + - pci1179,0623 > + > + reg: > + maxItems: 1 > + > + i2c-parent: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + description: | Don't need '|' if no formatting to preserve. > + A phandle to the parent I2C node and the slave address of the device > + used to do configure qps615 to change FTS, tx amplitude etc. > + items: > + - description: Phandle to the I2C controller node > + - description: I2C slave address > + > + vdd18-supply: true > + > + vdd09-supply: true > + > + vddc-supply: true > + > + vddio1-supply: true > + > + vddio2-supply: true > + > + vddio18-supply: true > + > + reset-gpios: > + maxItems: 1 > + description: > + GPIO controlling the RESX# pin. Is the PERST# or something else? > + > + qps615,axi-clk-freq-hz: qps615 is not a vendor prefix. > + description: > + AXI clock rate which is internal bus of the switch > + The switch only runs in two frequencies i.e 250MHz and 125MHz. > + enum: [125000000, 250000000] > + > +allOf: > + - $ref: "#/$defs/qps615-node" > + > +patternProperties: > + "@1?[0-9a-f](,[0-7])?$": You have 3 ports. So isn't this fixed and limited to 0-2? > + description: child nodes describing the internal downstream ports > + the qps615 switch. Please be consistent with starting after the ':' or on the next line. And start with capital C. > + type: object > + $ref: "#/$defs/qps615-node" > + unevaluatedProperties: false > + > +$defs: > + qps615-node: > + type: object > + > + properties: > + qcom,l0s-entry-delay-ns: > + description: Aspm l0s entry delay. > + > + qcom,l1-entry-delay-ns: > + description: Aspm l1 entry delay. These should probably be common being standard PCIe things. Though, why are they needed? I'm sure the timing is defined by the PCIe spec, so they are not compliant? > + > + qcom,tx-amplitude-millivolt: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: Change Tx Margin setting for low power consumption. > + > + qcom,no-dfe-support: > + type: boolean > + description: Disable DFE (Decision Feedback Equalizer), which mitigates > + intersymbol interference and some reflections caused by impedance mismatches. > + > + qcom,nfts: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Number of Fast Training Sequence (FTS) used during L0s to L0 exit > + for bit and Symbol lock. Also something common. The problem I have with all these properties is you are using them on both the upstream and downstream sides of the PCIe links. They belong in either the device's node (downstream) or the bus's node (upstream). > + > + allOf: > + - $ref: /schemas/pci/pci-bus.yaml# pci-pci-bridge.yaml is more specific and closer to what this device is. > + > +unevaluatedProperties: false > + > +required: > + - vdd18-supply > + - vdd09-supply > + - vddc-supply > + - vddio1-supply > + - vddio2-supply > + - vddio18-supply > + - i2c-parent > + - reset-gpios > + > +examples: > + - | > + > + #include <dt-bindings/gpio/gpio.h> > + > + 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>; > + > + pcie@0,0 { > + compatible = "pci1179,0623"; > + reg = <0x10000 0x0 0x0 0x0 0x0>; > + device_type = "pci"; > + #address-cells = <3>; > + #size-cells = <2>; > + ranges; > + bus-range = <0x02 0xff>; > + > + i2c-parent = <&qup_i2c 0x77>; > + > + vdd18-supply = <&vdd>; > + vdd09-supply = <&vdd>; > + vddc-supply = <&vdd>; > + vddio1-supply = <&vdd>; > + vddio2-supply = <&vdd>; > + vddio18-supply = <&vdd>; > + > + reset-gpios = <&gpio 1 GPIO_ACTIVE_LOW>; > + > + pcie@1,0 { > + reg = <0x20800 0x0 0x0 0x0 0x0>; > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + ranges; > + bus-range = <0x03 0xff>; > + > + qcom,no-dfe-support; > + }; > + > + pcie@2,0 { > + reg = <0x21000 0x0 0x0 0x0 0x0>; > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + ranges; > + bus-range = <0x04 0xff>; > + > + qcom,nfts = <10>; > + }; > + > + pcie@3,0 { > + reg = <0x21800 0x0 0x0 0x0 0x0>; > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + ranges; > + bus-range = <0x05 0xff>; > + > + qcom,tx-amplitude-millivolt = <10>; > + pcie@0,0 { > + reg = <0x50000 0x0 0x0 0x0 0x0>; > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; There's a 2nd PCI-PCI bridge? > + ranges; > + > + qcom,l1-entry-delay-ns = <10>; > + }; > + > + pcie@0,1 { > + reg = <0x50100 0x0 0x0 0x0 0x0>; > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + ranges; > + > + qcom,l0s-entry-delay-ns = <10>; > + }; > + }; > + }; > + }; > + }; > > -- > 2.34.1 >
On Tue, Nov 12, 2024 at 08:31:33PM +0530, Krishna chaitanya chundru wrote: > Add binding describing the Qualcomm PCIe switch, QPS615, > which provides Ethernet MAC integrated to the 3rd downstream port > and two downstream PCIe ports. > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> > --- > .../devicetree/bindings/pci/qcom,qps615.yaml | 205 +++++++++++++++++++++ > 1 file changed, 205 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..e6a63a0bb0f3 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pci/qcom,qps615.yaml Isn't "qcom,qps615" a SoC name? This is supposed to be matching compatible, in your case probably qcom,qps615-whatever-this-is? ... > + qps615,axi-clk-freq-hz: That's a downstream code you send us. Anyway, why assigned clock rates do not work for you? You are re-implementing legacy property now under different name :/ > + description: > + AXI clock rate which is internal bus of the switch > + The switch only runs in two frequencies i.e 250MHz and 125MHz. > + enum: [125000000, 250000000] > + > +allOf: > + - $ref: "#/$defs/qps615-node" > + > +patternProperties: > + "@1?[0-9a-f](,[0-7])?$": > + description: child nodes describing the internal downstream ports > + the qps615 switch. > + type: object > + $ref: "#/$defs/qps615-node" > + unevaluatedProperties: false > + > +$defs: > + qps615-node: > + type: object > + > + properties: > + qcom,l0s-entry-delay-ns: > + description: Aspm l0s entry delay. > + > + qcom,l1-entry-delay-ns: > + description: Aspm l1 entry delay. > + > + qcom,tx-amplitude-millivolt: -microvolt does not work for you? > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: Change Tx Margin setting for low power consumption. > + > + qcom,no-dfe-support: > + type: boolean > + description: Disable DFE (Decision Feedback Equalizer), which mitigates > + intersymbol interference and some reflections caused by impedance mismatches. > + > + qcom,nfts: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Number of Fast Training Sequence (FTS) used during L0s to L0 exit > + for bit and Symbol lock. Use some of these properties in the example. I saw only one. > + > + allOf: > + - $ref: /schemas/pci/pci-bus.yaml# > + > +unevaluatedProperties: false > + > +required: > + - vdd18-supply > + - vdd09-supply > + - vddc-supply > + - vddio1-supply > + - vddio2-supply > + - vddio18-supply > + - i2c-parent > + - reset-gpios > + > +examples: > + - | > + Drop blank line > + #include <dt-bindings/gpio/gpio.h> > + > + pcie { > + #address-cells = <3>; > + #size-cells = <2>; > + > + pcie@0 { > + device_type = "pci"; > + reg = <0x0 0x0 0x0 0x0 0x0>; Best regards, Krzysztof
On 11/15/2024 9:48 PM, Rob Herring wrote: > On Tue, Nov 12, 2024 at 08:31:33PM +0530, Krishna chaitanya chundru wrote: >> Add binding describing the Qualcomm PCIe switch, QPS615, >> which provides Ethernet MAC integrated to the 3rd downstream port >> and two downstream PCIe ports. >> >> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> >> --- >> .../devicetree/bindings/pci/qcom,qps615.yaml | 205 +++++++++++++++++++++ >> 1 file changed, 205 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..e6a63a0bb0f3 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/pci/qcom,qps615.yaml >> @@ -0,0 +1,205 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/pci/qcom,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. The 3rd downstream port has integrated endpoint device of >> + Ethernet MAC. Other two downstream ports are supposed to connect >> + to external device. >> + >> + The QPS615 PCIe switch can be configured through I2C interface before >> + PCIe link is established to change FTS, ASPM related entry delays, >> + tx amplitude etc for better power efficiency and functionality. >> + >> +properties: >> + compatible: >> + enum: >> + - pci1179,0623 >> + >> + reg: >> + maxItems: 1 >> + >> + i2c-parent: >> + $ref: /schemas/types.yaml#/definitions/phandle-array >> + description: | > > Don't need '|' if no formatting to preserve. > ack >> + A phandle to the parent I2C node and the slave address of the device >> + used to do configure qps615 to change FTS, tx amplitude etc. >> + items: >> + - description: Phandle to the I2C controller node >> + - description: I2C slave address >> + >> + vdd18-supply: true >> + >> + vdd09-supply: true >> + >> + vddc-supply: true >> + >> + vddio1-supply: true >> + >> + vddio2-supply: true >> + >> + vddio18-supply: true >> + >> + reset-gpios: >> + maxItems: 1 >> + description: >> + GPIO controlling the RESX# pin. > > Is the PERST# or something else? > it is not PERST GPIO, it is similar to PERST in terms of functionality which brings switch out from reset. >> + >> + qps615,axi-clk-freq-hz: > > qps615 is not a vendor prefix. > >> + description: >> + AXI clock rate which is internal bus of the switch >> + The switch only runs in two frequencies i.e 250MHz and 125MHz. >> + enum: [125000000, 250000000] >> + >> +allOf: >> + - $ref: "#/$defs/qps615-node" >> + >> +patternProperties: >> + "@1?[0-9a-f](,[0-7])?$": > > You have 3 ports. So isn't this fixed and limited to 0-2? > sure I will change it to below as suggested "@1?[0-3](,[0-1])?$" >> + description: child nodes describing the internal downstream ports >> + the qps615 switch. > > Please be consistent with starting after the ':' or on the next line. > > And start with capital C. > > ack >> + type: object >> + $ref: "#/$defs/qps615-node" >> + unevaluatedProperties: false >> + >> +$defs: >> + qps615-node: >> + type: object >> + >> + properties: >> + qcom,l0s-entry-delay-ns: >> + description: Aspm l0s entry delay. >> + >> + qcom,l1-entry-delay-ns: >> + description: Aspm l1 entry delay. > > These should probably be common being standard PCIe things. Though, why > are they needed? I'm sure the timing is defined by the PCIe spec, so > they are not compliant? > Usually the firmware in the endpoints/switches should do this these configurations. But the qps615 PCIe switch doesn't have any firmware running to configure these. So the hardware exposes i2c interface to configure these before link training. >> + >> + qcom,tx-amplitude-millivolt: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: Change Tx Margin setting for low power consumption. >> + >> + qcom,no-dfe-support: >> + type: boolean >> + description: Disable DFE (Decision Feedback Equalizer), which mitigates >> + intersymbol interference and some reflections caused by impedance mismatches. >> + >> + qcom,nfts: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: >> + Number of Fast Training Sequence (FTS) used during L0s to L0 exit >> + for bit and Symbol lock. > > Also something common. > > The problem I have with all these properties is you are using them on > both the upstream and downstream sides of the PCIe links. They belong in > either the device's node (downstream) or the bus's node (upstream). > This switch allows us to configure both upstream, downstream ports and also embedded Ethernet port which is internal to the switch. These properties are applicable for all of those. >> + >> + allOf: >> + - $ref: /schemas/pci/pci-bus.yaml# > > pci-pci-bridge.yaml is more specific and closer to what this device is. > I tried this now, I was getting warning saying the compatible /local/mnt/workspace/skales/kobj/Documentation/devicetree/bindings/pci/qcom,qps615.example.dtb: pcie@0,0: compatible: ['pci1179,0623'] does not contain items matching the given schema from schema $id: http://devicetree.org/schemas/pci/qcom,qps615.yaml# /local/mnt/workspace/skales/kobj/Documentation/devicetree/bindings/pci/qcom,qps615.example.dtb: pcie@0,0: Unevaluated properties are not allowed ('#address-cells', '#size-cells', 'bus-range', 'device_type', 'ranges' were unexpected) I think pci-pci-bridge is expecting the compatible string in this format only "pciclass,0604". >> + >> +unevaluatedProperties: false >> + >> +required: >> + - vdd18-supply >> + - vdd09-supply >> + - vddc-supply >> + - vddio1-supply >> + - vddio2-supply >> + - vddio18-supply >> + - i2c-parent >> + - reset-gpios >> + >> +examples: >> + - | >> + >> + #include <dt-bindings/gpio/gpio.h> >> + >> + 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>; >> + >> + pcie@0,0 { >> + compatible = "pci1179,0623"; >> + reg = <0x10000 0x0 0x0 0x0 0x0>; >> + device_type = "pci"; >> + #address-cells = <3>; >> + #size-cells = <2>; >> + ranges; >> + bus-range = <0x02 0xff>; >> + >> + i2c-parent = <&qup_i2c 0x77>; >> + >> + vdd18-supply = <&vdd>; >> + vdd09-supply = <&vdd>; >> + vddc-supply = <&vdd>; >> + vddio1-supply = <&vdd>; >> + vddio2-supply = <&vdd>; >> + vddio18-supply = <&vdd>; >> + >> + reset-gpios = <&gpio 1 GPIO_ACTIVE_LOW>; >> + >> + pcie@1,0 { >> + reg = <0x20800 0x0 0x0 0x0 0x0>; >> + #address-cells = <3>; >> + #size-cells = <2>; >> + device_type = "pci"; >> + ranges; >> + bus-range = <0x03 0xff>; >> + >> + qcom,no-dfe-support; >> + }; >> + >> + pcie@2,0 { >> + reg = <0x21000 0x0 0x0 0x0 0x0>; >> + #address-cells = <3>; >> + #size-cells = <2>; >> + device_type = "pci"; >> + ranges; >> + bus-range = <0x04 0xff>; >> + >> + qcom,nfts = <10>; >> + }; >> + >> + pcie@3,0 { >> + reg = <0x21800 0x0 0x0 0x0 0x0>; >> + #address-cells = <3>; >> + #size-cells = <2>; >> + device_type = "pci"; >> + ranges; >> + bus-range = <0x05 0xff>; >> + >> + qcom,tx-amplitude-millivolt = <10>; >> + pcie@0,0 { >> + reg = <0x50000 0x0 0x0 0x0 0x0>; >> + #address-cells = <3>; >> + #size-cells = <2>; >> + device_type = "pci"; > > There's a 2nd PCI-PCI bridge? This the embedded ethernet port which is as part of DSP3. - Krishna Chaitanya. > >> + ranges; >> + >> + qcom,l1-entry-delay-ns = <10>; >> + }; >> + >> + pcie@0,1 { >> + reg = <0x50100 0x0 0x0 0x0 0x0>; >> + #address-cells = <3>; >> + #size-cells = <2>; >> + device_type = "pci"; >> + ranges; >> + >> + qcom,l0s-entry-delay-ns = <10>; >> + }; >> + }; >> + }; >> + }; >> + }; >> >> -- >> 2.34.1 >>
On 11/20/2024 1:34 PM, Krzysztof Kozlowski wrote: > On Tue, Nov 12, 2024 at 08:31:33PM +0530, Krishna chaitanya chundru wrote: >> Add binding describing the Qualcomm PCIe switch, QPS615, >> which provides Ethernet MAC integrated to the 3rd downstream port >> and two downstream PCIe ports. >> >> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> >> --- >> .../devicetree/bindings/pci/qcom,qps615.yaml | 205 +++++++++++++++++++++ >> 1 file changed, 205 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..e6a63a0bb0f3 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/pci/qcom,qps615.yaml > > Isn't "qcom,qps615" a SoC name? This is supposed to be matching > compatible, in your case probably qcom,qps615-whatever-this-is? > qcom,qcs615 is a soc name, qcom,qps615 is the pcie switch. ack I will change it to qcom,qps615-pcie > ... > >> + qps615,axi-clk-freq-hz: > > That's a downstream code you send us. > > Anyway, why assigned clock rates do not work for you? You are > re-implementing legacy property now under different name :/ > > The assigned clock rates comes in to the picture when we are using clock framework to control the clocks. For this switch there are no clocks needs to be control, the moment we power on the switch clocks are enabled by default. This switch provides a mechanism to control the frequency using i2c. And switch supports only two frequencies i.e 125MHz and 250MHZ by default it runs on 250MHz, we can do one i2c write with which switch runs in 125MHz. >> + description: >> + AXI clock rate which is internal bus of the switch >> + The switch only runs in two frequencies i.e 250MHz and 125MHz. >> + enum: [125000000, 250000000] >> + >> +allOf: >> + - $ref: "#/$defs/qps615-node" >> + >> +patternProperties: >> + "@1?[0-9a-f](,[0-7])?$": >> + description: child nodes describing the internal downstream ports >> + the qps615 switch. >> + type: object >> + $ref: "#/$defs/qps615-node" >> + unevaluatedProperties: false >> + >> +$defs: >> + qps615-node: >> + type: object >> + >> + properties: >> + qcom,l0s-entry-delay-ns: >> + description: Aspm l0s entry delay. >> + >> + qcom,l1-entry-delay-ns: >> + description: Aspm l1 entry delay. >> + >> + qcom,tx-amplitude-millivolt: > > -microvolt does not work for you? > let me check this if it is applicable we will change it to microvolt. >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: Change Tx Margin setting for low power consumption. >> + >> + qcom,no-dfe-support: >> + type: boolean >> + description: Disable DFE (Decision Feedback Equalizer), which mitigates >> + intersymbol interference and some reflections caused by impedance mismatches. >> + >> + qcom,nfts: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: >> + Number of Fast Training Sequence (FTS) used during L0s to L0 exit >> + for bit and Symbol lock. > > Use some of these properties in the example. I saw only one. > In the next patch I will try to use all the properties as suggested. >> + >> + allOf: >> + - $ref: /schemas/pci/pci-bus.yaml# >> + >> +unevaluatedProperties: false >> + >> +required: >> + - vdd18-supply >> + - vdd09-supply >> + - vddc-supply >> + - vddio1-supply >> + - vddio2-supply >> + - vddio18-supply >> + - i2c-parent >> + - reset-gpios >> + >> +examples: >> + - | >> + > > Drop blank line > ack. - Krishna Chaitanya. >> + #include <dt-bindings/gpio/gpio.h> >> + >> + pcie { >> + #address-cells = <3>; >> + #size-cells = <2>; >> + >> + pcie@0 { >> + device_type = "pci"; >> + reg = <0x0 0x0 0x0 0x0 0x0>; > > Best regards, > Krzysztof >
On 24/11/2024 02:41, Krishna Chaitanya Chundru wrote: >> ... >> >>> + qps615,axi-clk-freq-hz: >> >> That's a downstream code you send us. >> >> Anyway, why assigned clock rates do not work for you? You are >> re-implementing legacy property now under different name :/ >> >> The assigned clock rates comes in to the picture when we are using clock > framework to control the clocks. For this switch there are no clocks > needs to be control, the moment we power on the switch clocks are > enabled by default. This switch provides a mechanism to control the > frequency using i2c. And switch supports only two frequencies i.e frequency of what, since there are no clocks? > 125MHz and 250MHZ by default it runs on 250MHz, we can do one i2c > write with which switch runs in 125MHz. How doing a write is relevant? Or you want to say you can control clock? Best regards, Krzysztof
On 11/25/2024 1:10 PM, Krzysztof Kozlowski wrote: > On 24/11/2024 02:41, Krishna Chaitanya Chundru wrote: >>> ... >>> >>>> + qps615,axi-clk-freq-hz: >>> >>> That's a downstream code you send us. >>> >>> Anyway, why assigned clock rates do not work for you? You are >>> re-implementing legacy property now under different name :/ >>> >>> The assigned clock rates comes in to the picture when we are >>> using clock >> framework to control the clocks. For this switch there are no >> clocks needs to be control, the moment we power on the switch >> clocks are enabled by default. This switch provides a mechanism to >> control the frequency using i2c. And switch supports only two >> frequencies i.e > > > frequency of what, since there are no clocks? > The axi clock frequency internal to the switch, host can't control the enablement of the clocks it can control only the frequency. we already had a discussion on this on v2[1], and we taught you agreed on this property. [1] https://lore.kernel.org/netdev/d1af1eac-f9bd-7a8e-586b-5c2a76445145@codeaurora.org/T/#m3d5864c758f2e05fa15ba522aad6a37e3417bd9f - Krishna Chaitanya. >> 125MHz and 250MHZ by default it runs on 250MHz, we can do one i2c >> write with which switch runs in 125MHz. > > How doing a write is relevant? Or you want to say you can control > clock? > > > > Best regards, Krzysztof
On 26/11/2024 07:50, Krishna Chaitanya Chundru wrote: > > > On 11/25/2024 1:10 PM, Krzysztof Kozlowski wrote: >> On 24/11/2024 02:41, Krishna Chaitanya Chundru wrote: >>>> ... >>>> >>>>> + qps615,axi-clk-freq-hz: >>>> >>>> That's a downstream code you send us. >>>> >>>> Anyway, why assigned clock rates do not work for you? You are >>>> re-implementing legacy property now under different name :/ >>>> >>>> The assigned clock rates comes in to the picture when we are >>>> using clock >>> framework to control the clocks. For this switch there are no >>> clocks needs to be control, the moment we power on the switch >>> clocks are enabled by default. This switch provides a mechanism to >>> control the frequency using i2c. And switch supports only two >>> frequencies i.e >> >> >> frequency of what, since there are no clocks? >> > The axi clock frequency internal to the switch, host can't control > the enablement of the clocks it can control only the frequency. > > we already had a discussion on this on v2[1], and we taught you agreed > on this property. > > [1] > https://lore.kernel.org/netdev/d1af1eac-f9bd-7a8e-586b-5c2a76445145@codeaurora.org/T/#m3d5864c758f2e05fa15ba522aad6a37e3417bd9f > This points something else. I diged v2 and found many unanswered questions and unfinished discussion: https://lore.kernel.org/r/linux-arm-msm/20240803-qps615-v2-0-9560b7c71369@quicinc.com/T/#m7074ab9e5f89e29faf430c82f769e67d0e4072cf The property description did not improve, actually it got worse: you repeat constraints in free form text. Best regards, Krzysztof
On Tue, Nov 26, 2024 at 07:58:16AM +0100, Krzysztof Kozlowski wrote: > On 26/11/2024 07:50, Krishna Chaitanya Chundru wrote: > > > > > > On 11/25/2024 1:10 PM, Krzysztof Kozlowski wrote: > >> On 24/11/2024 02:41, Krishna Chaitanya Chundru wrote: > >>>> ... > >>>> > >>>>> + qps615,axi-clk-freq-hz: > >>>> > >>>> That's a downstream code you send us. > >>>> > >>>> Anyway, why assigned clock rates do not work for you? You are > >>>> re-implementing legacy property now under different name :/ > >>>> > >>>> The assigned clock rates comes in to the picture when we are > >>>> using clock > >>> framework to control the clocks. For this switch there are no > >>> clocks needs to be control, the moment we power on the switch > >>> clocks are enabled by default. This switch provides a mechanism to > >>> control the frequency using i2c. And switch supports only two > >>> frequencies i.e > >> > >> > >> frequency of what, since there are no clocks? > >> > > The axi clock frequency internal to the switch, host can't control > > the enablement of the clocks it can control only the frequency. > > > > we already had a discussion on this on v2[1], and we taught you agreed > > on this property. > > > > [1] > > https://lore.kernel.org/netdev/d1af1eac-f9bd-7a8e-586b-5c2a76445145@codeaurora.org/T/#m3d5864c758f2e05fa15ba522aad6a37e3417bd9f > > > > This points something else. I diged v2 and found many unanswered > questions and unfinished discussion: > The conversation is here: https://lore.kernel.org/linux-arm-msm/20240823094028.7xul4eoiexey5xjm@thinkpad/ But there was no explicit agreement on the usage of 'qps615,axi-clk-freq-hz'. If describing the PCI device's internal clock frequency is not applicable, then I'd recommend to change the clock rate in the driver itself based on the number of DSPs enabled (or based on other configuration). - Mani
On Thu, Nov 28, 2024 at 06:54:32PM +0530, Manivannan Sadhasivam wrote: > On Tue, Nov 26, 2024 at 07:58:16AM +0100, Krzysztof Kozlowski wrote: > > On 26/11/2024 07:50, Krishna Chaitanya Chundru wrote: > > > > > > > > > On 11/25/2024 1:10 PM, Krzysztof Kozlowski wrote: > > >> On 24/11/2024 02:41, Krishna Chaitanya Chundru wrote: > > >>>> ... > > >>>> > > >>>>> + qps615,axi-clk-freq-hz: > > >>>> > > >>>> That's a downstream code you send us. > > >>>> > > >>>> Anyway, why assigned clock rates do not work for you? You are > > >>>> re-implementing legacy property now under different name :/ > > >>>> > > >>>> The assigned clock rates comes in to the picture when we are > > >>>> using clock > > >>> framework to control the clocks. For this switch there are no > > >>> clocks needs to be control, the moment we power on the switch > > >>> clocks are enabled by default. This switch provides a mechanism to > > >>> control the frequency using i2c. And switch supports only two > > >>> frequencies i.e > > >> > > >> > > >> frequency of what, since there are no clocks? > > >> > > > The axi clock frequency internal to the switch, host can't control > > > the enablement of the clocks it can control only the frequency. > > > > > > we already had a discussion on this on v2[1], and we taught you agreed > > > on this property. > > > > > > [1] > > > https://lore.kernel.org/netdev/d1af1eac-f9bd-7a8e-586b-5c2a76445145@codeaurora.org/T/#m3d5864c758f2e05fa15ba522aad6a37e3417bd9f > > > > > > > This points something else. I diged v2 and found many unanswered > > questions and unfinished discussion: > > > > The conversation is here: > https://lore.kernel.org/linux-arm-msm/20240823094028.7xul4eoiexey5xjm@thinkpad/ > > But there was no explicit agreement on the usage of 'qps615,axi-clk-freq-hz'. > > If describing the PCI device's internal clock frequency is not applicable, then > I'd recommend to change the clock rate in the driver itself based on the number > of DSPs enabled (or based on other configuration). Sounds like the best approach. Otherwise it's not obvious, what is the criteria for selecting this or that clock value. > > - Mani > > -- > மணிவண்ணன் சதாசிவம்
On 11/28/2024 7:38 PM, Dmitry Baryshkov wrote: > On Thu, Nov 28, 2024 at 06:54:32PM +0530, Manivannan Sadhasivam wrote: >> On Tue, Nov 26, 2024 at 07:58:16AM +0100, Krzysztof Kozlowski wrote: >>> On 26/11/2024 07:50, Krishna Chaitanya Chundru wrote: >>>> >>>> >>>> On 11/25/2024 1:10 PM, Krzysztof Kozlowski wrote: >>>>> On 24/11/2024 02:41, Krishna Chaitanya Chundru wrote: >>>>>>> ... >>>>>>> >>>>>>>> + qps615,axi-clk-freq-hz: >>>>>>> >>>>>>> That's a downstream code you send us. >>>>>>> >>>>>>> Anyway, why assigned clock rates do not work for you? You are >>>>>>> re-implementing legacy property now under different name :/ >>>>>>> >>>>>>> The assigned clock rates comes in to the picture when we are >>>>>>> using clock >>>>>> framework to control the clocks. For this switch there are no >>>>>> clocks needs to be control, the moment we power on the switch >>>>>> clocks are enabled by default. This switch provides a mechanism to >>>>>> control the frequency using i2c. And switch supports only two >>>>>> frequencies i.e >>>>> >>>>> >>>>> frequency of what, since there are no clocks? >>>>> >>>> The axi clock frequency internal to the switch, host can't control >>>> the enablement of the clocks it can control only the frequency. >>>> >>>> we already had a discussion on this on v2[1], and we taught you agreed >>>> on this property. >>>> >>>> [1] >>>> https://lore.kernel.org/netdev/d1af1eac-f9bd-7a8e-586b-5c2a76445145@codeaurora.org/T/#m3d5864c758f2e05fa15ba522aad6a37e3417bd9f >>>> >>> >>> This points something else. I diged v2 and found many unanswered >>> questions and unfinished discussion: >>> >> >> The conversation is here: >> https://lore.kernel.org/linux-arm-msm/20240823094028.7xul4eoiexey5xjm@thinkpad/ >> >> But there was no explicit agreement on the usage of 'qps615,axi-clk-freq-hz'. >> >> If describing the PCI device's internal clock frequency is not applicable, then >> I'd recommend to change the clock rate in the driver itself based on the number >> of DSPs enabled (or based on other configuration). > > Sounds like the best approach. Otherwise it's not obvious, what is the > criteria for selecting this or that clock value. > I will remove this property in next patch and driver code to set this frequency in the next patch, we will try to come up with some logic later. - Krishna chaitanya. >> >> - Mani >> >> -- >> மணிவண்ணன் சதாசிவம் >
diff --git a/Documentation/devicetree/bindings/pci/qcom,qps615.yaml b/Documentation/devicetree/bindings/pci/qcom,qps615.yaml new file mode 100644 index 000000000000..e6a63a0bb0f3 --- /dev/null +++ b/Documentation/devicetree/bindings/pci/qcom,qps615.yaml @@ -0,0 +1,205 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pci/qcom,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. The 3rd downstream port has integrated endpoint device of + Ethernet MAC. Other two downstream ports are supposed to connect + to external device. + + The QPS615 PCIe switch can be configured through I2C interface before + PCIe link is established to change FTS, ASPM related entry delays, + tx amplitude etc for better power efficiency and functionality. + +properties: + compatible: + enum: + - pci1179,0623 + + reg: + maxItems: 1 + + i2c-parent: + $ref: /schemas/types.yaml#/definitions/phandle-array + description: | + A phandle to the parent I2C node and the slave address of the device + used to do configure qps615 to change FTS, tx amplitude etc. + items: + - description: Phandle to the I2C controller node + - description: I2C slave address + + vdd18-supply: true + + vdd09-supply: true + + vddc-supply: true + + vddio1-supply: true + + vddio2-supply: true + + vddio18-supply: true + + reset-gpios: + maxItems: 1 + description: + GPIO controlling the RESX# pin. + + qps615,axi-clk-freq-hz: + description: + AXI clock rate which is internal bus of the switch + The switch only runs in two frequencies i.e 250MHz and 125MHz. + enum: [125000000, 250000000] + +allOf: + - $ref: "#/$defs/qps615-node" + +patternProperties: + "@1?[0-9a-f](,[0-7])?$": + description: child nodes describing the internal downstream ports + the qps615 switch. + type: object + $ref: "#/$defs/qps615-node" + unevaluatedProperties: false + +$defs: + qps615-node: + type: object + + properties: + qcom,l0s-entry-delay-ns: + description: Aspm l0s entry delay. + + qcom,l1-entry-delay-ns: + description: Aspm l1 entry delay. + + qcom,tx-amplitude-millivolt: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Change Tx Margin setting for low power consumption. + + qcom,no-dfe-support: + type: boolean + description: Disable DFE (Decision Feedback Equalizer), which mitigates + intersymbol interference and some reflections caused by impedance mismatches. + + qcom,nfts: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Number of Fast Training Sequence (FTS) used during L0s to L0 exit + for bit and Symbol lock. + + allOf: + - $ref: /schemas/pci/pci-bus.yaml# + +unevaluatedProperties: false + +required: + - vdd18-supply + - vdd09-supply + - vddc-supply + - vddio1-supply + - vddio2-supply + - vddio18-supply + - i2c-parent + - reset-gpios + +examples: + - | + + #include <dt-bindings/gpio/gpio.h> + + 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>; + + pcie@0,0 { + compatible = "pci1179,0623"; + reg = <0x10000 0x0 0x0 0x0 0x0>; + device_type = "pci"; + #address-cells = <3>; + #size-cells = <2>; + ranges; + bus-range = <0x02 0xff>; + + i2c-parent = <&qup_i2c 0x77>; + + vdd18-supply = <&vdd>; + vdd09-supply = <&vdd>; + vddc-supply = <&vdd>; + vddio1-supply = <&vdd>; + vddio2-supply = <&vdd>; + vddio18-supply = <&vdd>; + + reset-gpios = <&gpio 1 GPIO_ACTIVE_LOW>; + + pcie@1,0 { + reg = <0x20800 0x0 0x0 0x0 0x0>; + #address-cells = <3>; + #size-cells = <2>; + device_type = "pci"; + ranges; + bus-range = <0x03 0xff>; + + qcom,no-dfe-support; + }; + + pcie@2,0 { + reg = <0x21000 0x0 0x0 0x0 0x0>; + #address-cells = <3>; + #size-cells = <2>; + device_type = "pci"; + ranges; + bus-range = <0x04 0xff>; + + qcom,nfts = <10>; + }; + + pcie@3,0 { + reg = <0x21800 0x0 0x0 0x0 0x0>; + #address-cells = <3>; + #size-cells = <2>; + device_type = "pci"; + ranges; + bus-range = <0x05 0xff>; + + qcom,tx-amplitude-millivolt = <10>; + pcie@0,0 { + reg = <0x50000 0x0 0x0 0x0 0x0>; + #address-cells = <3>; + #size-cells = <2>; + device_type = "pci"; + ranges; + + qcom,l1-entry-delay-ns = <10>; + }; + + pcie@0,1 { + reg = <0x50100 0x0 0x0 0x0 0x0>; + #address-cells = <3>; + #size-cells = <2>; + device_type = "pci"; + ranges; + + qcom,l0s-entry-delay-ns = <10>; + }; + }; + }; + }; + };
Add binding describing the Qualcomm PCIe switch, QPS615, which provides Ethernet MAC integrated to the 3rd downstream port and two downstream PCIe ports. Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> --- .../devicetree/bindings/pci/qcom,qps615.yaml | 205 +++++++++++++++++++++ 1 file changed, 205 insertions(+)