Hi Changhwan,
On 20/06/11 08:34, Changhwan Youn wrote:
> For full support of power modes, this patch adds implementation
> external GIC on EXYNOS4.
>
> External GIC of Exynos4 cannot support register banking so
> several interrupt related code for CPU1 should be different
> from that of CPU0.
I just realized that patch has made it to mainline... Unfortunately, it
seems quite broken to me:
> Signed-off-by: Changhwan Youn <chaos.youn@xxxxxxxxxxx>
> ---
> arch/arm/mach-exynos4/cpu.c | 10 ++++++++
> arch/arm/mach-exynos4/include/mach/entry-macro.S | 5 ++++
> arch/arm/mach-exynos4/include/mach/map.h | 1 +
> arch/arm/mach-exynos4/platsmp.c | 27 +++++++++++++++++++++-
> 4 files changed, 42 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
> index fa33294..40a866c 100644
> --- a/arch/arm/mach-exynos4/cpu.c
> +++ b/arch/arm/mach-exynos4/cpu.c
> @@ -16,6 +16,7 @@
>
> #include <asm/proc-fns.h>
> #include <asm/hardware/cache-l2x0.h>
> +#include <asm/hardware/gic.h>
>
> #include <plat/cpu.h>
> #include <plat/clock.h>
> @@ -159,11 +160,20 @@ void __init exynos4_init_clocks(int xtal)
> exynos4_setup_clocks();
> }
>
> +static void exynos4_gic_irq_eoi(struct irq_data *d)
> +{
> + struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
> +
> + gic_data->cpu_base = S5P_VA_GIC_CPU +
> + (EXYNOS4_GIC_BANK_OFFSET * smp_processor_id());
Here, you're overwriting a field that is shared among *all* the
interrupts in the system. What if an interrupt comes up on another CPU?
If you look at the implementation of gic_eoi_irq(), you'll definitely
see the race.
> +}
> +
> void __init exynos4_init_irq(void)
> {
> int irq;
>
> gic_init(0, IRQ_LOCALTIMER, S5P_VA_GIC_DIST, S5P_VA_GIC_CPU);
> + gic_arch_extn.irq_eoi = exynos4_gic_irq_eoi;
And here you're abusing the GIC extension feature.
I've also had a look at -next, and this has been extended further to
support 4412. The problem with that is without banking, you're painfully
working around the GIC driver. At that stage, I wonder if you wouldn't
be better off with a separate driver instead of abusing the existing one...
M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
