[PATCH 1/2] clk: qcom: ipq8074: populate fw_name for usb3phy-s

[PATCH 1/2] clk: qcom: ipq8074: populate fw_name for usb3phy-s

[Robert Marko]

Having only .name populated in parent_data for clocks which are only
globally searchable currently will not work as the clk core won't copy
that name if there is no .fw_name present as well.

So, populate .fw_name for usb3phy clocks in parent_data as they were
missed by me in ("clk: qcom: ipq8074: populate fw_name for all parents").

Fixes: ae55ad32e273 ("clk: qcom: ipq8074: convert to parent data")
Signed-off-by: Robert Marko <robimarko@gmail.com>
---
 drivers/clk/qcom/gcc-ipq8074.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/gcc-ipq8074.c b/drivers/clk/qcom/gcc-ipq8074.c
index 8374cc40915a..888e8df7bc92 100644
--- a/drivers/clk/qcom/gcc-ipq8074.c
+++ b/drivers/clk/qcom/gcc-ipq8074.c
@@ -934,7 +934,7 @@ static struct clk_rcg2 usb0_mock_utmi_clk_src = {
 };
 
 static const struct clk_parent_data gcc_usb3phy_0_cc_pipe_clk_xo[] = {
-	{ .name = "usb3phy_0_cc_pipe_clk" },
+	{ .fw_name = "usb3phy_0_cc_pipe_clk", .name = "usb3phy_0_cc_pipe_clk" },
 	{ .fw_name = "xo", .name = "xo" },
 };
 
@@ -1002,7 +1002,7 @@ static struct clk_rcg2 usb1_mock_utmi_clk_src = {
 };
 
 static const struct clk_parent_data gcc_usb3phy_1_cc_pipe_clk_xo[] = {
-	{ .name = "usb3phy_1_cc_pipe_clk" },
+	{ .fw_name = "usb3phy_1_cc_pipe_clk", .name = "usb3phy_1_cc_pipe_clk" },
 	{ .fw_name = "xo", .name = "xo" },
 };
 
-- 
2.39.0

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

[Robert Marko]

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(-)

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";
 			};
 		};
 
-- 
2.39.0

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

[Dmitry Baryshkov]

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]

-- 
With best wishes
Dmitry

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

[Robert Marko]

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

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

[Dmitry Baryshkov]

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.


-- 
With best wishes
Dmitry

Re: [PATCH 1/2] clk: qcom: ipq8074: populate fw_name for usb3phy-s

[Dmitry Baryshkov]

On Sun, 8 Jan 2023 at 15:04, Robert Marko <robimarko@gmail.com> wrote:
>
> Having only .name populated in parent_data for clocks which are only
> globally searchable currently will not work as the clk core won't copy
> that name if there is no .fw_name present as well.
>
> So, populate .fw_name for usb3phy clocks in parent_data as they were
> missed by me in ("clk: qcom: ipq8074: populate fw_name for all parents").
>
> Fixes: ae55ad32e273 ("clk: qcom: ipq8074: convert to parent data")
> Signed-off-by: Robert Marko <robimarko@gmail.com>

Ah, excuse me, this is what I asked for in the other patch. We can
only hope to see these clocks added to the gcc DT node.

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


-- 
With best wishes
Dmitry

Re: [PATCH 1/2] clk: qcom: ipq8074: populate fw_name for usb3phy-s

[Robert Marko]

On Sun, 8 Jan 2023 at 18:34, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Sun, 8 Jan 2023 at 15:04, Robert Marko <robimarko@gmail.com> wrote:
> >
> > Having only .name populated in parent_data for clocks which are only
> > globally searchable currently will not work as the clk core won't copy
> > that name if there is no .fw_name present as well.
> >
> > So, populate .fw_name for usb3phy clocks in parent_data as they were
> > missed by me in ("clk: qcom: ipq8074: populate fw_name for all parents").
> >
> > Fixes: ae55ad32e273 ("clk: qcom: ipq8074: convert to parent data")
> > Signed-off-by: Robert Marko <robimarko@gmail.com>
>
> Ah, excuse me, this is what I asked for in the other patch. We can
> only hope to see these clocks added to the gcc DT node.

Yes, there are PCI pipe parenting fixes waiting for review, so I plan to add
both to GCC node after that.

It would be great if you can take a look at the IPQ8074 PCI support series.

Regards,
Robert
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
>
> --
> With best wishes
> Dmitry

Re: (subset) [PATCH 1/2] clk: qcom: ipq8074: populate fw_name for usb3phy-s

[Bjorn Andersson]

On Sun, 8 Jan 2023 14:04:39 +0100, Robert Marko wrote:
> Having only .name populated in parent_data for clocks which are only
> globally searchable currently will not work as the clk core won't copy
> that name if there is no .fw_name present as well.
> 
> So, populate .fw_name for usb3phy clocks in parent_data as they were
> missed by me in ("clk: qcom: ipq8074: populate fw_name for all parents").
> 
> [...]

Applied, thanks!

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

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>