| Message ID | 20170512104707.5393eaa1@roar.ozlabs.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 09b6c1129f899c72d70b8bea36020644aa3b5a28 |
| Headers | show |
Le 12/05/2017 à 02:47, Nicholas Piggin a écrit : > On Thu, 11 May 2017 20:52:56 +0200 > christophe leroy <christophe.leroy@c-s.fr> wrote: > >> Le 11/05/2017 à 19:14, christophe leroy a écrit : >>> >>> >>> Le 11/05/2017 à 17:15, Nicholas Piggin a écrit : >>>> Cc: Scott Wood <oss@buserror.net> >>>> Cc: Christophe Leroy <christophe.leroy@c-s.fr> >>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >>>> --- >>>> arch/powerpc/xmon/xmon.c | 5 ++++- >>>> 1 file changed, 4 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c >>>> index f11f65634aab..ec420b0e6e88 100644 >>>> --- a/arch/powerpc/xmon/xmon.c >>>> +++ b/arch/powerpc/xmon/xmon.c >>>> @@ -1242,10 +1242,13 @@ bpt_cmds(void) >>>> { >>>> int cmd; >>>> unsigned long a; >>>> - int mode, i; >>>> + int i; >>>> struct bpt *bp; >>>> +#ifndef CONFIG_8xx >>> >>> Would be better to use CONFIG_PPC_8xx >>> >>> As stated in arch/powerpc/platform/Kconfig.cputype, CONFIG_8xx is temp >>> to handle compat with arch/ppc, and we are trying to get rid of it >> >> I had the same comment in https://patchwork.ozlabs.org/patch/700354/ >> I also suggested to move the relevant declarations inside the switch() > > How's this one? Looks good. Christophe > > --- > arch/powerpc/xmon/xmon.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c > index f11f65634aab..438fdb0fb142 100644 > --- a/arch/powerpc/xmon/xmon.c > +++ b/arch/powerpc/xmon/xmon.c > @@ -1242,14 +1242,16 @@ bpt_cmds(void) > { > int cmd; > unsigned long a; > - int mode, i; > + int i; > struct bpt *bp; > - const char badaddr[] = "Only kernel addresses are permitted " > - "for breakpoints\n"; > > cmd = inchar(); > switch (cmd) { > -#ifndef CONFIG_8xx > +#ifndef CONFIG_PPC_8xx > + int mode; > + const char badaddr[] = "Only kernel addresses are permitted " > + "for breakpoints\n"; > + > case 'd': /* bd - hardware data breakpoint */ > mode = 7; > cmd = inchar(); >
Nicholas Piggin <npiggin@gmail.com> writes: > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c > index f11f65634aab..438fdb0fb142 100644 > --- a/arch/powerpc/xmon/xmon.c > +++ b/arch/powerpc/xmon/xmon.c > @@ -1242,14 +1242,16 @@ bpt_cmds(void) > { > int cmd; > unsigned long a; > - int mode, i; > + int i; > struct bpt *bp; > - const char badaddr[] = "Only kernel addresses are permitted " > - "for breakpoints\n"; > > cmd = inchar(); > switch (cmd) { > -#ifndef CONFIG_8xx > +#ifndef CONFIG_PPC_8xx > + int mode; > + const char badaddr[] = "Only kernel addresses are permitted " > + "for breakpoints\n"; > + > case 'd': /* bd - hardware data breakpoint */ > mode = 7; > cmd = inchar(); GCC 7 rejects this: arch/powerpc/xmon/xmon.c: In function ‘bpt_cmds’: arch/powerpc/xmon/xmon.c:1252:13: error: statement will never be executed [-Werror=switch-unreachable] const char badaddr[] = "Only kernel addresses are permitted for breakpoints\n"; ^~~~~~~ I'll go back to the earlier version. cheers
From: Michael Ellerman > Sent: 26 May 2017 08:24 > Nicholas Piggin <npiggin@gmail.com> writes: > > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c > > index f11f65634aab..438fdb0fb142 100644 > > --- a/arch/powerpc/xmon/xmon.c > > +++ b/arch/powerpc/xmon/xmon.c > > @@ -1242,14 +1242,16 @@ bpt_cmds(void) > > { > > int cmd; > > unsigned long a; > > - int mode, i; > > + int i; > > struct bpt *bp; > > - const char badaddr[] = "Only kernel addresses are permitted " > > - "for breakpoints\n"; > > > > cmd = inchar(); > > switch (cmd) { > > -#ifndef CONFIG_8xx > > +#ifndef CONFIG_PPC_8xx > > + int mode; > > + const char badaddr[] = "Only kernel addresses are permitted " > > + "for breakpoints\n"; > > + > > case 'd': /* bd - hardware data breakpoint */ > > mode = 7; > > cmd = inchar(); > > GCC 7 rejects this: > > arch/powerpc/xmon/xmon.c: In function bpt_cmds: > arch/powerpc/xmon/xmon.c:1252:13: error: statement will never be executed [-Werror=switch- > unreachable] > const char badaddr[] = "Only kernel addresses are permitted for breakpoints\n"; > ^~~~~~~ Try 'static' ? David
David Laight <David.Laight@ACULAB.COM> writes: > From: Michael Ellerman >> Sent: 26 May 2017 08:24 >> Nicholas Piggin <npiggin@gmail.com> writes: >> > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c >> > index f11f65634aab..438fdb0fb142 100644 >> > --- a/arch/powerpc/xmon/xmon.c >> > +++ b/arch/powerpc/xmon/xmon.c >> > @@ -1242,14 +1242,16 @@ bpt_cmds(void) >> > { >> > int cmd; >> > unsigned long a; >> > - int mode, i; >> > + int i; >> > struct bpt *bp; >> > - const char badaddr[] = "Only kernel addresses are permitted " >> > - "for breakpoints\n"; >> > >> > cmd = inchar(); >> > switch (cmd) { >> > -#ifndef CONFIG_8xx >> > +#ifndef CONFIG_PPC_8xx >> > + int mode; >> > + const char badaddr[] = "Only kernel addresses are permitted " >> > + "for breakpoints\n"; >> > + >> > case 'd': /* bd - hardware data breakpoint */ >> > mode = 7; >> > cmd = inchar(); >> >> GCC 7 rejects this: >> >> arch/powerpc/xmon/xmon.c: In function bpt_cmds: >> arch/powerpc/xmon/xmon.c:1252:13: error: statement will never be executed [-Werror=switch- >> unreachable] >> const char badaddr[] = "Only kernel addresses are permitted for breakpoints\n"; >> ^~~~~~~ > > Try 'static' ? Yep that works, will rebase this again ... O_o cheers
On Fri, 2017-05-12 at 00:47:07 UTC, Nicholas Piggin wrote: > On Thu, 11 May 2017 20:52:56 +0200 > christophe leroy <christophe.leroy@c-s.fr> wrote: > > > Le 11/05/2017 à 19:14, christophe leroy a écrit : > > > > > > > > > Le 11/05/2017 à 17:15, Nicholas Piggin a écrit : > > >> Cc: Scott Wood <oss@buserror.net> > > >> Cc: Christophe Leroy <christophe.leroy@c-s.fr> > > >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > >> --- > > >> arch/powerpc/xmon/xmon.c | 5 ++++- > > >> 1 file changed, 4 insertions(+), 1 deletion(-) > > >> > > >> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c > > >> index f11f65634aab..ec420b0e6e88 100644 > > >> --- a/arch/powerpc/xmon/xmon.c > > >> +++ b/arch/powerpc/xmon/xmon.c > > >> @@ -1242,10 +1242,13 @@ bpt_cmds(void) > > >> { > > >> int cmd; > > >> unsigned long a; > > >> - int mode, i; > > >> + int i; > > >> struct bpt *bp; > > >> +#ifndef CONFIG_8xx > > > > > > Would be better to use CONFIG_PPC_8xx > > > > > > As stated in arch/powerpc/platform/Kconfig.cputype, CONFIG_8xx is temp > > > to handle compat with arch/ppc, and we are trying to get rid of it > > > > I had the same comment in https://patchwork.ozlabs.org/patch/700354/ > > I also suggested to move the relevant declarations inside the switch() Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/09b6c1129f899c72d70b8bea360206 cheers
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c index f11f65634aab..438fdb0fb142 100644 --- a/arch/powerpc/xmon/xmon.c +++ b/arch/powerpc/xmon/xmon.c @@ -1242,14 +1242,16 @@ bpt_cmds(void) { int cmd; unsigned long a; - int mode, i; + int i; struct bpt *bp; - const char badaddr[] = "Only kernel addresses are permitted " - "for breakpoints\n"; cmd = inchar(); switch (cmd) { -#ifndef CONFIG_8xx +#ifndef CONFIG_PPC_8xx + int mode; + const char badaddr[] = "Only kernel addresses are permitted " + "for breakpoints\n"; + case 'd': /* bd - hardware data breakpoint */ mode = 7; cmd = inchar();