On 3 October 2013 02:37, Tomasz Figa <tomasz.figa@xxxxxxxxx> wrote:
> Hi Vyacheslav, Tarek,
>
> On Tuesday 01 of October 2013 20:17:06 Vyacheslav Tyrtov wrote:
>> From: Tarek Dakhran <t.dakhran@xxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
with warm regards,
Chander Kashyap
--
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
