Re: [PATCH] clk: rockchip: switch PLLs to slow mode before reboot for rk3288

From: Heiko Stuebner
Date: Tue Nov 10 2015 - 05:57:11 EST


Hi Chris,

Am Dienstag, 10. November 2015, 17:35:31 schrieb Chris Zhong:
> We've been seeing some crashes at reboot test on rk3288-based systems,
> which boards have not reset pin connected to NPOR, they reboot by
> setting 0xfdb9 to RK3288_GLB_SRST_FST register. If the APLL works in
> a high frequency mode, some IPs might hang during soft reset.
> It appears that we can fix the problem by switching to slow mode before
> reboot, just like what we did before suspend.
>
> Signed-off-by: Chris Zhong <zyw@xxxxxxxxxxxxxx>
>
> ---
>
> drivers/clk/rockchip/clk-rk3288.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-
rk3288.c
> index 9040878..524662c 100644
> --- a/drivers/clk/rockchip/clk-rk3288.c
> +++ b/drivers/clk/rockchip/clk-rk3288.c
> @@ -16,6 +16,7 @@
> #include <linux/clk-provider.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> +#include <linux/reboot.h>
> #include <linux/syscore_ops.h>
> #include <dt-bindings/clock/rk3288-cru.h>
> #include "clk.h"
> @@ -855,6 +856,34 @@ static void rk3288_clk_sleep_init(void __iomem
*reg_base)
> static void rk3288_clk_sleep_init(void __iomem *reg_base) {}
> #endif
>
> +void __iomem *mode_con_reg;
> +static int rk3288_restart_notify(struct notifier_block *this,
> + unsigned long mode, void *cmd)
> +{
> + writel(0xf3030000, mode_con_reg);
> + return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block rk3288_restart_handler = {
> + .notifier_call = rk3288_restart_notify,
> + /* Switch PLLs other than DPLL (for SDRAM) to slow mode before reboot
> + * to avoid crashes in reset, so this priority must bigger than the one
> + * in rockchip_restart_handler.
> + */
> + .priority = 129,
> +};
> +
> +void __init rk3288_register_restart_notifier(void __iomem *reg)
> +{
> + int ret;
> +
> + mode_con_reg = reg;
> + ret = register_restart_handler(&rk3288_restart_handler);
> + if (ret)
> + pr_err("%s: cannot register restart handler, %d\n",
> + __func__, ret);
> +}
> +

restart_handlers are really _only_ supposed to be used for actions actually
restarting the system, not stuff that needs to be done before.

In the rockchip clock case, we already have the syscore_ops defined doing the
suspend/resume handling, so you could simply define a .shutdown callback which
also gets called from kernel_restart() [0], so runs at nearly the same time.

Heiko

[0] http://lxr.free-electrons.com/source/kernel/reboot.c#L214

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/