[PATCH 0/4] rockchip: fix serial output on rk3036

[PATCH 0/4] rockchip: fix serial output on rk3036

[Heiko Stuebner]

Recent changes to the 8250-dw variant revealed issues concerning
how the clock rates are handled on the rk3036 uart.

For one, there was an error in the clock declaration, but also the
shared uart-pll-select-mux also as default got supplied from the apll
that also supplies the cpu and thus gets frequency scaled.

The patches in this series remedy this and make the debug uart
function again on 4.10 + current merge window.


Heiko Stuebner (4):
  clk: rockchip: add "," to mux_pll_src_apll_dpll_gpll_usb480m_p on
    rk3036
  clk: rockchip: add SCLK_UARTPLL to rk3036 clock ids
  clk: rockchip: assign the SCLK_UARTPLL clock id on rk3036
  ARM: dts: rockchip: Make uartpll a child of the gpll on rk3036

 arch/arm/boot/dts/rk3036.dtsi          | 5 +++--
 drivers/clk/rockchip/clk-rk3036.c      | 4 ++--
 include/dt-bindings/clock/rk3036-cru.h | 1 +
 3 files changed, 6 insertions(+), 4 deletions(-)

-- 
2.11.0

[PATCH 1/4] clk: rockchip: add "," to mux_pll_src_apll_dpll_gpll_usb480m_p on rk3036

[Heiko Stuebner]

The mux_pll_src_apll_dpll_gpll_usb480m_p parent list was missing a ","
between the 3rd and 4th parent names, making them fall together and thus
lookups fail. Fix that.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/clk/rockchip/clk-rk3036.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk-rk3036.c b/drivers/clk/rockchip/clk-rk3036.c
index 924f560dcf80..dcde70f4c105 100644
--- a/drivers/clk/rockchip/clk-rk3036.c
+++ b/drivers/clk/rockchip/clk-rk3036.c
@@ -127,7 +127,7 @@ PNAME(mux_ddrphy_p)		= { "dpll_ddr", "gpll_ddr" };
 PNAME(mux_pll_src_3plls_p)	= { "apll", "dpll", "gpll" };
 PNAME(mux_timer_p)		= { "xin24m", "pclk_peri_src" };
 
-PNAME(mux_pll_src_apll_dpll_gpll_usb480m_p)	= { "apll", "dpll", "gpll" "usb480m" };
+PNAME(mux_pll_src_apll_dpll_gpll_usb480m_p)	= { "apll", "dpll", "gpll", "usb480m" };
 
 PNAME(mux_mmc_src_p)	= { "apll", "dpll", "gpll", "xin24m" };
 PNAME(mux_i2s_pre_p)	= { "i2s_src", "i2s_frac", "ext_i2s", "xin12m" };
-- 
2.11.0

Re: [PATCH 1/4] clk: rockchip: add "," to mux_pll_src_apll_dpll_gpll_usb480m_p on rk3036

[Stephen Boyd]

On 02/28, Heiko Stuebner wrote:
> The mux_pll_src_apll_dpll_gpll_usb480m_p parent list was missing a ","
> between the 3rd and 4th parent names, making them fall together and thus
> lookups fail. Fix that.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---

Subtle. Fixes tag?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 2/4] clk: rockchip: add SCLK_UARTPLL to rk3036 clock ids

[Heiko Stuebner]

Systems might need it, if they want to assign the pll-mux shared
by all uart clocks to some specific pll.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 include/dt-bindings/clock/rk3036-cru.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/dt-bindings/clock/rk3036-cru.h b/include/dt-bindings/clock/rk3036-cru.h
index de44109a3a04..58dd05af77e2 100644
--- a/include/dt-bindings/clock/rk3036-cru.h
+++ b/include/dt-bindings/clock/rk3036-cru.h
@@ -56,6 +56,7 @@
 #define SCLK_MACREF		152
 #define SCLK_MACPLL		153
 #define SCLK_SFC		160
+#define SCLK_UARTPLL		161
 
 /* aclk gates */
 #define ACLK_DMAC2		194
-- 
2.11.0

[PATCH 3/4] clk: rockchip: assign the SCLK_UARTPLL clock id on rk3036

[Heiko Stuebner]

Assign it to the matching clock in the clock driver.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/clk/rockchip/clk-rk3036.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk-rk3036.c b/drivers/clk/rockchip/clk-rk3036.c
index dcde70f4c105..d10934b63a0d 100644
--- a/drivers/clk/rockchip/clk-rk3036.c
+++ b/drivers/clk/rockchip/clk-rk3036.c
@@ -242,7 +242,7 @@ static struct rockchip_clk_branch rk3036_clk_branches[] __initdata = {
 			RK2928_CLKSEL_CON(2), 7, 1, MFLAGS,
 			RK2928_CLKGATE_CON(2), 5, GFLAGS),
 
-	MUX(0, "uart_pll_clk", mux_pll_src_apll_dpll_gpll_usb480m_p, 0,
+	MUX(SCLK_UARTPLL, "uart_pll_clk", mux_pll_src_apll_dpll_gpll_usb480m_p, 0,
 			RK2928_CLKSEL_CON(13), 10, 2, MFLAGS),
 	COMPOSITE_NOMUX(0, "uart0_src", "uart_pll_clk", 0,
 			RK2928_CLKSEL_CON(13), 0, 7, DFLAGS,
-- 
2.11.0

[PATCH 4/4] ARM: dts: rockchip: Make uartpll a child of the gpll on rk3036

[Heiko Stuebner]

The shared uart-pll is normally a child of the apll that can get changed
by cpu frequency scaling. So move it away to the more stable gpll to
make sure the uart doesn't break on cpu frequency changes.

This turned up during the 4.11 merge-window when commit
6a171b299379 ("serial: 8250_dw: Allow hardware flow control to be used")
added general termios enablement making the uart on rk3036 change
frequency and thus making it susceptible for the frequency scaling issue.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 arch/arm/boot/dts/rk3036.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
index ff9b90bfaefd..6442023854c5 100644
--- a/arch/arm/boot/dts/rk3036.dtsi
+++ b/arch/arm/boot/dts/rk3036.dtsi
@@ -310,8 +310,9 @@
 		rockchip,grf = <&grf>;
 		#clock-cells = <1>;
 		#reset-cells = <1>;
-		assigned-clocks = <&cru PLL_GPLL>;
-		assigned-clock-rates = <594000000>;
+		assigned-clocks = <&cru PLL_GPLL>, <&cru SCLK_UARTPLL>;
+		assigned-clock-rates = <594000000>, <0>;
+		assigned-clock-parents = <0>, <&cru PLL_GPLL>;
 	};
 
 	grf: syscon@20008000 {
-- 
2.11.0

Re: [PATCH 0/4] rockchip: fix serial output on rk3036

[Stephen Boyd]

On 02/28, Heiko Stuebner wrote:
> Recent changes to the 8250-dw variant revealed issues concerning
> how the clock rates are handled on the rk3036 uart.
> 
> For one, there was an error in the clock declaration, but also the
> shared uart-pll-select-mux also as default got supplied from the apll
> that also supplies the cpu and thus gets frequency scaled.
> 
> The patches in this series remedy this and make the debug uart
> function again on 4.10 + current merge window.
> 

What's the merge path? The last patch is sort of questionable
because it fixes a regression by changing assigned clocks in DT,
which doesn't really make sense from a DT perspective (it should
have been right already or can be configured from the clk driver
itself in software).

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 0/4] rockchip: fix serial output on rk3036

[Heiko Stuebner]

Am Montag, 27. Februar 2017, 23:32:40 CET schrieb Stephen Boyd:
> On 02/28, Heiko Stuebner wrote:
> > Recent changes to the 8250-dw variant revealed issues concerning
> > how the clock rates are handled on the rk3036 uart.
> > 
> > For one, there was an error in the clock declaration, but also the
> > shared uart-pll-select-mux also as default got supplied from the apll
> > that also supplies the cpu and thus gets frequency scaled.
> > 
> > The patches in this series remedy this and make the debug uart
> > function again on 4.10 + current merge window.
> 
> What's the merge path? The last patch is sort of questionable
> because it fixes a regression by changing assigned clocks in DT,
> which doesn't really make sense from a DT perspective (it should
> have been right already or can be configured from the clk driver
> itself in software).

yeah, I was (and somewhat still am) debating on the dt vs. clk driver 
positioning. As you can see, there are multiple sources, most of them are 
somewhat questionable. I.e. apll and dpll are the cpu + ddr supplies and we 
will want to do ddr-scaling at some point too.

So the gpll really is the only really sane option and I guess simply doing it 
in the driver (similar to what rk3188 does) might be better.
If some boards really want to select a different source, they then can do that 
via assigned-clocks in the board-dts.