[PATCH v7 0/8] PCI: Enable Power and configure the TC9563 PCIe switch

[PATCH v7 0/8] PCI: Enable Power and configure the TC9563 PCIe switch

[Krishna Chaitanya Chundru]

TC9563 is the PCIe switch which has one upstream and three downstream
ports. To one of the downstream ports ethernet MAC is connected as endpoint
device. Other two downstream ports are supposed to connect to external
device. One Host can connect to TC956x by upstream port.

TC9563 switch power is controlled by the GPIO's. After powering on
the switch will immediately participate in the link training. if the
host is also ready by that time PCIe link will established. 

The TC9563 needs to configured certain parameters like de-emphasis,
disable unused port etc before link is established.

As the controller starts link training before the probe of pwrctl driver,
the PCIe link may come up as soon as we power on the switch. Due to this
configuring the switch itself through i2c will not have any effect as
this configuration needs to done before link training. To avoid this
introduce two functions in pci_ops to start_link() & stop_link() which
will disable the link training if the PCIe link is not up yet.

This series depends on the https://lore.kernel.org/all/20250124101038.3871768-3-krishna.chundru@oss.qualcomm.com/

Note: The QPS615 PCIe switch is rebranded version of Toshiba switch TC9563 series.
There is no difference between both the switches, both has two open downstream ports
and one embedded downstream port to which Ethernet MAC is connected. As QPS615 is the
rebranded version of Toshiba switch rename qps615 with tc956x so that this driver
can be leveraged by all who are using Toshiba switch.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
Changes in v7:
- Rename stop_link() & start_link() to asser_perst() and change all
  occurances  (Bjorn).
- Remove PCIe link is active check & relevent patch,  assume this driver will
  be for the swicth connected directly to the root port, if it is
  connected in the DSP of another switch we can't control the link so driver will not have any impact
  we need make them as fixed regulators for now.
- Link to v6: https://lore.kernel.org/r/20250828-qps615_v4_1-v6-0-985f90a7dd03@oss.qualcomm.com

Changes in v6:
- Took v10 patch  https://lore.kernel.org/all/1822371399.1670864.1756212520886.JavaMail.zimbra@raptorengineeringinc.com/
  to my series as my change is dependent on it.
- Add Reviewed-by tag by Rob on dt-binding patch.
- Add Reviewed-by tag by Dmitry on devicetree.
- Fixed compilation errors.
- Fixed n-fts issue point by (Bjorn Helgaas).
- Fixed couple of nits by (Bjorn Helgas).
- Link to v5: https://lore.kernel.org/r/20250412-qps615_v4_1-v5-0-5b6a06132fec@oss.qualcomm.com
Changes from v4:
- Rename tc956x to tc9563, instead of using x which represents overlay board one
  use actual name (Konrad & Krzysztof).
- Remove the patches 9 & 10 from the series and this will be added by mani
- Couple of nits by Konrad
- Have defconfig change for TC956X by Dmitry
- Change the function name pcie_is_link_active to pcie_link_is_active
  replace all call sites of pciehp_check_link_active() with a call
  to the new function. return bool instead of int (Lukas)
- Add pincntrl property for reset gpio (Dmitry)
- Follow the example-schema order, remove ref for the
  tx-amplitude-microvolt, change the vendor prefix (Krzysztof)
- for USP node refer pci-bus-common.yaml and for remaining refer
  pci-pci-bridge.yaml(Mani)
- rebase to latest code and change pci dev retrieval logic due code
  changes in the latest tip.
- Link to v4: https://lore.kernel.org/r/20250225-qps615_v4_1-v4-0-e08633a7bdf8@oss.qualcomm.com
changes from v3:
- move common properties like l0s-delay, l1-delay and nfts to pci-host-common.yaml (bjorn H)
- remove axi-clk-frequency property (Krzysztof)
- Update the pattern properties (Rob)
- use pci-pci-bridge as the reference (Rob)
- change tx-amplitude-millivolt to tx-amplitude-microvolt  (Krzysztof)
- rename qps615_pwrctl_power_on to qps615_pwrctl_bring_up (Bart)
- move the checks for l0s_delay, l1_delay etc to helper functon to
  reduce a level of indentation (Bjorn H)
- move platform_set_drvdata to end after there is no error return (bjorn H)
- Replace GPIOD_ASIS to GPIOD_OUT_HIGH (Mani)
- Create a common api to check if link is up or not and use that to call
  stop_link() and start_link().
- couple of nits in comments, names etc from everyone
Link to v3: https://lore.kernel.org/all/20241112-qps615_pwr-v3-3-29a1e98aa2b0@quicinc.com/T/
Changes from v2:
- As per offline discussions with rob i2c-parent is best suitable to
  use i2c client device. So use i2c-parent as suggested and remove i2c
  client node reference from the dt-bindings & devicetree.
- Remove "PCI: Change the parent to correctly represent pcie hierarchy"
  as this requires seperate discussions.
- Remove bdf logic to identify the dsp's and usp's to make it generic
  by using the logic that downstream devices will always child of
  upstream node and dsp1, dsp2 will always in same order (Dmitry)
- Remove recursive function for parsing devicetree instead parse
  only for required devicetree nodes (Dmitry)
- Fix the issue in be & le conversion (Dmitry).
- Call put_device for i2c device once done with the usage (Dmitry)
- Use $defs to describe common properties between upstream port and
  downstream properties. and remove unneccessary if then. (Krzysztof)
- Place the qcom,qps615 compatibility in dt-binding document in alphabatic order (Krzysztof)
- Rename qcom,no-dfe to describe it as hardware capability and change
  qcom,nfts description to reflect hardware details (Krzysztof)
- Fix the indentation in the example in dt binding (Dmitry)
- Add more description to qcom,nfts (Dmitry)
- Remove nanosec from the property description (Dmitry)
- Link to v2: https://lore.kernel.org/r/linux-arm-msm/20240803-qps615-v2-0-9560b7c71369@quicinc.com/T/
Changes from v1:
- Instead of referencing whole i2c-bus add i2c-client node and reference it (Dmitry)
- Change the regulator's as per the schematics as per offline review
(Bjorn Andresson)
- Remove additional host check in bus.c (Bart)
- For stop_link op change return type from int to void (Bart)
- Remove firmware based approach for configuring sequence as suggested
by multiple reviewers.
- Introduce new dt-properties for the switch to configure the switch
as we are replacing the firmware based approach.
- The downstream ports add properties in the child nodes which will
represented in PCIe hierarchy format.
- Removed D3cold D0 sequence in suspend resume for now as it needs
separate discussion.
- Link to v1: https://lore.kernel.org/linux-pci/20240626-qps615-v1-4-2ade7bd91e02@quicinc.com/T/

---
Krishna Chaitanya Chundru (8):
      dt-bindings: PCI: Add binding for Toshiba TC9563 PCIe switch
      arm64: dts: qcom: qcs6490-rb3gen2: Add TC9563 PCIe switch node
      PCI: Add assert_perst() operation to control PCIe PERST#
      PCI: dwc: Add assert_perst() hook for dwc glue drivers
      PCI: dwc: Implement .assert_perst() hook
      PCI: qcom: Add support for assert_perst()
      arm64: defconfig: Enable TC9563 PWRCTL driver
      PCI: pwrctrl: Add power control driver for tc9563

 .../devicetree/bindings/pci/toshiba,tc9563.yaml    | 178 ++++++
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts       | 128 +++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi               |   2 +-
 arch/arm64/configs/defconfig                       |   1 +
 drivers/pci/controller/dwc/pcie-designware-host.c  |  19 +
 drivers/pci/controller/dwc/pcie-designware.h       |   9 +
 drivers/pci/controller/dwc/pcie-qcom.c             |  13 +
 drivers/pci/pwrctrl/Kconfig                        |  13 +
 drivers/pci/pwrctrl/Makefile                       |   2 +
 drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c           | 639 +++++++++++++++++++++
 include/linux/pci.h                                |   1 +
 11 files changed, 1004 insertions(+), 1 deletion(-)
---
base-commit: e53642b87a4f4b03a8d7e5f8507fc3cd0c595ea6
change-id: 20250212-qps615_v4_1-f8e62fa11786

Best regards,
-- 
Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

[PATCH v7 1/8] dt-bindings: PCI: Add binding for Toshiba TC9563 PCIe switch

[Krishna Chaitanya Chundru]

Add a device tree binding for the Toshiba TC9563 PCIe switch, which
provides an Ethernet MAC integrated to the 3rd downstream port and
two downstream PCIe ports.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
 .../devicetree/bindings/pci/toshiba,tc9563.yaml    | 178 +++++++++++++++++++++
 1 file changed, 178 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml b/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..82c902b67852d6c4b0305764a2231fe04e83458d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
@@ -0,0 +1,178 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/toshiba,tc9563.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Toshiba TC9563 PCIe switch
+
+maintainers:
+  - Krishna chaitanya chundru <quic_krichai@quicinc.com>
+
+description: |
+  Toshiba TC9563 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 TC9563 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
+
+  reset-gpios:
+    maxItems: 1
+    description:
+      GPIO controlling the RESX# pin.
+
+  vdd18-supply: true
+
+  vdd09-supply: true
+
+  vddc-supply: true
+
+  vddio1-supply: true
+
+  vddio2-supply: true
+
+  vddio18-supply: true
+
+  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 tc9563 to change FTS, tx amplitude etc.
+    items:
+      - description: Phandle to the I2C controller node
+      - description: I2C slave address
+
+patternProperties:
+  "^pcie@[1-3],0$":
+    description:
+      child nodes describing the internal downstream ports
+      the tc9563 switch.
+    type: object
+    allOf:
+      - $ref: "#/$defs/tc9563-node"
+      - $ref: /schemas/pci/pci-pci-bridge.yaml#
+    unevaluatedProperties: false
+
+$defs:
+  tc9563-node:
+    type: object
+
+    properties:
+      toshiba,tx-amplitude-microvolt:
+        description:
+          Change Tx Margin setting for low power consumption.
+
+      toshiba,no-dfe-support:
+        type: boolean
+        description:
+          Disable DFE (Decision Feedback Equalizer), which mitigates
+          intersymbol interference and some reflections caused by impedance mismatches.
+
+required:
+  - reset-gpios
+  - vdd18-supply
+  - vdd09-supply
+  - vddc-supply
+  - vddio1-supply
+  - vddio2-supply
+  - vddio18-supply
+  - i2c-parent
+
+allOf:
+  - $ref: "#/$defs/tc9563-node"
+  - $ref: /schemas/pci/pci-bus-common.yaml#
+
+unevaluatedProperties: false
+
+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 {
+                    compatible = "pciclass,0604";
+                    reg = <0x20800 0x0 0x0 0x0 0x0>;
+                    #address-cells = <3>;
+                    #size-cells = <2>;
+                    device_type = "pci";
+                    ranges;
+                    bus-range = <0x03 0xff>;
+
+                    toshiba,no-dfe-support;
+                };
+
+                pcie@2,0 {
+                    compatible = "pciclass,0604";
+                    reg = <0x21000 0x0 0x0 0x0 0x0>;
+                    #address-cells = <3>;
+                    #size-cells = <2>;
+                    device_type = "pci";
+                    ranges;
+                    bus-range = <0x04 0xff>;
+                };
+
+                pcie@3,0 {
+                    compatible = "pciclass,0604";
+                    reg = <0x21800 0x0 0x0 0x0 0x0>;
+                    #address-cells = <3>;
+                    #size-cells = <2>;
+                    device_type = "pci";
+                    ranges;
+                    bus-range = <0x05 0xff>;
+
+                    toshiba,tx-amplitude-microvolt = <10>;
+
+                    ethernet@0,0 {
+                        reg = <0x50000 0x0 0x0 0x0 0x0>;
+                    };
+
+                    ethernet@0,1 {
+                        reg = <0x50100 0x0 0x0 0x0 0x0>;
+                    };
+                };
+            };
+        };
+    };

-- 
2.34.1

[PATCH v7 2/8] arm64: dts: qcom: qcs6490-rb3gen2: Add TC9563 PCIe switch node

[Krishna Chaitanya Chundru]

Add a node for the TC9563 PCIe switch, which has three downstream ports.
Two embedded Ethernet devices are present on one of the downstream ports.
As all these ports are present in the node represent the downstream
ports and embedded endpoints.

Power to the TC9563 is supplied through two LDO regulators, controlled by
two GPIOs, which are added as fixed regulators. Configure the TC9563
through I2C.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Reviewed-by: Bjorn Andersson <andersson@kernel.org>
Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 128 +++++++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi         |   2 +-
 2 files changed, 129 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 18cea8812001421456dc85547c3c711e2c42182a..c8308de1116e07f83f345faca0f29b9da3c4f474 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -262,6 +262,30 @@ vph_pwr: vph-pwr-regulator {
 		regulator-max-microvolt = <3700000>;
 	};
 
+	vdd_ntn_0p9: regulator-vdd-ntn-0p9 {
+		compatible = "regulator-fixed";
+		regulator-name = "VDD_NTN_0P9";
+		gpio = <&pm8350c_gpios 2 GPIO_ACTIVE_HIGH>;
+		regulator-min-microvolt = <899400>;
+		regulator-max-microvolt = <899400>;
+		enable-active-high;
+		pinctrl-0 = <&ntn_0p9_en>;
+		pinctrl-names = "default";
+		regulator-enable-ramp-delay = <4300>;
+	};
+
+	vdd_ntn_1p8: regulator-vdd-ntn-1p8 {
+		compatible = "regulator-fixed";
+		regulator-name = "VDD_NTN_1P8";
+		gpio = <&pm8350c_gpios 3 GPIO_ACTIVE_HIGH>;
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		enable-active-high;
+		pinctrl-0 = <&ntn_1p8_en>;
+		pinctrl-names = "default";
+		regulator-enable-ramp-delay = <10000>;
+	};
+
 	wcn6750-pmu {
 		compatible = "qcom,wcn6750-pmu";
 		pinctrl-0 = <&bt_en>;
@@ -843,6 +867,78 @@ &pcie1_phy {
 	status = "okay";
 };
 
+&pcie1_port0 {
+	pcie@0,0 {
+		compatible = "pci1179,0623";
+		reg = <0x10000 0x0 0x0 0x0 0x0>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+
+		device_type = "pci";
+		ranges;
+		bus-range = <0x2 0xff>;
+
+		vddc-supply = <&vdd_ntn_0p9>;
+		vdd18-supply = <&vdd_ntn_1p8>;
+		vdd09-supply = <&vdd_ntn_0p9>;
+		vddio1-supply = <&vdd_ntn_1p8>;
+		vddio2-supply = <&vdd_ntn_1p8>;
+		vddio18-supply = <&vdd_ntn_1p8>;
+
+		i2c-parent = <&i2c0 0x77>;
+
+		reset-gpios = <&pm8350c_gpios 1 GPIO_ACTIVE_LOW>;
+
+		pinctrl-0 = <&tc9563_rsex_n>;
+		pinctrl-names = "default";
+
+		pcie@1,0 {
+			reg = <0x20800 0x0 0x0 0x0 0x0>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+
+			device_type = "pci";
+			ranges;
+			bus-range = <0x3 0xff>;
+		};
+
+		pcie@2,0 {
+			reg = <0x21000 0x0 0x0 0x0 0x0>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+
+			device_type = "pci";
+			ranges;
+			bus-range = <0x4 0xff>;
+		};
+
+		pcie@3,0 {
+			reg = <0x21800 0x0 0x0 0x0 0x0>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+			device_type = "pci";
+			ranges;
+			bus-range = <0x5 0xff>;
+
+			pci@0,0 {
+				reg = <0x50000 0x0 0x0 0x0 0x0>;
+				#address-cells = <3>;
+				#size-cells = <2>;
+				device_type = "pci";
+				ranges;
+			};
+
+			pci@0,1 {
+				reg = <0x50100 0x0 0x0 0x0 0x0>;
+				#address-cells = <3>;
+				#size-cells = <2>;
+				device_type = "pci";
+				ranges;
+			};
+		};
+	};
+};
+
 &pm7325_gpios {
 	kypd_vol_up_n: kypd-vol-up-n-state {
 		pins = "gpio6";
@@ -1119,6 +1215,38 @@ right_spkr: speaker@0,2 {
 	};
 };
 
+&pm8350c_gpios {
+	ntn_0p9_en: ntn-0p9-en-state {
+		pins = "gpio2";
+		function = "normal";
+
+		bias-disable;
+		input-disable;
+		output-enable;
+		power-source = <0>;
+	};
+
+	ntn_1p8_en: ntn-1p8-en-state {
+		pins = "gpio3";
+		function = "normal";
+
+		bias-disable;
+		input-disable;
+		output-enable;
+		power-source = <0>;
+	};
+
+	tc9563_rsex_n: tc9563-resx-state {
+		pins = "gpio1";
+		function = "normal";
+
+		bias-disable;
+		input-disable;
+		output-enable;
+		power-source = <0>;
+	};
+};
+
 &tlmm {
 	gpio-reserved-ranges = <32 2>, /* ADSP */
 			       <48 4>; /* NFC */
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 4b04dea57ec8cc652e37f1d93c410404adaadd5d..23cf2c8c72b0bab67467e4b60cd57a3e658efa68 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2424,7 +2424,7 @@ pcie1: pcie@1c08000 {
 
 			status = "disabled";
 
-			pcie@0 {
+			pcie1_port0: pcie@0 {
 				device_type = "pci";
 				reg = <0x0 0x0 0x0 0x0 0x0>;
 				bus-range = <0x01 0xff>;

-- 
2.34.1

[PATCH v7 3/8] PCI: Add assert_perst() operation to control PCIe PERST#

[Krishna Chaitanya Chundru]

Controller driver probes firsts, enables link training and scans the
bus. When the PCI bridge is found, its child DT nodes will be scanned
and pwrctrl devices will be created if needed. By the time pwrctrl
driver probe gets called link training is already enabled by controller
driver.

Certain devices like TC956x which uses PCI pwrctl framework needs to
configure the device before PCI link is up.

As the controller driver already enables link training as part of
its probe, the moment device is powered on, controller and device
participates in the link training and link can come up immediately
and may not have time to configure the device.

So we need to stop the link training by using assert_perst() by asserting
the PERST# and de-assert the PERST# after device is configured.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 include/linux/pci.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index d1fdf81fbe1e427aecbc951fa3fdf65c20450b05..ed5dac663e96e3a6ad2edffffc9fa8b348d0a677 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -829,6 +829,7 @@ struct pci_ops {
 	void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where);
 	int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val);
 	int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val);
+	int (*assert_perst)(struct pci_bus *bus, bool assert);
 };
 
 /*

-- 
2.34.1

[PATCH v7 4/8] PCI: dwc: Add assert_perst() hook for dwc glue drivers

[Krishna Chaitanya Chundru]

Add assert_perst() function hook for dwc glue drivers to register with
assert_perst() of pci ops, allowing for better control over the
link initialization and shutdown process.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-designware.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index e995f692a1ecd10130d3be3358827f801811387f..99a02f052e1c8410573ecd44340a9ba4f822e979 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -485,6 +485,7 @@ struct dw_pcie_ops {
 	enum dw_pcie_ltssm (*get_ltssm)(struct dw_pcie *pcie);
 	int	(*start_link)(struct dw_pcie *pcie);
 	void	(*stop_link)(struct dw_pcie *pcie);
+	int	(*assert_perst)(struct dw_pcie *pcie, bool assert);
 };
 
 struct debugfs_info {
@@ -787,6 +788,14 @@ static inline void dw_pcie_stop_link(struct dw_pcie *pci)
 		pci->ops->stop_link(pci);
 }
 
+static inline int dw_pcie_assert_perst(struct dw_pcie *pci, bool assert)
+{
+	if (pci->ops && pci->ops->assert_perst)
+		return pci->ops->assert_perst(pci, assert);
+
+	return 0;
+}
+
 static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
 {
 	u32 val;

-- 
2.34.1

[PATCH v7 5/8] PCI: dwc: Implement .assert_perst() hook

[Krishna Chaitanya Chundru]

Implement assert_perst() function op for dwc drivers.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index e92513c5bda51bde3a7157033ddbd73afa370d78..a209701e8037039191fb6c61b83978d8f561f7a7 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -856,16 +856,35 @@ static void __iomem *dw_pcie_ecam_conf_map_bus(struct pci_bus *bus, unsigned int
 	return pci->dbi_base + where;
 }
 
+static int dw_pcie_op_assert_perst(struct pci_bus *bus, bool assert)
+{
+	struct dw_pcie_rp *pp = bus->sysdata;
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+
+	return dw_pcie_assert_perst(pci, assert);
+}
+
+static int dw_pcie_ecam_op_assert_perst(struct pci_bus *bus, bool assert)
+{
+	struct pci_config_window *cfg = bus->sysdata;
+	struct dw_pcie_rp *pp = cfg->priv;
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+
+	return dw_pcie_assert_perst(pci, assert);
+}
+
 static struct pci_ops dw_pcie_ops = {
 	.map_bus = dw_pcie_own_conf_map_bus,
 	.read = pci_generic_config_read,
 	.write = pci_generic_config_write,
+	.assert_perst = dw_pcie_op_assert_perst,
 };
 
 static struct pci_ops dw_pcie_ecam_ops = {
 	.map_bus = dw_pcie_ecam_conf_map_bus,
 	.read = pci_generic_config_read,
 	.write = pci_generic_config_write,
+	.assert_perst = dw_pcie_ecam_op_assert_perst,
 };
 
 static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)

-- 
2.34.1

[PATCH v7 6/8] PCI: qcom: Add support for assert_perst()

[Krishna Chaitanya Chundru]

Add support for assert_perst() for switches like TC9563, which require
configuration before the PCIe link is established. Such devices use
this function op to assert the PERST# before configuring the device
and once the configuration is done they de-assert the PERST#.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 6948824642dcdcb1f59730479b5a3d196ebf66ee..a6ebcbc19d3c8b28bab53f516ae2a2b42701ca6f 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -640,6 +640,18 @@ static int qcom_pcie_post_init_1_0_0(struct qcom_pcie *pcie)
 	return 0;
 }
 
+static int qcom_pcie_assert_perst(struct dw_pcie *pci, bool assert)
+{
+	struct qcom_pcie *pcie = to_qcom_pcie(pci);
+
+	if (assert)
+		qcom_ep_reset_assert(pcie);
+	else
+		qcom_ep_reset_deassert(pcie);
+
+	return 0;
+}
+
 static void qcom_pcie_2_3_2_ltssm_enable(struct qcom_pcie *pcie)
 {
 	u32 val;
@@ -1448,6 +1460,7 @@ static const struct qcom_pcie_cfg cfg_fw_managed = {
 static const struct dw_pcie_ops dw_pcie_ops = {
 	.link_up = qcom_pcie_link_up,
 	.start_link = qcom_pcie_start_link,
+	.assert_perst = qcom_pcie_assert_perst,
 };
 
 static int qcom_pcie_icc_init(struct qcom_pcie *pcie)

-- 
2.34.1

[PATCH v7 7/8] arm64: defconfig: Enable TC9563 PWRCTL driver

[Krishna Chaitanya Chundru]

Enable TC9563 PCIe switch pwrctl driver by default. This is needed
to power the PCIe switch which is present in Qualcomm RB3gen2 platform.
Without this the switch will not powered up and we can't use the
endpoints connected to the switch.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index e3a2d37bd10423b028f59dc40d6e8ee1c610d6b8..fe5c9951c437a67ac76bf939a9e436eafa3820bf 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -249,6 +249,7 @@ CONFIG_PCIE_LAYERSCAPE_GEN4=y
 CONFIG_PCI_ENDPOINT=y
 CONFIG_PCI_ENDPOINT_CONFIGFS=y
 CONFIG_PCI_EPF_TEST=m
+CONFIG_PCI_PWRCTRL_TC9563=m
 CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
 CONFIG_FW_LOADER_USER_HELPER=y

-- 
2.34.1

[PATCH v7 8/8] PCI: pwrctrl: Add power control driver for tc9563

[Krishna Chaitanya Chundru]

TC9563 is a PCIe switch which has one upstream and three downstream
ports. To one of the downstream ports integrated ethernet MAC is connected
as endpoint device. Other two downstream ports are supposed to connect to
external device. One Host can connect to TC9563 by upstream port. TC9563
switch needs to be configured after powering on and before the PCIe link
was up.

The PCIe controller driver already enables link training at the host side
even before this driver probe happens, due to this when driver enables
power to the switch it participates in the link training and PCIe link
may come up before configuring the switch through I2C. Once the link is
up the configuration done through I2C will not have any effect. To prevent
the host from participating in link training, disable link training on the
host side to ensure the link does not come up before the switch is
configured via I2C.

Based on dt property and type of the port, tc9563 is configured through
I2C.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Reviewed-by: Bjorn Andersson <andersson@kernel.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/pci/pwrctrl/Kconfig              |  13 +
 drivers/pci/pwrctrl/Makefile             |   2 +
 drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c | 639 +++++++++++++++++++++++++++++++
 3 files changed, 654 insertions(+)

diff --git a/drivers/pci/pwrctrl/Kconfig b/drivers/pci/pwrctrl/Kconfig
index 6956c18548114ce12247b560f1ef159eb7e90b10..de8632549f88d5171fcad9879dfeb6250180b060 100644
--- a/drivers/pci/pwrctrl/Kconfig
+++ b/drivers/pci/pwrctrl/Kconfig
@@ -22,6 +22,19 @@ config PCI_PWRCTRL_SLOT
 	  PCI slots. The voltage regulators powering the rails of the PCI slots
 	  are expected to be defined in the devicetree node of the PCI bridge.
 
+config PCI_PWRCTRL_TC9563
+	tristate "PCI Power Control driver for TC9563 PCIe switch"
+	select PCI_PWRCTRL
+	help
+	  Say Y here to enable the PCI Power Control driver of TC9563 PCIe
+	  switch.
+
+	  This driver enables power and configures the TC9563 PCIe switch
+	  through i2c. TC9563 is a PCIe switch which has one upstream and three
+	  downstream ports. To one of the downstream ports integrated ethernet
+	  MAC is connected as endpoint device. Other two downstream ports are
+	  supposed to connect to external device.
+
 # deprecated
 config HAVE_PWRCTL
 	bool
diff --git a/drivers/pci/pwrctrl/Makefile b/drivers/pci/pwrctrl/Makefile
index a4e5808d7850ceb0ca272731e5539e1dfc564e43..13b02282106c2bdbf884f487534f7466047c7fcf 100644
--- a/drivers/pci/pwrctrl/Makefile
+++ b/drivers/pci/pwrctrl/Makefile
@@ -7,3 +7,5 @@ obj-$(CONFIG_PCI_PWRCTRL_PWRSEQ)	+= pci-pwrctrl-pwrseq.o
 
 obj-$(CONFIG_PCI_PWRCTRL_SLOT)		+= pci-pwrctrl-slot.o
 pci-pwrctrl-slot-y			:= slot.o
+
+obj-$(CONFIG_PCI_PWRCTRL_TC9563)	+= pci-pwrctrl-tc9563.o
diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
new file mode 100644
index 0000000000000000000000000000000000000000..2f09931ae671eac16c33a78dfd541fba5dfa446b
--- /dev/null
+++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
@@ -0,0 +1,639 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/pci.h>
+#include <linux/pci-pwrctrl.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+
+#include "../pci.h"
+
+#define TC9563_GPIO_CONFIG		0x801208
+#define TC9563_RESET_GPIO		0x801210
+
+#define TC9563_PORT_L0S_DELAY		0x82496c
+#define TC9563_PORT_L1_DELAY		0x824970
+
+#define TC9563_EMBEDDED_ETH_DELAY	0x8200d8
+#define TC9563_ETH_L1_DELAY_MASK	GENMASK(27, 18)
+#define TC9563_ETH_L1_DELAY_VALUE(x)	FIELD_PREP(TC9563_ETH_L1_DELAY_MASK, x)
+#define TC9563_ETH_L0S_DELAY_MASK	GENMASK(17, 13)
+#define TC9563_ETH_L0S_DELAY_VALUE(x)	FIELD_PREP(TC9563_ETH_L0S_DELAY_MASK, x)
+
+#define TC9563_NFTS_2_5_GT		0x824978
+#define TC9563_NFTS_5_GT		0x82497c
+
+#define TC9563_PORT_LANE_ACCESS_ENABLE	0x828000
+
+#define TC9563_PHY_RATE_CHANGE_OVERRIDE	0x828040
+#define TC9563_PHY_RATE_CHANGE		0x828050
+
+#define TC9563_TX_MARGIN		0x828234
+
+#define TC9563_DFE_ENABLE		0x828a04
+#define TC9563_DFE_EQ0_MODE		0x828a08
+#define TC9563_DFE_EQ1_MODE		0x828a0c
+#define TC9563_DFE_EQ2_MODE		0x828a14
+#define TC9563_DFE_PD_MASK		0x828254
+
+#define TC9563_PORT_SELECT		0x82c02c
+#define TC9563_PORT_ACCESS_ENABLE	0x82c030
+
+#define TC9563_POWER_CONTROL		0x82b09c
+#define TC9563_POWER_CONTROL_OVREN	0x82b2c8
+
+#define TC9563_GPIO_MASK		0xfffffff3
+
+#define TC9563_TX_MARGIN_MIN_VAL	400000
+
+struct tc9563_pwrctrl_reg_setting {
+	unsigned int offset;
+	unsigned int val;
+};
+
+enum tc9563_pwrctrl_ports {
+	TC9563_USP,
+	TC9563_DSP1,
+	TC9563_DSP2,
+	TC9563_DSP3,
+	TC9563_ETHERNET,
+	TC9563_MAX
+};
+
+struct tc9563_pwrctrl_cfg {
+	u32 l0s_delay;
+	u32 l1_delay;
+	u32 tx_amp;
+	u8 nfts[2]; /* GEN1 & GEN2 */
+	bool disable_dfe;
+	bool disable_port;
+};
+
+#define TC9563_PWRCTL_MAX_SUPPLY	6
+
+static const char *const tc9563_supply_names[TC9563_PWRCTL_MAX_SUPPLY] = {
+	"vddc",
+	"vdd18",
+	"vdd09",
+	"vddio1",
+	"vddio2",
+	"vddio18",
+};
+
+struct tc9563_pwrctrl_ctx {
+	struct regulator_bulk_data supplies[TC9563_PWRCTL_MAX_SUPPLY];
+	struct tc9563_pwrctrl_cfg cfg[TC9563_MAX];
+	struct gpio_desc *reset_gpio;
+	struct i2c_adapter *adapter;
+	struct i2c_client *client;
+	struct pci_pwrctrl pwrctrl;
+};
+
+/*
+ * downstream port power off sequence, hardcoding the address
+ * as we don't know register names for these register offsets.
+ */
+static const struct tc9563_pwrctrl_reg_setting common_pwroff_seq[] = {
+	{0x82900c, 0x1},
+	{0x829010, 0x1},
+	{0x829018, 0x0},
+	{0x829020, 0x1},
+	{0x82902c, 0x1},
+	{0x829030, 0x1},
+	{0x82903c, 0x1},
+	{0x829058, 0x0},
+	{0x82905c, 0x1},
+	{0x829060, 0x1},
+	{0x8290cc, 0x1},
+	{0x8290d0, 0x1},
+	{0x8290d8, 0x1},
+	{0x8290e0, 0x1},
+	{0x8290e8, 0x1},
+	{0x8290ec, 0x1},
+	{0x8290f4, 0x1},
+	{0x82910c, 0x1},
+	{0x829110, 0x1},
+	{0x829114, 0x1},
+};
+
+static const struct tc9563_pwrctrl_reg_setting dsp1_pwroff_seq[] = {
+	{TC9563_PORT_ACCESS_ENABLE, 0x2},
+	{TC9563_PORT_LANE_ACCESS_ENABLE, 0x3},
+	{TC9563_POWER_CONTROL, 0x014f4804},
+	{TC9563_POWER_CONTROL_OVREN, 0x1},
+	{TC9563_PORT_ACCESS_ENABLE, 0x4},
+};
+
+static const struct tc9563_pwrctrl_reg_setting dsp2_pwroff_seq[] = {
+	{TC9563_PORT_ACCESS_ENABLE, 0x8},
+	{TC9563_PORT_LANE_ACCESS_ENABLE, 0x1},
+	{TC9563_POWER_CONTROL, 0x014f4804},
+	{TC9563_POWER_CONTROL_OVREN, 0x1},
+	{TC9563_PORT_ACCESS_ENABLE, 0x8},
+};
+
+/*
+ * Since all transfers are initiated by the probe, no locks are necessary,
+ * as there are no concurrent calls.
+ */
+static int tc9563_pwrctrl_i2c_write(struct i2c_client *client,
+				    u32 reg_addr, u32 reg_val)
+{
+	struct i2c_msg msg;
+	u8 msg_buf[7];
+	int ret;
+
+	msg.addr = client->addr;
+	msg.len = 7;
+	msg.flags = 0;
+
+	/* Big Endian for reg addr */
+	put_unaligned_be24(reg_addr, &msg_buf[0]);
+
+	/* Little Endian for reg val */
+	put_unaligned_le32(reg_val, &msg_buf[3]);
+
+	msg.buf = msg_buf;
+	ret = i2c_transfer(client->adapter, &msg, 1);
+	return ret == 1 ? 0 : ret;
+}
+
+static int tc9563_pwrctrl_i2c_read(struct i2c_client *client,
+				   u32 reg_addr, u32 *reg_val)
+{
+	struct i2c_msg msg[2];
+	u8 wr_data[3];
+	u32 rd_data;
+	int ret;
+
+	msg[0].addr = client->addr;
+	msg[0].len = 3;
+	msg[0].flags = 0;
+
+	/* Big Endian for reg addr */
+	put_unaligned_be24(reg_addr, &wr_data[0]);
+
+	msg[0].buf = wr_data;
+
+	msg[1].addr = client->addr;
+	msg[1].len = 4;
+	msg[1].flags = I2C_M_RD;
+
+	msg[1].buf = (u8 *)&rd_data;
+
+	ret = i2c_transfer(client->adapter, &msg[0], 2);
+	if (ret == 2) {
+		*reg_val = get_unaligned_le32(&rd_data);
+		return 0;
+	}
+
+	/* If only one message successfully completed, return -EIO */
+	return ret == 1 ? -EIO : ret;
+}
+
+static int tc9563_pwrctrl_i2c_bulk_write(struct i2c_client *client,
+					 const struct tc9563_pwrctrl_reg_setting *seq, int len)
+{
+	int ret, i;
+
+	for (i = 0; i < len; i++) {
+		ret = tc9563_pwrctrl_i2c_write(client, seq[i].offset, seq[i].val);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int tc9563_pwrctrl_disable_port(struct tc9563_pwrctrl_ctx *ctx,
+				       enum tc9563_pwrctrl_ports port)
+{
+	struct tc9563_pwrctrl_cfg *cfg  = &ctx->cfg[port];
+	const struct tc9563_pwrctrl_reg_setting *seq;
+	int ret, len;
+
+	if (!cfg->disable_port)
+		return 0;
+
+	if (port == TC9563_DSP1) {
+		seq = dsp1_pwroff_seq;
+		len = ARRAY_SIZE(dsp1_pwroff_seq);
+	} else {
+		seq = dsp2_pwroff_seq;
+		len = ARRAY_SIZE(dsp2_pwroff_seq);
+	}
+
+	ret = tc9563_pwrctrl_i2c_bulk_write(ctx->client, seq, len);
+	if (ret)
+		return ret;
+
+	return tc9563_pwrctrl_i2c_bulk_write(ctx->client,
+					    common_pwroff_seq, ARRAY_SIZE(common_pwroff_seq));
+}
+
+static int tc9563_pwrctrl_set_l0s_l1_entry_delay(struct tc9563_pwrctrl_ctx *ctx,
+						 enum tc9563_pwrctrl_ports port, bool is_l1, u32 ns)
+{
+	u32 rd_val, units;
+	int ret;
+
+	if (ns < 256)
+		return 0;
+
+	/* convert to units of 256ns */
+	units = ns / 256;
+
+	if (port == TC9563_ETHERNET) {
+		ret = tc9563_pwrctrl_i2c_read(ctx->client, TC9563_EMBEDDED_ETH_DELAY, &rd_val);
+		if (ret)
+			return ret;
+
+		if (is_l1)
+			rd_val = u32_replace_bits(rd_val, units, TC9563_ETH_L1_DELAY_MASK);
+		else
+			rd_val = u32_replace_bits(rd_val, units, TC9563_ETH_L0S_DELAY_MASK);
+
+		return tc9563_pwrctrl_i2c_write(ctx->client, TC9563_EMBEDDED_ETH_DELAY, rd_val);
+	}
+
+	ret = tc9563_pwrctrl_i2c_write(ctx->client, TC9563_PORT_SELECT, BIT(port));
+	if (ret)
+		return ret;
+
+	return tc9563_pwrctrl_i2c_write(ctx->client,
+				       is_l1 ? TC9563_PORT_L1_DELAY : TC9563_PORT_L0S_DELAY, units);
+}
+
+static int tc9563_pwrctrl_set_tx_amplitude(struct tc9563_pwrctrl_ctx *ctx,
+					   enum tc9563_pwrctrl_ports port, u32 amp)
+{
+	int port_access;
+
+	if (amp < TC9563_TX_MARGIN_MIN_VAL)
+		return 0;
+
+	/* txmargin = (Amp(uV) - 400000) / 3125 */
+	amp = (amp - TC9563_TX_MARGIN_MIN_VAL) / 3125;
+
+	switch (port) {
+	case TC9563_USP:
+		port_access = 0x1;
+		break;
+	case TC9563_DSP1:
+		port_access = 0x2;
+		break;
+	case TC9563_DSP2:
+		port_access = 0x8;
+		break;
+	default:
+		return -EINVAL;
+	};
+
+	struct tc9563_pwrctrl_reg_setting tx_amp_seq[] = {
+		{TC9563_PORT_ACCESS_ENABLE, port_access},
+		{TC9563_PORT_LANE_ACCESS_ENABLE, 0x3},
+		{TC9563_TX_MARGIN, amp},
+	};
+
+	return tc9563_pwrctrl_i2c_bulk_write(ctx->client, tx_amp_seq, ARRAY_SIZE(tx_amp_seq));
+}
+
+static int tc9563_pwrctrl_disable_dfe(struct tc9563_pwrctrl_ctx *ctx,
+				      enum tc9563_pwrctrl_ports port)
+{
+	struct tc9563_pwrctrl_cfg *cfg  = &ctx->cfg[port];
+	int port_access, lane_access = 0x3;
+	u32 phy_rate = 0x21;
+
+	if (!cfg->disable_dfe)
+		return 0;
+
+	switch (port) {
+	case TC9563_USP:
+		phy_rate = 0x1;
+		port_access = 0x1;
+		break;
+	case TC9563_DSP1:
+		port_access = 0x2;
+		break;
+	case TC9563_DSP2:
+		port_access = 0x8;
+		lane_access = 0x1;
+		break;
+	default:
+		return -EINVAL;
+	};
+
+	struct tc9563_pwrctrl_reg_setting disable_dfe_seq[] = {
+		{TC9563_PORT_ACCESS_ENABLE, port_access},
+		{TC9563_PORT_LANE_ACCESS_ENABLE, lane_access},
+		{TC9563_DFE_ENABLE, 0x0},
+		{TC9563_DFE_EQ0_MODE, 0x411},
+		{TC9563_DFE_EQ1_MODE, 0x11},
+		{TC9563_DFE_EQ2_MODE, 0x11},
+		{TC9563_DFE_PD_MASK, 0x7},
+		{TC9563_PHY_RATE_CHANGE_OVERRIDE, 0x10},
+		{TC9563_PHY_RATE_CHANGE, phy_rate},
+		{TC9563_PHY_RATE_CHANGE, 0x0},
+		{TC9563_PHY_RATE_CHANGE_OVERRIDE, 0x0},
+	};
+
+	return tc9563_pwrctrl_i2c_bulk_write(ctx->client,
+					    disable_dfe_seq, ARRAY_SIZE(disable_dfe_seq));
+}
+
+static int tc9563_pwrctrl_set_nfts(struct tc9563_pwrctrl_ctx *ctx,
+				   enum tc9563_pwrctrl_ports port, u8 *nfts)
+{
+	struct tc9563_pwrctrl_reg_setting nfts_seq[] = {
+		{TC9563_NFTS_2_5_GT, nfts[0]},
+		{TC9563_NFTS_5_GT, nfts[1]},
+	};
+	int ret;
+
+	if (!nfts[0])
+		return 0;
+
+	ret =  tc9563_pwrctrl_i2c_write(ctx->client, TC9563_PORT_SELECT, BIT(port));
+	if (ret)
+		return ret;
+
+	return tc9563_pwrctrl_i2c_bulk_write(ctx->client, nfts_seq, ARRAY_SIZE(nfts_seq));
+}
+
+static int tc9563_pwrctrl_assert_deassert_reset(struct tc9563_pwrctrl_ctx *ctx, bool deassert)
+{
+	int ret, val;
+
+	ret = tc9563_pwrctrl_i2c_write(ctx->client, TC9563_GPIO_CONFIG, TC9563_GPIO_MASK);
+	if (ret)
+		return ret;
+
+	val = deassert ? 0xc : 0;
+
+	return tc9563_pwrctrl_i2c_write(ctx->client, TC9563_RESET_GPIO, val);
+}
+
+static int tc9563_pwrctrl_parse_device_dt(struct tc9563_pwrctrl_ctx *ctx, struct device_node *node,
+					  enum tc9563_pwrctrl_ports port)
+{
+	struct tc9563_pwrctrl_cfg *cfg;
+	int ret;
+
+	cfg = &ctx->cfg[port];
+
+	/* Disable port if the status of the port is disabled. */
+	if (!of_device_is_available(node)) {
+		cfg->disable_port = true;
+		return 0;
+	};
+
+	ret = of_property_read_u32(node, "aspm-l0s-entry-delay-ns", &cfg->l0s_delay);
+	if (ret && ret != -EINVAL)
+		return ret;
+
+	ret = of_property_read_u32(node, "aspm-l1-entry-delay-ns", &cfg->l1_delay);
+	if (ret && ret != -EINVAL)
+		return ret;
+
+	ret = of_property_read_u32(node, "qcom,tx-amplitude-microvolt", &cfg->tx_amp);
+	if (ret && ret != -EINVAL)
+		return ret;
+
+	ret = of_property_read_u8_array(node, "n-fts", cfg->nfts, 2);
+	if (ret && ret != -EINVAL)
+		return ret;
+
+	cfg->disable_dfe = of_property_read_bool(node, "qcom,no-dfe-support");
+
+	return 0;
+}
+
+static void tc9563_pwrctrl_power_off(struct tc9563_pwrctrl_ctx *ctx)
+{
+	gpiod_set_value(ctx->reset_gpio, 1);
+
+	regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+}
+
+static int tc9563_pwrctrl_bring_up(struct tc9563_pwrctrl_ctx *ctx)
+{
+	struct tc9563_pwrctrl_cfg *cfg;
+	int ret, i;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+	if (ret < 0)
+		return dev_err_probe(ctx->pwrctrl.dev, ret, "cannot enable regulators\n");
+
+	gpiod_set_value(ctx->reset_gpio, 0);
+
+	 /*
+	  * From TC9563 PORSYS rev 0.2, figure 1.1 POR boot sequence
+	  * wait for 10ms for the internal osc frequency to stabilize.
+	  */
+	fsleep(10000);
+
+	ret = tc9563_pwrctrl_assert_deassert_reset(ctx, false);
+	if (ret)
+		goto power_off;
+
+	for (i = 0; i < TC9563_MAX; i++) {
+		cfg = &ctx->cfg[i];
+		ret = tc9563_pwrctrl_disable_port(ctx, i);
+		if (ret) {
+			dev_err(ctx->pwrctrl.dev, "Disabling port failed\n");
+			goto power_off;
+		}
+
+		ret = tc9563_pwrctrl_set_l0s_l1_entry_delay(ctx, i, false, cfg->l0s_delay);
+		if (ret) {
+			dev_err(ctx->pwrctrl.dev, "Setting L0s entry delay failed\n");
+			goto power_off;
+		}
+
+		ret = tc9563_pwrctrl_set_l0s_l1_entry_delay(ctx, i, true, cfg->l1_delay);
+		if (ret) {
+			dev_err(ctx->pwrctrl.dev, "Setting L1 entry delay failed\n");
+			goto power_off;
+		}
+
+		ret = tc9563_pwrctrl_set_tx_amplitude(ctx, i, cfg->tx_amp);
+		if (ret) {
+			dev_err(ctx->pwrctrl.dev, "Setting Tx amplitude failed\n");
+			goto power_off;
+		}
+
+		ret = tc9563_pwrctrl_set_nfts(ctx, i, cfg->nfts);
+		if (ret) {
+			dev_err(ctx->pwrctrl.dev, "Setting N_FTS failed\n");
+			goto power_off;
+		}
+
+		ret = tc9563_pwrctrl_disable_dfe(ctx, i);
+		if (ret) {
+			dev_err(ctx->pwrctrl.dev, "Disabling DFE failed\n");
+			goto power_off;
+		}
+	}
+
+	ret = tc9563_pwrctrl_assert_deassert_reset(ctx, true);
+	if (!ret)
+		return 0;
+
+power_off:
+	tc9563_pwrctrl_power_off(ctx);
+	return ret;
+}
+
+static int tc9563_pwrctrl_probe(struct platform_device *pdev)
+{
+	struct pci_host_bridge *bridge = to_pci_host_bridge(pdev->dev.parent);
+	struct pci_bus *bus = bridge->bus;
+	struct device *dev = &pdev->dev;
+	enum tc9563_pwrctrl_ports port;
+	struct tc9563_pwrctrl_ctx *ctx;
+	struct device_node *i2c_node;
+	int ret, addr;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ret = of_property_read_u32_index(pdev->dev.of_node, "i2c-parent", 1, &addr);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to read i2c-parent property\n");
+
+	i2c_node = of_parse_phandle(dev->of_node, "i2c-parent", 0);
+	ctx->adapter = of_find_i2c_adapter_by_node(i2c_node);
+	of_node_put(i2c_node);
+	if (!ctx->adapter)
+		return dev_err_probe(dev, -EPROBE_DEFER, "Failed to find I2C adapter\n");
+
+	ctx->client = i2c_new_dummy_device(ctx->adapter, addr);
+	if (IS_ERR(ctx->client)) {
+		dev_err(dev, "Failed to create I2C client\n");
+		i2c_put_adapter(ctx->adapter);
+		return PTR_ERR(ctx->client);
+	}
+
+	for (int i = 0; i < TC9563_PWRCTL_MAX_SUPPLY; i++)
+		ctx->supplies[i].supply = tc9563_supply_names[i];
+
+	ret = devm_regulator_bulk_get(dev, TC9563_PWRCTL_MAX_SUPPLY, ctx->supplies);
+	if (ret) {
+		dev_err_probe(dev, ret,
+			      "failed to get supply regulator\n");
+		goto remove_i2c;
+	}
+
+	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(ctx->reset_gpio)) {
+		ret = dev_err_probe(dev, PTR_ERR(ctx->reset_gpio), "failed to get reset GPIO\n");
+		goto remove_i2c;
+	}
+
+	pci_pwrctrl_init(&ctx->pwrctrl, dev);
+
+	port = TC9563_USP;
+	ret = tc9563_pwrctrl_parse_device_dt(ctx, pdev->dev.of_node, port);
+	if (ret) {
+		dev_err(dev, "failed to parse device tree properties: %d\n", ret);
+		goto remove_i2c;
+	}
+
+	/*
+	 * Downstream ports are always children of the upstream port.
+	 * The first node represents DSP1, the second node represents DSP2, and so on.
+	 */
+	for_each_child_of_node_scoped(pdev->dev.of_node, child) {
+		ret = tc9563_pwrctrl_parse_device_dt(ctx, child, port++);
+		if (ret)
+			break;
+		/* Embedded ethernet device are under DSP3 */
+		if (port == TC9563_DSP3)
+			for_each_child_of_node_scoped(child, child1) {
+				ret = tc9563_pwrctrl_parse_device_dt(ctx, child1, port++);
+				if (ret)
+					break;
+			}
+	}
+	if (ret) {
+		dev_err(dev, "failed to parse device tree properties: %d\n", ret);
+		goto remove_i2c;
+	}
+
+	if (bridge->ops->assert_perst) {
+		ret = bridge->ops->assert_perst(bus, true);
+		if (ret)
+			goto remove_i2c;
+	}
+
+	ret = tc9563_pwrctrl_bring_up(ctx);
+	if (ret)
+		goto remove_i2c;
+
+	if (bridge->ops->assert_perst) {
+		ret = bridge->ops->assert_perst(bus, false);
+		if (ret)
+			goto power_off;
+	}
+
+	ret = devm_pci_pwrctrl_device_set_ready(dev, &ctx->pwrctrl);
+	if (ret)
+		goto power_off;
+
+	platform_set_drvdata(pdev, ctx);
+
+	return 0;
+
+power_off:
+	tc9563_pwrctrl_power_off(ctx);
+remove_i2c:
+	i2c_unregister_device(ctx->client);
+	i2c_put_adapter(ctx->adapter);
+	return ret;
+}
+
+static void tc9563_pwrctrl_remove(struct platform_device *pdev)
+{
+	struct tc9563_pwrctrl_ctx *ctx = platform_get_drvdata(pdev);
+
+	tc9563_pwrctrl_power_off(ctx);
+	i2c_unregister_device(ctx->client);
+	i2c_put_adapter(ctx->adapter);
+}
+
+static const struct of_device_id tc9563_pwrctrl_of_match[] = {
+	{ .compatible = "pci1179,0623"},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tc9563_pwrctrl_of_match);
+
+static struct platform_driver tc9563_pwrctrl_driver = {
+	.driver = {
+		.name = "pwrctrl-tc9563",
+		.of_match_table = tc9563_pwrctrl_of_match,
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+	},
+	.probe = tc9563_pwrctrl_probe,
+	.remove = tc9563_pwrctrl_remove,
+};
+module_platform_driver(tc9563_pwrctrl_driver);
+
+MODULE_AUTHOR("Krishna chaitanya chundru <quic_krichai@quicinc.com>");
+MODULE_DESCRIPTION("TC956x power control driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1

Re: [PATCH v7 8/8] PCI: pwrctrl: Add power control driver for tc9563

[Ilpo Järvinen]

On Wed, 29 Oct 2025, Krishna Chaitanya Chundru wrote:

> TC9563 is a PCIe switch which has one upstream and three downstream
> ports. To one of the downstream ports integrated ethernet MAC is connected
> as endpoint device. Other two downstream ports are supposed to connect to
> external device. One Host can connect to TC9563 by upstream port. TC9563
> switch needs to be configured after powering on and before the PCIe link
> was up.
> 
> The PCIe controller driver already enables link training at the host side
> even before this driver probe happens, due to this when driver enables
> power to the switch it participates in the link training and PCIe link
> may come up before configuring the switch through I2C. Once the link is
> up the configuration done through I2C will not have any effect. To prevent
> the host from participating in link training, disable link training on the
> host side to ensure the link does not come up before the switch is
> configured via I2C.
> 
> Based on dt property and type of the port, tc9563 is configured through
> I2C.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  drivers/pci/pwrctrl/Kconfig              |  13 +
>  drivers/pci/pwrctrl/Makefile             |   2 +
>  drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c | 639 +++++++++++++++++++++++++++++++
>  3 files changed, 654 insertions(+)
> 
> diff --git a/drivers/pci/pwrctrl/Kconfig b/drivers/pci/pwrctrl/Kconfig
> index 6956c18548114ce12247b560f1ef159eb7e90b10..de8632549f88d5171fcad9879dfeb6250180b060 100644
> --- a/drivers/pci/pwrctrl/Kconfig
> +++ b/drivers/pci/pwrctrl/Kconfig
> @@ -22,6 +22,19 @@ config PCI_PWRCTRL_SLOT
>  	  PCI slots. The voltage regulators powering the rails of the PCI slots
>  	  are expected to be defined in the devicetree node of the PCI bridge.
>  
> +config PCI_PWRCTRL_TC9563
> +	tristate "PCI Power Control driver for TC9563 PCIe switch"
> +	select PCI_PWRCTRL
> +	help
> +	  Say Y here to enable the PCI Power Control driver of TC9563 PCIe
> +	  switch.
> +
> +	  This driver enables power and configures the TC9563 PCIe switch
> +	  through i2c. TC9563 is a PCIe switch which has one upstream and three
> +	  downstream ports. To one of the downstream ports integrated ethernet
> +	  MAC is connected as endpoint device. Other two downstream ports are
> +	  supposed to connect to external device.
> +
>  # deprecated
>  config HAVE_PWRCTL
>  	bool
> diff --git a/drivers/pci/pwrctrl/Makefile b/drivers/pci/pwrctrl/Makefile
> index a4e5808d7850ceb0ca272731e5539e1dfc564e43..13b02282106c2bdbf884f487534f7466047c7fcf 100644
> --- a/drivers/pci/pwrctrl/Makefile
> +++ b/drivers/pci/pwrctrl/Makefile
> @@ -7,3 +7,5 @@ obj-$(CONFIG_PCI_PWRCTRL_PWRSEQ)	+= pci-pwrctrl-pwrseq.o
>  
>  obj-$(CONFIG_PCI_PWRCTRL_SLOT)		+= pci-pwrctrl-slot.o
>  pci-pwrctrl-slot-y			:= slot.o
> +
> +obj-$(CONFIG_PCI_PWRCTRL_TC9563)	+= pci-pwrctrl-tc9563.o
> diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..2f09931ae671eac16c33a78dfd541fba5dfa446b
> --- /dev/null
> +++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> @@ -0,0 +1,639 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/pci.h>
> +#include <linux/pci-pwrctrl.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/string.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
> +
> +#include "../pci.h"
> +
> +#define TC9563_GPIO_CONFIG		0x801208
> +#define TC9563_RESET_GPIO		0x801210
> +
> +#define TC9563_PORT_L0S_DELAY		0x82496c
> +#define TC9563_PORT_L1_DELAY		0x824970
> +
> +#define TC9563_EMBEDDED_ETH_DELAY	0x8200d8
> +#define TC9563_ETH_L1_DELAY_MASK	GENMASK(27, 18)

Add include.

> +#define TC9563_ETH_L1_DELAY_VALUE(x)	FIELD_PREP(TC9563_ETH_L1_DELAY_MASK, x)
> +#define TC9563_ETH_L0S_DELAY_MASK	GENMASK(17, 13)
> +#define TC9563_ETH_L0S_DELAY_VALUE(x)	FIELD_PREP(TC9563_ETH_L0S_DELAY_MASK, x)
> +
> +#define TC9563_NFTS_2_5_GT		0x824978
> +#define TC9563_NFTS_5_GT		0x82497c
> +
> +#define TC9563_PORT_LANE_ACCESS_ENABLE	0x828000
> +
> +#define TC9563_PHY_RATE_CHANGE_OVERRIDE	0x828040
> +#define TC9563_PHY_RATE_CHANGE		0x828050
> +
> +#define TC9563_TX_MARGIN		0x828234
> +
> +#define TC9563_DFE_ENABLE		0x828a04
> +#define TC9563_DFE_EQ0_MODE		0x828a08
> +#define TC9563_DFE_EQ1_MODE		0x828a0c
> +#define TC9563_DFE_EQ2_MODE		0x828a14
> +#define TC9563_DFE_PD_MASK		0x828254
> +
> +#define TC9563_PORT_SELECT		0x82c02c
> +#define TC9563_PORT_ACCESS_ENABLE	0x82c030
> +
> +#define TC9563_POWER_CONTROL		0x82b09c
> +#define TC9563_POWER_CONTROL_OVREN	0x82b2c8
> +
> +#define TC9563_GPIO_MASK		0xfffffff3
> +
> +#define TC9563_TX_MARGIN_MIN_VAL	400000

If this is time related, add unit into the name.

> +struct tc9563_pwrctrl_reg_setting {
> +	unsigned int offset;
> +	unsigned int val;
> +};
> +
> +enum tc9563_pwrctrl_ports {
> +	TC9563_USP,
> +	TC9563_DSP1,
> +	TC9563_DSP2,
> +	TC9563_DSP3,
> +	TC9563_ETHERNET,
> +	TC9563_MAX
> +};
> +
> +struct tc9563_pwrctrl_cfg {
> +	u32 l0s_delay;
> +	u32 l1_delay;
> +	u32 tx_amp;
> +	u8 nfts[2]; /* GEN1 & GEN2 */
> +	bool disable_dfe;
> +	bool disable_port;
> +};
> +
> +#define TC9563_PWRCTL_MAX_SUPPLY	6
> +
> +static const char *const tc9563_supply_names[TC9563_PWRCTL_MAX_SUPPLY] = {
> +	"vddc",
> +	"vdd18",
> +	"vdd09",
> +	"vddio1",
> +	"vddio2",
> +	"vddio18",
> +};
> +
> +struct tc9563_pwrctrl_ctx {
> +	struct regulator_bulk_data supplies[TC9563_PWRCTL_MAX_SUPPLY];
> +	struct tc9563_pwrctrl_cfg cfg[TC9563_MAX];
> +	struct gpio_desc *reset_gpio;
> +	struct i2c_adapter *adapter;
> +	struct i2c_client *client;
> +	struct pci_pwrctrl pwrctrl;
> +};
> +
> +/*
> + * downstream port power off sequence, hardcoding the address
> + * as we don't know register names for these register offsets.
> + */
> +static const struct tc9563_pwrctrl_reg_setting common_pwroff_seq[] = {
> +	{0x82900c, 0x1},
> +	{0x829010, 0x1},
> +	{0x829018, 0x0},
> +	{0x829020, 0x1},
> +	{0x82902c, 0x1},
> +	{0x829030, 0x1},
> +	{0x82903c, 0x1},
> +	{0x829058, 0x0},
> +	{0x82905c, 0x1},
> +	{0x829060, 0x1},
> +	{0x8290cc, 0x1},
> +	{0x8290d0, 0x1},
> +	{0x8290d8, 0x1},
> +	{0x8290e0, 0x1},
> +	{0x8290e8, 0x1},
> +	{0x8290ec, 0x1},
> +	{0x8290f4, 0x1},
> +	{0x82910c, 0x1},
> +	{0x829110, 0x1},
> +	{0x829114, 0x1},
> +};
> +
> +static const struct tc9563_pwrctrl_reg_setting dsp1_pwroff_seq[] = {
> +	{TC9563_PORT_ACCESS_ENABLE, 0x2},
> +	{TC9563_PORT_LANE_ACCESS_ENABLE, 0x3},
> +	{TC9563_POWER_CONTROL, 0x014f4804},
> +	{TC9563_POWER_CONTROL_OVREN, 0x1},
> +	{TC9563_PORT_ACCESS_ENABLE, 0x4},
> +};
> +
> +static const struct tc9563_pwrctrl_reg_setting dsp2_pwroff_seq[] = {
> +	{TC9563_PORT_ACCESS_ENABLE, 0x8},
> +	{TC9563_PORT_LANE_ACCESS_ENABLE, 0x1},
> +	{TC9563_POWER_CONTROL, 0x014f4804},
> +	{TC9563_POWER_CONTROL_OVREN, 0x1},
> +	{TC9563_PORT_ACCESS_ENABLE, 0x8},
> +};
> +
> +/*
> + * Since all transfers are initiated by the probe, no locks are necessary,
> + * as there are no concurrent calls.
> + */
> +static int tc9563_pwrctrl_i2c_write(struct i2c_client *client,
> +				    u32 reg_addr, u32 reg_val)
> +{
> +	struct i2c_msg msg;
> +	u8 msg_buf[7];
> +	int ret;
> +
> +	msg.addr = client->addr;
> +	msg.len = 7;
> +	msg.flags = 0;
> +
> +	/* Big Endian for reg addr */
> +	put_unaligned_be24(reg_addr, &msg_buf[0]);
> +
> +	/* Little Endian for reg val */
> +	put_unaligned_le32(reg_val, &msg_buf[3]);
> +
> +	msg.buf = msg_buf;
> +	ret = i2c_transfer(client->adapter, &msg, 1);
> +	return ret == 1 ? 0 : ret;
> +}
> +
> +static int tc9563_pwrctrl_i2c_read(struct i2c_client *client,
> +				   u32 reg_addr, u32 *reg_val)
> +{
> +	struct i2c_msg msg[2];
> +	u8 wr_data[3];
> +	u32 rd_data;
> +	int ret;
> +
> +	msg[0].addr = client->addr;
> +	msg[0].len = 3;
> +	msg[0].flags = 0;
> +
> +	/* Big Endian for reg addr */
> +	put_unaligned_be24(reg_addr, &wr_data[0]);
> +
> +	msg[0].buf = wr_data;
> +
> +	msg[1].addr = client->addr;
> +	msg[1].len = 4;
> +	msg[1].flags = I2C_M_RD;
> +
> +	msg[1].buf = (u8 *)&rd_data;
> +
> +	ret = i2c_transfer(client->adapter, &msg[0], 2);
> +	if (ret == 2) {
> +		*reg_val = get_unaligned_le32(&rd_data);
> +		return 0;
> +	}
> +
> +	/* If only one message successfully completed, return -EIO */
> +	return ret == 1 ? -EIO : ret;
> +}
> +
> +static int tc9563_pwrctrl_i2c_bulk_write(struct i2c_client *client,
> +					 const struct tc9563_pwrctrl_reg_setting *seq, int len)
> +{
> +	int ret, i;
> +
> +	for (i = 0; i < len; i++) {
> +		ret = tc9563_pwrctrl_i2c_write(client, seq[i].offset, seq[i].val);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int tc9563_pwrctrl_disable_port(struct tc9563_pwrctrl_ctx *ctx,
> +				       enum tc9563_pwrctrl_ports port)
> +{
> +	struct tc9563_pwrctrl_cfg *cfg  = &ctx->cfg[port];

Extra space.

> +	const struct tc9563_pwrctrl_reg_setting *seq;
> +	int ret, len;
> +
> +	if (!cfg->disable_port)
> +		return 0;
> +
> +	if (port == TC9563_DSP1) {
> +		seq = dsp1_pwroff_seq;
> +		len = ARRAY_SIZE(dsp1_pwroff_seq);
> +	} else {
> +		seq = dsp2_pwroff_seq;
> +		len = ARRAY_SIZE(dsp2_pwroff_seq);
> +	}
> +
> +	ret = tc9563_pwrctrl_i2c_bulk_write(ctx->client, seq, len);
> +	if (ret)
> +		return ret;
> +
> +	return tc9563_pwrctrl_i2c_bulk_write(ctx->client,
> +					    common_pwroff_seq, ARRAY_SIZE(common_pwroff_seq));
> +}
> +
> +static int tc9563_pwrctrl_set_l0s_l1_entry_delay(struct tc9563_pwrctrl_ctx *ctx,
> +						 enum tc9563_pwrctrl_ports port, bool is_l1, u32 ns)
> +{
> +	u32 rd_val, units;
> +	int ret;
> +
> +	if (ns < 256)
> +		return 0;
> +
> +	/* convert to units of 256ns */
> +	units = ns / 256;

Name this 256 with a define.

> +
> +	if (port == TC9563_ETHERNET) {
> +		ret = tc9563_pwrctrl_i2c_read(ctx->client, TC9563_EMBEDDED_ETH_DELAY, &rd_val);
> +		if (ret)
> +			return ret;
> +
> +		if (is_l1)
> +			rd_val = u32_replace_bits(rd_val, units, TC9563_ETH_L1_DELAY_MASK);
> +		else
> +			rd_val = u32_replace_bits(rd_val, units, TC9563_ETH_L0S_DELAY_MASK);

Move the if into the last parameter using ? : operator like you seem to 
already do below when writing.

> +
> +		return tc9563_pwrctrl_i2c_write(ctx->client, TC9563_EMBEDDED_ETH_DELAY, rd_val);
> +	}
> +
> +	ret = tc9563_pwrctrl_i2c_write(ctx->client, TC9563_PORT_SELECT, BIT(port));
> +	if (ret)
> +		return ret;
> +
> +	return tc9563_pwrctrl_i2c_write(ctx->client,
> +				       is_l1 ? TC9563_PORT_L1_DELAY : TC9563_PORT_L0S_DELAY, units);
> +}
> +
> +static int tc9563_pwrctrl_set_tx_amplitude(struct tc9563_pwrctrl_ctx *ctx,
> +					   enum tc9563_pwrctrl_ports port, u32 amp)
> +{
> +	int port_access;
> +
> +	if (amp < TC9563_TX_MARGIN_MIN_VAL)
> +		return 0;
> +
> +	/* txmargin = (Amp(uV) - 400000) / 3125 */
> +	amp = (amp - TC9563_TX_MARGIN_MIN_VAL) / 3125;
> +
> +	switch (port) {
> +	case TC9563_USP:
> +		port_access = 0x1;
> +		break;
> +	case TC9563_DSP1:
> +		port_access = 0x2;
> +		break;
> +	case TC9563_DSP2:
> +		port_access = 0x8;
> +		break;
> +	default:
> +		return -EINVAL;
> +	};
> +
> +	struct tc9563_pwrctrl_reg_setting tx_amp_seq[] = {
> +		{TC9563_PORT_ACCESS_ENABLE, port_access},
> +		{TC9563_PORT_LANE_ACCESS_ENABLE, 0x3},
> +		{TC9563_TX_MARGIN, amp},
> +	};
> +
> +	return tc9563_pwrctrl_i2c_bulk_write(ctx->client, tx_amp_seq, ARRAY_SIZE(tx_amp_seq));

Include for ARRAY_SIZE().

> +}
> +
> +static int tc9563_pwrctrl_disable_dfe(struct tc9563_pwrctrl_ctx *ctx,
> +				      enum tc9563_pwrctrl_ports port)
> +{
> +	struct tc9563_pwrctrl_cfg *cfg  = &ctx->cfg[port];

Extra space.

> +	int port_access, lane_access = 0x3;
> +	u32 phy_rate = 0x21;
> +
> +	if (!cfg->disable_dfe)
> +		return 0;
> +
> +	switch (port) {
> +	case TC9563_USP:
> +		phy_rate = 0x1;
> +		port_access = 0x1;
> +		break;
> +	case TC9563_DSP1:
> +		port_access = 0x2;
> +		break;
> +	case TC9563_DSP2:
> +		port_access = 0x8;
> +		lane_access = 0x1;
> +		break;
> +	default:
> +		return -EINVAL;
> +	};

You seem to have these kind of switches in a few places and some similar 
if statements, maybe move all this hw info data into an array of structs 
so you don't need to do switch/case everywhere.

> +	struct tc9563_pwrctrl_reg_setting disable_dfe_seq[] = {
> +		{TC9563_PORT_ACCESS_ENABLE, port_access},
> +		{TC9563_PORT_LANE_ACCESS_ENABLE, lane_access},
> +		{TC9563_DFE_ENABLE, 0x0},
> +		{TC9563_DFE_EQ0_MODE, 0x411},
> +		{TC9563_DFE_EQ1_MODE, 0x11},
> +		{TC9563_DFE_EQ2_MODE, 0x11},
> +		{TC9563_DFE_PD_MASK, 0x7},
> +		{TC9563_PHY_RATE_CHANGE_OVERRIDE, 0x10},
> +		{TC9563_PHY_RATE_CHANGE, phy_rate},
> +		{TC9563_PHY_RATE_CHANGE, 0x0},
> +		{TC9563_PHY_RATE_CHANGE_OVERRIDE, 0x0},
> +	};
> +
> +	return tc9563_pwrctrl_i2c_bulk_write(ctx->client,
> +					    disable_dfe_seq, ARRAY_SIZE(disable_dfe_seq));
> +}
> +
> +static int tc9563_pwrctrl_set_nfts(struct tc9563_pwrctrl_ctx *ctx,
> +				   enum tc9563_pwrctrl_ports port, u8 *nfts)
> +{
> +	struct tc9563_pwrctrl_reg_setting nfts_seq[] = {
> +		{TC9563_NFTS_2_5_GT, nfts[0]},
> +		{TC9563_NFTS_5_GT, nfts[1]},
> +	};
> +	int ret;
> +
> +	if (!nfts[0])
> +		return 0;
> +
> +	ret =  tc9563_pwrctrl_i2c_write(ctx->client, TC9563_PORT_SELECT, BIT(port));
> +	if (ret)
> +		return ret;
> +
> +	return tc9563_pwrctrl_i2c_bulk_write(ctx->client, nfts_seq, ARRAY_SIZE(nfts_seq));
> +}
> +
> +static int tc9563_pwrctrl_assert_deassert_reset(struct tc9563_pwrctrl_ctx *ctx, bool deassert)
> +{
> +	int ret, val;
> +
> +	ret = tc9563_pwrctrl_i2c_write(ctx->client, TC9563_GPIO_CONFIG, TC9563_GPIO_MASK);
> +	if (ret)
> +		return ret;
> +
> +	val = deassert ? 0xc : 0;

Can this 0xc be named?

Perhaps TC9563_GPIO_MASK could be constructed as inverse of it? (I'm not 
sure if these those are connected but it looks suspiciously so).

> +
> +	return tc9563_pwrctrl_i2c_write(ctx->client, TC9563_RESET_GPIO, val);
> +}
> +
> +static int tc9563_pwrctrl_parse_device_dt(struct tc9563_pwrctrl_ctx *ctx, struct device_node *node,
> +					  enum tc9563_pwrctrl_ports port)
> +{
> +	struct tc9563_pwrctrl_cfg *cfg;
> +	int ret;
> +
> +	cfg = &ctx->cfg[port];

This is boilerplace, do it when declaring the variable.

> +
> +	/* Disable port if the status of the port is disabled. */
> +	if (!of_device_is_available(node)) {
> +		cfg->disable_port = true;
> +		return 0;
> +	};
> +
> +	ret = of_property_read_u32(node, "aspm-l0s-entry-delay-ns", &cfg->l0s_delay);
> +	if (ret && ret != -EINVAL)
> +		return ret;
> +
> +	ret = of_property_read_u32(node, "aspm-l1-entry-delay-ns", &cfg->l1_delay);
> +	if (ret && ret != -EINVAL)
> +		return ret;
> +
> +	ret = of_property_read_u32(node, "qcom,tx-amplitude-microvolt", &cfg->tx_amp);
> +	if (ret && ret != -EINVAL)
> +		return ret;
> +
> +	ret = of_property_read_u8_array(node, "n-fts", cfg->nfts, 2);

2 -> ARRAY_SIZE().

> +	if (ret && ret != -EINVAL)
> +		return ret;
> +
> +	cfg->disable_dfe = of_property_read_bool(node, "qcom,no-dfe-support");
> +
> +	return 0;
> +}
> +
> +static void tc9563_pwrctrl_power_off(struct tc9563_pwrctrl_ctx *ctx)
> +{
> +	gpiod_set_value(ctx->reset_gpio, 1);
> +
> +	regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +}
> +
> +static int tc9563_pwrctrl_bring_up(struct tc9563_pwrctrl_ctx *ctx)
> +{
> +	struct tc9563_pwrctrl_cfg *cfg;
> +	int ret, i;
> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (ret < 0)
> +		return dev_err_probe(ctx->pwrctrl.dev, ret, "cannot enable regulators\n");
> +
> +	gpiod_set_value(ctx->reset_gpio, 0);
> +
> +	 /*
> +	  * From TC9563 PORSYS rev 0.2, figure 1.1 POR boot sequence
> +	  * wait for 10ms for the internal osc frequency to stabilize.
> +	  */
> +	fsleep(10000);

Add define for it (with the unit in the name) and put comment next to that 
define.


(10 * USEC_PER_MSEC)

> +
> +	ret = tc9563_pwrctrl_assert_deassert_reset(ctx, false);
> +	if (ret)
> +		goto power_off;
> +
> +	for (i = 0; i < TC9563_MAX; i++) {
> +		cfg = &ctx->cfg[i];
> +		ret = tc9563_pwrctrl_disable_port(ctx, i);
> +		if (ret) {
> +			dev_err(ctx->pwrctrl.dev, "Disabling port failed\n");
> +			goto power_off;
> +		}
> +
> +		ret = tc9563_pwrctrl_set_l0s_l1_entry_delay(ctx, i, false, cfg->l0s_delay);
> +		if (ret) {
> +			dev_err(ctx->pwrctrl.dev, "Setting L0s entry delay failed\n");
> +			goto power_off;
> +		}
> +
> +		ret = tc9563_pwrctrl_set_l0s_l1_entry_delay(ctx, i, true, cfg->l1_delay);
> +		if (ret) {
> +			dev_err(ctx->pwrctrl.dev, "Setting L1 entry delay failed\n");
> +			goto power_off;
> +		}
> +
> +		ret = tc9563_pwrctrl_set_tx_amplitude(ctx, i, cfg->tx_amp);
> +		if (ret) {
> +			dev_err(ctx->pwrctrl.dev, "Setting Tx amplitude failed\n");
> +			goto power_off;
> +		}
> +
> +		ret = tc9563_pwrctrl_set_nfts(ctx, i, cfg->nfts);

I wonder why you need to pass cfg-> variables to these functions (except 
for the l0s/l1 variation) as the values is available through 
ctx->cfg[port]->ntfs in the function itself.

> +		if (ret) {
> +			dev_err(ctx->pwrctrl.dev, "Setting N_FTS failed\n");
> +			goto power_off;
> +		}
> +
> +		ret = tc9563_pwrctrl_disable_dfe(ctx, i);
> +		if (ret) {
> +			dev_err(ctx->pwrctrl.dev, "Disabling DFE failed\n");
> +			goto power_off;
> +		}
> +	}
> +
> +	ret = tc9563_pwrctrl_assert_deassert_reset(ctx, true);
> +	if (!ret)
> +		return 0;
> +
> +power_off:
> +	tc9563_pwrctrl_power_off(ctx);
> +	return ret;
> +}
> +
> +static int tc9563_pwrctrl_probe(struct platform_device *pdev)
> +{
> +	struct pci_host_bridge *bridge = to_pci_host_bridge(pdev->dev.parent);
> +	struct pci_bus *bus = bridge->bus;
> +	struct device *dev = &pdev->dev;
> +	enum tc9563_pwrctrl_ports port;
> +	struct tc9563_pwrctrl_ctx *ctx;
> +	struct device_node *i2c_node;
> +	int ret, addr;
> +
> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	ret = of_property_read_u32_index(pdev->dev.of_node, "i2c-parent", 1, &addr);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to read i2c-parent property\n");
> +
> +	i2c_node = of_parse_phandle(dev->of_node, "i2c-parent", 0);
> +	ctx->adapter = of_find_i2c_adapter_by_node(i2c_node);
> +	of_node_put(i2c_node);
> +	if (!ctx->adapter)
> +		return dev_err_probe(dev, -EPROBE_DEFER, "Failed to find I2C adapter\n");
> +
> +	ctx->client = i2c_new_dummy_device(ctx->adapter, addr);
> +	if (IS_ERR(ctx->client)) {
> +		dev_err(dev, "Failed to create I2C client\n");
> +		i2c_put_adapter(ctx->adapter);
> +		return PTR_ERR(ctx->client);
> +	}
> +
> +	for (int i = 0; i < TC9563_PWRCTL_MAX_SUPPLY; i++)

I'd prefer using ARRAY_SIZE() as you're iterating over an array here.

> +		ctx->supplies[i].supply = tc9563_supply_names[i];
> +
> +	ret = devm_regulator_bulk_get(dev, TC9563_PWRCTL_MAX_SUPPLY, ctx->supplies);
> +	if (ret) {
> +		dev_err_probe(dev, ret,
> +			      "failed to get supply regulator\n");

Fits to one line.

> +		goto remove_i2c;
> +	}
> +
> +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(ctx->reset_gpio)) {
> +		ret = dev_err_probe(dev, PTR_ERR(ctx->reset_gpio), "failed to get reset GPIO\n");
> +		goto remove_i2c;
> +	}
> +
> +	pci_pwrctrl_init(&ctx->pwrctrl, dev);
> +
> +	port = TC9563_USP;
> +	ret = tc9563_pwrctrl_parse_device_dt(ctx, pdev->dev.of_node, port);
> +	if (ret) {
> +		dev_err(dev, "failed to parse device tree properties: %d\n", ret);
> +		goto remove_i2c;
> +	}
> +
> +	/*
> +	 * Downstream ports are always children of the upstream port.
> +	 * The first node represents DSP1, the second node represents DSP2, and so on.
> +	 */
> +	for_each_child_of_node_scoped(pdev->dev.of_node, child) {
> +		ret = tc9563_pwrctrl_parse_device_dt(ctx, child, port++);
> +		if (ret)
> +			break;
> +		/* Embedded ethernet device are under DSP3 */
> +		if (port == TC9563_DSP3)

Add braces.

I assume this check okay with the port being post-incremented? I'd prefer 
doing this code like this:

		port++;
		/*
		 * Downstream ports are always children of the upstream port.
		 * The first node represents DSP1, the second node represents 
		 * DSP2, and so on.
		 */
		ret = tc9563_pwrctrl_parse_device_dt(ctx, child, port - 1);
		if (ret)
			break;
		/* Embedded ethernet device are under DSP3 */
		if (port == TC9563_DSP3) {

While this is functionally same as yours, the explicit "port - 1" makes 
the difference between tc9563_pwrctrl_ports and DT indexing more obvious
(the code doesn't look buggy on the first sight like it does with the 
post-incrementing. I was sure the indexing is buggy until I read that 
comment that was too far away from the line it was relevant to).

> +			for_each_child_of_node_scoped(child, child1) {
> +				ret = tc9563_pwrctrl_parse_device_dt(ctx, child1, port++);
> +				if (ret)
> +					break;
> +			}
> +	}
> +	if (ret) {
> +		dev_err(dev, "failed to parse device tree properties: %d\n", ret);
> +		goto remove_i2c;
> +	}
> +
> +	if (bridge->ops->assert_perst) {
> +		ret = bridge->ops->assert_perst(bus, true);
> +		if (ret)
> +			goto remove_i2c;
> +	}
> +
> +	ret = tc9563_pwrctrl_bring_up(ctx);
> +	if (ret)
> +		goto remove_i2c;
> +
> +	if (bridge->ops->assert_perst) {
> +		ret = bridge->ops->assert_perst(bus, false);
> +		if (ret)
> +			goto power_off;
> +	}
> +
> +	ret = devm_pci_pwrctrl_device_set_ready(dev, &ctx->pwrctrl);
> +	if (ret)
> +		goto power_off;
> +
> +	platform_set_drvdata(pdev, ctx);
> +
> +	return 0;
> +
> +power_off:
> +	tc9563_pwrctrl_power_off(ctx);
> +remove_i2c:
> +	i2c_unregister_device(ctx->client);
> +	i2c_put_adapter(ctx->adapter);
> +	return ret;
> +}
> +
> +static void tc9563_pwrctrl_remove(struct platform_device *pdev)
> +{
> +	struct tc9563_pwrctrl_ctx *ctx = platform_get_drvdata(pdev);
> +
> +	tc9563_pwrctrl_power_off(ctx);
> +	i2c_unregister_device(ctx->client);
> +	i2c_put_adapter(ctx->adapter);
> +}
> +
> +static const struct of_device_id tc9563_pwrctrl_of_match[] = {
> +	{ .compatible = "pci1179,0623"},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, tc9563_pwrctrl_of_match);
> +
> +static struct platform_driver tc9563_pwrctrl_driver = {
> +	.driver = {
> +		.name = "pwrctrl-tc9563",
> +		.of_match_table = tc9563_pwrctrl_of_match,
> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
> +	},
> +	.probe = tc9563_pwrctrl_probe,
> +	.remove = tc9563_pwrctrl_remove,
> +};
> +module_platform_driver(tc9563_pwrctrl_driver);
> +
> +MODULE_AUTHOR("Krishna chaitanya chundru <quic_krichai@quicinc.com>");
> +MODULE_DESCRIPTION("TC956x power control driver");
> +MODULE_LICENSE("GPL");
> 
> 

-- 
 i.

Re: [PATCH v7 7/8] arm64: defconfig: Enable TC9563 PWRCTL driver

[Bartosz Golaszewski]

On Wed, Oct 29, 2025 at 12:30 PM Krishna Chaitanya Chundru
<krishna.chundru@oss.qualcomm.com> wrote:
>
> Enable TC9563 PCIe switch pwrctl driver by default. This is needed
> to power the PCIe switch which is present in Qualcomm RB3gen2 platform.
> Without this the switch will not powered up and we can't use the
> endpoints connected to the switch.
>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  arch/arm64/configs/defconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index e3a2d37bd10423b028f59dc40d6e8ee1c610d6b8..fe5c9951c437a67ac76bf939a9e436eafa3820bf 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -249,6 +249,7 @@ CONFIG_PCIE_LAYERSCAPE_GEN4=y
>  CONFIG_PCI_ENDPOINT=y
>  CONFIG_PCI_ENDPOINT_CONFIGFS=y
>  CONFIG_PCI_EPF_TEST=m
> +CONFIG_PCI_PWRCTRL_TC9563=m
>  CONFIG_DEVTMPFS=y
>  CONFIG_DEVTMPFS_MOUNT=y
>  CONFIG_FW_LOADER_USER_HELPER=y
>
> --
> 2.34.1
>

Can't we just do the following in the respective Kconfig entry?

config PCI_PWRCTRL_TC9563
    tristate ...
    default m if ARCH_QCOM

Bart

Re: [PATCH v7 0/8] PCI: Enable Power and configure the TC9563 PCIe switch

[Bjorn Helgaas]

On Wed, Oct 29, 2025 at 04:59:53PM +0530, Krishna Chaitanya Chundru wrote:
> TC9563 is the PCIe switch which has one upstream and three downstream
> ports. To one of the downstream ports ethernet MAC is connected as endpoint
> device. Other two downstream ports are supposed to connect to external
> device. One Host can connect to TC956x by upstream port.
> 
> TC9563 switch power is controlled by the GPIO's. After powering on
> the switch will immediately participate in the link training. if the
> host is also ready by that time PCIe link will established. 
> 
> The TC9563 needs to configured certain parameters like de-emphasis,
> disable unused port etc before link is established.
> 
> As the controller starts link training before the probe of pwrctl driver,
> the PCIe link may come up as soon as we power on the switch. Due to this
> configuring the switch itself through i2c will not have any effect as
> this configuration needs to done before link training. To avoid this
> introduce two functions in pci_ops to start_link() & stop_link() which
> will disable the link training if the PCIe link is not up yet.
> 
> This series depends on the https://lore.kernel.org/all/20250124101038.3871768-3-krishna.chundru@oss.qualcomm.com/

What does this series apply to?  It doesn't apply cleanly to v6.18-rc1
(the normal base for topic branches) or v6.18-rc3 or pci/next.

I tried first applying the patches from
https://lore.kernel.org/all/20250124101038.3871768-3-krishna.chundru@oss.qualcomm.com/,
but those don't apply to -rc1 or -rc3 either.

Bjorn

Re: [PATCH v7 7/8] arm64: defconfig: Enable TC9563 PWRCTL driver

[Dmitry Baryshkov]

On Wed, Oct 29, 2025 at 05:00:00PM +0530, Krishna Chaitanya Chundru wrote:
> Enable TC9563 PCIe switch pwrctl driver by default. This is needed
> to power the PCIe switch which is present in Qualcomm RB3gen2 platform.
> Without this the switch will not powered up and we can't use the
> endpoints connected to the switch.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  arch/arm64/configs/defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 

There is some broken logic in your commit order. How comes defconfig
changes come before the driver which actually defines that Kconfig
entry?

Please reorder your patches _logically_:
- DT bindings
- driver changes
- DTS changes
- defconfig changes

-- 
With best wishes
Dmitry

Re: [PATCH v7 0/8] PCI: Enable Power and configure the TC9563 PCIe switch

[Dmitry Baryshkov]

On Wed, Oct 29, 2025 at 06:23:23PM -0500, Bjorn Helgaas wrote:
> On Wed, Oct 29, 2025 at 04:59:53PM +0530, Krishna Chaitanya Chundru wrote:
> > TC9563 is the PCIe switch which has one upstream and three downstream
> > ports. To one of the downstream ports ethernet MAC is connected as endpoint
> > device. Other two downstream ports are supposed to connect to external
> > device. One Host can connect to TC956x by upstream port.
> > 
> > TC9563 switch power is controlled by the GPIO's. After powering on
> > the switch will immediately participate in the link training. if the
> > host is also ready by that time PCIe link will established. 
> > 
> > The TC9563 needs to configured certain parameters like de-emphasis,
> > disable unused port etc before link is established.
> > 
> > As the controller starts link training before the probe of pwrctl driver,
> > the PCIe link may come up as soon as we power on the switch. Due to this
> > configuring the switch itself through i2c will not have any effect as
> > this configuration needs to done before link training. To avoid this
> > introduce two functions in pci_ops to start_link() & stop_link() which
> > will disable the link training if the PCIe link is not up yet.
> > 
> > This series depends on the https://lore.kernel.org/all/20250124101038.3871768-3-krishna.chundru@oss.qualcomm.com/
> 
> What does this series apply to?  It doesn't apply cleanly to v6.18-rc1
> (the normal base for topic branches) or v6.18-rc3 or pci/next.

Juding by the base-commit in the cover letter, it is the following tree:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=e53642b87a4f4b03a8d7e5f8507fc3cd0c595ea6

Merge tag 'v6.18-rc3-smb-server-fixes' of git://git.samba.org/ksmbd

Not that I have an idea, _why_ that tree was used, was it really used or
why there are no dependencies mentioned in the footer of the cover
letter.

> 
> I tried first applying the patches from
> https://lore.kernel.org/all/20250124101038.3871768-3-krishna.chundru@oss.qualcomm.com/,
> but those don't apply to -rc1 or -rc3 either.
> 
> Bjorn

-- 
With best wishes
Dmitry

Re: [PATCH v7 7/8] arm64: defconfig: Enable TC9563 PWRCTL driver

[Krishna Chaitanya Chundru]

On 10/29/2025 6:45 PM, Bartosz Golaszewski wrote:
> On Wed, Oct 29, 2025 at 12:30 PM Krishna Chaitanya Chundru
> <krishna.chundru@oss.qualcomm.com> wrote:
>> Enable TC9563 PCIe switch pwrctl driver by default. This is needed
>> to power the PCIe switch which is present in Qualcomm RB3gen2 platform.
>> Without this the switch will not powered up and we can't use the
>> endpoints connected to the switch.
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> ---
>>   arch/arm64/configs/defconfig | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
>> index e3a2d37bd10423b028f59dc40d6e8ee1c610d6b8..fe5c9951c437a67ac76bf939a9e436eafa3820bf 100644
>> --- a/arch/arm64/configs/defconfig
>> +++ b/arch/arm64/configs/defconfig
>> @@ -249,6 +249,7 @@ CONFIG_PCIE_LAYERSCAPE_GEN4=y
>>   CONFIG_PCI_ENDPOINT=y
>>   CONFIG_PCI_ENDPOINT_CONFIGFS=y
>>   CONFIG_PCI_EPF_TEST=m
>> +CONFIG_PCI_PWRCTRL_TC9563=m
>>   CONFIG_DEVTMPFS=y
>>   CONFIG_DEVTMPFS_MOUNT=y
>>   CONFIG_FW_LOADER_USER_HELPER=y
>>
>> --
>> 2.34.1
>>
> Can't we just do the following in the respective Kconfig entry?
>
> config PCI_PWRCTRL_TC9563
>      tristate ...
>      default m if ARCH_QCOM

Ack, I will do in next series

- Krishna Chaitanya.

> Bart

Re: [PATCH v7 7/8] arm64: defconfig: Enable TC9563 PWRCTL driver

[Krishna Chaitanya Chundru]

On 10/30/2025 5:34 AM, Dmitry Baryshkov wrote:
> On Wed, Oct 29, 2025 at 05:00:00PM +0530, Krishna Chaitanya Chundru wrote:
>> Enable TC9563 PCIe switch pwrctl driver by default. This is needed
>> to power the PCIe switch which is present in Qualcomm RB3gen2 platform.
>> Without this the switch will not powered up and we can't use the
>> endpoints connected to the switch.
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> ---
>>   arch/arm64/configs/defconfig | 1 +
>>   1 file changed, 1 insertion(+)
>>
> There is some broken logic in your commit order. How comes defconfig
> changes come before the driver which actually defines that Kconfig
> entry?
>
> Please reorder your patches _logically_:
> - DT bindings
> - driver changes
> - DTS changes
> - defconfig changes

Ack.

- Krishna Chaitanya.

>

Re: [PATCH v7 0/8] PCI: Enable Power and configure the TC9563 PCIe switch

[Krishna Chaitanya Chundru]

On 10/30/2025 4:53 AM, Bjorn Helgaas wrote:
> On Wed, Oct 29, 2025 at 04:59:53PM +0530, Krishna Chaitanya Chundru wrote:
>> TC9563 is the PCIe switch which has one upstream and three downstream
>> ports. To one of the downstream ports ethernet MAC is connected as endpoint
>> device. Other two downstream ports are supposed to connect to external
>> device. One Host can connect to TC956x by upstream port.
>>
>> TC9563 switch power is controlled by the GPIO's. After powering on
>> the switch will immediately participate in the link training. if the
>> host is also ready by that time PCIe link will established.
>>
>> The TC9563 needs to configured certain parameters like de-emphasis,
>> disable unused port etc before link is established.
>>
>> As the controller starts link training before the probe of pwrctl driver,
>> the PCIe link may come up as soon as we power on the switch. Due to this
>> configuring the switch itself through i2c will not have any effect as
>> this configuration needs to done before link training. To avoid this
>> introduce two functions in pci_ops to start_link() & stop_link() which
>> will disable the link training if the PCIe link is not up yet.
>>
>> This series depends on the https://lore.kernel.org/all/20250124101038.3871768-3-krishna.chundru@oss.qualcomm.com/
> What does this series apply to?  It doesn't apply cleanly to v6.18-rc1
> (the normal base for topic branches) or v6.18-rc3 or pci/next.
I sent this on top of rc3 as we have some dependencies with latest 
changes i.e ecam changes in dwc driver.
> I tried first applying the patches from
> https://lore.kernel.org/all/20250124101038.3871768-3-krishna.chundru@oss.qualcomm.com/,
> but those don't apply to -rc1 or -rc3 either.

This needs to be applied on the dts schema in github, it is already 
applied I will remove this reference in next
series.

- Krishna Chaitanya.

>
> Bjorn

Re: [PATCH v7 8/8] PCI: pwrctrl: Add power control driver for tc9563

[Krishna Chaitanya Chundru]

On 10/29/2025 6:38 PM, Ilpo Järvinen wrote:
> On Wed, 29 Oct 2025, Krishna Chaitanya Chundru wrote:
>
>> TC9563 is a PCIe switch which has one upstream and three downstream
>> ports. To one of the downstream ports integrated ethernet MAC is connected
>> as endpoint device. Other two downstream ports are supposed to connect to
>> external device. One Host can connect to TC9563 by upstream port. TC9563
>> switch needs to be configured after powering on and before the PCIe link
>> was up.
>>
>> The PCIe controller driver already enables link training at the host side
>> even before this driver probe happens, due to this when driver enables
>> power to the switch it participates in the link training and PCIe link
>> may come up before configuring the switch through I2C. Once the link is
>> up the configuration done through I2C will not have any effect. To prevent
>> the host from participating in link training, disable link training on the
>> host side to ensure the link does not come up before the switch is
>> configured via I2C.
>>
>> Based on dt property and type of the port, tc9563 is configured through
>> I2C.
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
>> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>> ---
>>   drivers/pci/pwrctrl/Kconfig              |  13 +
>>   drivers/pci/pwrctrl/Makefile             |   2 +
>>   drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c | 639 +++++++++++++++++++++++++++++++
>>   3 files changed, 654 insertions(+)
>>
>> diff --git a/drivers/pci/pwrctrl/Kconfig b/drivers/pci/pwrctrl/Kconfig
>> index 6956c18548114ce12247b560f1ef159eb7e90b10..de8632549f88d5171fcad9879dfeb6250180b060 100644
>> --- a/drivers/pci/pwrctrl/Kconfig
>> +++ b/drivers/pci/pwrctrl/Kconfig
>> @@ -22,6 +22,19 @@ config PCI_PWRCTRL_SLOT
>>   	  PCI slots. The voltage regulators powering the rails of the PCI slots
>>   	  are expected to be defined in the devicetree node of the PCI bridge.
>>   
>> +config PCI_PWRCTRL_TC9563
>> +	tristate "PCI Power Control driver for TC9563 PCIe switch"
>> +	select PCI_PWRCTRL
>> +	help
>> +	  Say Y here to enable the PCI Power Control driver of TC9563 PCIe
>> +	  switch.
>> +
>> +	  This driver enables power and configures the TC9563 PCIe switch
>> +	  through i2c. TC9563 is a PCIe switch which has one upstream and three
>> +	  downstream ports. To one of the downstream ports integrated ethernet
>> +	  MAC is connected as endpoint device. Other two downstream ports are
>> +	  supposed to connect to external device.
>> +
>>   # deprecated
>>   config HAVE_PWRCTL
>>   	bool
>> diff --git a/drivers/pci/pwrctrl/Makefile b/drivers/pci/pwrctrl/Makefile
>> index a4e5808d7850ceb0ca272731e5539e1dfc564e43..13b02282106c2bdbf884f487534f7466047c7fcf 100644
>> --- a/drivers/pci/pwrctrl/Makefile
>> +++ b/drivers/pci/pwrctrl/Makefile
>> @@ -7,3 +7,5 @@ obj-$(CONFIG_PCI_PWRCTRL_PWRSEQ)	+= pci-pwrctrl-pwrseq.o
>>   
>>   obj-$(CONFIG_PCI_PWRCTRL_SLOT)		+= pci-pwrctrl-slot.o
>>   pci-pwrctrl-slot-y			:= slot.o
>> +
>> +obj-$(CONFIG_PCI_PWRCTRL_TC9563)	+= pci-pwrctrl-tc9563.o
>> diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..2f09931ae671eac16c33a78dfd541fba5dfa446b
>> --- /dev/null
>> +++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
>> @@ -0,0 +1,639 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/delay.h>
>> +#include <linux/device.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/i2c.h>
>> +#include <linux/mod_devicetable.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/pci.h>
>> +#include <linux/pci-pwrctrl.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/string.h>
>> +#include <linux/types.h>
>> +#include <linux/unaligned.h>
>> +
>> +#include "../pci.h"
>> +
>> +#define TC9563_GPIO_CONFIG		0x801208
>> +#define TC9563_RESET_GPIO		0x801210
>> +
>> +#define TC9563_PORT_L0S_DELAY		0x82496c
>> +#define TC9563_PORT_L1_DELAY		0x824970
>> +
>> +#define TC9563_EMBEDDED_ETH_DELAY	0x8200d8
>> +#define TC9563_ETH_L1_DELAY_MASK	GENMASK(27, 18)
> Add include.
>
>> +#define TC9563_ETH_L1_DELAY_VALUE(x)	FIELD_PREP(TC9563_ETH_L1_DELAY_MASK, x)
>> +#define TC9563_ETH_L0S_DELAY_MASK	GENMASK(17, 13)
>> +#define TC9563_ETH_L0S_DELAY_VALUE(x)	FIELD_PREP(TC9563_ETH_L0S_DELAY_MASK, x)
>> +
>> +#define TC9563_NFTS_2_5_GT		0x824978
>> +#define TC9563_NFTS_5_GT		0x82497c
>> +
>> +#define TC9563_PORT_LANE_ACCESS_ENABLE	0x828000
>> +
>> +#define TC9563_PHY_RATE_CHANGE_OVERRIDE	0x828040
>> +#define TC9563_PHY_RATE_CHANGE		0x828050
>> +
>> +#define TC9563_TX_MARGIN		0x828234
>> +
>> +#define TC9563_DFE_ENABLE		0x828a04
>> +#define TC9563_DFE_EQ0_MODE		0x828a08
>> +#define TC9563_DFE_EQ1_MODE		0x828a0c
>> +#define TC9563_DFE_EQ2_MODE		0x828a14
>> +#define TC9563_DFE_PD_MASK		0x828254
>> +
>> +#define TC9563_PORT_SELECT		0x82c02c
>> +#define TC9563_PORT_ACCESS_ENABLE	0x82c030
>> +
>> +#define TC9563_POWER_CONTROL		0x82b09c
>> +#define TC9563_POWER_CONTROL_OVREN	0x82b2c8
>> +
>> +#define TC9563_GPIO_MASK		0xfffffff3
>> +
>> +#define TC9563_TX_MARGIN_MIN_VAL	400000
> If this is time related, add unit into the name.
>
>> +struct tc9563_pwrctrl_reg_setting {
>> +	unsigned int offset;
>> +	unsigned int val;
>> +};
>> +
>> +enum tc9563_pwrctrl_ports {
>> +	TC9563_USP,
>> +	TC9563_DSP1,
>> +	TC9563_DSP2,
>> +	TC9563_DSP3,
>> +	TC9563_ETHERNET,
>> +	TC9563_MAX
>> +};
>> +
>> +struct tc9563_pwrctrl_cfg {
>> +	u32 l0s_delay;
>> +	u32 l1_delay;
>> +	u32 tx_amp;
>> +	u8 nfts[2]; /* GEN1 & GEN2 */
>> +	bool disable_dfe;
>> +	bool disable_port;
>> +};
>> +
>> +#define TC9563_PWRCTL_MAX_SUPPLY	6
>> +
>> +static const char *const tc9563_supply_names[TC9563_PWRCTL_MAX_SUPPLY] = {
>> +	"vddc",
>> +	"vdd18",
>> +	"vdd09",
>> +	"vddio1",
>> +	"vddio2",
>> +	"vddio18",
>> +};
>> +
>> +struct tc9563_pwrctrl_ctx {
>> +	struct regulator_bulk_data supplies[TC9563_PWRCTL_MAX_SUPPLY];
>> +	struct tc9563_pwrctrl_cfg cfg[TC9563_MAX];
>> +	struct gpio_desc *reset_gpio;
>> +	struct i2c_adapter *adapter;
>> +	struct i2c_client *client;
>> +	struct pci_pwrctrl pwrctrl;
>> +};
>> +
>> +/*
>> + * downstream port power off sequence, hardcoding the address
>> + * as we don't know register names for these register offsets.
>> + */
>> +static const struct tc9563_pwrctrl_reg_setting common_pwroff_seq[] = {
>> +	{0x82900c, 0x1},
>> +	{0x829010, 0x1},
>> +	{0x829018, 0x0},
>> +	{0x829020, 0x1},
>> +	{0x82902c, 0x1},
>> +	{0x829030, 0x1},
>> +	{0x82903c, 0x1},
>> +	{0x829058, 0x0},
>> +	{0x82905c, 0x1},
>> +	{0x829060, 0x1},
>> +	{0x8290cc, 0x1},
>> +	{0x8290d0, 0x1},
>> +	{0x8290d8, 0x1},
>> +	{0x8290e0, 0x1},
>> +	{0x8290e8, 0x1},
>> +	{0x8290ec, 0x1},
>> +	{0x8290f4, 0x1},
>> +	{0x82910c, 0x1},
>> +	{0x829110, 0x1},
>> +	{0x829114, 0x1},
>> +};
>> +
>> +static const struct tc9563_pwrctrl_reg_setting dsp1_pwroff_seq[] = {
>> +	{TC9563_PORT_ACCESS_ENABLE, 0x2},
>> +	{TC9563_PORT_LANE_ACCESS_ENABLE, 0x3},
>> +	{TC9563_POWER_CONTROL, 0x014f4804},
>> +	{TC9563_POWER_CONTROL_OVREN, 0x1},
>> +	{TC9563_PORT_ACCESS_ENABLE, 0x4},
>> +};
>> +
>> +static const struct tc9563_pwrctrl_reg_setting dsp2_pwroff_seq[] = {
>> +	{TC9563_PORT_ACCESS_ENABLE, 0x8},
>> +	{TC9563_PORT_LANE_ACCESS_ENABLE, 0x1},
>> +	{TC9563_POWER_CONTROL, 0x014f4804},
>> +	{TC9563_POWER_CONTROL_OVREN, 0x1},
>> +	{TC9563_PORT_ACCESS_ENABLE, 0x8},
>> +};
>> +
>> +/*
>> + * Since all transfers are initiated by the probe, no locks are necessary,
>> + * as there are no concurrent calls.
>> + */
>> +static int tc9563_pwrctrl_i2c_write(struct i2c_client *client,
>> +				    u32 reg_addr, u32 reg_val)
>> +{
>> +	struct i2c_msg msg;
>> +	u8 msg_buf[7];
>> +	int ret;
>> +
>> +	msg.addr = client->addr;
>> +	msg.len = 7;
>> +	msg.flags = 0;
>> +
>> +	/* Big Endian for reg addr */
>> +	put_unaligned_be24(reg_addr, &msg_buf[0]);
>> +
>> +	/* Little Endian for reg val */
>> +	put_unaligned_le32(reg_val, &msg_buf[3]);
>> +
>> +	msg.buf = msg_buf;
>> +	ret = i2c_transfer(client->adapter, &msg, 1);
>> +	return ret == 1 ? 0 : ret;
>> +}
>> +
>> +static int tc9563_pwrctrl_i2c_read(struct i2c_client *client,
>> +				   u32 reg_addr, u32 *reg_val)
>> +{
>> +	struct i2c_msg msg[2];
>> +	u8 wr_data[3];
>> +	u32 rd_data;
>> +	int ret;
>> +
>> +	msg[0].addr = client->addr;
>> +	msg[0].len = 3;
>> +	msg[0].flags = 0;
>> +
>> +	/* Big Endian for reg addr */
>> +	put_unaligned_be24(reg_addr, &wr_data[0]);
>> +
>> +	msg[0].buf = wr_data;
>> +
>> +	msg[1].addr = client->addr;
>> +	msg[1].len = 4;
>> +	msg[1].flags = I2C_M_RD;
>> +
>> +	msg[1].buf = (u8 *)&rd_data;
>> +
>> +	ret = i2c_transfer(client->adapter, &msg[0], 2);
>> +	if (ret == 2) {
>> +		*reg_val = get_unaligned_le32(&rd_data);
>> +		return 0;
>> +	}
>> +
>> +	/* If only one message successfully completed, return -EIO */
>> +	return ret == 1 ? -EIO : ret;
>> +}
>> +
>> +static int tc9563_pwrctrl_i2c_bulk_write(struct i2c_client *client,
>> +					 const struct tc9563_pwrctrl_reg_setting *seq, int len)
>> +{
>> +	int ret, i;
>> +
>> +	for (i = 0; i < len; i++) {
>> +		ret = tc9563_pwrctrl_i2c_write(client, seq[i].offset, seq[i].val);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int tc9563_pwrctrl_disable_port(struct tc9563_pwrctrl_ctx *ctx,
>> +				       enum tc9563_pwrctrl_ports port)
>> +{
>> +	struct tc9563_pwrctrl_cfg *cfg  = &ctx->cfg[port];
> Extra space.
>
>> +	const struct tc9563_pwrctrl_reg_setting *seq;
>> +	int ret, len;
>> +
>> +	if (!cfg->disable_port)
>> +		return 0;
>> +
>> +	if (port == TC9563_DSP1) {
>> +		seq = dsp1_pwroff_seq;
>> +		len = ARRAY_SIZE(dsp1_pwroff_seq);
>> +	} else {
>> +		seq = dsp2_pwroff_seq;
>> +		len = ARRAY_SIZE(dsp2_pwroff_seq);
>> +	}
>> +
>> +	ret = tc9563_pwrctrl_i2c_bulk_write(ctx->client, seq, len);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return tc9563_pwrctrl_i2c_bulk_write(ctx->client,
>> +					    common_pwroff_seq, ARRAY_SIZE(common_pwroff_seq));
>> +}
>> +
>> +static int tc9563_pwrctrl_set_l0s_l1_entry_delay(struct tc9563_pwrctrl_ctx *ctx,
>> +						 enum tc9563_pwrctrl_ports port, bool is_l1, u32 ns)
>> +{
>> +	u32 rd_val, units;
>> +	int ret;
>> +
>> +	if (ns < 256)
>> +		return 0;
>> +
>> +	/* convert to units of 256ns */
>> +	units = ns / 256;
> Name this 256 with a define.
>
>> +
>> +	if (port == TC9563_ETHERNET) {
>> +		ret = tc9563_pwrctrl_i2c_read(ctx->client, TC9563_EMBEDDED_ETH_DELAY, &rd_val);
>> +		if (ret)
>> +			return ret;
>> +
>> +		if (is_l1)
>> +			rd_val = u32_replace_bits(rd_val, units, TC9563_ETH_L1_DELAY_MASK);
>> +		else
>> +			rd_val = u32_replace_bits(rd_val, units, TC9563_ETH_L0S_DELAY_MASK);
> Move the if into the last parameter using ? : operator like you seem to
> already do below when writing.
We tried it previously but we are seeing compilation errors if we use ? 
: way.
>> +
>> +		return tc9563_pwrctrl_i2c_write(ctx->client, TC9563_EMBEDDED_ETH_DELAY, rd_val);
>> +	}
>> +
>> +	ret = tc9563_pwrctrl_i2c_write(ctx->client, TC9563_PORT_SELECT, BIT(port));
>> +	if (ret)
>> +		return ret;
>> +
>> +	return tc9563_pwrctrl_i2c_write(ctx->client,
>> +				       is_l1 ? TC9563_PORT_L1_DELAY : TC9563_PORT_L0S_DELAY, units);
>> +}
>> +
>> +static int tc9563_pwrctrl_set_tx_amplitude(struct tc9563_pwrctrl_ctx *ctx,
>> +					   enum tc9563_pwrctrl_ports port, u32 amp)
>> +{
>> +	int port_access;
>> +
>> +	if (amp < TC9563_TX_MARGIN_MIN_VAL)
>> +		return 0;
>> +
>> +	/* txmargin = (Amp(uV) - 400000) / 3125 */
>> +	amp = (amp - TC9563_TX_MARGIN_MIN_VAL) / 3125;
>> +
>> +	switch (port) {
>> +	case TC9563_USP:
>> +		port_access = 0x1;
>> +		break;
>> +	case TC9563_DSP1:
>> +		port_access = 0x2;
>> +		break;
>> +	case TC9563_DSP2:
>> +		port_access = 0x8;
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	};
>> +
>> +	struct tc9563_pwrctrl_reg_setting tx_amp_seq[] = {
>> +		{TC9563_PORT_ACCESS_ENABLE, port_access},
>> +		{TC9563_PORT_LANE_ACCESS_ENABLE, 0x3},
>> +		{TC9563_TX_MARGIN, amp},
>> +	};
>> +
>> +	return tc9563_pwrctrl_i2c_bulk_write(ctx->client, tx_amp_seq, ARRAY_SIZE(tx_amp_seq));
> Include for ARRAY_SIZE().
>
>> +}
>> +
>> +static int tc9563_pwrctrl_disable_dfe(struct tc9563_pwrctrl_ctx *ctx,
>> +				      enum tc9563_pwrctrl_ports port)
>> +{
>> +	struct tc9563_pwrctrl_cfg *cfg  = &ctx->cfg[port];
> Extra space.
>
>> +	int port_access, lane_access = 0x3;
>> +	u32 phy_rate = 0x21;
>> +
>> +	if (!cfg->disable_dfe)
>> +		return 0;
>> +
>> +	switch (port) {
>> +	case TC9563_USP:
>> +		phy_rate = 0x1;
>> +		port_access = 0x1;
>> +		break;
>> +	case TC9563_DSP1:
>> +		port_access = 0x2;
>> +		break;
>> +	case TC9563_DSP2:
>> +		port_access = 0x8;
>> +		lane_access = 0x1;
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	};
> You seem to have these kind of switches in a few places and some similar
> if statements, maybe move all this hw info data into an array of structs
> so you don't need to do switch/case everywhere.

I got your point, there are only two places we used switch and that two 
different values.
So adding array of structures might not give any extra benefit as we are 
using in this
functions only. For now we will continue with this method only.

>> +	struct tc9563_pwrctrl_reg_setting disable_dfe_seq[] = {
>> +		{TC9563_PORT_ACCESS_ENABLE, port_access},
>> +		{TC9563_PORT_LANE_ACCESS_ENABLE, lane_access},
>> +		{TC9563_DFE_ENABLE, 0x0},
>> +		{TC9563_DFE_EQ0_MODE, 0x411},
>> +		{TC9563_DFE_EQ1_MODE, 0x11},
>> +		{TC9563_DFE_EQ2_MODE, 0x11},
>> +		{TC9563_DFE_PD_MASK, 0x7},
>> +		{TC9563_PHY_RATE_CHANGE_OVERRIDE, 0x10},
>> +		{TC9563_PHY_RATE_CHANGE, phy_rate},
>> +		{TC9563_PHY_RATE_CHANGE, 0x0},
>> +		{TC9563_PHY_RATE_CHANGE_OVERRIDE, 0x0},
>> +	};
>> +
>> +	return tc9563_pwrctrl_i2c_bulk_write(ctx->client,
>> +					    disable_dfe_seq, ARRAY_SIZE(disable_dfe_seq));
>> +}
>> +
>> +static int tc9563_pwrctrl_set_nfts(struct tc9563_pwrctrl_ctx *ctx,
>> +				   enum tc9563_pwrctrl_ports port, u8 *nfts)
>> +{
>> +	struct tc9563_pwrctrl_reg_setting nfts_seq[] = {
>> +		{TC9563_NFTS_2_5_GT, nfts[0]},
>> +		{TC9563_NFTS_5_GT, nfts[1]},
>> +	};
>> +	int ret;
>> +
>> +	if (!nfts[0])
>> +		return 0;
>> +
>> +	ret =  tc9563_pwrctrl_i2c_write(ctx->client, TC9563_PORT_SELECT, BIT(port));
>> +	if (ret)
>> +		return ret;
>> +
>> +	return tc9563_pwrctrl_i2c_bulk_write(ctx->client, nfts_seq, ARRAY_SIZE(nfts_seq));
>> +}
>> +
>> +static int tc9563_pwrctrl_assert_deassert_reset(struct tc9563_pwrctrl_ctx *ctx, bool deassert)
>> +{
>> +	int ret, val;
>> +
>> +	ret = tc9563_pwrctrl_i2c_write(ctx->client, TC9563_GPIO_CONFIG, TC9563_GPIO_MASK);
>> +	if (ret)
>> +		return ret;
>> +
>> +	val = deassert ? 0xc : 0;
> Can this 0xc be named?
>
> Perhaps TC9563_GPIO_MASK could be constructed as inverse of it? (I'm not
> sure if these those are connected but it looks suspiciously so).
>
>> +
>> +	return tc9563_pwrctrl_i2c_write(ctx->client, TC9563_RESET_GPIO, val);
>> +}
>> +
>> +static int tc9563_pwrctrl_parse_device_dt(struct tc9563_pwrctrl_ctx *ctx, struct device_node *node,
>> +					  enum tc9563_pwrctrl_ports port)
>> +{
>> +	struct tc9563_pwrctrl_cfg *cfg;
>> +	int ret;
>> +
>> +	cfg = &ctx->cfg[port];
> This is boilerplace, do it when declaring the variable.
>
>> +
>> +	/* Disable port if the status of the port is disabled. */
>> +	if (!of_device_is_available(node)) {
>> +		cfg->disable_port = true;
>> +		return 0;
>> +	};
>> +
>> +	ret = of_property_read_u32(node, "aspm-l0s-entry-delay-ns", &cfg->l0s_delay);
>> +	if (ret && ret != -EINVAL)
>> +		return ret;
>> +
>> +	ret = of_property_read_u32(node, "aspm-l1-entry-delay-ns", &cfg->l1_delay);
>> +	if (ret && ret != -EINVAL)
>> +		return ret;
>> +
>> +	ret = of_property_read_u32(node, "qcom,tx-amplitude-microvolt", &cfg->tx_amp);
>> +	if (ret && ret != -EINVAL)
>> +		return ret;
>> +
>> +	ret = of_property_read_u8_array(node, "n-fts", cfg->nfts, 2);
> 2 -> ARRAY_SIZE().
>
>> +	if (ret && ret != -EINVAL)
>> +		return ret;
>> +
>> +	cfg->disable_dfe = of_property_read_bool(node, "qcom,no-dfe-support");
>> +
>> +	return 0;
>> +}
>> +
>> +static void tc9563_pwrctrl_power_off(struct tc9563_pwrctrl_ctx *ctx)
>> +{
>> +	gpiod_set_value(ctx->reset_gpio, 1);
>> +
>> +	regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
>> +}
>> +
>> +static int tc9563_pwrctrl_bring_up(struct tc9563_pwrctrl_ctx *ctx)
>> +{
>> +	struct tc9563_pwrctrl_cfg *cfg;
>> +	int ret, i;
>> +
>> +	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
>> +	if (ret < 0)
>> +		return dev_err_probe(ctx->pwrctrl.dev, ret, "cannot enable regulators\n");
>> +
>> +	gpiod_set_value(ctx->reset_gpio, 0);
>> +
>> +	 /*
>> +	  * From TC9563 PORSYS rev 0.2, figure 1.1 POR boot sequence
>> +	  * wait for 10ms for the internal osc frequency to stabilize.
>> +	  */
>> +	fsleep(10000);
> Add define for it (with the unit in the name) and put comment next to that
> define.
>
>
> (10 * USEC_PER_MSEC)
>
>> +
>> +	ret = tc9563_pwrctrl_assert_deassert_reset(ctx, false);
>> +	if (ret)
>> +		goto power_off;
>> +
>> +	for (i = 0; i < TC9563_MAX; i++) {
>> +		cfg = &ctx->cfg[i];
>> +		ret = tc9563_pwrctrl_disable_port(ctx, i);
>> +		if (ret) {
>> +			dev_err(ctx->pwrctrl.dev, "Disabling port failed\n");
>> +			goto power_off;
>> +		}
>> +
>> +		ret = tc9563_pwrctrl_set_l0s_l1_entry_delay(ctx, i, false, cfg->l0s_delay);
>> +		if (ret) {
>> +			dev_err(ctx->pwrctrl.dev, "Setting L0s entry delay failed\n");
>> +			goto power_off;
>> +		}
>> +
>> +		ret = tc9563_pwrctrl_set_l0s_l1_entry_delay(ctx, i, true, cfg->l1_delay);
>> +		if (ret) {
>> +			dev_err(ctx->pwrctrl.dev, "Setting L1 entry delay failed\n");
>> +			goto power_off;
>> +		}
>> +
>> +		ret = tc9563_pwrctrl_set_tx_amplitude(ctx, i, cfg->tx_amp);
>> +		if (ret) {
>> +			dev_err(ctx->pwrctrl.dev, "Setting Tx amplitude failed\n");
>> +			goto power_off;
>> +		}
>> +
>> +		ret = tc9563_pwrctrl_set_nfts(ctx, i, cfg->nfts);
> I wonder why you need to pass cfg-> variables to these functions (except
> for the l0s/l1 variation) as the values is available through
> ctx->cfg[port]->ntfs in the function itself.
>
>> +		if (ret) {
>> +			dev_err(ctx->pwrctrl.dev, "Setting N_FTS failed\n");
>> +			goto power_off;
>> +		}
>> +
>> +		ret = tc9563_pwrctrl_disable_dfe(ctx, i);
>> +		if (ret) {
>> +			dev_err(ctx->pwrctrl.dev, "Disabling DFE failed\n");
>> +			goto power_off;
>> +		}
>> +	}
>> +
>> +	ret = tc9563_pwrctrl_assert_deassert_reset(ctx, true);
>> +	if (!ret)
>> +		return 0;
>> +
>> +power_off:
>> +	tc9563_pwrctrl_power_off(ctx);
>> +	return ret;
>> +}
>> +
>> +static int tc9563_pwrctrl_probe(struct platform_device *pdev)
>> +{
>> +	struct pci_host_bridge *bridge = to_pci_host_bridge(pdev->dev.parent);
>> +	struct pci_bus *bus = bridge->bus;
>> +	struct device *dev = &pdev->dev;
>> +	enum tc9563_pwrctrl_ports port;
>> +	struct tc9563_pwrctrl_ctx *ctx;
>> +	struct device_node *i2c_node;
>> +	int ret, addr;
>> +
>> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>> +	if (!ctx)
>> +		return -ENOMEM;
>> +
>> +	ret = of_property_read_u32_index(pdev->dev.of_node, "i2c-parent", 1, &addr);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "Failed to read i2c-parent property\n");
>> +
>> +	i2c_node = of_parse_phandle(dev->of_node, "i2c-parent", 0);
>> +	ctx->adapter = of_find_i2c_adapter_by_node(i2c_node);
>> +	of_node_put(i2c_node);
>> +	if (!ctx->adapter)
>> +		return dev_err_probe(dev, -EPROBE_DEFER, "Failed to find I2C adapter\n");
>> +
>> +	ctx->client = i2c_new_dummy_device(ctx->adapter, addr);
>> +	if (IS_ERR(ctx->client)) {
>> +		dev_err(dev, "Failed to create I2C client\n");
>> +		i2c_put_adapter(ctx->adapter);
>> +		return PTR_ERR(ctx->client);
>> +	}
>> +
>> +	for (int i = 0; i < TC9563_PWRCTL_MAX_SUPPLY; i++)
> I'd prefer using ARRAY_SIZE() as you're iterating over an array here.
>
>> +		ctx->supplies[i].supply = tc9563_supply_names[i];
>> +
>> +	ret = devm_regulator_bulk_get(dev, TC9563_PWRCTL_MAX_SUPPLY, ctx->supplies);
>> +	if (ret) {
>> +		dev_err_probe(dev, ret,
>> +			      "failed to get supply regulator\n");
> Fits to one line.
>
>> +		goto remove_i2c;
>> +	}
>> +
>> +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
>> +	if (IS_ERR(ctx->reset_gpio)) {
>> +		ret = dev_err_probe(dev, PTR_ERR(ctx->reset_gpio), "failed to get reset GPIO\n");
>> +		goto remove_i2c;
>> +	}
>> +
>> +	pci_pwrctrl_init(&ctx->pwrctrl, dev);
>> +
>> +	port = TC9563_USP;
>> +	ret = tc9563_pwrctrl_parse_device_dt(ctx, pdev->dev.of_node, port);
>> +	if (ret) {
>> +		dev_err(dev, "failed to parse device tree properties: %d\n", ret);
>> +		goto remove_i2c;
>> +	}
>> +
>> +	/*
>> +	 * Downstream ports are always children of the upstream port.
>> +	 * The first node represents DSP1, the second node represents DSP2, and so on.
>> +	 */
>> +	for_each_child_of_node_scoped(pdev->dev.of_node, child) {
>> +		ret = tc9563_pwrctrl_parse_device_dt(ctx, child, port++);
>> +		if (ret)
>> +			break;
>> +		/* Embedded ethernet device are under DSP3 */
>> +		if (port == TC9563_DSP3)
> Add braces.
>
> I assume this check okay with the port being post-incremented? I'd prefer
> doing this code like this:
>
> 		port++;
> 		/*
> 		 * Downstream ports are always children of the upstream port.
> 		 * The first node represents DSP1, the second node represents
> 		 * DSP2, and so on.
> 		 */
> 		ret = tc9563_pwrctrl_parse_device_dt(ctx, child, port - 1);
> 		if (ret)
> 			break;
> 		/* Embedded ethernet device are under DSP3 */
> 		if (port == TC9563_DSP3) {
>
> While this is functionally same as yours, the explicit "port - 1" makes
> the difference between tc9563_pwrctrl_ports and DT indexing more obvious
> (the code doesn't look buggy on the first sight like it does with the
> post-incrementing. I was sure the indexing is buggy until I read that
> comment that was too far away from the line it was relevant to).

Ack.

Ack to rest of all the comments.

- Krishna Chaitanya.
>> +			for_each_child_of_node_scoped(child, child1) {
>> +				ret = tc9563_pwrctrl_parse_device_dt(ctx, child1, port++);
>> +				if (ret)
>> +					break;
>> +			}
>> +	}
>> +	if (ret) {
>> +		dev_err(dev, "failed to parse device tree properties: %d\n", ret);
>> +		goto remove_i2c;
>> +	}
>> +
>> +	if (bridge->ops->assert_perst) {
>> +		ret = bridge->ops->assert_perst(bus, true);
>> +		if (ret)
>> +			goto remove_i2c;
>> +	}
>> +
>> +	ret = tc9563_pwrctrl_bring_up(ctx);
>> +	if (ret)
>> +		goto remove_i2c;
>> +
>> +	if (bridge->ops->assert_perst) {
>> +		ret = bridge->ops->assert_perst(bus, false);
>> +		if (ret)
>> +			goto power_off;
>> +	}
>> +
>> +	ret = devm_pci_pwrctrl_device_set_ready(dev, &ctx->pwrctrl);
>> +	if (ret)
>> +		goto power_off;
>> +
>> +	platform_set_drvdata(pdev, ctx);
>> +
>> +	return 0;
>> +
>> +power_off:
>> +	tc9563_pwrctrl_power_off(ctx);
>> +remove_i2c:
>> +	i2c_unregister_device(ctx->client);
>> +	i2c_put_adapter(ctx->adapter);
>> +	return ret;
>> +}
>> +
>> +static void tc9563_pwrctrl_remove(struct platform_device *pdev)
>> +{
>> +	struct tc9563_pwrctrl_ctx *ctx = platform_get_drvdata(pdev);
>> +
>> +	tc9563_pwrctrl_power_off(ctx);
>> +	i2c_unregister_device(ctx->client);
>> +	i2c_put_adapter(ctx->adapter);
>> +}
>> +
>> +static const struct of_device_id tc9563_pwrctrl_of_match[] = {
>> +	{ .compatible = "pci1179,0623"},
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, tc9563_pwrctrl_of_match);
>> +
>> +static struct platform_driver tc9563_pwrctrl_driver = {
>> +	.driver = {
>> +		.name = "pwrctrl-tc9563",
>> +		.of_match_table = tc9563_pwrctrl_of_match,
>> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
>> +	},
>> +	.probe = tc9563_pwrctrl_probe,
>> +	.remove = tc9563_pwrctrl_remove,
>> +};
>> +module_platform_driver(tc9563_pwrctrl_driver);
>> +
>> +MODULE_AUTHOR("Krishna chaitanya chundru <quic_krichai@quicinc.com>");
>> +MODULE_DESCRIPTION("TC956x power control driver");
>> +MODULE_LICENSE("GPL");
>>
>>

Re: [PATCH v7 1/8] dt-bindings: PCI: Add binding for Toshiba TC9563 PCIe switch

[Manivannan Sadhasivam]

On Wed, Oct 29, 2025 at 04:59:54PM +0530, Krishna Chaitanya Chundru wrote:
> Add a device tree binding for the Toshiba TC9563 PCIe switch, which
> provides an Ethernet MAC integrated to the 3rd downstream port and
> two downstream PCIe ports.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> ---
>  .../devicetree/bindings/pci/toshiba,tc9563.yaml    | 178 +++++++++++++++++++++
>  1 file changed, 178 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml b/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..82c902b67852d6c4b0305764a2231fe04e83458d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
> @@ -0,0 +1,178 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/toshiba,tc9563.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Toshiba TC9563 PCIe switch
> +
> +maintainers:
> +  - Krishna chaitanya chundru <quic_krichai@quicinc.com>
> +
> +description: |
> +  Toshiba TC9563 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 TC9563 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
> +
> +  reset-gpios:
> +    maxItems: 1
> +    description:
> +      GPIO controlling the RESX# pin.

PCI binding already defines PERST# as 'reset-gpios'. Since this pin is called
RESX in the device reference manual, it'd be better to rename it to
'resx-gpios'.

With this,

Acked-by: Manivannan Sadhasivam <mani@kernel.org>

- Mani

> +
> +  vdd18-supply: true
> +
> +  vdd09-supply: true
> +
> +  vddc-supply: true
> +
> +  vddio1-supply: true
> +
> +  vddio2-supply: true
> +
> +  vddio18-supply: true
> +
> +  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 tc9563 to change FTS, tx amplitude etc.
> +    items:
> +      - description: Phandle to the I2C controller node
> +      - description: I2C slave address
> +
> +patternProperties:
> +  "^pcie@[1-3],0$":
> +    description:
> +      child nodes describing the internal downstream ports
> +      the tc9563 switch.
> +    type: object
> +    allOf:
> +      - $ref: "#/$defs/tc9563-node"
> +      - $ref: /schemas/pci/pci-pci-bridge.yaml#
> +    unevaluatedProperties: false
> +
> +$defs:
> +  tc9563-node:
> +    type: object
> +
> +    properties:
> +      toshiba,tx-amplitude-microvolt:
> +        description:
> +          Change Tx Margin setting for low power consumption.
> +
> +      toshiba,no-dfe-support:
> +        type: boolean
> +        description:
> +          Disable DFE (Decision Feedback Equalizer), which mitigates
> +          intersymbol interference and some reflections caused by impedance mismatches.
> +
> +required:
> +  - reset-gpios
> +  - vdd18-supply
> +  - vdd09-supply
> +  - vddc-supply
> +  - vddio1-supply
> +  - vddio2-supply
> +  - vddio18-supply
> +  - i2c-parent
> +
> +allOf:
> +  - $ref: "#/$defs/tc9563-node"
> +  - $ref: /schemas/pci/pci-bus-common.yaml#
> +
> +unevaluatedProperties: false
> +
> +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 {
> +                    compatible = "pciclass,0604";
> +                    reg = <0x20800 0x0 0x0 0x0 0x0>;
> +                    #address-cells = <3>;
> +                    #size-cells = <2>;
> +                    device_type = "pci";
> +                    ranges;
> +                    bus-range = <0x03 0xff>;
> +
> +                    toshiba,no-dfe-support;
> +                };
> +
> +                pcie@2,0 {
> +                    compatible = "pciclass,0604";
> +                    reg = <0x21000 0x0 0x0 0x0 0x0>;
> +                    #address-cells = <3>;
> +                    #size-cells = <2>;
> +                    device_type = "pci";
> +                    ranges;
> +                    bus-range = <0x04 0xff>;
> +                };
> +
> +                pcie@3,0 {
> +                    compatible = "pciclass,0604";
> +                    reg = <0x21800 0x0 0x0 0x0 0x0>;
> +                    #address-cells = <3>;
> +                    #size-cells = <2>;
> +                    device_type = "pci";
> +                    ranges;
> +                    bus-range = <0x05 0xff>;
> +
> +                    toshiba,tx-amplitude-microvolt = <10>;
> +
> +                    ethernet@0,0 {
> +                        reg = <0x50000 0x0 0x0 0x0 0x0>;
> +                    };
> +
> +                    ethernet@0,1 {
> +                        reg = <0x50100 0x0 0x0 0x0 0x0>;
> +                    };
> +                };
> +            };
> +        };
> +    };
> 
> -- 
> 2.34.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v7 7/8] arm64: defconfig: Enable TC9563 PWRCTL driver

[Manivannan Sadhasivam]

On Wed, Oct 29, 2025 at 02:15:37PM +0100, Bartosz Golaszewski wrote:
> On Wed, Oct 29, 2025 at 12:30 PM Krishna Chaitanya Chundru
> <krishna.chundru@oss.qualcomm.com> wrote:
> >
> > Enable TC9563 PCIe switch pwrctl driver by default. This is needed
> > to power the PCIe switch which is present in Qualcomm RB3gen2 platform.
> > Without this the switch will not powered up and we can't use the
> > endpoints connected to the switch.
> >
> > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > ---
> >  arch/arm64/configs/defconfig | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> > index e3a2d37bd10423b028f59dc40d6e8ee1c610d6b8..fe5c9951c437a67ac76bf939a9e436eafa3820bf 100644
> > --- a/arch/arm64/configs/defconfig
> > +++ b/arch/arm64/configs/defconfig
> > @@ -249,6 +249,7 @@ CONFIG_PCIE_LAYERSCAPE_GEN4=y
> >  CONFIG_PCI_ENDPOINT=y
> >  CONFIG_PCI_ENDPOINT_CONFIGFS=y
> >  CONFIG_PCI_EPF_TEST=m
> > +CONFIG_PCI_PWRCTRL_TC9563=m
> >  CONFIG_DEVTMPFS=y
> >  CONFIG_DEVTMPFS_MOUNT=y
> >  CONFIG_FW_LOADER_USER_HELPER=y
> >
> > --
> > 2.34.1
> >
> 
> Can't we just do the following in the respective Kconfig entry?
> 
> config PCI_PWRCTRL_TC9563
>     tristate ...
>     default m if ARCH_QCOM

I believe the driver is supposed to support all Toshiba TC9563 switches, but
since we have some hooks for toggling PERST# and they are only added for QCOM
platforms atm, it is OK to select the driver in Kconfig.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v7 0/8] PCI: Enable Power and configure the TC9563 PCIe switch

[Bjorn Helgaas]

On Thu, Oct 30, 2025 at 09:13:29AM +0530, Krishna Chaitanya Chundru wrote:
> On 10/30/2025 4:53 AM, Bjorn Helgaas wrote:
> > On Wed, Oct 29, 2025 at 04:59:53PM +0530, Krishna Chaitanya Chundru wrote:
> > > TC9563 is the PCIe switch which has one upstream and three downstream
> > > ports. To one of the downstream ports ethernet MAC is connected as endpoint
> > > device. Other two downstream ports are supposed to connect to external
> > > device. One Host can connect to TC956x by upstream port.
> > > 
> > > TC9563 switch power is controlled by the GPIO's. After powering on
> > > the switch will immediately participate in the link training. if the
> > > host is also ready by that time PCIe link will established.
> > > 
> > > The TC9563 needs to configured certain parameters like de-emphasis,
> > > disable unused port etc before link is established.
> > > 
> > > As the controller starts link training before the probe of pwrctl driver,
> > > the PCIe link may come up as soon as we power on the switch. Due to this
> > > configuring the switch itself through i2c will not have any effect as
> > > this configuration needs to done before link training. To avoid this
> > > introduce two functions in pci_ops to start_link() & stop_link() which
> > > will disable the link training if the PCIe link is not up yet.
> > > 
> > > This series depends on the https://lore.kernel.org/all/20250124101038.3871768-3-krishna.chundru@oss.qualcomm.com/
> > What does this series apply to?  It doesn't apply cleanly to v6.18-rc1
> > (the normal base for topic branches) or v6.18-rc3 or pci/next.
>
> I sent this on top of rc3 as we have some dependencies with latest changes
> i.e ecam changes in dwc driver.

Oops, sorry, my fault.  I must have been trying to apply the v6 series
(not this v7) on -rc3.  This v7 *does* apply cleanly to -rc3.

But all the other topic branches are based on -rc1, so I think the
best thing is to make this one apply on -rc1 as well, and I will deal
with the resulting conflicts when merging into pci/next and ultimately
into Linus's tree.

Bjorn