All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/2] Consolidate asmlinkage defines
@ 2014-12-03  8:36 Masahiro Yamada
  2014-12-03  8:36 ` [U-Boot] [PATCH 1/2] x86: move arch-specific asmlinkage to <asm/linkage.h> Masahiro Yamada
  2014-12-03  8:36 ` [U-Boot] [PATCH 2/2] ARM: remove redundant asmlinkage define Masahiro Yamada
  0 siblings, 2 replies; 8+ messages in thread
From: Masahiro Yamada @ 2014-12-03  8:36 UTC (permalink / raw)
  To: u-boot


include/common.h is horribly dirty.

It looks like people tend to put things to include/common.h
without much thought.

When you add something to include/common.h,
I think you should consider if you are doing the right thing.



Masahiro Yamada (2):
  x86: move arch-specific asmlinkage to <asm/linkage.h>
  ARM: remove redundant asmlinkage define

 arch/x86/include/asm/arch-ivybridge/pei_data.h | 2 ++
 arch/x86/include/asm/config.h                  | 1 -
 arch/x86/include/asm/linkage.h                 | 6 ++++++
 arch/x86/lib/bios.c                            | 1 +
 arch/x86/lib/bios.h                            | 2 ++
 include/common.h                               | 6 ------
 include/linux/linkage.h                        | 2 ++
 7 files changed, 13 insertions(+), 7 deletions(-)
 create mode 100644 arch/x86/include/asm/linkage.h

-- 
1.9.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH 1/2] x86: move arch-specific asmlinkage to <asm/linkage.h>
  2014-12-03  8:36 [U-Boot] [PATCH 0/2] Consolidate asmlinkage defines Masahiro Yamada
@ 2014-12-03  8:36 ` Masahiro Yamada
  2014-12-07 21:44   ` Simon Glass
  2014-12-03  8:36 ` [U-Boot] [PATCH 2/2] ARM: remove redundant asmlinkage define Masahiro Yamada
  1 sibling, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2014-12-03  8:36 UTC (permalink / raw)
  To: u-boot

Commit 65dd74a674d6 (x86: ivybridge: Implement SDRAM init) introduced
x86-specific asmlinkage into arch/x86/include/asm/config.h.

Commit ed0a2fbf14f7 (x86: Add a definition of asmlinkage) added the
same macro define again, this time, into include/common.h.
(Please do not add arch-specific stuff to include/common.h any more;
it is already too cluttered.)

The generic asmlinkage is defined in <linux/linkage.h>.  If you want
to override it with an arch-specific one, the best way is to add it
to <asm/linkage.h> like Linux Kernel.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
Cc: Simon Glass <sjg@chromium.org>
---

 arch/x86/include/asm/arch-ivybridge/pei_data.h | 2 ++
 arch/x86/include/asm/config.h                  | 1 -
 arch/x86/include/asm/linkage.h                 | 6 ++++++
 arch/x86/lib/bios.c                            | 1 +
 arch/x86/lib/bios.h                            | 2 ++
 include/common.h                               | 3 ---
 include/linux/linkage.h                        | 2 ++
 7 files changed, 13 insertions(+), 4 deletions(-)
 create mode 100644 arch/x86/include/asm/linkage.h

diff --git a/arch/x86/include/asm/arch-ivybridge/pei_data.h b/arch/x86/include/asm/arch-ivybridge/pei_data.h
index 5026c8b..9453336 100644
--- a/arch/x86/include/asm/arch-ivybridge/pei_data.h
+++ b/arch/x86/include/asm/arch-ivybridge/pei_data.h
@@ -7,6 +7,8 @@
 #ifndef ASM_ARCH_PEI_DATA_H
 #define ASM_ARCH_PEI_DATA_H
 
+#include <linux/linkage.h>
+
 struct pch_usb3_controller_settings {
 	/* 0: Disable, 1: Enable, 2: Auto, 3: Smart Auto */
 	uint16_t mode;
diff --git a/arch/x86/include/asm/config.h b/arch/x86/include/asm/config.h
index c97d988..ff15828 100644
--- a/arch/x86/include/asm/config.h
+++ b/arch/x86/include/asm/config.h
@@ -10,6 +10,5 @@
 #define CONFIG_SYS_GENERIC_BOARD
 #define CONFIG_LMB
 #define CONFIG_SYS_BOOT_RAMDISK_HIGH
-#define asmlinkage __attribute__((regparm(0)))
 
 #endif
diff --git a/arch/x86/include/asm/linkage.h b/arch/x86/include/asm/linkage.h
new file mode 100644
index 0000000..bdca72e
--- /dev/null
+++ b/arch/x86/include/asm/linkage.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_X86_LINKAGE_H
+#define _ASM_X86_LINKAGE_H
+
+#define asmlinkage CPP_ASMLINKAGE __attribute__((regparm(0)))
+
+#endif /* _ASM_X86_LINKAGE_H */
diff --git a/arch/x86/lib/bios.c b/arch/x86/lib/bios.c
index 298fca6..d1f8933 100644
--- a/arch/x86/lib/bios.c
+++ b/arch/x86/lib/bios.c
@@ -9,6 +9,7 @@
 #include <common.h>
 #include <bios_emul.h>
 #include <vbe.h>
+#include <linux/linkage.h>
 #include <asm/cache.h>
 #include <asm/processor.h>
 #include <asm/i8259.h>
diff --git a/arch/x86/lib/bios.h b/arch/x86/lib/bios.h
index 8491b4a..668f4b5 100644
--- a/arch/x86/lib/bios.h
+++ b/arch/x86/lib/bios.h
@@ -10,6 +10,8 @@
 #ifndef _X86_LIB_BIOS_H
 #define _X86_LIB_BIOS_H
 
+#include <linux/linkage.h>
+
 #define REALMODE_BASE		0x600
 
 #ifdef __ASSEMBLY__
diff --git a/include/common.h b/include/common.h
index 94c354b..f1ab2cf 100644
--- a/include/common.h
+++ b/include/common.h
@@ -73,9 +73,6 @@ typedef volatile unsigned char	vu_char;
 #ifdef CONFIG_ARM
 #define asmlinkage	/* nothing */
 #endif
-#ifdef CONFIG_X86
-#define asmlinkage __attribute__((regparm(0)))
-#endif
 #ifdef CONFIG_BLACKFIN
 #include <asm/blackfin.h>
 #endif
diff --git a/include/linux/linkage.h b/include/linux/linkage.h
index 7435fcd..5797498 100644
--- a/include/linux/linkage.h
+++ b/include/linux/linkage.h
@@ -17,7 +17,9 @@
 #define CPP_ASMLINKAGE
 #endif
 
+#ifndef asmlinkage
 #define asmlinkage CPP_ASMLINKAGE
+#endif
 
 #define SYMBOL_NAME_STR(X)	#X
 #define SYMBOL_NAME(X)		X
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH 2/2] ARM: remove redundant asmlinkage define
  2014-12-03  8:36 [U-Boot] [PATCH 0/2] Consolidate asmlinkage defines Masahiro Yamada
  2014-12-03  8:36 ` [U-Boot] [PATCH 1/2] x86: move arch-specific asmlinkage to <asm/linkage.h> Masahiro Yamada
@ 2014-12-03  8:36 ` Masahiro Yamada
  2014-12-15 18:27   ` Simon Glass
  1 sibling, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2014-12-03  8:36 UTC (permalink / raw)
  To: u-boot

Use asmlinkage defined in include/linux/linkage.h if necessary.
Actually no ARM board uses asmlinkage, so this commit has no impact.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
---

 include/common.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/common.h b/include/common.h
index f1ab2cf..ecbbcee 100644
--- a/include/common.h
+++ b/include/common.h
@@ -70,9 +70,6 @@ typedef volatile unsigned char	vu_char;
 #ifdef	CONFIG_4xx
 #include <asm/ppc4xx.h>
 #endif
-#ifdef CONFIG_ARM
-#define asmlinkage	/* nothing */
-#endif
 #ifdef CONFIG_BLACKFIN
 #include <asm/blackfin.h>
 #endif
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH 1/2] x86: move arch-specific asmlinkage to <asm/linkage.h>
  2014-12-03  8:36 ` [U-Boot] [PATCH 1/2] x86: move arch-specific asmlinkage to <asm/linkage.h> Masahiro Yamada
@ 2014-12-07 21:44   ` Simon Glass
  2014-12-08  2:01     ` Masahiro Yamada
  0 siblings, 1 reply; 8+ messages in thread
From: Simon Glass @ 2014-12-07 21:44 UTC (permalink / raw)
  To: u-boot

Hi Masahiro,

On 3 December 2014 at 01:36, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
> Commit 65dd74a674d6 (x86: ivybridge: Implement SDRAM init) introduced
> x86-specific asmlinkage into arch/x86/include/asm/config.h.
>
> Commit ed0a2fbf14f7 (x86: Add a definition of asmlinkage) added the
> same macro define again, this time, into include/common.h.
> (Please do not add arch-specific stuff to include/common.h any more;
> it is already too cluttered.)
>
> The generic asmlinkage is defined in <linux/linkage.h>.  If you want
> to override it with an arch-specific one, the best way is to add it
> to <asm/linkage.h> like Linux Kernel.
>
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>
>  arch/x86/include/asm/arch-ivybridge/pei_data.h | 2 ++
>  arch/x86/include/asm/config.h                  | 1 -
>  arch/x86/include/asm/linkage.h                 | 6 ++++++
>  arch/x86/lib/bios.c                            | 1 +
>  arch/x86/lib/bios.h                            | 2 ++
>  include/common.h                               | 3 ---
>  include/linux/linkage.h                        | 2 ++
>  7 files changed, 13 insertions(+), 4 deletions(-)
>  create mode 100644 arch/x86/include/asm/linkage.h
>
> diff --git a/arch/x86/include/asm/arch-ivybridge/pei_data.h b/arch/x86/include/asm/arch-ivybridge/pei_data.h
> index 5026c8b..9453336 100644
> --- a/arch/x86/include/asm/arch-ivybridge/pei_data.h
> +++ b/arch/x86/include/asm/arch-ivybridge/pei_data.h
> @@ -7,6 +7,8 @@
>  #ifndef ASM_ARCH_PEI_DATA_H
>  #define ASM_ARCH_PEI_DATA_H
>
> +#include <linux/linkage.h>
> +
>  struct pch_usb3_controller_settings {
>         /* 0: Disable, 1: Enable, 2: Auto, 3: Smart Auto */
>         uint16_t mode;
> diff --git a/arch/x86/include/asm/config.h b/arch/x86/include/asm/config.h
> index c97d988..ff15828 100644
> --- a/arch/x86/include/asm/config.h
> +++ b/arch/x86/include/asm/config.h
> @@ -10,6 +10,5 @@
>  #define CONFIG_SYS_GENERIC_BOARD
>  #define CONFIG_LMB
>  #define CONFIG_SYS_BOOT_RAMDISK_HIGH
> -#define asmlinkage __attribute__((regparm(0)))
>
>  #endif
> diff --git a/arch/x86/include/asm/linkage.h b/arch/x86/include/asm/linkage.h
> new file mode 100644
> index 0000000..bdca72e
> --- /dev/null
> +++ b/arch/x86/include/asm/linkage.h
> @@ -0,0 +1,6 @@
> +#ifndef _ASM_X86_LINKAGE_H
> +#define _ASM_X86_LINKAGE_H
> +
> +#define asmlinkage CPP_ASMLINKAGE __attribute__((regparm(0)))

Why CPP_ASMLINKAGE here?

> +
> +#endif /* _ASM_X86_LINKAGE_H */
> diff --git a/arch/x86/lib/bios.c b/arch/x86/lib/bios.c
> index 298fca6..d1f8933 100644
> --- a/arch/x86/lib/bios.c
> +++ b/arch/x86/lib/bios.c
> @@ -9,6 +9,7 @@
>  #include <common.h>
>  #include <bios_emul.h>
>  #include <vbe.h>
> +#include <linux/linkage.h>
>  #include <asm/cache.h>
>  #include <asm/processor.h>
>  #include <asm/i8259.h>
> diff --git a/arch/x86/lib/bios.h b/arch/x86/lib/bios.h
> index 8491b4a..668f4b5 100644
> --- a/arch/x86/lib/bios.h
> +++ b/arch/x86/lib/bios.h
> @@ -10,6 +10,8 @@
>  #ifndef _X86_LIB_BIOS_H
>  #define _X86_LIB_BIOS_H
>
> +#include <linux/linkage.h>
> +
>  #define REALMODE_BASE          0x600
>
>  #ifdef __ASSEMBLY__
> diff --git a/include/common.h b/include/common.h
> index 94c354b..f1ab2cf 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -73,9 +73,6 @@ typedef volatile unsigned char        vu_char;
>  #ifdef CONFIG_ARM
>  #define asmlinkage     /* nothing */
>  #endif
> -#ifdef CONFIG_X86
> -#define asmlinkage __attribute__((regparm(0)))
> -#endif
>  #ifdef CONFIG_BLACKFIN
>  #include <asm/blackfin.h>
>  #endif
> diff --git a/include/linux/linkage.h b/include/linux/linkage.h
> index 7435fcd..5797498 100644
> --- a/include/linux/linkage.h
> +++ b/include/linux/linkage.h
> @@ -17,7 +17,9 @@
>  #define CPP_ASMLINKAGE
>  #endif
>
> +#ifndef asmlinkage
>  #define asmlinkage CPP_ASMLINKAGE
> +#endif
>
>  #define SYMBOL_NAME_STR(X)     #X
>  #define SYMBOL_NAME(X)         X
> --
> 1.9.1
>

Tested on chromebook_link:

Tested-by: Simon Glass <sjg@chromium.org>

With the above question answered, I'd like to apply this as it is a
clean-up. Is it OK to so this independently of the ARM patch?

Regards,
Simon

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH 1/2] x86: move arch-specific asmlinkage to <asm/linkage.h>
  2014-12-07 21:44   ` Simon Glass
@ 2014-12-08  2:01     ` Masahiro Yamada
  2014-12-10  5:12       ` Simon Glass
  0 siblings, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2014-12-08  2:01 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On Sun, 7 Dec 2014 14:44:21 -0700
Simon Glass <sjg@chromium.org> wrote:

> >  #endif
> > diff --git a/arch/x86/include/asm/linkage.h b/arch/x86/include/asm/linkage.h
> > new file mode 100644
> > index 0000000..bdca72e
> > --- /dev/null
> > +++ b/arch/x86/include/asm/linkage.h
> > @@ -0,0 +1,6 @@
> > +#ifndef _ASM_X86_LINKAGE_H
> > +#define _ASM_X86_LINKAGE_H
> > +
> > +#define asmlinkage CPP_ASMLINKAGE __attribute__((regparm(0)))
> 
> Why CPP_ASMLINKAGE here?


The intention of the generic asmlinkage (defined in <linux/linkage.h>)
is to add 'extern "C"' if __cplusplus is defined.
The x86-specific asmlinkage should be supposed to add "__attribute__((regparm(0)))"
onto that rather than replacing it.


> 
> Tested on chromebook_link:
> 
> Tested-by: Simon Glass <sjg@chromium.org>
> 
> With the above question answered, I'd like to apply this as it is a
> clean-up. Is it OK to so this independently of the ARM patch?


It must be accompanied with the ARM patch, otherwise the latter
will get a conflict.

Will you apply both to u-boot-x86?
I think it is OK because 2/2 is trivial enough.


Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH 1/2] x86: move arch-specific asmlinkage to <asm/linkage.h>
  2014-12-08  2:01     ` Masahiro Yamada
@ 2014-12-10  5:12       ` Simon Glass
  2014-12-15 18:26         ` Simon Glass
  0 siblings, 1 reply; 8+ messages in thread
From: Simon Glass @ 2014-12-10  5:12 UTC (permalink / raw)
  To: u-boot

Hi Tom,

On 7 December 2014 at 19:01, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
> Hi Simon,
>
> On Sun, 7 Dec 2014 14:44:21 -0700
> Simon Glass <sjg@chromium.org> wrote:
>
>> >  #endif
>> > diff --git a/arch/x86/include/asm/linkage.h b/arch/x86/include/asm/linkage.h
>> > new file mode 100644
>> > index 0000000..bdca72e
>> > --- /dev/null
>> > +++ b/arch/x86/include/asm/linkage.h
>> > @@ -0,0 +1,6 @@
>> > +#ifndef _ASM_X86_LINKAGE_H
>> > +#define _ASM_X86_LINKAGE_H
>> > +
>> > +#define asmlinkage CPP_ASMLINKAGE __attribute__((regparm(0)))
>>
>> Why CPP_ASMLINKAGE here?
>
>
> The intention of the generic asmlinkage (defined in <linux/linkage.h>)
> is to add 'extern "C"' if __cplusplus is defined.
> The x86-specific asmlinkage should be supposed to add "__attribute__((regparm(0)))"
> onto that rather than replacing it.
>
>
>>
>> Tested on chromebook_link:
>>
>> Tested-by: Simon Glass <sjg@chromium.org>
>>
>> With the above question answered, I'd like to apply this as it is a
>> clean-up. Is it OK to so this independently of the ARM patch?
>
>
> It must be accompanied with the ARM patch, otherwise the latter
> will get a conflict.
>
> Will you apply both to u-boot-x86?
> I think it is OK because 2/2 is trivial enough.

Do you agree with this?

Regards,
Simon

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH 1/2] x86: move arch-specific asmlinkage to <asm/linkage.h>
  2014-12-10  5:12       ` Simon Glass
@ 2014-12-15 18:26         ` Simon Glass
  0 siblings, 0 replies; 8+ messages in thread
From: Simon Glass @ 2014-12-15 18:26 UTC (permalink / raw)
  To: u-boot

On 9 December 2014 at 22:12, Simon Glass <sjg@chromium.org> wrote:
> Hi Tom,
>
> On 7 December 2014 at 19:01, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
>> Hi Simon,
>>
>> On Sun, 7 Dec 2014 14:44:21 -0700
>> Simon Glass <sjg@chromium.org> wrote:
>>
>>> >  #endif
>>> > diff --git a/arch/x86/include/asm/linkage.h b/arch/x86/include/asm/linkage.h
>>> > new file mode 100644
>>> > index 0000000..bdca72e
>>> > --- /dev/null
>>> > +++ b/arch/x86/include/asm/linkage.h
>>> > @@ -0,0 +1,6 @@
>>> > +#ifndef _ASM_X86_LINKAGE_H
>>> > +#define _ASM_X86_LINKAGE_H
>>> > +
>>> > +#define asmlinkage CPP_ASMLINKAGE __attribute__((regparm(0)))
>>>
>>> Why CPP_ASMLINKAGE here?
>>
>>
>> The intention of the generic asmlinkage (defined in <linux/linkage.h>)
>> is to add 'extern "C"' if __cplusplus is defined.
>> The x86-specific asmlinkage should be supposed to add "__attribute__((regparm(0)))"
>> onto that rather than replacing it.
>>
>>
>>>
>>> Tested on chromebook_link:
>>>
>>> Tested-by: Simon Glass <sjg@chromium.org>
>>>
>>> With the above question answered, I'd like to apply this as it is a
>>> clean-up. Is it OK to so this independently of the ARM patch?
>>
>>
>> It must be accompanied with the ARM patch, otherwise the latter
>> will get a conflict.
>>
>> Will you apply both to u-boot-x86?
>> I think it is OK because 2/2 is trivial enough.
>
> Do you agree with this?

OK, I'm going ahead.

Applied to u-boot-x86, thanks!

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH 2/2] ARM: remove redundant asmlinkage define
  2014-12-03  8:36 ` [U-Boot] [PATCH 2/2] ARM: remove redundant asmlinkage define Masahiro Yamada
@ 2014-12-15 18:27   ` Simon Glass
  0 siblings, 0 replies; 8+ messages in thread
From: Simon Glass @ 2014-12-15 18:27 UTC (permalink / raw)
  To: u-boot

On 3 December 2014 at 01:36, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
> Use asmlinkage defined in include/linux/linkage.h if necessary.
> Actually no ARM board uses asmlinkage, so this commit has no impact.
>
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
> ---
>
>  include/common.h | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/include/common.h b/include/common.h
> index f1ab2cf..ecbbcee 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -70,9 +70,6 @@ typedef volatile unsigned char        vu_char;
>  #ifdef CONFIG_4xx
>  #include <asm/ppc4xx.h>
>  #endif
> -#ifdef CONFIG_ARM
> -#define asmlinkage     /* nothing */
> -#endif
>  #ifdef CONFIG_BLACKFIN
>  #include <asm/blackfin.h>
>  #endif

This goes with the x86 clean-up and is trivial, so I will take this
through x86 as suggested on patch 1/2.

Applied to u-boot-x86, thanks!

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-12-15 18:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-03  8:36 [U-Boot] [PATCH 0/2] Consolidate asmlinkage defines Masahiro Yamada
2014-12-03  8:36 ` [U-Boot] [PATCH 1/2] x86: move arch-specific asmlinkage to <asm/linkage.h> Masahiro Yamada
2014-12-07 21:44   ` Simon Glass
2014-12-08  2:01     ` Masahiro Yamada
2014-12-10  5:12       ` Simon Glass
2014-12-15 18:26         ` Simon Glass
2014-12-03  8:36 ` [U-Boot] [PATCH 2/2] ARM: remove redundant asmlinkage define Masahiro Yamada
2014-12-15 18:27   ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.