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 devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html