| Message ID | 20180829155046.29359-3-m.szyprowski@samsung.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Cleanup suspend/resume code in Samsung clock drivers | expand |
Hi Marek, On 2018년 08월 30일 00:50, Marek Szyprowski wrote: > Replace common suspend/resume handling code by generic helper. > No functional change. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > drivers/clk/samsung/clk-s3c2410.c | 43 ++----------------------------- > 1 file changed, 2 insertions(+), 41 deletions(-) > Looks good to me. Acked-by: Chanwoo Choi <cw00.choi@samsung.com> > diff --git a/drivers/clk/samsung/clk-s3c2410.c b/drivers/clk/samsung/clk-s3c2410.c > index a9c887475054..8cb868f06257 100644 > --- a/drivers/clk/samsung/clk-s3c2410.c > +++ b/drivers/clk/samsung/clk-s3c2410.c > @@ -11,7 +11,6 @@ > #include <linux/clk-provider.h> > #include <linux/of.h> > #include <linux/of_address.h> > -#include <linux/syscore_ops.h> > > #include <dt-bindings/clock/s3c2410.h> > > @@ -40,9 +39,6 @@ enum s3c2410_plls { > > static void __iomem *reg_base; > > -#ifdef CONFIG_PM_SLEEP > -static struct samsung_clk_reg_dump *s3c2410_save; > - > /* > * list of controller registers to be saved and restored during a > * suspend/resume cycle. > @@ -57,42 +53,6 @@ static unsigned long s3c2410_clk_regs[] __initdata = { > CAMDIVN, > }; > > -static int s3c2410_clk_suspend(void) > -{ > - samsung_clk_save(reg_base, s3c2410_save, > - ARRAY_SIZE(s3c2410_clk_regs)); > - > - return 0; > -} > - > -static void s3c2410_clk_resume(void) > -{ > - samsung_clk_restore(reg_base, s3c2410_save, > - ARRAY_SIZE(s3c2410_clk_regs)); > -} > - > -static struct syscore_ops s3c2410_clk_syscore_ops = { > - .suspend = s3c2410_clk_suspend, > - .resume = s3c2410_clk_resume, > -}; > - > -static void __init s3c2410_clk_sleep_init(void) > -{ > - s3c2410_save = samsung_clk_alloc_reg_dump(s3c2410_clk_regs, > - ARRAY_SIZE(s3c2410_clk_regs)); > - if (!s3c2410_save) { > - pr_warn("%s: failed to allocate sleep save data, no sleep support!\n", > - __func__); > - return; > - } > - > - register_syscore_ops(&s3c2410_clk_syscore_ops); > - return; > -} > -#else > -static void __init s3c2410_clk_sleep_init(void) {} > -#endif > - > PNAME(fclk_p) = { "mpll", "div_slow" }; > > static struct samsung_mux_clock s3c2410_common_muxes[] __initdata = { > @@ -461,7 +421,8 @@ void __init s3c2410_common_clk_init(struct device_node *np, unsigned long xti_f, > ARRAY_SIZE(s3c244x_common_aliases)); > } > > - s3c2410_clk_sleep_init(); > + samsung_clk_sleep_init(reg_base, s3c2410_clk_regs, > + ARRAY_SIZE(s3c2410_clk_regs)); > > samsung_clk_of_add_provider(np, ctx); > } >
On Wed, 29 Aug 2018 at 17:51, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > Replace common suspend/resume handling code by generic helper. > No functional change. There is functional change in case alloc fails (kzalloc(), samsung_clk_alloc_reg_dump()). Previously it would be warned and ignored. Now it will panic. You could mention this but anyway the change looks nice! Thanks! Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof
Hi Krzysztof, On 2018-08-30 08:39, Krzysztof Kozlowski wrote: > On Wed, 29 Aug 2018 at 17:51, Marek Szyprowski <m.szyprowski@samsung.com> wrote: >> Replace common suspend/resume handling code by generic helper. >> No functional change. > There is functional change in case alloc fails (kzalloc(), > samsung_clk_alloc_reg_dump()). Previously it would be warned and > ignored. Now it will panic. You could mention this but anyway the > change looks nice! Thanks! No problem to replace panic() in samsung_clk_sleep_init() with a pair of warn and return error as this is probably now a preferred behavior. This is still truly hypothetical discussion because system, which fails to allocate memory at boot is useless anyway. Best regards
On Thu, 30 Aug 2018 at 13:26, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > Hi Krzysztof, > > On 2018-08-30 08:39, Krzysztof Kozlowski wrote: > > On Wed, 29 Aug 2018 at 17:51, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > >> Replace common suspend/resume handling code by generic helper. > >> No functional change. > > There is functional change in case alloc fails (kzalloc(), > > samsung_clk_alloc_reg_dump()). Previously it would be warned and > > ignored. Now it will panic. You could mention this but anyway the > > change looks nice! Thanks! > > No problem to replace panic() in samsung_clk_sleep_init() with a pair > of warn and return error as this is probably now a preferred behavior. > This is still truly hypothetical discussion because system, which > fails to allocate memory at boot is useless anyway. I do not mind keeping this as is with small notice in commit msg about change in case of error path. Indeed it is truly hypothetical case so there is no point to rewrite samsung_clk_sleep_init for this. Best regards, Krzysztof
diff --git a/drivers/clk/samsung/clk-s3c2410.c b/drivers/clk/samsung/clk-s3c2410.c index a9c887475054..8cb868f06257 100644 --- a/drivers/clk/samsung/clk-s3c2410.c +++ b/drivers/clk/samsung/clk-s3c2410.c @@ -11,7 +11,6 @@ #include <linux/clk-provider.h> #include <linux/of.h> #include <linux/of_address.h> -#include <linux/syscore_ops.h> #include <dt-bindings/clock/s3c2410.h> @@ -40,9 +39,6 @@ enum s3c2410_plls { static void __iomem *reg_base; -#ifdef CONFIG_PM_SLEEP -static struct samsung_clk_reg_dump *s3c2410_save; - /* * list of controller registers to be saved and restored during a * suspend/resume cycle. @@ -57,42 +53,6 @@ static unsigned long s3c2410_clk_regs[] __initdata = { CAMDIVN, }; -static int s3c2410_clk_suspend(void) -{ - samsung_clk_save(reg_base, s3c2410_save, - ARRAY_SIZE(s3c2410_clk_regs)); - - return 0; -} - -static void s3c2410_clk_resume(void) -{ - samsung_clk_restore(reg_base, s3c2410_save, - ARRAY_SIZE(s3c2410_clk_regs)); -} - -static struct syscore_ops s3c2410_clk_syscore_ops = { - .suspend = s3c2410_clk_suspend, - .resume = s3c2410_clk_resume, -}; - -static void __init s3c2410_clk_sleep_init(void) -{ - s3c2410_save = samsung_clk_alloc_reg_dump(s3c2410_clk_regs, - ARRAY_SIZE(s3c2410_clk_regs)); - if (!s3c2410_save) { - pr_warn("%s: failed to allocate sleep save data, no sleep support!\n", - __func__); - return; - } - - register_syscore_ops(&s3c2410_clk_syscore_ops); - return; -} -#else -static void __init s3c2410_clk_sleep_init(void) {} -#endif - PNAME(fclk_p) = { "mpll", "div_slow" }; static struct samsung_mux_clock s3c2410_common_muxes[] __initdata = { @@ -461,7 +421,8 @@ void __init s3c2410_common_clk_init(struct device_node *np, unsigned long xti_f, ARRAY_SIZE(s3c244x_common_aliases)); } - s3c2410_clk_sleep_init(); + samsung_clk_sleep_init(reg_base, s3c2410_clk_regs, + ARRAY_SIZE(s3c2410_clk_regs)); samsung_clk_of_add_provider(np, ctx); }
Replace common suspend/resume handling code by generic helper. No functional change. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- drivers/clk/samsung/clk-s3c2410.c | 43 ++----------------------------- 1 file changed, 2 insertions(+), 41 deletions(-)