| Message ID | 1336457231-32513-1-git-send-email-Shaohui.Xie@freescale.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Kumar Gala |
| Headers | show |
> -----Original Message----- > From: linuxppc-dev-bounces+bharat.bhushan=freescale.com@lists.ozlabs.org > [mailto:linuxppc-dev-bounces+bharat.bhushan=freescale.com@lists.ozlabs.org] On > Behalf Of Shaohui Xie > Sent: Tuesday, May 08, 2012 11:37 AM > To: linux-watchdog@vger.kernel.org; linuxppc-dev@lists.ozlabs.org > Cc: Xie Shaohui-B21989 > Subject: [PATCH 1/2] powerpc/watchdog: move booke watchdog param related code to > prom.c > > Currently, BOOKE watchdog code for checking "wdt" and "wdt_period" is in > setup_32.c, it cannot be used in 64-bit, so move it to a common place prom.c, > which will be shared by 32-bit and 64-bit. > > Also, replace the simple_strtoul with kstrtol. > > Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com> > --- > arch/powerpc/kernel/prom.c | 27 +++++++++++++++++++++++++++ > arch/powerpc/kernel/setup_32.c | 24 ------------------------ > 2 files changed, 27 insertions(+), 24 deletions(-) Is not setup-common.c is better place to move this? Thanks -Bharat
>> -----Original Message----- >> From: >> linuxppc-dev-bounces+bharat.bhushan=freescale.com@lists.ozlabs.org >> [mailto:linuxppc-dev-bounces+bharat.bhushan=freescale.com@lists.ozlabs >> .org] On Behalf Of Shaohui Xie >> Sent: Tuesday, May 08, 2012 11:37 AM >> To: linux-watchdog@vger.kernel.org; linuxppc-dev@lists.ozlabs.org >> Cc: Xie Shaohui-B21989 >> Subject: [PATCH 1/2] powerpc/watchdog: move booke watchdog param >> related code to prom.c >> >> Currently, BOOKE watchdog code for checking "wdt" and "wdt_period" is >> in setup_32.c, it cannot be used in 64-bit, so move it to a common >> place prom.c, which will be shared by 32-bit and 64-bit. >> >> Also, replace the simple_strtoul with kstrtol. >> >> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com> >> --- >> arch/powerpc/kernel/prom.c | 27 +++++++++++++++++++++++++++ >> arch/powerpc/kernel/setup_32.c | 24 ------------------------ >> 2 files changed, 27 insertions(+), 24 deletions(-) > >Is not setup-common.c is better place to move this? Move out from setup_32.c does not mean it have to go into setup-common.c, I need better reason to do this. Best Regards, Shaohui Xie
> -----Original Message----- > From: Xie Shaohui-B21989 > Sent: Wednesday, May 09, 2012 8:50 AM > To: Bhushan Bharat-R65777; linux-watchdog@vger.kernel.org; linuxppc- > dev@lists.ozlabs.org > Subject: RE: [PATCH 1/2] powerpc/watchdog: move booke watchdog param related > code to prom.c > > >> -----Original Message----- > >> From: > >> linuxppc-dev-bounces+bharat.bhushan=freescale.com@lists.ozlabs.org > >> [mailto:linuxppc-dev-bounces+bharat.bhushan=freescale.com@lists.ozlab > >> s > >> .org] On Behalf Of Shaohui Xie > >> Sent: Tuesday, May 08, 2012 11:37 AM > >> To: linux-watchdog@vger.kernel.org; linuxppc-dev@lists.ozlabs.org > >> Cc: Xie Shaohui-B21989 > >> Subject: [PATCH 1/2] powerpc/watchdog: move booke watchdog param > >> related code to prom.c > >> > >> Currently, BOOKE watchdog code for checking "wdt" and "wdt_period" is > >> in setup_32.c, it cannot be used in 64-bit, so move it to a common > >> place prom.c, which will be shared by 32-bit and 64-bit. > >> > >> Also, replace the simple_strtoul with kstrtol. > >> > >> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com> > >> --- > >> arch/powerpc/kernel/prom.c | 27 +++++++++++++++++++++++++++ > >> arch/powerpc/kernel/setup_32.c | 24 ------------------------ > >> 2 files changed, 27 insertions(+), 24 deletions(-) > > > >Is not setup-common.c is better place to move this? > > Move out from setup_32.c does not mean it have to go into setup-common.c, I need > better reason to do this. > What I think that setup_32.c is for 32 bit, setup_64.c is for 64 bit and setup-common.c is for both. I am not saying that you move this to setup-common.c. I am asking why you have not used setup-common.c ? I am ok even with prom.c. Thanks -Bharat
>> -----Original Message----- >> From: Xie Shaohui-B21989 >> Sent: Wednesday, May 09, 2012 8:50 AM >> To: Bhushan Bharat-R65777; linux-watchdog@vger.kernel.org; linuxppc- >> dev@lists.ozlabs.org >> Subject: RE: [PATCH 1/2] powerpc/watchdog: move booke watchdog param >> related code to prom.c >> >> >> -----Original Message----- >> >> From: >> >> linuxppc-dev-bounces+bharat.bhushan=freescale.com@lists.ozlabs.org >> >> [mailto:linuxppc-dev-bounces+bharat.bhushan=freescale.com@lists.ozl >> >> ab >> >> s >> >> .org] On Behalf Of Shaohui Xie >> >> Sent: Tuesday, May 08, 2012 11:37 AM >> >> To: linux-watchdog@vger.kernel.org; linuxppc-dev@lists.ozlabs.org >> >> Cc: Xie Shaohui-B21989 >> >> Subject: [PATCH 1/2] powerpc/watchdog: move booke watchdog param >> >> related code to prom.c >> >> >> >> Currently, BOOKE watchdog code for checking "wdt" and "wdt_period" >> >> is in setup_32.c, it cannot be used in 64-bit, so move it to a >> >> common place prom.c, which will be shared by 32-bit and 64-bit. >> >> >> >> Also, replace the simple_strtoul with kstrtol. >> >> >> >> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com> >> >> --- >> >> arch/powerpc/kernel/prom.c | 27 +++++++++++++++++++++++++++ >> >> arch/powerpc/kernel/setup_32.c | 24 ------------------------ >> >> 2 files changed, 27 insertions(+), 24 deletions(-) >> > >> >Is not setup-common.c is better place to move this? >> >> Move out from setup_32.c does not mean it have to go into >> setup-common.c, I need better reason to do this. >> > >What I think that setup_32.c is for 32 bit, setup_64.c is for 64 bit and >setup-common.c is for both. > >I am not saying that you move this to setup-common.c. I am asking why you >have not used setup-common.c ? I am ok even with prom.c. > [Xie Shaohui] I'm not a fan of prom.c, I did this because I see same kind of early parameters checking is did in this file only, so I thought maybe I should put them together. And seems setup-common.c is not the place to do command line checking (I'm not sure about this). Best Regards, Shaohui Xie
> >> >> .org] On Behalf Of Shaohui Xie > >> >> Sent: Tuesday, May 08, 2012 11:37 AM > >> >> To: linux-watchdog@vger.kernel.org; linuxppc-dev@lists.ozlabs.org > >> >> Cc: Xie Shaohui-B21989 > >> >> Subject: [PATCH 1/2] powerpc/watchdog: move booke watchdog param > >> >> related code to prom.c > >> >> > >> >> Currently, BOOKE watchdog code for checking "wdt" and "wdt_period" > >> >> is in setup_32.c, it cannot be used in 64-bit, so move it to a > >> >> common place prom.c, which will be shared by 32-bit and 64-bit. > >> >> > >> >> Also, replace the simple_strtoul with kstrtol. > >> >> > >> >> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com> > >> >> --- > >> >> arch/powerpc/kernel/prom.c | 27 +++++++++++++++++++++++++++ > >> >> arch/powerpc/kernel/setup_32.c | 24 ------------------------ > >> >> 2 files changed, 27 insertions(+), 24 deletions(-) > >> > > >> >Is not setup-common.c is better place to move this? > >> > >> Move out from setup_32.c does not mean it have to go into > >> setup-common.c, I need better reason to do this. > >> > > > >What I think that setup_32.c is for 32 bit, setup_64.c is for 64 bit > >and setup-common.c is for both. > > > >I am not saying that you move this to setup-common.c. I am asking why > >you have not used setup-common.c ? I am ok even with prom.c. > > > [Xie Shaohui] I'm not a fan of prom.c, I did this because I see same kind of > early parameters checking is did in this file only, so I thought maybe I should > put them together. And seems setup-common.c is not the place to do command line > checking (I'm not sure about this). > Ok, so you are also not sure. Let us see what other guys things of this. Thanks -Bharat
Hi, All, Is there any concern for this patch, it's been a long time. Thanks! Best Regards, Shaohui Xie >-----Original Message----- >From: Xie Shaohui-B21989 >Sent: Tuesday, May 08, 2012 2:07 PM >To: linux-watchdog@vger.kernel.org; linuxppc-dev@lists.ozlabs.org >Cc: Xie Shaohui-B21989 >Subject: [PATCH 1/2] powerpc/watchdog: move booke watchdog param related >code to prom.c > >Currently, BOOKE watchdog code for checking "wdt" and "wdt_period" is in >setup_32.c, it cannot be used in 64-bit, so move it to a common place >prom.c, which will be shared by 32-bit and 64-bit. > >Also, replace the simple_strtoul with kstrtol. > >Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com> >--- > arch/powerpc/kernel/prom.c | 27 +++++++++++++++++++++++++++ > arch/powerpc/kernel/setup_32.c | 24 ------------------------ > 2 files changed, 27 insertions(+), 24 deletions(-) > >diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index >f191bf0..49e1bdf 100644 >--- a/arch/powerpc/kernel/prom.c >+++ b/arch/powerpc/kernel/prom.c >@@ -84,6 +84,33 @@ static int __init early_parse_mem(char *p) } >early_param("mem", early_parse_mem); > >+#ifdef CONFIG_BOOKE_WDT >+extern u32 booke_wdt_enabled; >+extern u32 booke_wdt_period; >+ >+/* Checks wdt=x and wdt_period=xx command-line option */ notrace int >+__init early_parse_wdt(char *p) { >+ if (p && strncmp(p, "0", 1) != 0) >+ booke_wdt_enabled = 1; >+ >+ return 0; >+} >+early_param("wdt", early_parse_wdt); >+ >+int __init early_parse_wdt_period(char *p) { >+ unsigned long ret; >+ if (p) { >+ if (!kstrtol(p, 0, &ret)) >+ booke_wdt_period = ret; >+ } >+ >+ return 0; >+} >+early_param("wdt_period", early_parse_wdt_period); >+#endif /* CONFIG_BOOKE_WDT */ >+ > /* > * overlaps_initrd - check for overlap with page aligned extension of > * initrd. >diff --git a/arch/powerpc/kernel/setup_32.c >b/arch/powerpc/kernel/setup_32.c index ec8a53f..a8f54ec 100644 >--- a/arch/powerpc/kernel/setup_32.c >+++ b/arch/powerpc/kernel/setup_32.c >@@ -149,30 +149,6 @@ notrace void __init machine_init(u64 dt_ptr) > ppc_md.progress("id mach(): done", 0x200); } > >-#ifdef CONFIG_BOOKE_WDT >-extern u32 booke_wdt_enabled; >-extern u32 booke_wdt_period; >- >-/* Checks wdt=x and wdt_period=xx command-line option */ -notrace int >__init early_parse_wdt(char *p) -{ >- if (p && strncmp(p, "0", 1) != 0) >- booke_wdt_enabled = 1; >- >- return 0; >-} >-early_param("wdt", early_parse_wdt); >- >-int __init early_parse_wdt_period (char *p) -{ >- if (p) >- booke_wdt_period = simple_strtoul(p, NULL, 0); >- >- return 0; >-} >-early_param("wdt_period", early_parse_wdt_period); >-#endif /* CONFIG_BOOKE_WDT */ >- > /* Checks "l2cr=xxxx" command-line option */ int __init >ppc_setup_l2cr(char *str) { >-- >1.6.4
On May 8, 2012, at 10:46 PM, Bhushan Bharat-R65777 wrote: >>>>>> .org] On Behalf Of Shaohui Xie >>>>>> Sent: Tuesday, May 08, 2012 11:37 AM >>>>>> To: linux-watchdog@vger.kernel.org; linuxppc-dev@lists.ozlabs.org >>>>>> Cc: Xie Shaohui-B21989 >>>>>> Subject: [PATCH 1/2] powerpc/watchdog: move booke watchdog param >>>>>> related code to prom.c >>>>>> >>>>>> Currently, BOOKE watchdog code for checking "wdt" and "wdt_period" >>>>>> is in setup_32.c, it cannot be used in 64-bit, so move it to a >>>>>> common place prom.c, which will be shared by 32-bit and 64-bit. >>>>>> >>>>>> Also, replace the simple_strtoul with kstrtol. >>>>>> >>>>>> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com> >>>>>> --- >>>>>> arch/powerpc/kernel/prom.c | 27 +++++++++++++++++++++++++++ >>>>>> arch/powerpc/kernel/setup_32.c | 24 ------------------------ >>>>>> 2 files changed, 27 insertions(+), 24 deletions(-) >>>>> >>>>> Is not setup-common.c is better place to move this? >>>> >>>> Move out from setup_32.c does not mean it have to go into >>>> setup-common.c, I need better reason to do this. >>>> >>> >>> What I think that setup_32.c is for 32 bit, setup_64.c is for 64 bit >>> and setup-common.c is for both. >>> >>> I am not saying that you move this to setup-common.c. I am asking why >>> you have not used setup-common.c ? I am ok even with prom.c. >>> >> [Xie Shaohui] I'm not a fan of prom.c, I did this because I see same kind of >> early parameters checking is did in this file only, so I thought maybe I should >> put them together. And seems setup-common.c is not the place to do command line >> checking (I'm not sure about this). >> > > Ok, so you are also not sure. > Let us see what other guys things of this. Put it in setup-common.c. prom.c has normally been mostly OF/dts related parsing. - k
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index f191bf0..49e1bdf 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -84,6 +84,33 @@ static int __init early_parse_mem(char *p) } early_param("mem", early_parse_mem); +#ifdef CONFIG_BOOKE_WDT +extern u32 booke_wdt_enabled; +extern u32 booke_wdt_period; + +/* Checks wdt=x and wdt_period=xx command-line option */ +notrace int __init early_parse_wdt(char *p) +{ + if (p && strncmp(p, "0", 1) != 0) + booke_wdt_enabled = 1; + + return 0; +} +early_param("wdt", early_parse_wdt); + +int __init early_parse_wdt_period(char *p) +{ + unsigned long ret; + if (p) { + if (!kstrtol(p, 0, &ret)) + booke_wdt_period = ret; + } + + return 0; +} +early_param("wdt_period", early_parse_wdt_period); +#endif /* CONFIG_BOOKE_WDT */ + /* * overlaps_initrd - check for overlap with page aligned extension of * initrd. diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c index ec8a53f..a8f54ec 100644 --- a/arch/powerpc/kernel/setup_32.c +++ b/arch/powerpc/kernel/setup_32.c @@ -149,30 +149,6 @@ notrace void __init machine_init(u64 dt_ptr) ppc_md.progress("id mach(): done", 0x200); } -#ifdef CONFIG_BOOKE_WDT -extern u32 booke_wdt_enabled; -extern u32 booke_wdt_period; - -/* Checks wdt=x and wdt_period=xx command-line option */ -notrace int __init early_parse_wdt(char *p) -{ - if (p && strncmp(p, "0", 1) != 0) - booke_wdt_enabled = 1; - - return 0; -} -early_param("wdt", early_parse_wdt); - -int __init early_parse_wdt_period (char *p) -{ - if (p) - booke_wdt_period = simple_strtoul(p, NULL, 0); - - return 0; -} -early_param("wdt_period", early_parse_wdt_period); -#endif /* CONFIG_BOOKE_WDT */ - /* Checks "l2cr=xxxx" command-line option */ int __init ppc_setup_l2cr(char *str) {
Currently, BOOKE watchdog code for checking "wdt" and "wdt_period" is in setup_32.c, it cannot be used in 64-bit, so move it to a common place prom.c, which will be shared by 32-bit and 64-bit. Also, replace the simple_strtoul with kstrtol. Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com> --- arch/powerpc/kernel/prom.c | 27 +++++++++++++++++++++++++++ arch/powerpc/kernel/setup_32.c | 24 ------------------------ 2 files changed, 27 insertions(+), 24 deletions(-)