diff mbox series

[2/2] arm64: dts: qcom: ipq8074: correct USB3 QMP PHY-s clock output names

Message ID 20230108130440.670181-2-robimarko@gmail.com
State New
Headers
Series [1/2] clk: qcom: ipq8074: populate fw_name for usb3phy-s |

Commit Message

Robert Marko Jan. 8, 2023, 1:04 p.m. UTC 
  It seems that clock-output-names for the USB3 QMP PHY-s where set without
actually checking what is the GCC clock driver expecting, so clock core
could never actually find the parents for usb0_pipe_clk_src and
usb1_pipe_clk_src clocks in the GCC driver.

So, correct the names to be what the driver expects so that parenting
works.

Before:
gcc_usb0_pipe_clk_src                0        0        0   125000000          0     0  50000         Y
gcc_usb1_pipe_clk_src                0        0        0   125000000          0     0  50000         Y

After:
 usb3phy_0_cc_pipe_clk                1        1        0   125000000          0     0  50000         Y
    usb0_pipe_clk_src                 1        1        0   125000000          0     0  50000         Y
       gcc_usb0_pipe_clk              1        1        0   125000000          0     0  50000         Y
 usb3phy_1_cc_pipe_clk                1        1        0   125000000          0     0  50000         Y
    usb1_pipe_clk_src                 1        1        0   125000000          0     0  50000         Y
       gcc_usb1_pipe_clk              1        1        0   125000000          0     0  50000         Y

Fixes: 5e09bc51d07b ("arm64: dts: ipq8074: enable USB support")
Signed-off-by: Robert Marko <robimarko@gmail.com>
---
 arch/arm64/boot/dts/qcom/ipq8074.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Dmitry Baryshkov Jan. 8, 2023, 5:32 p.m. UTC | #1 
On Sun, 8 Jan 2023 at 15:04, Robert Marko <robimarko@gmail.com> wrote:
>
> It seems that clock-output-names for the USB3 QMP PHY-s where set without
> actually checking what is the GCC clock driver expecting, so clock core
> could never actually find the parents for usb0_pipe_clk_src and
> usb1_pipe_clk_src clocks in the GCC driver.
>
> So, correct the names to be what the driver expects so that parenting
> works.
>
> Before:
> gcc_usb0_pipe_clk_src                0        0        0   125000000          0     0  50000         Y
> gcc_usb1_pipe_clk_src                0        0        0   125000000          0     0  50000         Y
>
> After:
>  usb3phy_0_cc_pipe_clk                1        1        0   125000000          0     0  50000         Y
>     usb0_pipe_clk_src                 1        1        0   125000000          0     0  50000         Y
>        gcc_usb0_pipe_clk              1        1        0   125000000          0     0  50000         Y
>  usb3phy_1_cc_pipe_clk                1        1        0   125000000          0     0  50000         Y
>     usb1_pipe_clk_src                 1        1        0   125000000          0     0  50000         Y
>        gcc_usb1_pipe_clk              1        1        0   125000000          0     0  50000         Y
>
> Fixes: 5e09bc51d07b ("arm64: dts: ipq8074: enable USB support")
> Signed-off-by: Robert Marko <robimarko@gmail.com>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Nevertheless, could you please add .fw_name to these entries in gcc
driver (as you did for other clocks in 35dc8e101a8e ("clk: qcom:
ipq8074: populate fw_name for all parents")) and add all pipe clocks
to the gcc node? This way you can drop clock-output-names from the PHY
nodes.

> ---
>  arch/arm64/boot/dts/qcom/ipq8074.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

[skipped]
Robert Marko Jan. 8, 2023, 6:04 p.m. UTC | #2 
On Sun, 8 Jan 2023 at 18:32, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Sun, 8 Jan 2023 at 15:04, Robert Marko <robimarko@gmail.com> wrote:
> >
> > It seems that clock-output-names for the USB3 QMP PHY-s where set without
> > actually checking what is the GCC clock driver expecting, so clock core
> > could never actually find the parents for usb0_pipe_clk_src and
> > usb1_pipe_clk_src clocks in the GCC driver.
> >
> > So, correct the names to be what the driver expects so that parenting
> > works.
> >
> > Before:
> > gcc_usb0_pipe_clk_src                0        0        0   125000000          0     0  50000         Y
> > gcc_usb1_pipe_clk_src                0        0        0   125000000          0     0  50000         Y
> >
> > After:
> >  usb3phy_0_cc_pipe_clk                1        1        0   125000000          0     0  50000         Y
> >     usb0_pipe_clk_src                 1        1        0   125000000          0     0  50000         Y
> >        gcc_usb0_pipe_clk              1        1        0   125000000          0     0  50000         Y
> >  usb3phy_1_cc_pipe_clk                1        1        0   125000000          0     0  50000         Y
> >     usb1_pipe_clk_src                 1        1        0   125000000          0     0  50000         Y
> >        gcc_usb1_pipe_clk              1        1        0   125000000          0     0  50000         Y
> >
> > Fixes: 5e09bc51d07b ("arm64: dts: ipq8074: enable USB support")
> > Signed-off-by: Robert Marko <robimarko@gmail.com>
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
> Nevertheless, could you please add .fw_name to these entries in gcc
> driver (as you did for other clocks in 35dc8e101a8e ("clk: qcom:
> ipq8074: populate fw_name for all parents")) and add all pipe clocks
> to the gcc node? This way you can drop clock-output-names from the PHY
> nodes.

As you noticed they are in the GCC patch already, after the PCI PIPE
parenting fixes
are merged I plan to add them to the GCC node to avoid global lookup.

Regards,
Robert
>
> > ---
> >  arch/arm64/boot/dts/qcom/ipq8074.dtsi | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
>
> [skipped]
>
> --
> With best wishes
> Dmitry
Dmitry Baryshkov Jan. 8, 2023, 6:07 p.m. UTC | #3 
On Sun, 8 Jan 2023 at 20:04, Robert Marko <robimarko@gmail.com> wrote:
>
> On Sun, 8 Jan 2023 at 18:32, Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On Sun, 8 Jan 2023 at 15:04, Robert Marko <robimarko@gmail.com> wrote:
> > >
> > > It seems that clock-output-names for the USB3 QMP PHY-s where set without
> > > actually checking what is the GCC clock driver expecting, so clock core
> > > could never actually find the parents for usb0_pipe_clk_src and
> > > usb1_pipe_clk_src clocks in the GCC driver.
> > >
> > > So, correct the names to be what the driver expects so that parenting
> > > works.
> > >
> > > Before:
> > > gcc_usb0_pipe_clk_src                0        0        0   125000000          0     0  50000         Y
> > > gcc_usb1_pipe_clk_src                0        0        0   125000000          0     0  50000         Y
> > >
> > > After:
> > >  usb3phy_0_cc_pipe_clk                1        1        0   125000000          0     0  50000         Y
> > >     usb0_pipe_clk_src                 1        1        0   125000000          0     0  50000         Y
> > >        gcc_usb0_pipe_clk              1        1        0   125000000          0     0  50000         Y
> > >  usb3phy_1_cc_pipe_clk                1        1        0   125000000          0     0  50000         Y
> > >     usb1_pipe_clk_src                 1        1        0   125000000          0     0  50000         Y
> > >        gcc_usb1_pipe_clk              1        1        0   125000000          0     0  50000         Y
> > >
> > > Fixes: 5e09bc51d07b ("arm64: dts: ipq8074: enable USB support")
> > > Signed-off-by: Robert Marko <robimarko@gmail.com>
> >
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >
> > Nevertheless, could you please add .fw_name to these entries in gcc
> > driver (as you did for other clocks in 35dc8e101a8e ("clk: qcom:
> > ipq8074: populate fw_name for all parents")) and add all pipe clocks
> > to the gcc node? This way you can drop clock-output-names from the PHY
> > nodes.
>
> As you noticed they are in the GCC patch already, after the PCI PIPE
> parenting fixes
> are merged I plan to add them to the GCC node to avoid global lookup.

Good! I think the pcie fixes are already in Bjorn's tree. And you
might send the dts fix anyway, in the worst case the driver will just
ignore the DT clocks.
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
index a0f7460020c2..3d1fccdf37c7 100644
--- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
@@ -263,7 +263,7 @@  usb1_ssphy: phy@58200 {
 				#clock-cells = <0>;
 				clocks = <&gcc GCC_USB1_PIPE_CLK>;
 				clock-names = "pipe0";
-				clock-output-names = "gcc_usb1_pipe_clk_src";
+				clock-output-names = "usb3phy_1_cc_pipe_clk";
 			};
 		};
 
@@ -306,7 +306,7 @@  usb0_ssphy: phy@78200 {
 				#clock-cells = <0>;
 				clocks = <&gcc GCC_USB0_PIPE_CLK>;
 				clock-names = "pipe0";
-				clock-output-names = "gcc_usb0_pipe_clk_src";
+				clock-output-names = "usb3phy_0_cc_pipe_clk";
 			};
 		};