diff mbox series

[RFC,2/7] arm64: dts: qcom: qcs6490-rb3gen2: Add qps615 node

Message ID 20240626-qps615-v1-2-2ade7bd91e02@quicinc.com (mailing list archive)
State RFC
Delegated to: Krzysztof WilczyƄski
Headers show
Series PCI: enable Power and configure the QPS615 PCIe switch | expand

Commit Message

Krishna Chaitanya Chundru June 26, 2024, 12:37 p.m. UTC
QPS615 switch power is controlled by GPIO's. Once the GPIO's are
enabled, switch power will be on.

Make all GPIO's as fixed regulators and inter link them so that
enabling the regulator will enable power to the switch by enabling
GPIO's.

Enable i2c0 which is required to configure the switch.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 55 ++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

Comments

Bjorn Andersson June 26, 2024, 5:11 p.m. UTC | #1
On Wed, Jun 26, 2024 at 06:07:50PM GMT, Krishna chaitanya chundru wrote:
> QPS615 switch power is controlled by GPIO's. Once the GPIO's are
> enabled, switch power will be on.
> 
> Make all GPIO's as fixed regulators and inter link them so that
> enabling the regulator will enable power to the switch by enabling
> GPIO's.
> 
> Enable i2c0 which is required to configure the switch.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 55 ++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index a085ff5b5fb2..5b453896a6c9 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> @@ -511,6 +511,61 @@ vreg_bob_3p296: bob {
>  			regulator-max-microvolt = <3960000>;
>  		};
>  	};
> +
> +	qps615_0p9_vreg: qps615-0p9-vreg {

Keep nodes sorted by address, node name, then label.

Perhaps name the nodes "regulator-<name>" to group static regulators
together.

> +		compatible = "regulator-fixed";
> +		regulator-name = "qps615_0p9_vreg";

Is this the name used for the output of the regulator in the schematics?

> +		gpio = <&pm8350c_gpios 2 0>;

Replace that 0 with GPIO_ACTIVE_HIGH.

> +		regulator-min-microvolt = <1000000>;
> +		regulator-max-microvolt = <1000000>;
> +		enable-active-high;
> +		regulator-enable-ramp-delay = <4300>;
> +	};
> +
> +	qps615_1p8_vreg: qps615-1p8-vreg {
> +		compatible = "regulator-fixed";
> +		regulator-name = "qps615_1p8_vreg";
> +		gpio = <&pm8350c_gpios 3 0>;
> +		vin-supply = <&qps615_0p9_vreg>;

This makes me curious, &qps615_0p9_vreg provides 1V, which we feed into
another regulator (which typically would be an LDO) to provide 1.8V...

> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		enable-active-high;
> +		regulator-enable-ramp-delay = <10000>;
> +	};
> +
> +	qps615_rsex_vreg: qps615-rsex-vreg {
> +		compatible = "regulator-fixed";
> +		regulator-name = "qps615_rsex_vreg";
> +		gpio = <&pm8350c_gpios 1 0>;
> +		vin-supply = <&qps615_1p8_vreg>;

...which is then fed to another LDO(?) which provides 1.8V.

Please double check the schematics and make sure this represents what's
on the board. Feel free to ping me offline and I can help review the
schematics.

> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		enable-active-high;
> +		regulator-enable-ramp-delay = <10000>;
> +	};
> +};
> +
> +&i2c0 {
> +	clock-frequency = <100000>;
> +	status = "okay";
> +};
> +
> +&pcie1 {
> +	pcie@0 {
> +		device_type = "pci";
> +		reg = <0x0 0x0 0x0 0x0 0x0>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +
> +		bus-range = <0x01 0xff>;
> +
> +		qps615@0 {
> +			compatible = "pci1179,0623";
> +			reg = <0x1000 0x0 0x0 0x0 0x0>;
> +			vdda-supply = <&qps615_rsex_vreg>;

I presume you didn't "make qcom/qcs6490-rb3gen2.dtb CHECK_DTBS=1" this,
as "vdda-supply" is not listed as a valid supply in the binding (also
the driver is looking for vdd-supply...)

Regards,
Bjorn

> +			switch-i2c-cntrl = <&i2c0>;
> +		};
> +	};
>  };
>  
>  &gcc {
> 
> -- 
> 2.42.0
>
Dmitry Baryshkov June 26, 2024, 5:51 p.m. UTC | #2
On Wed, Jun 26, 2024 at 06:07:50PM GMT, Krishna chaitanya chundru wrote:
> QPS615 switch power is controlled by GPIO's. Once the GPIO's are
> enabled, switch power will be on.
> 
> Make all GPIO's as fixed regulators and inter link them so that
> enabling the regulator will enable power to the switch by enabling
> GPIO's.
> 
> Enable i2c0 which is required to configure the switch.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 55 ++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index a085ff5b5fb2..5b453896a6c9 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> @@ -511,6 +511,61 @@ vreg_bob_3p296: bob {
>  			regulator-max-microvolt = <3960000>;
>  		};
>  	};
> +
> +	qps615_0p9_vreg: qps615-0p9-vreg {
> +		compatible = "regulator-fixed";
> +		regulator-name = "qps615_0p9_vreg";
> +		gpio = <&pm8350c_gpios 2 0>;
> +		regulator-min-microvolt = <1000000>;
> +		regulator-max-microvolt = <1000000>;
> +		enable-active-high;
> +		regulator-enable-ramp-delay = <4300>;
> +	};
> +
> +	qps615_1p8_vreg: qps615-1p8-vreg {
> +		compatible = "regulator-fixed";
> +		regulator-name = "qps615_1p8_vreg";
> +		gpio = <&pm8350c_gpios 3 0>;
> +		vin-supply = <&qps615_0p9_vreg>;
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		enable-active-high;
> +		regulator-enable-ramp-delay = <10000>;
> +	};
> +
> +	qps615_rsex_vreg: qps615-rsex-vreg {
> +		compatible = "regulator-fixed";
> +		regulator-name = "qps615_rsex_vreg";
> +		gpio = <&pm8350c_gpios 1 0>;
> +		vin-supply = <&qps615_1p8_vreg>;
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		enable-active-high;
> +		regulator-enable-ramp-delay = <10000>;
> +	};
> +};
> +
> +&i2c0 {
> +	clock-frequency = <100000>;
> +	status = "okay";
> +};
> +
> +&pcie1 {
> +	pcie@0 {
> +		device_type = "pci";
> +		reg = <0x0 0x0 0x0 0x0 0x0>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +
> +		bus-range = <0x01 0xff>;
> +
> +		qps615@0 {
> +			compatible = "pci1179,0623";
> +			reg = <0x1000 0x0 0x0 0x0 0x0>;
> +			vdda-supply = <&qps615_rsex_vreg>;
> +			switch-i2c-cntrl = <&i2c0>;

We already have proper bindings for I2C devices. The QPS615 obviously
has and I2C device inside. Please add it to DT instead of referencing
the whole bus.

> +		};
> +	};
>  };
>  
>  &gcc {
> 
> -- 
> 2.42.0
>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index a085ff5b5fb2..5b453896a6c9 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -511,6 +511,61 @@  vreg_bob_3p296: bob {
 			regulator-max-microvolt = <3960000>;
 		};
 	};
+
+	qps615_0p9_vreg: qps615-0p9-vreg {
+		compatible = "regulator-fixed";
+		regulator-name = "qps615_0p9_vreg";
+		gpio = <&pm8350c_gpios 2 0>;
+		regulator-min-microvolt = <1000000>;
+		regulator-max-microvolt = <1000000>;
+		enable-active-high;
+		regulator-enable-ramp-delay = <4300>;
+	};
+
+	qps615_1p8_vreg: qps615-1p8-vreg {
+		compatible = "regulator-fixed";
+		regulator-name = "qps615_1p8_vreg";
+		gpio = <&pm8350c_gpios 3 0>;
+		vin-supply = <&qps615_0p9_vreg>;
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		enable-active-high;
+		regulator-enable-ramp-delay = <10000>;
+	};
+
+	qps615_rsex_vreg: qps615-rsex-vreg {
+		compatible = "regulator-fixed";
+		regulator-name = "qps615_rsex_vreg";
+		gpio = <&pm8350c_gpios 1 0>;
+		vin-supply = <&qps615_1p8_vreg>;
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		enable-active-high;
+		regulator-enable-ramp-delay = <10000>;
+	};
+};
+
+&i2c0 {
+	clock-frequency = <100000>;
+	status = "okay";
+};
+
+&pcie1 {
+	pcie@0 {
+		device_type = "pci";
+		reg = <0x0 0x0 0x0 0x0 0x0>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+
+		bus-range = <0x01 0xff>;
+
+		qps615@0 {
+			compatible = "pci1179,0623";
+			reg = <0x1000 0x0 0x0 0x0 0x0>;
+			vdda-supply = <&qps615_rsex_vreg>;
+			switch-i2c-cntrl = <&i2c0>;
+		};
+	};
 };
 
 &gcc {