On Mon, Apr 14, 2025 at 02:49:00PM +0200, Caleb Connolly wrote:
>
>
> On 4/14/25 07:39, Krishna Chaitanya Chundru wrote:
> > Move phy, perst, to root port from the controller node.
> >
> > Rename perst-gpios to reset-gpios to align with the expected naming
> > convention of pci-bus-common.yaml.
> >
> > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@xxxxxxxxxxxxxxxx>
> > ---
> > arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 5 ++++-
> > arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 5 ++++-
> > arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 5 ++++-
> > arch/arm64/boot/dts/qcom/sc7280.dtsi | 6 ++----
> > 4 files changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > index 7a36c90ad4ec8b52f30b22b1621404857d6ef336..3dd58986ad5da0f898537a51715bb5d0fecbe100 100644
> > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > @@ -709,8 +709,11 @@ &mdss_edp_phy {
> > status = "okay";
> > };
> > +&pcie1_port0 {
> > + reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> > +};
> > +
> > &pcie1 {
> > - perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> > pinctrl-0 = <&pcie1_reset_n>, <&pcie1_wake_n>;
> > pinctrl-names = "default";
> > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> > index 2ba4ea60cb14736c9cfbf9f4a9048f20a4c921f2..ff11d85d015bdab6a90bd8a0eb9113a339866953 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> > @@ -472,10 +472,13 @@ &pcie1 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&pcie1_clkreq_n>, <&ssd_rst_l>, <&pe_wake_odl>;
> > - perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> > vddpe-3v3-supply = <&pp3300_ssd>;
> > };
> > +&pcie1_port0 {
> > + reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> > +};
> > +
> > &pm8350c_pwm {
> > status = "okay";
> > };
> > diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> > index 7370aa0dbf0e3f9e7a3e38c3f00686e1d3dcbc9f..3209bb15dfec36299cabae07d34f3dc82db6de77 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> > @@ -414,9 +414,12 @@ &lpass_va_macro {
> > vdd-micb-supply = <&vreg_bob>;
> > };
> > +&pcie1_port0 {
> > + reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> > +};
> > +
> > &pcie1 {
> > status = "okay";
> > - perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> > vddpe-3v3-supply = <&nvme_3v3_regulator>;
> > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > index 0f2caf36910b65c398c9e03800a8ce0a8a1f8fc7..376fabf3b4eac34d75bb79ef902c9d83490c45f7 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > @@ -2271,9 +2271,6 @@ pcie1: pcie@1c08000 {
> > power-domains = <&gcc GCC_PCIE_1_GDSC>;
> > - phys = <&pcie1_phy>;
> > - phy-names = "pciephy";
>
> Isn't this a huge API breakage that will break using new DT with old
> kernels? It will also break U-boot which uses upstream DT.
>
That's right.
> This is bad enough, but at least let's break it a clean break by changing
> all platforms at once.
>
Even though converting all platforms is the end goal, I don't think converting
all of them in a single patch is going to do any good.
> As I understand it, we seem to allow these breakages in DT for now (and this
> patch landing will confirm that), but perhaps we could at least track them
> somewhere or at acknowledge the breakage with a tag in the commit message
> pointing to the relevant dt-bindings patch.
>
I'll leave the decision of breaking old kernels to DT maintainers, but I'd
atleast prefer to use this in upcoming targets. So the binding and driver
changes can go any time.
- Mani
--
மணிவண்ணன் சதாசிவம்
