2012-02-19 15:12:57

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 1/5] blackfin: s/#if CONFIG/#ifdef CONFIG/

Signed-off-by: Geert Uytterhoeven <[email protected]>
Cc: Mike Frysinger <[email protected]>
Cc: [email protected]
---
arch/blackfin/mach-common/entry.S | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/blackfin/mach-common/entry.S b/arch/blackfin/mach-common/entry.S
index e413729..48bb434 100644
--- a/arch/blackfin/mach-common/entry.S
+++ b/arch/blackfin/mach-common/entry.S
@@ -1244,7 +1244,7 @@ ENTRY(_software_trace_buff)
.endr
#endif /* CONFIG_DEBUG_BFIN_HWTRACE_EXPAND */

-#if CONFIG_EARLY_PRINTK
+#ifdef CONFIG_EARLY_PRINTK
__INIT
ENTRY(_early_trap)
SAVE_ALL_SYS
--
1.7.0.4


2012-02-19 15:12:11

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 2/5] microblaze: s/#if[!] CONFIG/#if[n]def CONFIG/

Signed-off-by: Geert Uytterhoeven <[email protected]>
Cc: Michal Simek <[email protected]>
Cc: [email protected]
---
arch/microblaze/include/asm/exceptions.h | 2 +-
arch/microblaze/include/asm/irqflags.h | 2 +-
arch/microblaze/kernel/entry-nommu.S | 2 +-
arch/microblaze/kernel/entry.S | 4 ++--
arch/microblaze/kernel/setup.c | 4 ++--
5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/microblaze/include/asm/exceptions.h b/arch/microblaze/include/asm/exceptions.h
index e6a8dde..48c197b 100644
--- a/arch/microblaze/include/asm/exceptions.h
+++ b/arch/microblaze/include/asm/exceptions.h
@@ -25,7 +25,7 @@
/* Define MSR enable bit for HW exceptions */
#define HWEX_MSR_BIT (1 << 8)

-#if CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
+#ifdef CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
#define __enable_hw_exceptions() \
__asm__ __volatile__ (" msrset r0, %0; \
nop;" \
diff --git a/arch/microblaze/include/asm/irqflags.h b/arch/microblaze/include/asm/irqflags.h
index c9a6262..3cfe473 100644
--- a/arch/microblaze/include/asm/irqflags.h
+++ b/arch/microblaze/include/asm/irqflags.h
@@ -12,7 +12,7 @@
#include <linux/types.h>
#include <asm/registers.h>

-#if CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
+#ifdef CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR

static inline notrace unsigned long arch_local_irq_save(void)
{
diff --git a/arch/microblaze/kernel/entry-nommu.S b/arch/microblaze/kernel/entry-nommu.S
index 34b526f..84f1f87 100644
--- a/arch/microblaze/kernel/entry-nommu.S
+++ b/arch/microblaze/kernel/entry-nommu.S
@@ -18,7 +18,7 @@
#include <asm/percpu.h>
#include <asm/signal.h>

-#if CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
+#ifdef CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
.macro disable_irq
msrclr r0, MSR_IE
.endm
diff --git a/arch/microblaze/kernel/entry.S b/arch/microblaze/kernel/entry.S
index 66e34a3..add11b5 100644
--- a/arch/microblaze/kernel/entry.S
+++ b/arch/microblaze/kernel/entry.S
@@ -49,7 +49,7 @@ syscall_debug_table:
* This is mucky, but necessary using microblaze version that
* allows msr ops to write to BIP
*/
-#if CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
+#ifdef CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
.macro clear_bip
msrclr r0, MSR_BIP
.endm
@@ -993,7 +993,7 @@ ENTRY(_reset)
/* These are compiled and loaded into high memory, then
* copied into place in mach_early_setup */
.section .init.ivt, "ax"
-#if CONFIG_MANUAL_RESET_VECTOR
+#ifdef CONFIG_MANUAL_RESET_VECTOR
.org 0x0
brai CONFIG_MANUAL_RESET_VECTOR
#endif
diff --git a/arch/microblaze/kernel/setup.c b/arch/microblaze/kernel/setup.c
index 604cd9d..e046199 100644
--- a/arch/microblaze/kernel/setup.c
+++ b/arch/microblaze/kernel/setup.c
@@ -164,7 +164,7 @@ void __init machine_early_init(const char *cmdline, unsigned int ram,
printk("New klimit: 0x%08x\n", (unsigned)klimit);
#endif

-#if CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
+#ifdef CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
if (msr)
printk("!!!Your kernel has setup MSR instruction but "
"CPU don't have it %x\n", msr);
@@ -177,7 +177,7 @@ void __init machine_early_init(const char *cmdline, unsigned int ram,
/* Do not copy reset vectors. offset = 0x2 means skip the first
* two instructions. dst is pointer to MB vectors which are placed
* in block ram. If you want to copy reset vector setup offset to 0x0 */
-#if !CONFIG_MANUAL_RESET_VECTOR
+#ifndef CONFIG_MANUAL_RESET_VECTOR
offset = 0x2;
#endif
dst = (unsigned long *) (offset * sizeof(u32));
--
1.7.0.4

2012-02-19 15:12:09

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 3/5] x86: s/#if CONFIG/#ifdef CONFIG/

Signed-off-by: Geert Uytterhoeven <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>"
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: [email protected]
---
arch/x86/boot/compressed/head_32.S | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index a055993..a37b117 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -173,7 +173,7 @@ relocated:
call decompress_kernel
addl $20, %esp

-#if CONFIG_RELOCATABLE
+#ifdef CONFIG_RELOCATABLE
/*
* Find the address of the relocations.
*/
--
1.7.0.4

2012-02-19 15:12:08

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 4/5] input: s/#if CONFIG/#ifdef CONFIG/

On m68k:

drivers/input/misc/twl4030-vibra.c:175:5: warning: "CONFIG_PM" is not defined

Signed-off-by: Geert Uytterhoeven <[email protected]>
Cc: Dmitry Torokhov <[email protected]>
Cc: [email protected]
---
drivers/input/misc/twl4030-vibra.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/input/misc/twl4030-vibra.c b/drivers/input/misc/twl4030-vibra.c
index 3765137..2739f32 100644
--- a/drivers/input/misc/twl4030-vibra.c
+++ b/drivers/input/misc/twl4030-vibra.c
@@ -172,7 +172,7 @@ static void twl4030_vibra_close(struct input_dev *input)
}

/*** Module ***/
-#if CONFIG_PM
+#ifdef CONFIG_PM
static int twl4030_vibra_suspend(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
--
1.7.0.4

2012-02-19 15:12:06

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 5/5] fbdev: s/#if CONFIG/#ifdef CONFIG/

Signed-off-by: Geert Uytterhoeven <[email protected]>
Cc: Florian Tobias Schandinat <[email protected]>
Cc: [email protected]
---
drivers/video/au1100fb.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/au1100fb.c b/drivers/video/au1100fb.c
index de9da67..4da1895 100644
--- a/drivers/video/au1100fb.c
+++ b/drivers/video/au1100fb.c
@@ -532,7 +532,7 @@ static int __devinit au1100fb_drv_probe(struct platform_device *dev)
for (page = (unsigned long)fbdev->fb_mem;
page < PAGE_ALIGN((unsigned long)fbdev->fb_mem + fbdev->fb_len);
page += PAGE_SIZE) {
-#if CONFIG_DMA_NONCOHERENT
+#ifdef CONFIG_DMA_NONCOHERENT
SetPageReserved(virt_to_page(CAC_ADDR((void *)page)));
#else
SetPageReserved(virt_to_page(page));
--
1.7.0.4

Subject: Re: [PATCH 5/5] fbdev: s/#if CONFIG/#ifdef CONFIG/

On 02/19/2012 03:11 PM, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> Cc: Florian Tobias Schandinat <[email protected]>

Acked-by: Florian Tobias Schandinat <[email protected]>
(I assume you want that to go mainline via Jiri's tree)


Best regards,

Florian Tobias Schandinat

> Cc: [email protected]
> ---
> drivers/video/au1100fb.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/au1100fb.c b/drivers/video/au1100fb.c
> index de9da67..4da1895 100644
> --- a/drivers/video/au1100fb.c
> +++ b/drivers/video/au1100fb.c
> @@ -532,7 +532,7 @@ static int __devinit au1100fb_drv_probe(struct platform_device *dev)
> for (page = (unsigned long)fbdev->fb_mem;
> page < PAGE_ALIGN((unsigned long)fbdev->fb_mem + fbdev->fb_len);
> page += PAGE_SIZE) {
> -#if CONFIG_DMA_NONCOHERENT
> +#ifdef CONFIG_DMA_NONCOHERENT
> SetPageReserved(virt_to_page(CAC_ADDR((void *)page)));
> #else
> SetPageReserved(virt_to_page(page));

2012-02-20 00:27:37

by John Williams

[permalink] [raw]
Subject: Re: [microblaze-linux] [PATCH 2/5] microblaze: s/#if[!] CONFIG/#if[n]def CONFIG/

Hi Geert,

NACK - please see comments below

On Mon, Feb 20, 2012 at 1:11 AM, Geert Uytterhoeven
<[email protected]> wrote:
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> Cc: Michal Simek <[email protected]>
> Cc: [email protected]
> ---
> ?arch/microblaze/include/asm/exceptions.h | ? ?2 +-
> ?arch/microblaze/include/asm/irqflags.h ? | ? ?2 +-
> ?arch/microblaze/kernel/entry-nommu.S ? ? | ? ?2 +-
> ?arch/microblaze/kernel/entry.S ? ? ? ? ? | ? ?4 ++--
> ?arch/microblaze/kernel/setup.c ? ? ? ? ? | ? ?4 ++--
> ?5 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch/microblaze/include/asm/exceptions.h b/arch/microblaze/include/asm/exceptions.h
> index e6a8dde..48c197b 100644
> --- a/arch/microblaze/include/asm/exceptions.h
> +++ b/arch/microblaze/include/asm/exceptions.h
> @@ -25,7 +25,7 @@
> ?/* Define MSR enable bit for HW exceptions */
> ?#define HWEX_MSR_BIT (1 << 8)
>
> -#if CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
> +#ifdef CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR

We actually care about the value of this one - it is always defined,
but is either 0 or 1 depending upon the presence or absence of this
feature in the target CPU.

These CPU feature-related configs are defined in
arch/microblaze/platform/generic/Kconfig.auto

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/microblaze/platform/generic/Kconfig.auto;h=25a6f019e94d5d9a1c884dfef9e906435685f980;hb=HEAD

not as bools, but as integers. Kconfig.auto is actually a placeholder
for a design file which is emitted from the MicroBlaze CPU / SoC
design tool flow and copied into place by the user.

In my view these should remain integers rather than booleans for a
couple of reasons:
* breakage for all current users
* MicroBlaze is an evolving architecture, there are other cases of
seemingly boolean HW parametesr growing to become integers, such as
the USE_FPU option. Once upon a time it was yes/no, now there's 2
different flavours of FPU as well as 'none'. MSR is just as likely to
change in future.

> -#if CONFIG_MANUAL_RESET_VECTOR
> +#ifdef CONFIG_MANUAL_RESET_VECTOR

This guy is also always defined (Kconfig 'hex' type), we just need to
do something different for a non-zero value.

Regards,

John

2012-02-20 08:26:04

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 3/5] x86: s/#if CONFIG/#ifdef CONFIG/


* Geert Uytterhoeven <[email protected]> wrote:

> Signed-off-by: Geert Uytterhoeven <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>"
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: [email protected]
> ---
> arch/x86/boot/compressed/head_32.S | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
> index a055993..a37b117 100644
> --- a/arch/x86/boot/compressed/head_32.S
> +++ b/arch/x86/boot/compressed/head_32.S
> @@ -173,7 +173,7 @@ relocated:
> call decompress_kernel
> addl $20, %esp
>
> -#if CONFIG_RELOCATABLE
> +#ifdef CONFIG_RELOCATABLE
> /*
> * Find the address of the relocations.
> */

I'd really prefer to read an actual "this is safe, because this
value is defined in the Kconfig as ..." type of commit log
instead of nothing, which would ensure us that you didnt just do
a (fundamentally unsafe) sed job over the kernel tree but
actually have *read* the Kconfig entries in question to ensure
the correctness of your patches...

Thanks,

Ingo

2012-02-20 11:59:13

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [microblaze-linux] [PATCH 2/5] microblaze: s/#if[!] CONFIG/#if[n]def CONFIG/

On Mon, Feb 20, 2012 at 01:27, John Williams
<[email protected]> wrote:
> NACK - please see comments below
>
> On Mon, Feb 20, 2012 at 1:11 AM, Geert Uytterhoeven
> <[email protected]> wrote:
>> Signed-off-by: Geert Uytterhoeven <[email protected]>
>> Cc: Michal Simek <[email protected]>
>> Cc: [email protected]
>> ---
>>  arch/microblaze/include/asm/exceptions.h |    2 +-
>>  arch/microblaze/include/asm/irqflags.h   |    2 +-
>>  arch/microblaze/kernel/entry-nommu.S     |    2 +-
>>  arch/microblaze/kernel/entry.S           |    4 ++--
>>  arch/microblaze/kernel/setup.c           |    4 ++--
>>  5 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/microblaze/include/asm/exceptions.h b/arch/microblaze/include/asm/exceptions.h
>> index e6a8dde..48c197b 100644
>> --- a/arch/microblaze/include/asm/exceptions.h
>> +++ b/arch/microblaze/include/asm/exceptions.h
>> @@ -25,7 +25,7 @@
>>  /* Define MSR enable bit for HW exceptions */
>>  #define HWEX_MSR_BIT (1 << 8)
>>
>> -#if CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
>> +#ifdef CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
>
> We actually care about the value of this one - it is always defined,
> but is either 0 or 1 depending upon the presence or absence of this
> feature in the target CPU.
>
> These CPU feature-related configs are defined in
> arch/microblaze/platform/generic/Kconfig.auto
>
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/microblaze/platform/generic/Kconfig.auto;h=25a6f019e94d5d9a1c884dfef9e906435685f980;hb=HEAD
>
> not as bools, but as integers.  Kconfig.auto is actually a placeholder
> for a design file which is emitted from the MicroBlaze CPU / SoC
> design tool flow and copied into place by the user.
>
> In my view these should remain integers rather than booleans for a
> couple of reasons:
>  * breakage for all current users
>  * MicroBlaze is an evolving architecture, there are other cases of
> seemingly boolean HW parametesr growing to become integers, such as
> the USE_FPU option.  Once upon a time it was yes/no, now there's 2
> different flavours of FPU as well as 'none'.  MSR is just as likely to
> change in future.
>
>> -#if CONFIG_MANUAL_RESET_VECTOR
>> +#ifdef CONFIG_MANUAL_RESET_VECTOR
>
> This guy is also always defined (Kconfig 'hex' type), we just need to
> do something different for a non-zero value.

Sorry, I missed these are not bool values.

To avoid this from happening again, perhaps it's a good idea to
explicitly write e.g.

#if CONFIG_MANUAL_RESET_VECTOR != 0

like is done for other int config values?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

2012-02-20 23:54:13

by John Williams

[permalink] [raw]
Subject: Re: [microblaze-linux] [PATCH 2/5] microblaze: s/#if[!] CONFIG/#if[n]def CONFIG/

Hi Geert,

On Mon, Feb 20, 2012 at 9:59 PM, Geert Uytterhoeven
<[email protected]> wrote:
> On Mon, Feb 20, 2012 at 01:27, John Williams
> <[email protected]> wrote:
>> NACK - please see comments below
>>
>> On Mon, Feb 20, 2012 at 1:11 AM, Geert Uytterhoeven
>> <[email protected]> wrote:
>>> Signed-off-by: Geert Uytterhoeven <[email protected]>
>>> Cc: Michal Simek <[email protected]>
>>> Cc: [email protected]
>>> ---
>>> ?arch/microblaze/include/asm/exceptions.h | ? ?2 +-
>>> ?arch/microblaze/include/asm/irqflags.h ? | ? ?2 +-
>>> ?arch/microblaze/kernel/entry-nommu.S ? ? | ? ?2 +-
>>> ?arch/microblaze/kernel/entry.S ? ? ? ? ? | ? ?4 ++--
>>> ?arch/microblaze/kernel/setup.c ? ? ? ? ? | ? ?4 ++--
>>> ?5 files changed, 7 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/arch/microblaze/include/asm/exceptions.h b/arch/microblaze/include/asm/exceptions.h
>>> index e6a8dde..48c197b 100644
>>> --- a/arch/microblaze/include/asm/exceptions.h
>>> +++ b/arch/microblaze/include/asm/exceptions.h
>>> @@ -25,7 +25,7 @@
>>> ?/* Define MSR enable bit for HW exceptions */
>>> ?#define HWEX_MSR_BIT (1 << 8)
>>>
>>> -#if CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
>>> +#ifdef CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
>>
>> We actually care about the value of this one - it is always defined,
>> but is either 0 or 1 depending upon the presence or absence of this
>> feature in the target CPU.
>>
>> These CPU feature-related configs are defined in
>> arch/microblaze/platform/generic/Kconfig.auto
>>
>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/microblaze/platform/generic/Kconfig.auto;h=25a6f019e94d5d9a1c884dfef9e906435685f980;hb=HEAD
>>
>> not as bools, but as integers. ?Kconfig.auto is actually a placeholder
>> for a design file which is emitted from the MicroBlaze CPU / SoC
>> design tool flow and copied into place by the user.
>>
>> In my view these should remain integers rather than booleans for a
>> couple of reasons:
>> ?* breakage for all current users
>> ?* MicroBlaze is an evolving architecture, there are other cases of
>> seemingly boolean HW parametesr growing to become integers, such as
>> the USE_FPU option. ?Once upon a time it was yes/no, now there's 2
>> different flavours of FPU as well as 'none'. ?MSR is just as likely to
>> change in future.
>>
>>> -#if CONFIG_MANUAL_RESET_VECTOR
>>> +#ifdef CONFIG_MANUAL_RESET_VECTOR
>>
>> This guy is also always defined (Kconfig 'hex' type), we just need to
>> do something different for a non-zero value.
>
> Sorry, I missed these are not bool values.

No problem.

> To avoid this from happening again, perhaps it's a good idea to
> explicitly write e.g.
>
> #if CONFIG_MANUAL_RESET_VECTOR != 0
>
> like is done for other int config values?

Yes I think that's probably worth doing.

Thanks,

John