diff mbox

mach-s3c64xx: Remove all defintions not related to regs-clocks

Message ID 1405361664-5210-1-git-send-email-xerofoify@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nick July 14, 2014, 6:14 p.m. UTC
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(-)

Comments

Randy Dunlap July 14, 2014, 6:23 p.m. UTC | #1
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?
Randy Dunlap July 14, 2014, 6:34 p.m. UTC | #2
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 */
>
Paul Bolle July 14, 2014, 6:40 p.m. UTC | #3
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

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Randy Dunlap July 14, 2014, 6:45 p.m. UTC | #4
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.
Nick July 14, 2014, 6:56 p.m. UTC | #5
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
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Valdis Kl ē tnieks July 14, 2014, 9:43 p.m. UTC | #6
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..
Nick July 17, 2014, 1:13 a.m. UTC | #7
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
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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 */