The radix trees used by interrupt controllers for their irq reverse
mapping (currently only the XICS found on pSeries) have a complex
locking scheme dating back to before the advent of the lockless radix
tree.
This takes advantage of the lockless radix tree and of the fact that
the items of the tree are pointers to a static array (irq_map)
elements which can never go under us to simplify the locking.
Concurrency between readers and writers is handled by the intrinsic
properties of the lockless radix tree. Concurrency between writers is
handled with a global mutex.
Signed-off-by: Sebastien Dugue <sebastien.dugue@bull.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <michael@ellerman.id.au>
Signed-off-by: Paul Mackerras <paulus@samba.org>
irq_radix_revmap() currently serves 2 purposes, irq mapping lookup
and insertion which happen in interrupt and process context respectively.
Separate the function into its 2 components, one for lookup only and one
for insertion only.
Fix the only user of the revmap tree (XICS) to use the new functions.
Also, move the insertion into the radix tree of those irqs that were
requested before it was initialized at said tree initialization.
Mutual exclusion between the tree initialization and readers/writers is
handled via a state variable (revmap_trees_allocated) set to 1 when the tree
has been initialized and set to 2 after the already requested irqs have been
inserted in the tree by the init path. This state is checked before any reader
or writer access just like we used to check for tree.gfp_mask != 0 before.
Finally, now that we're not any longer inserting nodes into the radix-tree
in interrupt context, turn the GFP_ATOMIC allocations into GFP_KERNEL ones.
Signed-off-by: Sebastien Dugue <sebastien.dugue@bull.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <michael@ellerman.id.au>
Signed-off-by: Paul Mackerras <paulus@samba.org>
Now that arch/ppc is gone and CONFIG_PPC_MERGE is always set, remove
the dead code associated with !CONFIG_PPC_MERGE from arch/powerpc
and include/asm-powerpc.
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
debugfs_create_file() returns a non-NULL (non-zero) value in case of
success, not a NULL value.
This fixes this non-critical boot-time debugging error message:
[ 1.316386] calling irq_debugfs_init+0x0/0x50
[ 1.316399] initcall irq_debugfs_init+0x0/0x50 returned -12 after 0 msecs
[ 1.316411] initcall irq_debugfs_init+0x0/0x50 returned with error code -12
Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
Acked-by: Michael Ellerman <michael@ellerman.id.au>
Signed-off-by: Paul Mackerras <paulus@samba.org>
When I changed irq_alloc_host() to take an of_node
(52964f87c64e6c6ea671b5bf3030fb1494090a48: "Add an optional
device_node pointer to the irq_host"), I botched the reference
counting semantics.
Stephen pointed out that it's irq_alloc_host()'s business if
it needs to take an additional reference to the device_node,
the caller shouldn't need to care.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Signed-off-by: Paul Mackerras <paulus@samba.org>
For the additonal exception levels (critical, debug, machine check) on
40x/book-e we were using "static" allocations of the stack in the
associated head.S.
Move to a runtime allocation to make the code a bit easier to read as
we mimic how we handle IRQ stacks. Its also a bit easier to setup the
stack with a "dummy" thread_info in C code.
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
Acked-by: Paul Mackerras <paulus@samba.org>
This patch adds full support for ftrace for PowerPC (both 64 and 32 bit).
This includes dynamic tracing and function filtering.
Signed-off-by: Steven Rostedt <srostedt@redhat.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
This makes it possible to use separate stacks for hard and soft IRQs
on 32-bit powerpc as well as on 64-bit. The code for 32-bit is just
the 32-bit analog of the 64-bit code.
* Added allocation and initialization of the irq stacks. We limit the
stacks to be in lowmem for ppc32.
* Implemented ppc32 versions of call_do_softirq() and call_handle_irq()
to switch the stack pointers
* Reworked how we do stack overflow detection. We now keep around the
limit of the stack in the thread_struct and compare against the limit
to see if we've overflowed. We can now use this on ppc64 if desired.
[ paulus@samba.org: Fixed bug on 6xx where we need to reload r9 with the
thread_info pointer. ]
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
This adds the low level irq tracing hooks to the powerpc architecture
needed to enable full lockdep functionality.
This is partly based on Johannes Berg's initial version. I removed
the asm trampoline that isn't needed (thus improving performance) and
modified all sorts of bits and pieces, reworking most of the assembly,
etc...
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
This fixes the handling of the preempt count when switching
interrupt stacks so that HW interrupt properly get the softirq
mask copied over from the previous stack.
It also initializes the softirq stack preempt_count to 0 instead
of SOFTIRQ_OFFSET, like x86, as __do_softirq() does the increment,
and we hit some lockdep checks if we have it twice.
That means we do run for a little while off the softirq stack
with the preempt-count set to 0, which could be deadly if we
try to take a softirq at that point, however we do so with
interrupts disabled, so I think we are ok.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
A subtle bug sneaked into iSeries recently. On this platform, we must
not normally clear MSR:EE (the hardware external interrupt enable)
except for short periods of time. Taking an interrupt while
soft-disabled doesn't cause us to clear it for example.
The iSeries kernel expects to mostly run with MSR:EE enabled at all
times except in a few exception entry/exit code paths. Thus
local_irq_enable() doesn't check if it needs to hard-enable as it
expects this to be unnecessary on iSeries.
However, hard_irq_disable() _does_ cause MSR:EE to be cleared,
including on iSeries. A call to it was recently added to the
context switch code, thus causing interrupts to become disabled
for a long periods of time, causing the iSeries watchdog to kick
in under some circumstances and other nasty things.
This patch fixes it by making local_irq_enable() properly re-enable
MSR:EE on iSeries. It basically removes a return statement here
to make iSeries use the same code path as everybody else. That does
mean that we might occasionally get spurious decrementer interrupts
but I don't think that matters.
Another option would have been to make hard_irq_disable() a nop
on iSeries but I didn't like it much, in case we have good reasons
to hard-disable.
Part of the patch is fixes to make sure the hard_enabled PACA field
is properly set on iSeries as it used not to be before, since it
was mostly unused.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
* 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc: (408 commits)
[POWERPC] Add memchr() to the bootwrapper
[POWERPC] Implement logging of unhandled signals
[POWERPC] Add legacy serial support for OPB with flattened device tree
[POWERPC] Use 1TB segments
[POWERPC] XilinxFB: Allow fixed framebuffer base address
[POWERPC] XilinxFB: Add support for custom screen resolution
[POWERPC] XilinxFB: Use pdata to pass around framebuffer parameters
[POWERPC] PCI: Add 64-bit physical address support to setup_indirect_pci
[POWERPC] 4xx: Kilauea defconfig file
[POWERPC] 4xx: Kilauea DTS
[POWERPC] 4xx: Add AMCC Kilauea eval board support to platforms/40x
[POWERPC] 4xx: Add AMCC 405EX support to cputable.c
[POWERPC] Adjust TASK_SIZE on ppc32 systems to 3GB that are capable
[POWERPC] Use PAGE_OFFSET to tell if an address is user/kernel in SW TLB handlers
[POWERPC] 85xx: Enable FP emulation in MPC8560 ADS defconfig
[POWERPC] 85xx: Killed <asm/mpc85xx.h>
[POWERPC] 85xx: Add cpm nodes for 8541/8555 CDS
[POWERPC] 85xx: Convert mpc8560ads to the new CPM binding.
[POWERPC] mpc8272ads: Remove muram from the CPM reg property.
[POWERPC] Make clockevents work on PPC601 processors
...
Fixed up conflict in Documentation/powerpc/booting-without-of.txt manually.
As noted by Christoph Hellwig, pktgen was the only user so
it can now be removed.
[ Add missing cases caught by Adrian Bunk. -DaveM ]
Signed-off-by: Robert Olsson <robert.olsson@its.uu.se>
Signed-off-by: David S. Miller <davem@davemloft.net>
This patch introduces zalloc_maybe_bootmem and uses it so that we don't
have to mark a whole (largish) routine as __init_ref_ok.
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Paul Mackerras <paulus@samba.org>
This adds a debugfs file "powerpc/virq_mapping", which shows the virtual
to real mapping of irq numbers. Enable it with CONFIG_VIRQ_DEBUG.
Signed-off-by: Zhang Wei <wei.zhang@freescale.com>
Signed-off-by: Chen Gong <G.Chen@freescale.com>
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Signed-off-by: Paul Mackerras <paulus@samba.org>
Although no one uses the hwirq value for legacy irqs at the moment, we
should really setup the correct value in the irq_map.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Signed-off-by: Paul Mackerras <paulus@samba.org>
The most common match semantic is an exact match based on the device node.
So provide a default implementation that does this, and hook it up if no
match routine is specified.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Signed-off-by: Paul Mackerras <paulus@samba.org>
Currently if you don't specify a match callback for your irq_host it's
assumed you match everything. This is a kind of opt-out approach, and
turns out to be the exception rather than the rule.
So change the semantics to be opt-in, ie. you don't match anything unless
you provide a match callback. This in itself isn't very useful, but will
allow us to provide a default match implementation in a subsequent patch.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Signed-off-by: Paul Mackerras <paulus@samba.org>
The majority of irq_host implementations (3 out of 4) are associated
with a device_node, and need to stash it somewhere. Rather than having
it somewhere different for each host, add an optional device_node pointer
to the irq_host structure.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Signed-off-by: Paul Mackerras <paulus@samba.org>
Fixed the following warnings:
WARNING: vmlinux.o(.text+0x2934): Section mismatch: reference to .init.text:__alloc_bootmem (between 'irq_alloc_host' and 'irq_set_default_host')
WARNING: vmlinux.o(.text+0xb2aa): Section mismatch: reference to .init.data:boot_command_line (between 'register_early_udbg_console' and 'udbg_printf')
WARNING: vmlinux.o(.text+0xb2b2): Section mismatch: reference to .init.data:boot_command_line (between 'register_early_udbg_console' and 'udbg_printf')
WARNING: vmlinux.o(.text+0xe354): Section mismatch: reference to .init.text:__alloc_bootmem (between 'pcibios_alloc_controller' and 'pci_domain_nr')
WARNING: vmlinux.o(.text+0x12768): Section mismatch: reference to .init.text:update_bridge_resource (between 'quirk_fsl_pcie_transparent' and 'indirect_read_config')
WARNING: vmlinux.o(.text+0x127a8): Section mismatch: reference to .init.text:update_bridge_resource (between 'quirk_fsl_pcie_transparent' and 'indirect_read_config')
WARNING: vmlinux.o(.text+0x17566c): Section mismatch: reference to .init.text:pcibios_fixup_bus (between 'pci_scan_child_bus' and 'pci_scan_bus_parented')
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
Check to make sure ppc_md.init_IRQ has been set before calling it.
Signed-off-by: Sonny Rao <sonny@burdell.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
This patch adds irq_create_direct_mapping(). This routine is
an alternative to irq_create_mapping(), for irq controllers that
can use linux virq numbers directly as hardware numbers.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
A future patch will need the logic at the end of irq_create_mapping()
which setups a virq and installs it in the irq_map. So split it out
into a new function irq_setup_virq().
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
Uninline virq_to_hw and export it so modules can use it. The alternative
would be to export the irq_map array instead, but it's an infrequently
called function, and keeping the array unexported seems considerably
cleaner.
Signed-off-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Paul Mackerras <paulus@samba.org>
APUS (the Amiga Power-Up System) is not supported under arch/powerpc
and it's unlikely it ever will be. Therefore, this patch removes the
fragments of APUS support code from arch/powerpc which have been
copied from arch/ppc.
A few APUS references are left in asm-powerpc in .h files which are
still used from arch/ppc.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Paul Mackerras <paulus@samba.org>
There is a thinko in the irq code, it uses IRQ_NONE to indicate no irq,
whereas it should be using NO_IRQ. IRQ_NONE is returned from irq
handlers to say "not handled".
As it happens they currently have the same value (0), so this is just for
future proof-ness.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
This patch renames the raw hard_irq_{enable,disable} into
__hard_irq_{enable,disable} and introduces a higher level hard_irq_disable()
function that can be used by any code to enforce that IRQs are fully disabled,
not only lazy disabled.
The difference with the __ versions is that it will update some per-processor
fields so that the kernel keeps track and properly re-enables them in the next
local_irq_disable();
This prepares powerpc for my next patch that introduces hard_irq_disable()
generically.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Rip out the existing powerpc msi stubs. These were the start of an
implementation based on ppc_md calls, but were never used in mainline.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Signed-off-by: Paul Mackerras <paulus@samba.org>
The PS3 HV will deliver soft-disabled interrupts at the next HV call or
interrupt. Add an HV call to local_irq_restore() to force the timely
delivery of any pending interrupts.
This fixes the system slowdown bug reported here
http://bugzilla.kernel.org/show_bug.cgi?id=8260
Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
Use DEFINE_SPINLOCK instead of initializing spinlocks to
SPIN_LOCK_UNLOCKED, since DEFINE_SPINLOCK is better for lockdep.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Paul Mackerras <paulus@samba.org>
In some cases when we are not using msi we need a way to ensure that the
hardware does not have an msi capability enabled. Currently the code has been
calling disable_msi_mode to try and achieve that. However disable_msi_mode
has several other side effects and is only available when msi support is
compiled in so it isn't really appropriate.
Instead this patch implements pci_msi_off which disables all msi and msix
capabilities unconditionally with no additional side effects.
pci_disable_device was redundantly clearing the bus master enable flag and
clearing the msi enable bit. A device that is not allowed to perform bus
mastering operations cannot generate intx or msi interrupt messages as those
are essentially a special case of dma, and require bus mastering. So the call
in pci_disable_device to disable msi capabilities was redundant.
quirk_pcie_pxh also called disable_msi_mode and is updated to use pci_msi_off.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Cc: Michael Ellerman <michael@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Greg KH <greg@kroah.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
kernel/irq.c contains a comment that speaks of -1 and -2 as interrupt
numbers, but this is actually dependent on configuration options now.
Replace by NO_IRQ and NO_IRQ_ENABLED.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
This patch adds irq remapping hook. On interrupt mechanism on Beat,
when an irq outlet which has an id which is formerly used is created,
remapping the irq is required.
Signed-off-by: Kou Ishizaki <kou.ishizaki@toshiba.co.jp>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
Rewrite local_get_flags and local_irq_disable to use r13 explicitly,
to avoid the risk that gcc will split get_paca()->soft_enabled into a
sequence unsafe against preemption. Similar care in local_irq_restore.
Signed-off-by: Hugh Dickins <hugh@veritas.com>
Signed-off-by: Paul Mackerras <paulus@samba.org>
It makes for a friendlier API if irq_dispose_mapping(NO_IRQ) is a
nop, rather than triggering a WARN_ON.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
This implements a lazy strategy for disabling interrupts. This means
that local_irq_disable() et al. just clear the 'interrupts are
enabled' flag in the paca. If an interrupt comes along, the interrupt
entry code notices that interrupts are supposed to be disabled, and
clears the EE bit in SRR1, clears the 'interrupts are hard-enabled'
flag in the paca, and returns. This means that interrupts only
actually get disabled in the processor when an interrupt comes along.
When interrupts are enabled by local_irq_enable() et al., the code
sets the interrupts-enabled flag in the paca, and then checks whether
interrupts got hard-disabled. If so, it also sets the EE bit in the
MSR to hard-enable the interrupts.
This has the potential to improve performance, and also makes it
easier to make a kernel that can boot on iSeries and on other 64-bit
machines, since this lazy-disable strategy is very similar to the
soft-disable strategy that iSeries already uses.
This version renames paca->proc_enabled to paca->soft_enabled, and
changes a couple of soft-disables in the kexec code to hard-disables,
which should fix the crash that Michael Ellerman saw. This doesn't
yet use a reserved CR field for the soft_enabled and hard_enabled
flags. This applies on top of Stephen Rothwell's patches to make it
possible to build a combined iSeries/other kernel.
Signed-off-by: Paul Mackerras <paulus@samba.org>
Remove struct pt_regs * from all handlers.
Also remove the regs argument from get_irq() functions.
Compile tested with arch/powerpc/config/* and
arch/ppc/configs/prep_defconfig
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Signed-off-by: Paul Mackerras <paulus@samba.org>
Maintain a per-CPU global "struct pt_regs *" variable which can be used instead
of passing regs around manually through all ~1800 interrupt handlers in the
Linux kernel.
The regs pointer is used in few places, but it potentially costs both stack
space and code to pass it around. On the FRV arch, removing the regs parameter
from all the genirq function results in a 20% speed up of the IRQ exit path
(ie: from leaving timer_interrupt() to leaving do_IRQ()).
Where appropriate, an arch may override the generic storage facility and do
something different with the variable. On FRV, for instance, the address is
maintained in GR28 at all times inside the kernel as part of general exception
handling.
Having looked over the code, it appears that the parameter may be handed down
through up to twenty or so layers of functions. Consider a USB character
device attached to a USB hub, attached to a USB controller that posts its
interrupts through a cascaded auxiliary interrupt controller. A character
device driver may want to pass regs to the sysrq handler through the input
layer which adds another few layers of parameter passing.
I've build this code with allyesconfig for x86_64 and i386. I've runtested the
main part of the code on FRV and i386, though I can't test most of the drivers.
I've also done partial conversion for powerpc and MIPS - these at least compile
with minimal configurations.
This will affect all archs. Mostly the changes should be relatively easy.
Take do_IRQ(), store the regs pointer at the beginning, saving the old one:
struct pt_regs *old_regs = set_irq_regs(regs);
And put the old one back at the end:
set_irq_regs(old_regs);
Don't pass regs through to generic_handle_irq() or __do_IRQ().
In timer_interrupt(), this sort of change will be necessary:
- update_process_times(user_mode(regs));
- profile_tick(CPU_PROFILING, regs);
+ update_process_times(user_mode(get_irq_regs()));
+ profile_tick(CPU_PROFILING);
I'd like to move update_process_times()'s use of get_irq_regs() into itself,
except that i386, alone of the archs, uses something other than user_mode().
Some notes on the interrupt handling in the drivers:
(*) input_dev() is now gone entirely. The regs pointer is no longer stored in
the input_dev struct.
(*) finish_unlinks() in drivers/usb/host/ohci-q.c needs checking. It does
something different depending on whether it's been supplied with a regs
pointer or not.
(*) Various IRQ handler function pointers have been moved to type
irq_handler_t.
Signed-Off-By: David Howells <dhowells@redhat.com>
(cherry picked from 1b16e7ac850969f38b375e511e3fa2f474a33867 commit)
There's a bug in irq_alloc_virt() if it's asked for more than 1 interrupt,
if it can't find a slot it might look past the end of the irq_map.
To be clear: the bug is that the continue affects the inner for loop,
not the outer one, so i becomes j + 1 and then we continue the inner
loop without checking if i is still <= limit.
This fixes it. No one in the kernel actually calls this with count >
1, so it's not critical.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
When reworking the powerpc irq code, I figured out that we were using
the radix tree in a racy way. As a temporary fix, I put a spinlock in
there. However, this can have a significant impact on performances. This
patch reworks that to use a smarter technique based on the fact that
what we need is in fact a rwlock with extremely rare writers (thus
optimized for the read path).
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
The code for using the radix tree for reverse mapping of interrupts has
a typo that causes it to create incorrect mappings if the software and
hardware numbers happen to be different. This would, among others, cause
the IDE interrupt to fail on js20's. This fixes it.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
Sebastien Dugue
Kumar Gala
Benjamin Herrenschmidt
Paul Mackerras
Ingo Molnar
Emil Medve
Michael Ellerman
Steven Rostedt
Benjamin Herrenschmidt
Jean Delvare
Linus Torvalds
Robert Olsson
Stephen Rothwell
Scott Wood
Sonny Rao
Olof Johansson
David Gibson
Takao Shinohara
Thomas Gleixner
Eric W. Biederman
Johannes Berg
Ishizaki Kou
Hugh Dickins
Al Viro
Olaf Hering
David Howells