[PATCH] clk: vf610: improve handling case that cpu frequency can't be changed

[PATCH] clk: vf610: improve handling case that cpu frequency can't be changed

[Heiner Kallweit]

Currently we get a nasty error message if the cpu clock can't be
changed:
DDRC is clocked by PLL1, can't switch CPU clockinitcall vf610_switch_cpu_clock+0x1/0x198 failed: Invalid argument

So let's do the following:
- factor out the check from vf610_switch_cpu_clock_to_500mhz() and
  vf610_switch_cpu_clock_to_400mhz
- if clock can't be changed, don't treat it as an error
- don't call clock notifier chain if clock can't be changed
- add trailing newline to the warning message

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/clk/imx/clk-vf610.c | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/clk/imx/clk-vf610.c b/drivers/clk/imx/clk-vf610.c
index 1b1b881..d70f441 100644
--- a/drivers/clk/imx/clk-vf610.c
+++ b/drivers/clk/imx/clk-vf610.c
@@ -459,26 +459,30 @@ enum {
 	DDRMC_CR117_AXI0_FITYPEREG_SYNC = 0b01 << 16,
 };
 
-static int vf610_switch_cpu_clock_to_500mhz(void)
+static bool vf610_cpu_clk_changeable(void)
 {
-	int ret;
-
 	/*
 	 * When switching A5 CPU to 500Mhz we expect DDRC to be
 	 * clocked by PLL2_PFD2 and the system to be configured in
 	 * asynchronous mode.
-	 *
-	 * We also can't just use default PFD1 output of PLL1 due to
-	 * Errata e6235, so we have to re-clock the PLL itself and use
-	 * its output to clock the CPU directly.
 	 */
-
 	if (clk_get_parent(clk[VF610_CLK_DDR_SEL]) != clk[VF610_CLK_PLL2_PFD2]) {
-		pr_warn("DDRC is clocked by PLL1, can't switch CPU clock");
-		return -EINVAL;
+		pr_warn("DDRC is clocked by PLL1, can't switch CPU clock\n");
+		return false;
 	}
 
+	return true;
+}
+
+static int vf610_switch_cpu_clock_to_500mhz(void)
+{
+	int ret;
+
 	/*
+	 * We can't just use default PFD1 output of PLL1 due to
+	 * Errata e6235, so we have to re-clock the PLL itself and use
+	 * its output to clock the CPU directly.
+	 *
 	 * Code below alters the frequency of PLL1, and doing so would
 	 * require us to wait for PLL1 lock before proceeding to
 	 * select it as a clock source again.
@@ -533,11 +537,6 @@ static int vf610_switch_cpu_clock_to_400mhz(void)
 	uint32_t cr117;
 	void * __iomem ddrmc = IOMEM(VF610_DDR_BASE_ADDR);
 
-	if (clk_get_parent(clk[VF610_CLK_DDR_SEL]) != clk[VF610_CLK_PLL2_PFD2]) {
-		pr_warn("DDRC is clocked by PLL1, can't switch CPU clock");
-		return -EINVAL;
-	}
-
 	ret = clk_set_parent(clk[VF610_CLK_PLL2_PFD_SEL], clk[VF610_CLK_PLL2_PFD2]);
 	if (ret < 0) {
 		pr_crit("Unable to re-parent '%s'\n",
@@ -595,10 +594,14 @@ static int vf610_switch_cpu_clock(void)
 		return 0;
 
 	case VF610_SPEED_500:
+		if (!vf610_cpu_clk_changeable())
+			return 0;
 		ret = vf610_switch_cpu_clock_to_500mhz();
 		break;
 
 	case VF610_SPEED_400:
+		if (!vf610_cpu_clk_changeable())
+			return 0;
 		ret = vf610_switch_cpu_clock_to_400mhz();
 		break;
 	}
-- 
2.21.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] clk: vf610: improve handling case that cpu frequency can't be changed

[Andrey Smirnov]

On Sat, Apr 27, 2019 at 3:09 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> Currently we get a nasty error message if the cpu clock can't be
> changed:
> DDRC is clocked by PLL1, can't switch CPU clockinitcall vf610_switch_cpu_clock+0x1/0x198 failed: Invalid argument
>
> So let's do the following:
> - factor out the check from vf610_switch_cpu_clock_to_500mhz() and
>   vf610_switch_cpu_clock_to_400mhz
> - if clock can't be changed, don't treat it as an error
> - don't call clock notifier chain if clock can't be changed
> - add trailing newline to the warning message
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Minor nit: it might've been a bit simpler to create
vf610_cpu_clk_unchangable(), so you wouldn't have to invert the result
in every usecase.

Regardless the code is fine as is as, so:

Reviewed-by: Andrey Smirnov <andrew.smirnov@gmail.com>

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] clk: vf610: improve handling case that cpu frequency can't be changed

[Sascha Hauer]

On Sat, Apr 27, 2019 at 12:09:05PM +0200, Heiner Kallweit wrote:
> Currently we get a nasty error message if the cpu clock can't be
> changed:
> DDRC is clocked by PLL1, can't switch CPU clockinitcall vf610_switch_cpu_clock+0x1/0x198 failed: Invalid argument
> 
> So let's do the following:
> - factor out the check from vf610_switch_cpu_clock_to_500mhz() and
>   vf610_switch_cpu_clock_to_400mhz
> - if clock can't be changed, don't treat it as an error
> - don't call clock notifier chain if clock can't be changed
> - add trailing newline to the warning message
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/clk/imx/clk-vf610.c | 33 ++++++++++++++++++---------------
>  1 file changed, 18 insertions(+), 15 deletions(-)

Applied, thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox