[U-Boot] [PATCH] Add ability for arch code to make changes before we boot
Added a arch_preboot() function that cpu specific code can implement to allow for various modifications to the state of the machine right before we boot. This can be useful to setup register state to a specific configuration.
Signed-off-by: Kumar Gala galak@kernel.crashing.org --- common/cmd_bootm.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 86c8122..766889a 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -166,6 +166,13 @@ void __arch_lmb_reserve(struct lmb *lmb) } void arch_lmb_reserve(struct lmb *lmb) __attribute__((weak, alias("__arch_lmb_reserve")));
+/* Allow for arch specific config before we boot */ +void __arch_preboot(void) +{ + /* please define platform specific arch_preboot() */ +} +void arch_preboot(void) __attribute__((weak, alias("__arch_preboot"))); + #if defined(__ARM__) #define IH_INITRD_ARCH IH_ARCH_ARM #elif defined(__avr32__) @@ -543,6 +550,7 @@ int do_bootm_subcommand (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) break; case BOOTM_STATE_OS_GO: disable_interrupts(); + arch_preboot(); boot_fn(BOOTM_STATE_OS_GO, argc, argv, &images); break; } @@ -673,6 +681,8 @@ int do_bootm (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) return 1; }
+ arch_preboot(); + boot_fn(0, argc, argv, &images);
show_boot_progress (-9);
In future Book-E implementations IVORs will most likely go away and be replaced with fixed offsets. The IVPR will continue to exist to allow for relocation of the interrupt vectors.
This code adds support to setup the IVORs as their fixed offset values per the ISA 2.06 spec when we transition from u-boot to another OS either via 'bootm' or a cpu release.
Signed-off-by: Kumar Gala galak@kernel.crashing.org --- cpu/mpc85xx/cpu_init.c | 7 ++++ cpu/mpc85xx/fixed_ivor.S | 79 +++++++++++++++++++++++++++++++++++++++++++ cpu/mpc85xx/release.S | 3 ++ cpu/mpc85xx/start.S | 6 +++ include/asm-ppc/processor.h | 12 ++++++ 5 files changed, 107 insertions(+), 0 deletions(-) create mode 100644 cpu/mpc85xx/fixed_ivor.S
diff --git a/cpu/mpc85xx/cpu_init.c b/cpu/mpc85xx/cpu_init.c index 41de694..76db914 100644 --- a/cpu/mpc85xx/cpu_init.c +++ b/cpu/mpc85xx/cpu_init.c @@ -374,3 +374,10 @@ int cpu_init_r(void) #endif return 0; } + +extern void setup_ivors(void); + +void arch_preboot(void) +{ + setup_ivors(); +} diff --git a/cpu/mpc85xx/fixed_ivor.S b/cpu/mpc85xx/fixed_ivor.S new file mode 100644 index 0000000..dc725c9 --- /dev/null +++ b/cpu/mpc85xx/fixed_ivor.S @@ -0,0 +1,79 @@ +/* + * Copyright 2009 Freescale Semiconductor, Inc. + * + * Kumar Gala kumar.gala@freescale.com + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +/* This file is intended to be included by other asm code since + * we will want to execute this on both the primary core when + * it does a bootm and the secondary core's that get released + * out of the spin table */ + +#define SET_IVOR(vector_number, vector_offset) \ + li r3,vector_offset@l; \ + mtspr SPRN_IVOR##vector_number,r3; + +#define SET_GIVOR(vector_number, vector_offset) \ + li r3,vector_offset@l; \ + mtspr SPRN_GIVOR##vector_number,r3; + + SET_IVOR(0, 0x020) /* Critical Input */ + SET_IVOR(1, 0x000) /* Machine Check */ + SET_IVOR(2, 0x060) /* Data Storage */ + SET_IVOR(3, 0x080) /* Instruction Storage */ + SET_IVOR(4, 0x0a0) /* External Input */ + SET_IVOR(5, 0x0c0) /* Alignment */ + SET_IVOR(6, 0x0e0) /* Program */ + SET_IVOR(7, 0x100) /* FP Unavailable */ + SET_IVOR(8, 0x120) /* System Call */ + SET_IVOR(9, 0x140) /* Auxiliary Processor Unavailable */ + SET_IVOR(10, 0x160) /* Decrementer */ + SET_IVOR(11, 0x180) /* Fixed Interval Timer */ + SET_IVOR(12, 0x1a0) /* Watchdog Timer */ + SET_IVOR(13, 0x1c0) /* Data TLB Error */ + SET_IVOR(14, 0x1e0) /* Instruction TLB Error */ + SET_IVOR(15, 0x040) /* Debug */ + +/* e500v1 & e500v2 only */ +#ifndef CONFIG_E500MC + SET_IVOR(32, 0x200) /* SPE Unavailable */ + SET_IVOR(33, 0x220) /* Embedded FP Data */ + SET_IVOR(34, 0x240) /* Embedded FP Round */ +#endif + + SET_IVOR(35, 0x260) /* Performance monitor */ + +/* e500mc only */ +#ifdef CONFIG_E500MC + SET_IVOR(36, 0x280) /* Processor doorbell */ + SET_IVOR(37, 0x2a0) /* Processor doorbell critical */ + SET_IVOR(38, 0x2c0) /* Guest Processor doorbell */ + SET_IVOR(39, 0x2e0) /* Guest Processor critical & machine check */ + SET_IVOR(40, 0x300) /* Hypervisor system call */ + SET_IVOR(41, 0x320) /* Hypervisor Priviledge */ + + SET_GIVOR(2, 0x060) /* Guest Data Storage */ + SET_GIVOR(3, 0x080) /* Guest Instruction Storage */ + SET_GIVOR(4, 0x0a0) /* Guest External Input */ + SET_GIVOR(8, 0x120) /* Guest System Call */ + SET_GIVOR(13, 0x1c0) /* Guest Data TLB Error */ + SET_GIVOR(14, 0x1e0) /* Guest Instruction TLB Error */ +#endif diff --git a/cpu/mpc85xx/release.S b/cpu/mpc85xx/release.S index 2d4f219..b839ba0 100644 --- a/cpu/mpc85xx/release.S +++ b/cpu/mpc85xx/release.S @@ -167,6 +167,9 @@ __secondary_start_page: andi. r11,r4,1 bne 2b isync + + /* setup IVORs to match fixed offsets */ +#include "fixed_ivor.S"
/* get the upper bits of the addr */ lwz r11,ENTRY_ADDR_UPPER(r10) diff --git a/cpu/mpc85xx/start.S b/cpu/mpc85xx/start.S index 4f7236f..e21a4eb 100644 --- a/cpu/mpc85xx/start.S +++ b/cpu/mpc85xx/start.S @@ -1122,3 +1122,9 @@ flush_dcache: isync
blr + +.globl setup_ivors +setup_ivors: + +#include "fixed_ivor.S" + blr diff --git a/include/asm-ppc/processor.h b/include/asm-ppc/processor.h index 5547245..a2b74bf 100644 --- a/include/asm-ppc/processor.h +++ b/include/asm-ppc/processor.h @@ -468,6 +468,16 @@ #define SPRN_IVOR13 0x19d /* Interrupt Vector Offset Register 13 */ #define SPRN_IVOR14 0x19e /* Interrupt Vector Offset Register 14 */ #define SPRN_IVOR15 0x19f /* Interrupt Vector Offset Register 15 */ +#define SPRN_IVOR38 0x1b0 /* Interrupt Vector Offset Register 38 */ +#define SPRN_IVOR39 0x1b1 /* Interrupt Vector Offset Register 39 */ +#define SPRN_IVOR40 0x1b2 /* Interrupt Vector Offset Register 40 */ +#define SPRN_IVOR41 0x1b3 /* Interrupt Vector Offset Register 41 */ +#define SPRN_GIVOR2 0x1b8 /* Guest Interrupt Vector Offset Register 2 */ +#define SPRN_GIVOR3 0x1b9 /* Guest Interrupt Vector Offset Register 3 */ +#define SPRN_GIVOR4 0x1ba /* Guest Interrupt Vector Offset Register 4 */ +#define SPRN_GIVOR8 0x1bb /* Guest Interrupt Vector Offset Register 8 */ +#define SPRN_GIVOR13 0x1bc /* Guest Interrupt Vector Offset Register 13 */ +#define SPRN_GIVOR14 0x1bd /* Guest Interrupt Vector Offset Register 14 */
/* e500 definitions */ #define SPRN_L1CFG0 0x203 /* L1 Cache Configuration Register 0 */ @@ -513,6 +523,8 @@ #define SPRN_IVOR33 0x211 /* Interrupt Vector Offset Register 33 */ #define SPRN_IVOR34 0x212 /* Interrupt Vector Offset Register 34 */ #define SPRN_IVOR35 0x213 /* Interrupt Vector Offset Register 35 */ +#define SPRN_IVOR36 0x214 /* Interrupt Vector Offset Register 36 */ +#define SPRN_IVOR37 0x215 /* Interrupt Vector Offset Register 37 */ #define SPRN_SPEFSCR 0x200 /* SPE & Embedded FP Status & Control */
#define SPRN_MCSRR0 0x23a /* Machine Check Save and Restore Register 0 */
On Aug 14, 2009, at 2:00 PM, Kumar Gala wrote:
Added a arch_preboot() function that cpu specific code can implement to allow for various modifications to the state of the machine right before we boot. This can be useful to setup register state to a specific configuration.
Signed-off-by: Kumar Gala galak@kernel.crashing.org
common/cmd_bootm.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
If you can review and ack this if its ok that would be great. I'd like to send this via the 85xx next tree as the patch for fixed IVORs depends on it.
- k
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 86c8122..766889a 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -166,6 +166,13 @@ void __arch_lmb_reserve(struct lmb *lmb) } void arch_lmb_reserve(struct lmb *lmb) __attribute__((weak, alias("__arch_lmb_reserve")));
+/* Allow for arch specific config before we boot */ +void __arch_preboot(void) +{
- /* please define platform specific arch_preboot() */
+} +void arch_preboot(void) __attribute__((weak, alias("__arch_preboot")));
#if defined(__ARM__) #define IH_INITRD_ARCH IH_ARCH_ARM #elif defined(__avr32__) @@ -543,6 +550,7 @@ int do_bootm_subcommand (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) break; case BOOTM_STATE_OS_GO: disable_interrupts();
arch_preboot(); boot_fn(BOOTM_STATE_OS_GO, argc, argv, &images); break;@@ -673,6 +681,8 @@ int do_bootm (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) return 1; }
arch_preboot();
boot_fn(0, argc, argv, &images);
show_boot_progress (-9);
-- 1.6.0.6
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On 14:00 Fri 14 Aug , Kumar Gala wrote:
Added a arch_preboot() function that cpu specific code can implement to allow for various modifications to the state of the machine right before we boot. This can be useful to setup register state to a specific configuration.
Signed-off-by: Kumar Gala galak@kernel.crashing.org
common/cmd_bootm.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 86c8122..766889a 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -166,6 +166,13 @@ void __arch_lmb_reserve(struct lmb *lmb) } void arch_lmb_reserve(struct lmb *lmb) __attribute__((weak, alias("__arch_lmb_reserve")));
+/* Allow for arch specific config before we boot */ +void __arch_preboot(void) +{
- /* please define platform specific arch_preboot() */
+} +void arch_preboot(void) __attribute__((weak, alias("__arch_preboot")));
why not a section so you will be able to have multiple callback so it can be arch or soc or board too in any combination
Best Regards, J.
On Aug 14, 2009, at 3:13 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
On 14:00 Fri 14 Aug , Kumar Gala wrote:
Added a arch_preboot() function that cpu specific code can implement to allow for various modifications to the state of the machine right before we boot. This can be useful to setup register state to a specific configuration.
Signed-off-by: Kumar Gala galak@kernel.crashing.org
common/cmd_bootm.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 86c8122..766889a 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -166,6 +166,13 @@ void __arch_lmb_reserve(struct lmb *lmb) } void arch_lmb_reserve(struct lmb *lmb) __attribute__((weak, alias("__arch_lmb_reserve")));
+/* Allow for arch specific config before we boot */ +void __arch_preboot(void) +{
- /* please define platform specific arch_preboot() */
+} +void arch_preboot(void) __attribute__((weak, alias("__arch_preboot")));
why not a section so you will be able to have multiple callback so it can be arch or soc or board too in any combination
Because the use I have for it is a simple single case at this point.
If and when we need more complexity we can add it.
- k
On 15:45 Fri 14 Aug , Kumar Gala wrote:
On Aug 14, 2009, at 3:13 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
On 14:00 Fri 14 Aug , Kumar Gala wrote:
Added a arch_preboot() function that cpu specific code can implement to allow for various modifications to the state of the machine right before we boot. This can be useful to setup register state to a specific configuration.
Signed-off-by: Kumar Gala galak@kernel.crashing.org
common/cmd_bootm.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 86c8122..766889a 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -166,6 +166,13 @@ void __arch_lmb_reserve(struct lmb *lmb) } void arch_lmb_reserve(struct lmb *lmb) __attribute__((weak, alias("__arch_lmb_reserve")));
+/* Allow for arch specific config before we boot */ +void __arch_preboot(void) +{
- /* please define platform specific arch_preboot() */
+} +void arch_preboot(void) __attribute__((weak, alias("__arch_preboot")));
why not a section so you will be able to have multiple callback so it can be arch or soc or board too in any combination
Because the use I have for it is a simple single case at this point.
If and when we need more complexity we can add it.
we have already it as example the usb drivers that need to be stop before start an os I forget some but I'm sure to have seen oher
Best Regards, J.
On Aug 15, 2009, at 9:24 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
On 15:45 Fri 14 Aug , Kumar Gala wrote:
On Aug 14, 2009, at 3:13 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
On 14:00 Fri 14 Aug , Kumar Gala wrote:
Added a arch_preboot() function that cpu specific code can implement to allow for various modifications to the state of the machine right before we boot. This can be useful to setup register state to a specific configuration.
Signed-off-by: Kumar Gala galak@kernel.crashing.org
common/cmd_bootm.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 86c8122..766889a 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -166,6 +166,13 @@ void __arch_lmb_reserve(struct lmb *lmb) } void arch_lmb_reserve(struct lmb *lmb) __attribute__((weak, alias("__arch_lmb_reserve")));
+/* Allow for arch specific config before we boot */ +void __arch_preboot(void) +{
- /* please define platform specific arch_preboot() */
+} +void arch_preboot(void) __attribute__((weak, alias("__arch_preboot")));
why not a section so you will be able to have multiple callback so it can be arch or soc or board too in any combination
Because the use I have for it is a simple single case at this point.
If and when we need more complexity we can add it.
we have already it as example the usb drivers that need to be stop before start an os I forget some but I'm sure to have seen oher
We already handle USB in the bootm code. I don't see any reason to go add a bunch of code for the one case we handle.
- k
On Aug 14, 2009, at 2:00 PM, Kumar Gala wrote:
Added a arch_preboot() function that cpu specific code can implement to allow for various modifications to the state of the machine right before we boot. This can be useful to setup register state to a specific configuration.
Signed-off-by: Kumar Gala galak@kernel.crashing.org
common/cmd_bootm.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
Wolfgang,
Can you look at either commenting on this or picking it up for 'next'. Its one patch the current '85xx next' tree depends on.
- k
Dear Kumar Gala,
In message 1250276442-28463-1-git-send-email-galak@kernel.crashing.org you wrote:
Added a arch_preboot() function that cpu specific code can implement to allow for various modifications to the state of the machine right before we boot. This can be useful to setup register state to a specific configuration.
Signed-off-by: Kumar Gala galak@kernel.crashing.org
common/cmd_bootm.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 86c8122..766889a 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -166,6 +166,13 @@ void __arch_lmb_reserve(struct lmb *lmb) } void arch_lmb_reserve(struct lmb *lmb) __attribute__((weak, alias("__arch_lmb_reserve")));
+/* Allow for arch specific config before we boot */ +void __arch_preboot(void)
As this is only used when booting an OS (and not for example when starting a standalone program) we should probably write:
/* Allow for arch specific code before booting the OS */ void __arch_preboot_os() ...
+{
- /* please define platform specific arch_preboot() */
+} +void arch_preboot(void) __attribute__((weak, alias("__arch_preboot")));
#if defined(__ARM__) #define IH_INITRD_ARCH IH_ARCH_ARM #elif defined(__avr32__) @@ -543,6 +550,7 @@ int do_bootm_subcommand (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) break; case BOOTM_STATE_OS_GO:
Hm... maybe this could / should be handled in BOOTM_STATE_OS_PREP state?
disable_interrupts();
arch_preboot();
And maybe we should mode disable_interrupts() into the default implementation of arch_preboot_os() ?
boot_fn(BOOTM_STATE_OS_GO, argc, argv, &images); break;} @@ -673,6 +681,8 @@ int do_bootm (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) return 1; }
- arch_preboot();
- boot_fn(0, argc, argv, &images);
But this would change behaviour here. Eventually to the better?
Best regards,
Wolfgang Denk
On Aug 26, 2009, at 3:46 PM, Wolfgang Denk wrote:
Dear Kumar Gala,
In message <1250276442-28463-1-git-send-email-galak@kernel.crashing.org
you wrote: Added a arch_preboot() function that cpu specific code can implement to allow for various modifications to the state of the machine right before we boot. This can be useful to setup register state to a specific configuration.
Signed-off-by: Kumar Gala galak@kernel.crashing.org
common/cmd_bootm.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 86c8122..766889a 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -166,6 +166,13 @@ void __arch_lmb_reserve(struct lmb *lmb) } void arch_lmb_reserve(struct lmb *lmb) __attribute__((weak, alias("__arch_lmb_reserve")));
+/* Allow for arch specific config before we boot */ +void __arch_preboot(void)
As this is only used when booting an OS (and not for example when starting a standalone program) we should probably write:
/* Allow for arch specific code before booting the OS */ void __arch_preboot_os() ...
no problem, will change the name.
+{
- /* please define platform specific arch_preboot() */
+} +void arch_preboot(void) __attribute__((weak, alias("__arch_preboot")));
#if defined(__ARM__) #define IH_INITRD_ARCH IH_ARCH_ARM #elif defined(__avr32__) @@ -543,6 +550,7 @@ int do_bootm_subcommand (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) break; case BOOTM_STATE_OS_GO:
Hm... maybe this could / should be handled in BOOTM_STATE_OS_PREP state?
I'd prefer to keep arch_preboot_os() as close to boot_fn() as possible
disable_interrupts();
arch_preboot();And maybe we should mode disable_interrupts() into the default implementation of arch_preboot_os() ?
Got no issue with moving disable_interrupts into the default implementation of arch_preboot_os()
boot_fn(BOOTM_STATE_OS_GO, argc, argv, &images); break;} @@ -673,6 +681,8 @@ int do_bootm (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) return 1; }
- arch_preboot();
- boot_fn(0, argc, argv, &images);
But this would change behaviour here. Eventually to the better?
Not sure if I see how this would be better (or worse for that matter).
- k
On Aug 26, 2009, at 10:27 PM, Kumar Gala wrote:
On Aug 26, 2009, at 3:46 PM, Wolfgang Denk wrote:
Dear Kumar Gala,
In message <1250276442-28463-1-git-send-email-galak@kernel.crashing.org
you wrote: Added a arch_preboot() function that cpu specific code can implement to allow for various modifications to the state of the machine right before we boot. This can be useful to setup register state to a specific configuration.
Signed-off-by: Kumar Gala galak@kernel.crashing.org
common/cmd_bootm.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 86c8122..766889a 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -166,6 +166,13 @@ void __arch_lmb_reserve(struct lmb *lmb) } void arch_lmb_reserve(struct lmb *lmb) __attribute__((weak, alias("__arch_lmb_reserve")));
+/* Allow for arch specific config before we boot */ +void __arch_preboot(void)
As this is only used when booting an OS (and not for example when starting a standalone program) we should probably write:
/* Allow for arch specific code before booting the OS */ void __arch_preboot_os() ...
no problem, will change the name.
+{
- /* please define platform specific arch_preboot() */
+} +void arch_preboot(void) __attribute__((weak, alias("__arch_preboot")));
#if defined(__ARM__) #define IH_INITRD_ARCH IH_ARCH_ARM #elif defined(__avr32__) @@ -543,6 +550,7 @@ int do_bootm_subcommand (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) break; case BOOTM_STATE_OS_GO:
Hm... maybe this could / should be handled in BOOTM_STATE_OS_PREP state?
I'd prefer to keep arch_preboot_os() as close to boot_fn() as possible
disable_interrupts();
arch_preboot();And maybe we should mode disable_interrupts() into the default implementation of arch_preboot_os() ?
Got no issue with moving disable_interrupts into the default implementation of arch_preboot_os()
Now that I look at how do_bootm() calls and uses disable_interrupts() this is a bit more tricky. I'd prefer to leave disable_interrupts() alone.
- k
participants (3)
-
Jean-Christophe PLAGNIOL-VILLARD -
Kumar Gala -
Wolfgang Denk