| Message ID | 1405361664-5210-1-git-send-email-xerofoify@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/14/14 11:14, Nicholas Krause wrote: > This patch addresses the fix me message in this file that states to > remove all definitions not related to reg-clocks in this header > file. > > Signed-off-by: Nicholas Krause <xerofoify@gmail.com> > --- > arch/arm/mach-s3c64xx/include/mach/regs-clock.h | 22 +--------------------- > 1 file changed, 1 insertion(+), 21 deletions(-) > > diff --git a/arch/arm/mach-s3c64xx/include/mach/regs-clock.h b/arch/arm/mach-s3c64xx/include/mach/regs-clock.h > index 4f44aac..46e64cc 100644 > --- a/arch/arm/mach-s3c64xx/include/mach/regs-clock.h > +++ b/arch/arm/mach-s3c64xx/include/mach/regs-clock.h > @@ -15,24 +15,4 @@ > #ifndef __PLAT_REGS_CLOCK_H > #define __PLAT_REGS_CLOCK_H __FILE__ > > -/* > - * FIXME: Remove remaining definitions > - */ > - > -#define S3C_CLKREG(x) (S3C_VA_SYS + (x)) > - > -#define S3C_PCLK_GATE S3C_CLKREG(0x34) > -#define S3C6410_CLK_SRC2 S3C_CLKREG(0x10C) > -#define S3C_MEM_SYS_CFG S3C_CLKREG(0x120) > - > -/* PCLK GATE Registers */ > -#define S3C_CLKCON_PCLK_UART3 (1<<4) > -#define S3C_CLKCON_PCLK_UART2 (1<<3) > -#define S3C_CLKCON_PCLK_UART1 (1<<2) > -#define S3C_CLKCON_PCLK_UART0 (1<<1) > - > -/* MEM_SYS_CFG */ > -#define MEM_SYS_CFG_INDEP_CF 0x4000 > -#define MEM_SYS_CFG_EBI_FIX_PRI_CFCON 0x30 > - > -#endif /* _PLAT_REGS_CLOCK_H */ > +/#endif /* _PLAT_REGS_CLOCK_H */ I don't know about the other changes, but that last line looks like an error. Did you build anything that #includes this header file?
On 07/14/14 11:14, Nicholas Krause wrote: > This patch addresses the fix me message in this file that states to > remove all definitions not related to reg-clocks in this header > file. > > Signed-off-by: Nicholas Krause <xerofoify@gmail.com> > --- > arch/arm/mach-s3c64xx/include/mach/regs-clock.h | 22 +--------------------- > 1 file changed, 1 insertion(+), 21 deletions(-) > > diff --git a/arch/arm/mach-s3c64xx/include/mach/regs-clock.h b/arch/arm/mach-s3c64xx/include/mach/regs-clock.h > index 4f44aac..46e64cc 100644 > --- a/arch/arm/mach-s3c64xx/include/mach/regs-clock.h > +++ b/arch/arm/mach-s3c64xx/include/mach/regs-clock.h > @@ -15,24 +15,4 @@ > #ifndef __PLAT_REGS_CLOCK_H > #define __PLAT_REGS_CLOCK_H __FILE__ > > -/* > - * FIXME: Remove remaining definitions > - */ > - > -#define S3C_CLKREG(x) (S3C_VA_SYS + (x)) > - > -#define S3C_PCLK_GATE S3C_CLKREG(0x34) There is another header file that uses the #define S3C_PCLK_GATE above. Deleting it here should be causing a problem here: ./arch/arm/mach-s3c64xx/include/mach/pm-core.h:24: u32 tmp = __raw_readl(S3 C_PCLK_GATE); ./arch/arm/mach-s3c64xx/include/mach/pm-core.h:37: __raw_writel(tmp, S3C_PC LK_GATE); Please check things like this in advance. > -#define S3C6410_CLK_SRC2 S3C_CLKREG(0x10C) > -#define S3C_MEM_SYS_CFG S3C_CLKREG(0x120) > - > -/* PCLK GATE Registers */ > -#define S3C_CLKCON_PCLK_UART3 (1<<4) used by pm-core.h > -#define S3C_CLKCON_PCLK_UART2 (1<<3) used by pm-core.h > -#define S3C_CLKCON_PCLK_UART1 (1<<2) used by pm-core.h > -#define S3C_CLKCON_PCLK_UART0 (1<<1) used by pm-core.h > - > -/* MEM_SYS_CFG */ > -#define MEM_SYS_CFG_INDEP_CF 0x4000 > -#define MEM_SYS_CFG_EBI_FIX_PRI_CFCON 0x30 > - > -#endif /* _PLAT_REGS_CLOCK_H */ > +/#endif /* _PLAT_REGS_CLOCK_H */ >
On Mon, 2014-07-14 at 11:23 -0700, Randy Dunlap wrote: > On 07/14/14 11:14, Nicholas Krause wrote: > > This patch addresses the fix me message in this file that states to > > remove all definitions not related to reg-clocks in this header > > file. > > > > Signed-off-by: Nicholas Krause <xerofoify@gmail.com> > > --- > > arch/arm/mach-s3c64xx/include/mach/regs-clock.h | 22 +--------------------- > > 1 file changed, 1 insertion(+), 21 deletions(-) > > > > diff --git a/arch/arm/mach-s3c64xx/include/mach/regs-clock.h b/arch/arm/mach-s3c64xx/include/mach/regs-clock.h > > index 4f44aac..46e64cc 100644 > > --- a/arch/arm/mach-s3c64xx/include/mach/regs-clock.h > > +++ b/arch/arm/mach-s3c64xx/include/mach/regs-clock.h > > @@ -15,24 +15,4 @@ > > #ifndef __PLAT_REGS_CLOCK_H > > #define __PLAT_REGS_CLOCK_H __FILE__ > > > > -/* > > - * FIXME: Remove remaining definitions > > - */ > > - > > -#define S3C_CLKREG(x) (S3C_VA_SYS + (x)) > > - > > -#define S3C_PCLK_GATE S3C_CLKREG(0x34) > > -#define S3C6410_CLK_SRC2 S3C_CLKREG(0x10C) > > -#define S3C_MEM_SYS_CFG S3C_CLKREG(0x120) > > - > > -/* PCLK GATE Registers */ > > -#define S3C_CLKCON_PCLK_UART3 (1<<4) > > -#define S3C_CLKCON_PCLK_UART2 (1<<3) > > -#define S3C_CLKCON_PCLK_UART1 (1<<2) > > -#define S3C_CLKCON_PCLK_UART0 (1<<1) > > - > > -/* MEM_SYS_CFG */ > > -#define MEM_SYS_CFG_INDEP_CF 0x4000 > > -#define MEM_SYS_CFG_EBI_FIX_PRI_CFCON 0x30 > > - > > -#endif /* _PLAT_REGS_CLOCK_H */ > > +/#endif /* _PLAT_REGS_CLOCK_H */ > > I don't know about the other changes, but that last line looks like an error. > Did you build anything that #includes this header file? I just did git grep -n S3C_CLKCON_PCLK_UART It was just a guess. I did not cherry pick the defines that covers! The output of that command is, I think, pretty clear. Nicholas, I think you've been told already to stop doing what you're doing again here. I try not to use colorful language on the net, but you're really pushing me here. Please go find something other to do than grepping the kernel tree for FIXMEs! Paul Bolle
On 07/14/14 11:40, Paul Bolle wrote: > On Mon, 2014-07-14 at 11:23 -0700, Randy Dunlap wrote: >> On 07/14/14 11:14, Nicholas Krause wrote: >>> This patch addresses the fix me message in this file that states to >>> remove all definitions not related to reg-clocks in this header >>> file. >>> >>> Signed-off-by: Nicholas Krause <xerofoify@gmail.com> >>> --- >>> arch/arm/mach-s3c64xx/include/mach/regs-clock.h | 22 +--------------------- >>> 1 file changed, 1 insertion(+), 21 deletions(-) >>> >>> diff --git a/arch/arm/mach-s3c64xx/include/mach/regs-clock.h b/arch/arm/mach-s3c64xx/include/mach/regs-clock.h >>> index 4f44aac..46e64cc 100644 >>> --- a/arch/arm/mach-s3c64xx/include/mach/regs-clock.h >>> +++ b/arch/arm/mach-s3c64xx/include/mach/regs-clock.h >>> @@ -15,24 +15,4 @@ >>> #ifndef __PLAT_REGS_CLOCK_H >>> #define __PLAT_REGS_CLOCK_H __FILE__ >>> >>> -/* >>> - * FIXME: Remove remaining definitions >>> - */ >>> - >>> -#define S3C_CLKREG(x) (S3C_VA_SYS + (x)) >>> - >>> -#define S3C_PCLK_GATE S3C_CLKREG(0x34) >>> -#define S3C6410_CLK_SRC2 S3C_CLKREG(0x10C) >>> -#define S3C_MEM_SYS_CFG S3C_CLKREG(0x120) >>> - >>> -/* PCLK GATE Registers */ >>> -#define S3C_CLKCON_PCLK_UART3 (1<<4) >>> -#define S3C_CLKCON_PCLK_UART2 (1<<3) >>> -#define S3C_CLKCON_PCLK_UART1 (1<<2) >>> -#define S3C_CLKCON_PCLK_UART0 (1<<1) >>> - >>> -/* MEM_SYS_CFG */ >>> -#define MEM_SYS_CFG_INDEP_CF 0x4000 >>> -#define MEM_SYS_CFG_EBI_FIX_PRI_CFCON 0x30 >>> - >>> -#endif /* _PLAT_REGS_CLOCK_H */ >>> +/#endif /* _PLAT_REGS_CLOCK_H */ >> >> I don't know about the other changes, but that last line looks like an error. >> Did you build anything that #includes this header file? > > I just did > git grep -n S3C_CLKCON_PCLK_UART > > It was just a guess. I did not cherry pick the defines that covers! The > output of that command is, I think, pretty clear. > > Nicholas, I think you've been told already to stop doing what you're > doing again here. I try not to use colorful language on the net, but > you're really pushing me here. > > Please go find something other to do than grepping the kernel tree for > FIXMEs! Yes. FIXMEs are not a suggested starting point for kernel beginners. If they were easy to fix, they would already be fixed or most likely would never have been introduced.
On Mon, Jul 14, 2014 at 2:45 PM, Randy Dunlap <rdunlap@infradead.org> wrote: > On 07/14/14 11:40, Paul Bolle wrote: >> On Mon, 2014-07-14 at 11:23 -0700, Randy Dunlap wrote: >>> On 07/14/14 11:14, Nicholas Krause wrote: >>>> This patch addresses the fix me message in this file that states to >>>> remove all definitions not related to reg-clocks in this header >>>> file. >>>> >>>> Signed-off-by: Nicholas Krause <xerofoify@gmail.com> >>>> --- >>>> arch/arm/mach-s3c64xx/include/mach/regs-clock.h | 22 +--------------------- >>>> 1 file changed, 1 insertion(+), 21 deletions(-) >>>> >>>> diff --git a/arch/arm/mach-s3c64xx/include/mach/regs-cloclinux-kernel@vger.kernel.orgk.h b/arch/arm/mach-s3c64xx/include/mach/regs-clock.h >>>> index 4f44aac..46e64cc 100644 >>>> --- a/arch/arm/mach-s3c64xx/include/mach/regs-clock.h >>>> +++ b/arch/arm/mach-s3c64xx/include/mach/regs-clock.h >>>> @@ -15,24 +15,4 @@ >>>> #ifndef __PLAT_REGS_CLOCK_H >>>> #define __PLAT_REGS_CLOCK_H __FILE__ >>>> >>>> -/* >>>> - * FIXME: Remove remaining definitions >>>> - */ >>>> - >>>> -#define S3C_CLKREG(x) (S3C_VA_SYS + (x)) >>>> - >>>> -#define S3C_PCLK_GATE S3C_CLKREG(0x34) >>>> -#define S3C6410_CLK_SRC2 S3C_CLKREG(0x10C) >>>> -#define S3C_MEM_SYS_CFG S3C_CLKREG(0x120) >>>> - >>>> -/* PCLK GATE Registers */ >>>> -#define S3C_CLKCON_PCLK_UART3 (1<<4) >>>> -#define S3C_CLKCON_PCLK_UART2 (1<<3) >>>> -#define S3C_CLKCON_PCLK_UART1 (1<<2) >>>> -#define S3C_CLKCON_PCLK_UART0 (1<<1) >>>> - >>>> -/* MEM_SYS_CFG */ >>>> -#define MEM_SYS_CFG_INDEP_CF 0x4000 >>>> -#define MEM_SYS_CFG_EBI_FIX_PRI_CFCON 0x30 >>>> - >>>> -#endif /* _PLAT_REGS_CLOCK_H */ >>>> +/#endif /* _PLAT_REGS_CLOCK_H */ >>> >>> I don't know about the other changes, but that last line looks like an error. >>> Did you build anything that #includes this header file? >> >> I just did >> git grep -n S3C_CLKCON_PCLK_UART >> >> It was just a guess. I did not cherry pick the defines that covers! The >> output of that command is, I think, pretty clear. >> >> Nicholas, I think you've been told already to stop doing what you're >> doing again here. I try not to use colorful language on the net, but >> you're really pushing me here. >> >> Please go find something other to do than grepping the kernel tree for >> FIXMEs! >linux-kernel@vger.kernel.org > Yes. > > FIXMEs are not a suggested starting point for kernel beginners. > If they were easy to fix, they would already be fixed or most likely > would never have been introduced. > > > -- > ~Randy Randy, My mistake I didn't check the tree for files linking to these definitions. I will be more careful ,next time. Niclk
On Mon, 14 Jul 2014 14:14:24 -0400, Nicholas Krause said: > This patch addresses the fix me message in this file that states to > remove all definitions not related to reg-clocks in this header > file. > > Signed-off-by: Nicholas Krause <xerofoify@gmail.com> > --- > arch/arm/mach-s3c64xx/include/mach/regs-clock.h | 22 +--------------------- > 1 file changed, 1 insertion(+), 21 deletions(-) > -#endif /* _PLAT_REGS_CLOCK_H */ > +/#endif /* _PLAT_REGS_CLOCK_H */ You're kidding us, right? Given that this was obviously *not* compile tested at all, why should we believe that in fact all those #defines are in fact no longer used? Hint: There's a very high chance that somebody stuck that FIXME in there rather than just cleaning it up themselves precisely *because* it was still in use in an inconvenient to fix place in the tree..
On Mon, Jul 14, 2014 at 5:43 PM, <Valdis.Kletnieks@vt.edu> wrote: > On Mon, 14 Jul 2014 14:14:24 -0400, Nicholas Krause said: >> This patch addresses the fix me message in this file that states to >> remove all definitions not related to reg-clocks in this header >> file. >> >> Signed-off-by: Nicholas Krause <xerofoify@gmail.com> >> --- >> arch/arm/mach-s3c64xx/include/mach/regs-clock.h | 22 +--------------------- >> 1 file changed, 1 insertion(+), 21 deletions(-) > >> -#endif /* _PLAT_REGS_CLOCK_H */ >> +/#endif /* _PLAT_REGS_CLOCK_H */ > > You're kidding us, right? Given that this was obviously *not* compile > tested at all, why should we believe that in fact all those #defines > are in fact no longer used? > > Hint: There's a very high chance that somebody stuck that FIXME in there rather > than just cleaning it up themselves precisely *because* it was still in use in > an inconvenient to fix place in the tree.. > I build the arch/arm directory I seem to me hitting a error. arch/arm/xen/enlighten.c:17:29: fatal error: asm/system_misc.h: No such file or directory #include <asm/system_misc.h> I am also looking into fixing this issue. Cheers Nick
diff --git a/arch/arm/mach-s3c64xx/include/mach/regs-clock.h b/arch/arm/mach-s3c64xx/include/mach/regs-clock.h index 4f44aac..46e64cc 100644 --- a/arch/arm/mach-s3c64xx/include/mach/regs-clock.h +++ b/arch/arm/mach-s3c64xx/include/mach/regs-clock.h @@ -15,24 +15,4 @@ #ifndef __PLAT_REGS_CLOCK_H #define __PLAT_REGS_CLOCK_H __FILE__ -/* - * FIXME: Remove remaining definitions - */ - -#define S3C_CLKREG(x) (S3C_VA_SYS + (x)) - -#define S3C_PCLK_GATE S3C_CLKREG(0x34) -#define S3C6410_CLK_SRC2 S3C_CLKREG(0x10C) -#define S3C_MEM_SYS_CFG S3C_CLKREG(0x120) - -/* PCLK GATE Registers */ -#define S3C_CLKCON_PCLK_UART3 (1<<4) -#define S3C_CLKCON_PCLK_UART2 (1<<3) -#define S3C_CLKCON_PCLK_UART1 (1<<2) -#define S3C_CLKCON_PCLK_UART0 (1<<1) - -/* MEM_SYS_CFG */ -#define MEM_SYS_CFG_INDEP_CF 0x4000 -#define MEM_SYS_CFG_EBI_FIX_PRI_CFCON 0x30 - -#endif /* _PLAT_REGS_CLOCK_H */ +/#endif /* _PLAT_REGS_CLOCK_H */
This patch addresses the fix me message in this file that states to remove all definitions not related to reg-clocks in this header file. Signed-off-by: Nicholas Krause <xerofoify@gmail.com> --- arch/arm/mach-s3c64xx/include/mach/regs-clock.h | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-)