| Message ID | 20250225-qps615_v4_1-v4-2-e08633a7bdf8@oss.qualcomm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | PCI: Enable Power and configure the TC956x PCIe switch | expand |
On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote: > Add a node for the TC956x PCIe switch, which has three downstream ports. > Two embedded Ethernet devices are present on one of the downstream ports. > > Power to the TC956x is supplied through two LDO regulators, controlled by > two GPIOs, which are added as fixed regulators. Configure the TC956x > 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> > --- > arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++ > arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- > 2 files changed, 117 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > index 7a36c90ad4ec..13dbb24a3179 100644 > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > @@ -218,6 +218,31 @@ vph_pwr: vph-pwr-regulator { > regulator-min-microvolt = <3700000>; > 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>; > + }; > + > }; > > &apps_rsc { > @@ -735,6 +760,75 @@ &pcie1_phy { > status = "okay"; > }; > > +&pcie1_port { > + pcie@0,0 { > + compatible = "pci1179,0623", "pciclass,0604"; > + 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>; > + > + pcie@1,0 { PCIe bus can be autodetected. Is there a reason for describing all the ports and a full topology? If so, it should be stated in the commit message. > + 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 {
On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote: > Add a node for the TC956x PCIe switch, which has three downstream ports. > Two embedded Ethernet devices are present on one of the downstream ports. > > Power to the TC956x is supplied through two LDO regulators, controlled by > two GPIOs, which are added as fixed regulators. Configure the TC956x > 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> > --- > arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++ > arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- > 2 files changed, 117 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > index 7a36c90ad4ec..13dbb24a3179 100644 > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > @@ -218,6 +218,31 @@ vph_pwr: vph-pwr-regulator { > regulator-min-microvolt = <3700000>; > 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>; > + }; > + > }; > > &apps_rsc { > @@ -735,6 +760,75 @@ &pcie1_phy { > status = "okay"; > }; > > +&pcie1_port { > + pcie@0,0 { > + compatible = "pci1179,0623", "pciclass,0604"; > + 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>; Please add pinctrl blck for this reset line.
On 2/25/2025 5:19 PM, Dmitry Baryshkov wrote: > On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote: >> Add a node for the TC956x PCIe switch, which has three downstream ports. >> Two embedded Ethernet devices are present on one of the downstream ports. >> >> Power to the TC956x is supplied through two LDO regulators, controlled by >> two GPIOs, which are added as fixed regulators. Configure the TC956x >> 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> >> --- >> arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++ >> arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- >> 2 files changed, 117 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts >> index 7a36c90ad4ec..13dbb24a3179 100644 >> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts >> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts >> @@ -218,6 +218,31 @@ vph_pwr: vph-pwr-regulator { >> regulator-min-microvolt = <3700000>; >> 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>; >> + }; >> + >> }; >> >> &apps_rsc { >> @@ -735,6 +760,75 @@ &pcie1_phy { >> status = "okay"; >> }; >> >> +&pcie1_port { >> + pcie@0,0 { >> + compatible = "pci1179,0623", "pciclass,0604"; >> + 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>; >> + >> + pcie@1,0 { > > PCIe bus can be autodetected. Is there a reason for describing all the > ports and a full topology? If so, it should be stated in the commit > message. > As these ports are fixed we are defining them here, I will mention this in the commit message. It is similar to how we added pcieport for all the platforms, I tried to add full topology here. And if we want to configure any ports like l0s entry delay, l1 entry delay etc in future we need these full topology to be present. - Krishna Chaitanya. >> + 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 { >
On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote: > Add a node for the TC956x PCIe switch, which has three downstream ports. > Two embedded Ethernet devices are present on one of the downstream ports. > > Power to the TC956x is supplied through two LDO regulators, controlled by > two GPIOs, which are added as fixed regulators. Configure the TC956x > 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> > --- > arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++ > arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- > 2 files changed, 117 insertions(+), 1 deletion(-) > > @@ -735,6 +760,75 @@ &pcie1_phy { > status = "okay"; > }; > > +&pcie1_port { > + pcie@0,0 { > + compatible = "pci1179,0623", "pciclass,0604"; > + 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>; > + I think I've responded here, but I'm not sure where the message went: please add pinctrl entry for this pin.
On Mon, Mar 17, 2025 at 03:05:17PM +0530, Krishna Chaitanya Chundru wrote: > > > On 2/25/2025 5:19 PM, Dmitry Baryshkov wrote: > > On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote: > > > Add a node for the TC956x PCIe switch, which has three downstream ports. > > > Two embedded Ethernet devices are present on one of the downstream ports. > > > > > > Power to the TC956x is supplied through two LDO regulators, controlled by > > > two GPIOs, which are added as fixed regulators. Configure the TC956x > > > 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> > > > --- > > > arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++ > > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- > > > 2 files changed, 117 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > > > index 7a36c90ad4ec..13dbb24a3179 100644 > > > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > > > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > > > @@ -218,6 +218,31 @@ vph_pwr: vph-pwr-regulator { > > > regulator-min-microvolt = <3700000>; > > > 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>; > > > + }; > > > + > > > }; > > > &apps_rsc { > > > @@ -735,6 +760,75 @@ &pcie1_phy { > > > status = "okay"; > > > }; > > > +&pcie1_port { > > > + pcie@0,0 { > > > + compatible = "pci1179,0623", "pciclass,0604"; > > > + 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>; > > > + > > > + pcie@1,0 { > > > > PCIe bus can be autodetected. Is there a reason for describing all the > > ports and a full topology? If so, it should be stated in the commit > > message. > > > As these ports are fixed we are defining them here, I will mention this > in the commit message. It is similar to how we added pcieport for all > the platforms, I tried to add full topology here. And if we want to > configure any ports like l0s entry delay, l1 entry delay etc in future > we need these full topology to be present. Ack > > - Krishna Chaitanya. > > > + 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 { > >
On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote: > On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote: >> Add a node for the TC956x PCIe switch, which has three downstream ports. >> Two embedded Ethernet devices are present on one of the downstream ports. >> >> Power to the TC956x is supplied through two LDO regulators, controlled by >> two GPIOs, which are added as fixed regulators. Configure the TC956x >> 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> >> --- >> arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++ >> arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- >> 2 files changed, 117 insertions(+), 1 deletion(-) >> >> @@ -735,6 +760,75 @@ &pcie1_phy { >> status = "okay"; >> }; >> >> +&pcie1_port { >> + pcie@0,0 { >> + compatible = "pci1179,0623", "pciclass,0604"; >> + 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>; >> + > > I think I've responded here, but I'm not sure where the message went: > please add pinctrl entry for this pin. > Do we need to also add pinctrl property for this node and refer the pinctrl entry for this pin? - Krishna Chaitanya.
On Tue, 18 Mar 2025 at 18:11, Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com> wrote: > > > > On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote: > > On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote: > >> Add a node for the TC956x PCIe switch, which has three downstream ports. > >> Two embedded Ethernet devices are present on one of the downstream ports. > >> > >> Power to the TC956x is supplied through two LDO regulators, controlled by > >> two GPIOs, which are added as fixed regulators. Configure the TC956x > >> 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> > >> --- > >> arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++ > >> arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- > >> 2 files changed, 117 insertions(+), 1 deletion(-) > >> > >> @@ -735,6 +760,75 @@ &pcie1_phy { > >> status = "okay"; > >> }; > >> > >> +&pcie1_port { > >> + pcie@0,0 { > >> + compatible = "pci1179,0623", "pciclass,0604"; > >> + 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>; > >> + > > > > I think I've responded here, but I'm not sure where the message went: > > please add pinctrl entry for this pin. > > > Do we need to also add pinctrl property for this node and refer the > pinctrl entry for this pin? I think that is what I've asked for, was that not?
On 3/18/2025 10:30 PM, Dmitry Baryshkov wrote: > On Tue, 18 Mar 2025 at 18:11, Krishna Chaitanya Chundru > <krishna.chundru@oss.qualcomm.com> wrote: >> >> >> >> On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote: >>> On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote: >>>> Add a node for the TC956x PCIe switch, which has three downstream ports. >>>> Two embedded Ethernet devices are present on one of the downstream ports. >>>> >>>> Power to the TC956x is supplied through two LDO regulators, controlled by >>>> two GPIOs, which are added as fixed regulators. Configure the TC956x >>>> 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> >>>> --- >>>> arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++ >>>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- >>>> 2 files changed, 117 insertions(+), 1 deletion(-) >>>> >>>> @@ -735,6 +760,75 @@ &pcie1_phy { >>>> status = "okay"; >>>> }; >>>> >>>> +&pcie1_port { >>>> + pcie@0,0 { >>>> + compatible = "pci1179,0623", "pciclass,0604"; >>>> + 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>; >>>> + >>> >>> I think I've responded here, but I'm not sure where the message went: >>> please add pinctrl entry for this pin. >>> >> Do we need to also add pinctrl property for this node and refer the >> pinctrl entry for this pin? > > I think that is what I've asked for, was that not? Currently there is no pincntrl property defined for this. - Krishna Chaitanya. >
On Wed, Mar 19, 2025 at 09:14:22AM +0530, Krishna Chaitanya Chundru wrote: > > > On 3/18/2025 10:30 PM, Dmitry Baryshkov wrote: > > On Tue, 18 Mar 2025 at 18:11, Krishna Chaitanya Chundru > > <krishna.chundru@oss.qualcomm.com> wrote: > > > > > > > > > > > > On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote: > > > > On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote: > > > > > Add a node for the TC956x PCIe switch, which has three downstream ports. > > > > > Two embedded Ethernet devices are present on one of the downstream ports. > > > > > > > > > > Power to the TC956x is supplied through two LDO regulators, controlled by > > > > > two GPIOs, which are added as fixed regulators. Configure the TC956x > > > > > 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> > > > > > --- > > > > > arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++ > > > > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- > > > > > 2 files changed, 117 insertions(+), 1 deletion(-) > > > > > > > > > > @@ -735,6 +760,75 @@ &pcie1_phy { > > > > > status = "okay"; > > > > > }; > > > > > > > > > > +&pcie1_port { > > > > > + pcie@0,0 { > > > > > + compatible = "pci1179,0623", "pciclass,0604"; > > > > > + 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>; > > > > > + > > > > > > > > I think I've responded here, but I'm not sure where the message went: > > > > please add pinctrl entry for this pin. > > > > > > > Do we need to also add pinctrl property for this node and refer the > > > pinctrl entry for this pin? > > > > I think that is what I've asked for, was that not? > Currently there is no pincntrl property defined for this. Does it need to be defined separately / specially?
On 3/19/2025 3:43 PM, Dmitry Baryshkov wrote: > On Wed, Mar 19, 2025 at 09:14:22AM +0530, Krishna Chaitanya Chundru wrote: >> >> >> On 3/18/2025 10:30 PM, Dmitry Baryshkov wrote: >>> On Tue, 18 Mar 2025 at 18:11, Krishna Chaitanya Chundru >>> <krishna.chundru@oss.qualcomm.com> wrote: >>>> >>>> >>>> >>>> On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote: >>>>> On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote: >>>>>> Add a node for the TC956x PCIe switch, which has three downstream ports. >>>>>> Two embedded Ethernet devices are present on one of the downstream ports. >>>>>> >>>>>> Power to the TC956x is supplied through two LDO regulators, controlled by >>>>>> two GPIOs, which are added as fixed regulators. Configure the TC956x >>>>>> 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> >>>>>> --- >>>>>> arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++ >>>>>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- >>>>>> 2 files changed, 117 insertions(+), 1 deletion(-) >>>>>> >>>>>> @@ -735,6 +760,75 @@ &pcie1_phy { >>>>>> status = "okay"; >>>>>> }; >>>>>> >>>>>> +&pcie1_port { >>>>>> + pcie@0,0 { >>>>>> + compatible = "pci1179,0623", "pciclass,0604"; >>>>>> + 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>; >>>>>> + >>>>> >>>>> I think I've responded here, but I'm not sure where the message went: >>>>> please add pinctrl entry for this pin. >>>>> >>>> Do we need to also add pinctrl property for this node and refer the >>>> pinctrl entry for this pin? >>> >>> I think that is what I've asked for, was that not? >> Currently there is no pincntrl property defined for this. > > Does it need to be defined separately / specially? > yes we need to define this property now. - Krishna Chaitanya.
On Wed, Mar 19, 2025 at 03:46:00PM +0530, Krishna Chaitanya Chundru wrote: > > > On 3/19/2025 3:43 PM, Dmitry Baryshkov wrote: > > On Wed, Mar 19, 2025 at 09:14:22AM +0530, Krishna Chaitanya Chundru wrote: > > > > > > > > > On 3/18/2025 10:30 PM, Dmitry Baryshkov wrote: > > > > On Tue, 18 Mar 2025 at 18:11, Krishna Chaitanya Chundru > > > > <krishna.chundru@oss.qualcomm.com> wrote: > > > > > > > > > > > > > > > > > > > > On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote: > > > > > > On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote: > > > > > > > Add a node for the TC956x PCIe switch, which has three downstream ports. > > > > > > > Two embedded Ethernet devices are present on one of the downstream ports. > > > > > > > > > > > > > > Power to the TC956x is supplied through two LDO regulators, controlled by > > > > > > > two GPIOs, which are added as fixed regulators. Configure the TC956x > > > > > > > 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> > > > > > > > --- > > > > > > > arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++ > > > > > > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- > > > > > > > 2 files changed, 117 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > @@ -735,6 +760,75 @@ &pcie1_phy { > > > > > > > status = "okay"; > > > > > > > }; > > > > > > > > > > > > > > +&pcie1_port { > > > > > > > + pcie@0,0 { > > > > > > > + compatible = "pci1179,0623", "pciclass,0604"; > > > > > > > + 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>; > > > > > > > + > > > > > > > > > > > > I think I've responded here, but I'm not sure where the message went: > > > > > > please add pinctrl entry for this pin. > > > > > > > > > > > Do we need to also add pinctrl property for this node and refer the > > > > > pinctrl entry for this pin? > > > > > > > > I think that is what I've asked for, was that not? > > > Currently there is no pincntrl property defined for this. > > > > Does it need to be defined separately / specially? > > > yes we need to define this property now. Could you please point out existing schema files defining those properties?
On 3/19/2025 3:51 PM, Dmitry Baryshkov wrote: > On Wed, Mar 19, 2025 at 03:46:00PM +0530, Krishna Chaitanya Chundru wrote: >> >> >> On 3/19/2025 3:43 PM, Dmitry Baryshkov wrote: >>> On Wed, Mar 19, 2025 at 09:14:22AM +0530, Krishna Chaitanya Chundru wrote: >>>> >>>> >>>> On 3/18/2025 10:30 PM, Dmitry Baryshkov wrote: >>>>> On Tue, 18 Mar 2025 at 18:11, Krishna Chaitanya Chundru >>>>> <krishna.chundru@oss.qualcomm.com> wrote: >>>>>> >>>>>> >>>>>> >>>>>> On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote: >>>>>>> On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote: >>>>>>>> Add a node for the TC956x PCIe switch, which has three downstream ports. >>>>>>>> Two embedded Ethernet devices are present on one of the downstream ports. >>>>>>>> >>>>>>>> Power to the TC956x is supplied through two LDO regulators, controlled by >>>>>>>> two GPIOs, which are added as fixed regulators. Configure the TC956x >>>>>>>> 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> >>>>>>>> --- >>>>>>>> arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++ >>>>>>>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- >>>>>>>> 2 files changed, 117 insertions(+), 1 deletion(-) >>>>>>>> >>>>>>>> @@ -735,6 +760,75 @@ &pcie1_phy { >>>>>>>> status = "okay"; >>>>>>>> }; >>>>>>>> >>>>>>>> +&pcie1_port { >>>>>>>> + pcie@0,0 { >>>>>>>> + compatible = "pci1179,0623", "pciclass,0604"; >>>>>>>> + 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>; >>>>>>>> + >>>>>>> >>>>>>> I think I've responded here, but I'm not sure where the message went: >>>>>>> please add pinctrl entry for this pin. >>>>>>> >>>>>> Do we need to also add pinctrl property for this node and refer the >>>>>> pinctrl entry for this pin? >>>>> >>>>> I think that is what I've asked for, was that not? >>>> Currently there is no pincntrl property defined for this. >>> >>> Does it need to be defined separately / specially? >>> >> yes we need to define this property now. > > Could you please point out existing schema files defining those > properties? sorry I was not able to get which schema file you are requesting for, if it is tc956x it is in this series only. What I understood from these conversation is we need to define pinctrl property and refer the reset gpio pin in next series. If it was wrong please correct me. - Krishna Chaitanya. >
On Wed, Mar 19, 2025 at 04:16:33PM +0530, Krishna Chaitanya Chundru wrote: > > > On 3/19/2025 3:51 PM, Dmitry Baryshkov wrote: > > On Wed, Mar 19, 2025 at 03:46:00PM +0530, Krishna Chaitanya Chundru wrote: > > > > > > > > > On 3/19/2025 3:43 PM, Dmitry Baryshkov wrote: > > > > On Wed, Mar 19, 2025 at 09:14:22AM +0530, Krishna Chaitanya Chundru wrote: > > > > > > > > > > > > > > > On 3/18/2025 10:30 PM, Dmitry Baryshkov wrote: > > > > > > On Tue, 18 Mar 2025 at 18:11, Krishna Chaitanya Chundru > > > > > > <krishna.chundru@oss.qualcomm.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote: > > > > > > > > On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote: > > > > > > > > > Add a node for the TC956x PCIe switch, which has three downstream ports. > > > > > > > > > Two embedded Ethernet devices are present on one of the downstream ports. > > > > > > > > > > > > > > > > > > Power to the TC956x is supplied through two LDO regulators, controlled by > > > > > > > > > two GPIOs, which are added as fixed regulators. Configure the TC956x > > > > > > > > > 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> > > > > > > > > > --- > > > > > > > > > arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++ > > > > > > > > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- > > > > > > > > > 2 files changed, 117 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > > > > > @@ -735,6 +760,75 @@ &pcie1_phy { > > > > > > > > > status = "okay"; > > > > > > > > > }; > > > > > > > > > > > > > > > > > > +&pcie1_port { > > > > > > > > > + pcie@0,0 { > > > > > > > > > + compatible = "pci1179,0623", "pciclass,0604"; > > > > > > > > > + 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>; > > > > > > > > > + > > > > > > > > > > > > > > > > I think I've responded here, but I'm not sure where the message went: > > > > > > > > please add pinctrl entry for this pin. > > > > > > > > > > > > > > > Do we need to also add pinctrl property for this node and refer the > > > > > > > pinctrl entry for this pin? > > > > > > > > > > > > I think that is what I've asked for, was that not? > > > > > Currently there is no pincntrl property defined for this. > > > > > > > > Does it need to be defined separately / specially? > > > > > > > yes we need to define this property now. > > > > Could you please point out existing schema files defining those > > properties? > sorry I was not able to get which schema file you are requesting for, > if it is tc956x it is in this series only. > > What I understood from these conversation is we need to define pinctrl > property and refer the reset gpio pin in next series. If it was wrong > please correct me. You claimed that pinctrl properties (there are several of those) are to be defined in the schema for TC956x. I asked you to point out other schema files which define those properties for the devices that use GPIO pins.
On 3/19/25 12:06 PM, Dmitry Baryshkov wrote: > On Wed, Mar 19, 2025 at 04:16:33PM +0530, Krishna Chaitanya Chundru wrote: >> >> >> On 3/19/2025 3:51 PM, Dmitry Baryshkov wrote: >>> On Wed, Mar 19, 2025 at 03:46:00PM +0530, Krishna Chaitanya Chundru wrote: >>>> >>>> >>>> On 3/19/2025 3:43 PM, Dmitry Baryshkov wrote: >>>>> On Wed, Mar 19, 2025 at 09:14:22AM +0530, Krishna Chaitanya Chundru wrote: >>>>>> >>>>>> >>>>>> On 3/18/2025 10:30 PM, Dmitry Baryshkov wrote: >>>>>>> On Tue, 18 Mar 2025 at 18:11, Krishna Chaitanya Chundru >>>>>>> <krishna.chundru@oss.qualcomm.com> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote: >>>>>>>>> On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote: >>>>>>>>>> Add a node for the TC956x PCIe switch, which has three downstream ports. >>>>>>>>>> Two embedded Ethernet devices are present on one of the downstream ports. >>>>>>>>>> >>>>>>>>>> Power to the TC956x is supplied through two LDO regulators, controlled by >>>>>>>>>> two GPIOs, which are added as fixed regulators. Configure the TC956x >>>>>>>>>> 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> >>>>>>>>>> --- >>>>>>>>>> arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++ >>>>>>>>>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- >>>>>>>>>> 2 files changed, 117 insertions(+), 1 deletion(-) >>>>>>>>>> >>>>>>>>>> @@ -735,6 +760,75 @@ &pcie1_phy { >>>>>>>>>> status = "okay"; >>>>>>>>>> }; >>>>>>>>>> >>>>>>>>>> +&pcie1_port { >>>>>>>>>> + pcie@0,0 { >>>>>>>>>> + compatible = "pci1179,0623", "pciclass,0604"; >>>>>>>>>> + 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>; >>>>>>>>>> + >>>>>>>>> >>>>>>>>> I think I've responded here, but I'm not sure where the message went: >>>>>>>>> please add pinctrl entry for this pin. >>>>>>>>> >>>>>>>> Do we need to also add pinctrl property for this node and refer the >>>>>>>> pinctrl entry for this pin? >>>>>>> >>>>>>> I think that is what I've asked for, was that not? >>>>>> Currently there is no pincntrl property defined for this. >>>>> >>>>> Does it need to be defined separately / specially? >>>>> >>>> yes we need to define this property now. >>> >>> Could you please point out existing schema files defining those >>> properties? >> sorry I was not able to get which schema file you are requesting for, >> if it is tc956x it is in this series only. >> >> What I understood from these conversation is we need to define pinctrl >> property and refer the reset gpio pin in next series. If it was wrong >> please correct me. > > You claimed that pinctrl properties (there are several of those) are to > be defined in the schema for TC956x. I asked you to point out other > schema files which define those properties for the devices that use > GPIO pins. pinctrl-x is part of common schema (see gh/devicetree-org/dt-schema/) Konrad
On Wed, Mar 19, 2025 at 03:12:50PM +0100, Konrad Dybcio wrote: > On 3/19/25 12:06 PM, Dmitry Baryshkov wrote: > > On Wed, Mar 19, 2025 at 04:16:33PM +0530, Krishna Chaitanya Chundru wrote: > >> > >> > >> On 3/19/2025 3:51 PM, Dmitry Baryshkov wrote: > >>> On Wed, Mar 19, 2025 at 03:46:00PM +0530, Krishna Chaitanya Chundru wrote: > >>>> > >>>> > >>>> On 3/19/2025 3:43 PM, Dmitry Baryshkov wrote: > >>>>> On Wed, Mar 19, 2025 at 09:14:22AM +0530, Krishna Chaitanya Chundru wrote: > >>>>>> > >>>>>> > >>>>>> On 3/18/2025 10:30 PM, Dmitry Baryshkov wrote: > >>>>>>> On Tue, 18 Mar 2025 at 18:11, Krishna Chaitanya Chundru > >>>>>>> <krishna.chundru@oss.qualcomm.com> wrote: > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> On 3/17/2025 4:57 PM, Dmitry Baryshkov wrote: > >>>>>>>>> On Tue, Feb 25, 2025 at 03:03:59PM +0530, Krishna Chaitanya Chundru wrote: > >>>>>>>>>> Add a node for the TC956x PCIe switch, which has three downstream ports. > >>>>>>>>>> Two embedded Ethernet devices are present on one of the downstream ports. > >>>>>>>>>> > >>>>>>>>>> Power to the TC956x is supplied through two LDO regulators, controlled by > >>>>>>>>>> two GPIOs, which are added as fixed regulators. Configure the TC956x > >>>>>>>>>> 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> > >>>>>>>>>> --- > >>>>>>>>>> arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 116 +++++++++++++++++++++++++++ > >>>>>>>>>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- > >>>>>>>>>> 2 files changed, 117 insertions(+), 1 deletion(-) > >>>>>>>>>> > >>>>>>>>>> @@ -735,6 +760,75 @@ &pcie1_phy { > >>>>>>>>>> status = "okay"; > >>>>>>>>>> }; > >>>>>>>>>> > >>>>>>>>>> +&pcie1_port { > >>>>>>>>>> + pcie@0,0 { > >>>>>>>>>> + compatible = "pci1179,0623", "pciclass,0604"; > >>>>>>>>>> + 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>; > >>>>>>>>>> + > >>>>>>>>> > >>>>>>>>> I think I've responded here, but I'm not sure where the message went: > >>>>>>>>> please add pinctrl entry for this pin. > >>>>>>>>> > >>>>>>>> Do we need to also add pinctrl property for this node and refer the > >>>>>>>> pinctrl entry for this pin? > >>>>>>> > >>>>>>> I think that is what I've asked for, was that not? > >>>>>> Currently there is no pincntrl property defined for this. > >>>>> > >>>>> Does it need to be defined separately / specially? > >>>>> > >>>> yes we need to define this property now. > >>> > >>> Could you please point out existing schema files defining those > >>> properties? > >> sorry I was not able to get which schema file you are requesting for, > >> if it is tc956x it is in this series only. > >> > >> What I understood from these conversation is we need to define pinctrl > >> property and refer the reset gpio pin in next series. If it was wrong > >> please correct me. > > > > You claimed that pinctrl properties (there are several of those) are to > > be defined in the schema for TC956x. I asked you to point out other > > schema files which define those properties for the devices that use > > GPIO pins. > > pinctrl-x is part of common schema (see gh/devicetree-org/dt-schema/) Right :-) So there is no need to define anything, they just need to be added to the DTS file.
diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts index 7a36c90ad4ec..13dbb24a3179 100644 --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts @@ -218,6 +218,31 @@ vph_pwr: vph-pwr-regulator { regulator-min-microvolt = <3700000>; 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>; + }; + }; &apps_rsc { @@ -735,6 +760,75 @@ &pcie1_phy { status = "okay"; }; +&pcie1_port { + pcie@0,0 { + compatible = "pci1179,0623", "pciclass,0604"; + 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>; + + 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>; + + pcie@0,0 { + reg = <0x50000 0x0 0x0 0x0 0x0>; + #address-cells = <3>; + #size-cells = <2>; + device_type = "pci"; + ranges; + }; + + pcie@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"; @@ -839,6 +933,28 @@ &sdhc_2 { status = "okay"; }; +&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>; + }; +}; + &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 0f2caf36910b..b2e2b1f26731 100644 --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi @@ -2284,7 +2284,7 @@ pcie1: pcie@1c08000 { status = "disabled"; - pcie@0 { + pcie1_port: pcie@0 { device_type = "pci"; reg = <0x0 0x0 0x0 0x0 0x0>; bus-range = <0x01 0xff>;