diff mbox series

[RFC,2/5] arm64: dts: qcom: sc7280: move pipe mux handling to phy

Message ID 20220421102041.17345-3-johan+linaro@kernel.org
State RFC
Headers show
Series phy: qcom-qmp: add support for pipe clock muxing | expand

Commit Message

Johan Hovold April 21, 2022, 10:20 a.m. UTC
The QMP PHY pipe clock remuxing is part of the PHY, which is both the
producer and the consumer of the pipe clock.

Update the PCIe controller and PHY node to reflect the new binding.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

Comments

Dmitry Baryshkov April 21, 2022, 10:59 a.m. UTC | #1
On 21/04/2022 13:20, Johan Hovold wrote:
> The QMP PHY pipe clock remuxing is part of the PHY, which is both the
> producer and the consumer of the pipe clock.
> 
> Update the PCIe controller and PHY node to reflect the new binding.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>   arch/arm64/boot/dts/qcom/sc7280.dtsi | 18 ++++++------------
>   1 file changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index c07765df9303..b3a9630262dc 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -1837,11 +1837,7 @@ pcie1: pci@1c08000 {
>   					<0 0 0 3 &intc 0 0 0 438 IRQ_TYPE_LEVEL_HIGH>,
>   					<0 0 0 4 &intc 0 0 0 439 IRQ_TYPE_LEVEL_HIGH>;
>   
> -			clocks = <&gcc GCC_PCIE_1_PIPE_CLK>,
> -				 <&gcc GCC_PCIE_1_PIPE_CLK_SRC>,
> -				 <&pcie1_lane 0>,
> -				 <&rpmhcc RPMH_CXO_CLK>,
> -				 <&gcc GCC_PCIE_1_AUX_CLK>,
> +			clocks = <&gcc GCC_PCIE_1_AUX_CLK>,
>   				 <&gcc GCC_PCIE_1_CFG_AHB_CLK>,
>   				 <&gcc GCC_PCIE_1_MSTR_AXI_CLK>,
>   				 <&gcc GCC_PCIE_1_SLV_AXI_CLK>,
> @@ -1849,11 +1845,7 @@ pcie1: pci@1c08000 {
>   				 <&gcc GCC_AGGRE_NOC_PCIE_TBU_CLK>,
>   				 <&gcc GCC_DDRSS_PCIE_SF_CLK>;
>   
> -			clock-names = "pipe",
> -				      "pipe_mux",
> -				      "phy_pipe",
> -				      "ref",
> -				      "aux",
> +			clock-names = "aux",
>   				      "cfg",
>   				      "bus_master",
>   				      "bus_slave",
> @@ -1910,8 +1902,10 @@ pcie1_lane: lanes@1c0e200 {
>   				      <0 0x01c0e600 0 0x170>,
>   				      <0 0x01c0e800 0 0x200>,
>   				      <0 0x01c0ee00 0 0xf4>;
> -				clocks = <&gcc GCC_PCIE_1_PIPE_CLK>;
> -				clock-names = "pipe0";
> +				clocks = <&gcc GCC_PCIE_1_PIPE_CLK>,
> +					 <&gcc GCC_PCIE_1_PIPE_CLK_SRC>,
> +					 <&rpmhcc RPMH_CXO_CLK>;
> +				clock-names = "pipe0", "mux", "ref";

This will not be compatible with earlier DTB files, which was a problem 
up to now.

>   
>   				#phy-cells = <0>;
>   				#clock-cells = <1>;
Johan Hovold April 22, 2022, 10:07 a.m. UTC | #2
On Thu, Apr 21, 2022 at 01:59:04PM +0300, Dmitry Baryshkov wrote:
> On 21/04/2022 13:20, Johan Hovold wrote:
> > The QMP PHY pipe clock remuxing is part of the PHY, which is both the
> > producer and the consumer of the pipe clock.
> > 
> > Update the PCIe controller and PHY node to reflect the new binding.
> > 
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > ---
> >   arch/arm64/boot/dts/qcom/sc7280.dtsi | 18 ++++++------------
> >   1 file changed, 6 insertions(+), 12 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > index c07765df9303..b3a9630262dc 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > @@ -1837,11 +1837,7 @@ pcie1: pci@1c08000 {
> >   					<0 0 0 3 &intc 0 0 0 438 IRQ_TYPE_LEVEL_HIGH>,
> >   					<0 0 0 4 &intc 0 0 0 439 IRQ_TYPE_LEVEL_HIGH>;
> >   
> > -			clocks = <&gcc GCC_PCIE_1_PIPE_CLK>,
> > -				 <&gcc GCC_PCIE_1_PIPE_CLK_SRC>,
> > -				 <&pcie1_lane 0>,
> > -				 <&rpmhcc RPMH_CXO_CLK>,
> > -				 <&gcc GCC_PCIE_1_AUX_CLK>,
> > +			clocks = <&gcc GCC_PCIE_1_AUX_CLK>,
> >   				 <&gcc GCC_PCIE_1_CFG_AHB_CLK>,
> >   				 <&gcc GCC_PCIE_1_MSTR_AXI_CLK>,
> >   				 <&gcc GCC_PCIE_1_SLV_AXI_CLK>,
> > @@ -1849,11 +1845,7 @@ pcie1: pci@1c08000 {
> >   				 <&gcc GCC_AGGRE_NOC_PCIE_TBU_CLK>,
> >   				 <&gcc GCC_DDRSS_PCIE_SF_CLK>;
> >   
> > -			clock-names = "pipe",
> > -				      "pipe_mux",
> > -				      "phy_pipe",
> > -				      "ref",
> > -				      "aux",
> > +			clock-names = "aux",
> >   				      "cfg",
> >   				      "bus_master",
> >   				      "bus_slave",
> > @@ -1910,8 +1902,10 @@ pcie1_lane: lanes@1c0e200 {
> >   				      <0 0x01c0e600 0 0x170>,
> >   				      <0 0x01c0e800 0 0x200>,
> >   				      <0 0x01c0ee00 0 0xf4>;
> > -				clocks = <&gcc GCC_PCIE_1_PIPE_CLK>;
> > -				clock-names = "pipe0";
> > +				clocks = <&gcc GCC_PCIE_1_PIPE_CLK>,
> > +					 <&gcc GCC_PCIE_1_PIPE_CLK_SRC>,
> > +					 <&rpmhcc RPMH_CXO_CLK>;
> > +				clock-names = "pipe0", "mux", "ref";
> 
> This will not be compatible with earlier DTB files, which was a problem 
> up to now.

That depends. The above wasn't added until 5.16 so we may still be able
to fix it.

The NAK you got from Rob earlier was when you removed clocks that have
been in the devicetree for several years:

	https://lore.kernel.org/all/YgQ+tGhLqwUCsTUo@robh.at.kernel.org/

and would still be needed by older kernels.

Worst case, we need to keep both sets for sc7280 (i.e. like we need to
do with the pipe clocks that have been around for years).

Johan
Dmitry Baryshkov April 22, 2022, 10:36 a.m. UTC | #3
On Fri, 22 Apr 2022 at 13:07, Johan Hovold <johan@kernel.org> wrote:
>
> On Thu, Apr 21, 2022 at 01:59:04PM +0300, Dmitry Baryshkov wrote:
> > On 21/04/2022 13:20, Johan Hovold wrote:
> > > The QMP PHY pipe clock remuxing is part of the PHY, which is both the
> > > producer and the consumer of the pipe clock.
> > >
> > > Update the PCIe controller and PHY node to reflect the new binding.
> > >
> > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > > ---
> > >   arch/arm64/boot/dts/qcom/sc7280.dtsi | 18 ++++++------------
> > >   1 file changed, 6 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > > index c07765df9303..b3a9630262dc 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > > @@ -1837,11 +1837,7 @@ pcie1: pci@1c08000 {
> > >                                     <0 0 0 3 &intc 0 0 0 438 IRQ_TYPE_LEVEL_HIGH>,
> > >                                     <0 0 0 4 &intc 0 0 0 439 IRQ_TYPE_LEVEL_HIGH>;
> > >
> > > -                   clocks = <&gcc GCC_PCIE_1_PIPE_CLK>,
> > > -                            <&gcc GCC_PCIE_1_PIPE_CLK_SRC>,
> > > -                            <&pcie1_lane 0>,
> > > -                            <&rpmhcc RPMH_CXO_CLK>,
> > > -                            <&gcc GCC_PCIE_1_AUX_CLK>,
> > > +                   clocks = <&gcc GCC_PCIE_1_AUX_CLK>,
> > >                              <&gcc GCC_PCIE_1_CFG_AHB_CLK>,
> > >                              <&gcc GCC_PCIE_1_MSTR_AXI_CLK>,
> > >                              <&gcc GCC_PCIE_1_SLV_AXI_CLK>,
> > > @@ -1849,11 +1845,7 @@ pcie1: pci@1c08000 {
> > >                              <&gcc GCC_AGGRE_NOC_PCIE_TBU_CLK>,
> > >                              <&gcc GCC_DDRSS_PCIE_SF_CLK>;
> > >
> > > -                   clock-names = "pipe",
> > > -                                 "pipe_mux",
> > > -                                 "phy_pipe",
> > > -                                 "ref",
> > > -                                 "aux",
> > > +                   clock-names = "aux",
> > >                                   "cfg",
> > >                                   "bus_master",
> > >                                   "bus_slave",
> > > @@ -1910,8 +1902,10 @@ pcie1_lane: lanes@1c0e200 {
> > >                                   <0 0x01c0e600 0 0x170>,
> > >                                   <0 0x01c0e800 0 0x200>,
> > >                                   <0 0x01c0ee00 0 0xf4>;
> > > -                           clocks = <&gcc GCC_PCIE_1_PIPE_CLK>;
> > > -                           clock-names = "pipe0";
> > > +                           clocks = <&gcc GCC_PCIE_1_PIPE_CLK>,
> > > +                                    <&gcc GCC_PCIE_1_PIPE_CLK_SRC>,
> > > +                                    <&rpmhcc RPMH_CXO_CLK>;
> > > +                           clock-names = "pipe0", "mux", "ref";
> >
> > This will not be compatible with earlier DTB files, which was a problem
> > up to now.
>
> That depends. The above wasn't added until 5.16 so we may still be able
> to fix it.

That would depend on Rob/Krzyshtof. But the whole process should be described.
The driver can nod depend on the clocks being there.

>
> The NAK you got from Rob earlier was when you removed clocks that have
> been in the devicetree for several years:
>
>         https://lore.kernel.org/all/YgQ+tGhLqwUCsTUo@robh.at.kernel.org/
>
> and would still be needed by older kernels.
>
> Worst case, we need to keep both sets for sc7280 (i.e. like we need to
> do with the pipe clocks that have been around for years).
>
> Johan
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index c07765df9303..b3a9630262dc 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -1837,11 +1837,7 @@  pcie1: pci@1c08000 {
 					<0 0 0 3 &intc 0 0 0 438 IRQ_TYPE_LEVEL_HIGH>,
 					<0 0 0 4 &intc 0 0 0 439 IRQ_TYPE_LEVEL_HIGH>;
 
-			clocks = <&gcc GCC_PCIE_1_PIPE_CLK>,
-				 <&gcc GCC_PCIE_1_PIPE_CLK_SRC>,
-				 <&pcie1_lane 0>,
-				 <&rpmhcc RPMH_CXO_CLK>,
-				 <&gcc GCC_PCIE_1_AUX_CLK>,
+			clocks = <&gcc GCC_PCIE_1_AUX_CLK>,
 				 <&gcc GCC_PCIE_1_CFG_AHB_CLK>,
 				 <&gcc GCC_PCIE_1_MSTR_AXI_CLK>,
 				 <&gcc GCC_PCIE_1_SLV_AXI_CLK>,
@@ -1849,11 +1845,7 @@  pcie1: pci@1c08000 {
 				 <&gcc GCC_AGGRE_NOC_PCIE_TBU_CLK>,
 				 <&gcc GCC_DDRSS_PCIE_SF_CLK>;
 
-			clock-names = "pipe",
-				      "pipe_mux",
-				      "phy_pipe",
-				      "ref",
-				      "aux",
+			clock-names = "aux",
 				      "cfg",
 				      "bus_master",
 				      "bus_slave",
@@ -1910,8 +1902,10 @@  pcie1_lane: lanes@1c0e200 {
 				      <0 0x01c0e600 0 0x170>,
 				      <0 0x01c0e800 0 0x200>,
 				      <0 0x01c0ee00 0 0xf4>;
-				clocks = <&gcc GCC_PCIE_1_PIPE_CLK>;
-				clock-names = "pipe0";
+				clocks = <&gcc GCC_PCIE_1_PIPE_CLK>,
+					 <&gcc GCC_PCIE_1_PIPE_CLK_SRC>,
+					 <&rpmhcc RPMH_CXO_CLK>;
+				clock-names = "pipe0", "mux", "ref";
 
 				#phy-cells = <0>;
 				#clock-cells = <1>;