[PATCH 5/6] ARM: EXYNOS: Minor fixes to enable EXYNOS5410 support

[Chander Kashyap]

On 3 October 2013 02:37, Tomasz Figa <tomasz.figa at gmail.com> wrote:
> Hi Vyacheslav, Tarek,
>
> On Tuesday 01 of October 2013 20:17:06 Vyacheslav Tyrtov wrote:
>> From: Tarek Dakhran <t.dakhran at samsung.com>
>>
>> Configure ARM_NR_BANKS as 16 for EXYNOS SoC.
>> Enable cci_control_port_by_index for ACE_PORT.
>> Add additional irqs for Exynos MCT.
>> Set irq base as 256 for EXYNOS5410 SoC.
>>
>> Signed-off-by: Vyacheslav Tyrtov <v.tyrtov at samsung.com>
>> ---
>>  arch/arm/Kconfig                  |  2 +-
>>  drivers/bus/arm-cci.c             |  7 +++++++
>>  drivers/clocksource/exynos_mct.c  |  8 +++++++-
>>  drivers/irqchip/exynos-combiner.c | 12 +++++++++++-
>>  4 files changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 3f7714d..7f88896 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -1080,7 +1080,7 @@ source arch/arm/mm/Kconfig
>>
>>  config ARM_NR_BANKS
>>       int
>> -     default 16 if ARCH_EP93XX
>> +     default 16 if ARCH_EP93XX || ARCH_EXYNOS
>
> Could you explain why this is needed, please?
>
>>       default 8
>>
>>  config IWMMXT
>> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
>> index 2009266..f2f5df1 100644
>> --- a/drivers/bus/arm-cci.c
>> +++ b/drivers/bus/arm-cci.c
>> @@ -363,8 +363,15 @@ int notrace __cci_control_port_by_index(u32 port,
>> bool enable) * interface (ie cci_disable_port_by_cpu(); control by
>> general purpose * indexing is therefore disabled for ACE ports.
>>        */
>> +
>> +     /*
>> +      * Using this way to enable cci_port on EXYNOS5410 SoC
>> +      */
>> +
>> +#ifndef CONFIG_SOC_EXYNOS5410
>>       if (ports[port].type == ACE_PORT)
>>               return -EPERM;
>> +#endif
>
> Huh? Could you explain a) why this is needed b) why this can't be detected
> at runtime? Any new code being added must be ready for multiplatform
> builds and this clearly isn't.
>
> I'd recommend extending the CCI binding with a boolean property that makes
> the driver bypass this check, but I'd like to see an answer to question a)
> first.
>
>>
>>       cci_port_control(port, enable);
>>       return 0;
>> diff --git a/drivers/clocksource/exynos_mct.c
>> b/drivers/clocksource/exynos_mct.c index 5b34768..33884d7 100644
>> --- a/drivers/clocksource/exynos_mct.c
>> +++ b/drivers/clocksource/exynos_mct.c
>> @@ -71,6 +71,12 @@ enum {
>>       MCT_L1_IRQ,
>>       MCT_L2_IRQ,
>>       MCT_L3_IRQ,

>> +#ifdef CONFIG_ARM_CCI
>> +     MCT_L4_IRQ,
>> +     MCT_L5_IRQ,
>> +     MCT_L6_IRQ,
>> +     MCT_L7_IRQ,
>> +#endif

The above change is not required as patches are already submitted to
boot all eight cores, which include this change also.

>
> This #ifdef is useless.
>
> Basically this whole enum is, as it is a remnant of legacy non-DT support,
> but it is a material for separate patch.
>
>>       MCT_NR_IRQS,
>>  };
>>
>> @@ -406,7 +412,7 @@ static int exynos4_local_timer_setup(struct
>> clock_event_device *evt) mevt = container_of(evt, struct
>> mct_clock_event_device, evt);
>>
>>       mevt->base = EXYNOS4_MCT_L_BASE(cpu);
>> -     sprintf(mevt->name, "mct_tick%d", cpu);
>> +     snprintf(mevt->name, 10, "mct_tick%d", cpu);
>
> Is this really necessary to enable EXYNOS5410 support?
>
>>
>>       evt->name = mevt->name;
>>       evt->cpumask = cpumask_of(cpu);
>> diff --git a/drivers/irqchip/exynos-combiner.c
>> b/drivers/irqchip/exynos-combiner.c index 868ed40..2e056fc 100644
>> --- a/drivers/irqchip/exynos-combiner.c
>> +++ b/drivers/irqchip/exynos-combiner.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/of_address.h>
>>  #include <linux/of_irq.h>
>>  #include <asm/mach/irq.h>
>> +#include <plat/cpu.h>
>>
>>  #include "irqchip.h"
>>
>> @@ -66,6 +67,11 @@ static void combiner_handle_cascade_irq(unsigned int
>> irq, struct irq_desc *desc) struct irq_chip *chip = irq_get_chip(irq);
>>       unsigned int cascade_irq, combiner_irq;
>>       unsigned long status;
>> +     if (unlikely(!chip || !chip_data)) {
>> +             printk_once(KERN_ALERT "%s: Chip not found for IRQ %d\n"
>> +                             , __func__, irq);
>> +             return;
>> +     }
>
> What is the reason for this change?
>
>>
>>       chained_irq_enter(chip, desc);
>>
>> @@ -226,7 +232,11 @@ static int __init combiner_of_init(struct
>> device_node *np, * get their IRQ from DT, remove this in order to get
>> dynamic * allocation.
>>        */
>> -     irq_base = 160;
>> +
>> +     if (soc_is_exynos5410())
>> +             irq_base = 256;
>> +     else
>> +             irq_base = 160;
>>
>>       combiner_init(combiner_base, np, max_nr, irq_base);
>
> There was a patch floating on the ML, possibly already merged, removing
> static IRQ base assignment for combiner (which is a remnant of legacy non-
> DT support) and moving the driver to normal linear IRQ domain. That patch
> is what you need instead of this change.

That's right Tomasz. Patch is already merged.
>
> Best regards,
> Tomasz
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
with warm regards,
Chander Kashyap