ARM: support for Moschip MCS814x SoCs

arch/arm/mach-mcs814x/clock.c new file +237 -0

new file mode 100644
index 0000000..eb30ae2
@@ -0,0 +1,271 @@
+/*
+ * Moschip MCS814x clock routines
+ *
+ * Copyright (C) 2012, Florian Fainelli <[email protected]>
+ *
+ * Licensed under GPLv2
+ */
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/export.h>
+#include <linux/spinlock.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/clkdev.h>
+#include <linux/clk.h>
+
+#include <mach/mcs814x.h>
+
+#include "common.h"
+
+#define KHZ	1000
+#define MHZ	(KHZ * KHZ)
+
+struct clk_ops {
+	unsigned long (*get_rate)(struct clk *clk);
+	int (*set_rate)(struct clk *clk, unsigned long rate);
+	struct clk *(*get_parent)(struct clk *clk);
+	int (*enable)(struct clk *clk, int enable);
+};
+
+struct clk {
+	struct clk *parent;		/* parent clk */
+	unsigned long rate;		/* clock rate in Hz */
+	unsigned long divider;		/* clock divider */
+	u32 usecount;			/* reference count */
+	struct clk_ops *ops;		/* clock operation */
+	u32 enable_reg;			/* clock enable register */
+	u32 enable_mask;		/* clock enable mask */
+};
+
+static unsigned long clk_divide_parent(struct clk *clk)
+{
+	if (clk->parent && clk->divider)
+		return clk_get_rate(clk->parent) / clk->divider;
+	else
+		return 0;
+}
+
+static int clk_local_onoff_enable(struct clk *clk, int enable)
+{
+	u32 tmp;
+
+	/* no enable_reg means the clock is always enabled */
+	if (!clk->enable_reg)
+		return 0;
+
+	tmp = __raw_readl(mcs814x_sysdbg_base + clk->enable_reg);
+	if (!enable)
+		tmp &= ~clk->enable_mask;
+	else
+		tmp |= clk->enable_mask;
+
+	__raw_writel(tmp, mcs814x_sysdbg_base + clk->enable_reg);
+
+	return 0;
+}
+
+static struct clk_ops default_clk_ops = {
+	.get_rate	= clk_divide_parent,
+	.enable		= clk_local_onoff_enable,
+};
+
+static DEFINE_SPINLOCK(clocks_lock);
+
+static const unsigned long cpu_freq_table[] = {
+	175000,
+	300000,
+	125000,
+	137500,
+	212500,
+	250000,
+	162500,
+	187500,
+	162500,
+	150000,
+	225000,
+	237500,
+	200000,
+	262500,
+	275000,
+	287500
+};
+
+static struct clk clk_cpu;
+
+/* System clock is fixed at 50Mhz */
+static struct clk clk_sys = {
+	.rate	= 50 * MHZ,
+};
+
+static struct clk clk_sdram;
+
+static struct clk clk_timer0 = {
+	.parent	= &clk_sdram,
+	.divider = 2,
+	.ops	= &default_clk_ops,
+};
+
+static struct clk clk_timer1_2 = {
+	.parent	= &clk_sys,
+};
+
+/* Watchdog clock is system clock / 128 */
+static struct clk clk_wdt = {
+	.parent	= &clk_sys,
+	.divider = 128,
+	.ops	= &default_clk_ops,
+};
+
+static struct clk clk_emac = {
+	.ops		= &default_clk_ops,
+	.enable_reg	= SYSDBG_SYSCTL,
+	.enable_mask	= SYSCTL_EMAC,
+};
+
+static struct clk clk_ephy = {
+	.ops		= &default_clk_ops,
+	.enable_reg	= SYSDBG_PLL_CTL,
+	.enable_mask	= ~SYSCTL_EPHY,	/* active low */
+};
+
+static struct clk clk_cipher = {
+	.ops		= &default_clk_ops,
+	.enable_reg	= SYSDBG_SYSCTL,
+	.enable_mask	= SYSCTL_CIPHER,
+};
+
+#define CLK(_dev, _con, _clk)	\
+{ .dev_id = (_dev), .con_id = (_con), .clk = (_clk) },
+
+static struct clk_lookup mcs814x_chip_clks[] = {
+	CLK("cpu", NULL, &clk_cpu)
+	CLK("sys", NULL, &clk_sys)
+	CLK("sdram", NULL, &clk_sdram)
+	/* 32-bits timer0 */
+	CLK("timer0", NULL, &clk_timer0)
+	/* 16-bits timer1 */
+	CLK("timer1", NULL, &clk_timer1_2)
+	/* 64-bits timer2, same as timer 1 */
+	CLK("timer2", NULL, &clk_timer1_2)
+	CLK(NULL, "wdt", &clk_wdt)
+	CLK(NULL, "emac", &clk_emac)
+	CLK(NULL, "ephy", &clk_ephy)
+	CLK(NULL, "cipher", &clk_cipher)
+};
+
+static void local_clk_disable(struct clk *clk)
+{
+	WARN_ON(!clk->usecount);
+
+	if (clk->usecount > 0) {
+		clk->usecount--;
+
+		if ((clk->usecount == 0) && (clk->ops->enable))
+			clk->ops->enable(clk, 0);
+
+		if (clk->parent)
+			local_clk_disable(clk->parent);
+	}
+}
+
+static int local_clk_enable(struct clk *clk)
+{
+	int ret = 0;
+
+	if (clk->parent)
+		ret = local_clk_enable(clk->parent);
+
+	if (ret)
+		return ret;
+
+	if ((clk->usecount == 0) && (clk->ops->enable))
+		ret = clk->ops->enable(clk, 1);
+
+	if (!ret)
+		clk->usecount++;
+	else if (clk->parent && clk->parent->ops->enable)
+		local_clk_disable(clk->parent);
+
+	return ret;
+}
+
+int clk_enable(struct clk *clk)
+{
+	int ret;
+	unsigned long flags;
+
+	spin_lock_irqsave(&clocks_lock, flags);
+	ret = local_clk_enable(clk);
+	spin_unlock_irqrestore(&clocks_lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL(clk_enable);
+
+void clk_disable(struct clk *clk)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&clocks_lock, flags);
+	local_clk_disable(clk);
+	spin_unlock_irqrestore(&clocks_lock, flags);
+}
+EXPORT_SYMBOL(clk_disable);
+
+unsigned long clk_get_rate(struct clk *clk)
+{
+	if (unlikely(IS_ERR_OR_NULL(clk)))
+		return 0;
+
+	if (clk->rate)
+		return clk->rate;
+
+	if (clk->ops && clk->ops->get_rate)
+		return clk->ops->get_rate(clk);
+
+	return clk_get_rate(clk->parent);
+}
+EXPORT_SYMBOL(clk_get_rate);
+
+struct clk *clk_get_parent(struct clk *clk)
+{
+	unsigned long flags;
+
+	if (unlikely(IS_ERR_OR_NULL(clk)))
+		return NULL;
+

arch/arm/mach-mcs814x/clock.c new file +34 -0

+struct cpu_mode {
+	const char *name;
+	int gpio_start;
+	int gpio_end;
+};
+
+static const struct cpu_mode cpu_modes[] = {
+	{
+		.name		= "I2S",
+		.gpio_start	= 4,
+		.gpio_end	= 8,
+	},
+	{
+		.name		= "UART",
+		.gpio_start	= 4,
+		.gpio_end	= 9,
+	},
+	{
+		.name		= "External MII",
+		.gpio_start	= 0,
+		.gpio_end	= 16,
+	},
+	{
+		.name		= "Normal",
+		.gpio_start	= -1,
+		.gpio_end	= -1,
+	},
+};
+
+void __init mcs814x_init_machine(void)
+{
+	u32 bs2, cpu_mode;
+	int gpio;
+

arch/arm/mach-mcs814x/include/mach/entry-macro.S new file +19 -0

new file mode 100644
index 0000000..cbad566
@@ -0,0 +1,29 @@
+#include <mach/mcs814x.h>
+                .macro  disable_fiq
+                .endm
+
+		.macro arch_ret_to_user, tmp1, tmp2
+		.endm
+
+		.macro  get_irqnr_preamble, base, tmp
+		ldr	\base, =mcs814x_intc_base@ base virtual address of INTC
+		ldr	\base, [\base]
+		.endm
+
+		.macro  get_irqnr_and_base, irqnr, irqstat, base, tmp
+		mov	\tmp, #MCS814X_IRQ_STS0	 @ load tmp with STS0 register offset
+		ldr	\irqstat, [\base, \tmp]	 @ load value at base + tmp
+		tst	\irqstat, \irqstat       @ test if no active IRQ's
+		beq	1002f                    @ if no active irqs return with status 0
+		mov	\irqnr, #0               @ start from irq zero
+		mov	\tmp,   #1               @ the mask initially 1

arch/arm/mach-mcs814x/include/mach/hardware.h new file +1 -0

new file mode 100644
index 0000000..529f648
@@ -0,0 +1,16 @@
+/*

arch/arm/mach-mcs814x/include/mach/hardware.h new file +4 -0

+#include "mcs814x.h"
+
+#endif
+

arch/arm/mach-mcs814x/include/mach/irqs.h new file +5 -0

new file mode 100644
index 0000000..78021d1
@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2003 Artec Design Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as

arch/arm/mach-mcs814x/timer.c new file +123 -0

new file mode 100644
index 0000000..e8408e4
@@ -0,0 +1,133 @@
+/*
+ * Moschip MCS814x timer routines
+ *
+ * Copyright (C) 2012, Florian Fainelli <[email protected]>
+ *
+ * Licensed under GPLv2
+ */
+#include <linux/kernel.h>
+#include <linux/interrupt.h>
+#include <linux/time.h>
+#include <linux/timex.h>
+#include <linux/irq.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#include <asm/mach/time.h>
+#include <mach/mcs814x.h>
+
+/* Timer block registers */
+#define TIMER_VAL	0x00
+#define TIMER_CTL	0x04
+
+static u32 last_reload;
+static u32 timer_correct;
+static u32 clock_rate;
+static u32 timer_reload_value;
+static void __iomem *mcs814x_timer_base;
+
+static inline unsigned long ticks2usecs(u32 x)
+{
+	return x / (clock_rate / 1000000);
+}
+
+/*
+ * Returns number of ms since last clock interrupt.  Note that interrupts
+ * will have been disabled by do_gettimeoffset()
+ */
+static unsigned long mcs814x_gettimeoffset(void)
+{
+	u32 ticks = __raw_readl(mcs814x_timer_base + TIMER_VAL);
+
+	if (ticks < last_reload)
+		return ticks2usecs(ticks + (u32)(0xffffffff - last_reload));
+	else
+		return ticks2usecs(ticks - last_reload);
+}
+
+
+static irqreturn_t mcs814x_timer_interrupt(int irq, void *dev_id)
+{
+	u32 count = __raw_readl(mcs814x_timer_base + TIMER_VAL);
+
+	/* take into account delay up to this moment */
+	last_reload = count + timer_correct + timer_reload_value;
+
+	if (last_reload < timer_reload_value)
+		last_reload = timer_reload_value;
+	else if (timer_correct == 0)
+		timer_correct = __raw_readl(mcs814x_timer_base + TIMER_VAL) -
+					count;
+
+	__raw_writel(last_reload, mcs814x_timer_base + TIMER_VAL);
+
+	timer_tick();
+
+	return IRQ_HANDLED;
+}
+
+static struct irqaction mcs814x_timer_irq = {
+	.name		= "mcs814x-timer",
+	.flags		= IRQF_DISABLED | IRQF_TIMER | IRQF_IRQPOLL,
+	.handler	= mcs814x_timer_interrupt,
+};
+
+static struct of_device_id mcs814x_timer_ids[] = {
+	{ .compatible = "moschip,mcs814x-timer" },
+	{ /* sentinel */ },
+};
+
+static void __init mcs814x_of_timer_init(void)
+{
+	struct device_node *np;
+	const unsigned int *intspec;
+
+	np = of_find_matching_node(NULL, mcs814x_timer_ids);
+	if (!np)
+		panic("unable to find compatible timer node in dtb");
+
+	mcs814x_timer_base = of_iomap(np, 0);
+	if (!mcs814x_timer_base)
+		panic("unable to remap timer cpu registers");
+
+	intspec = of_get_property(np, "interrupts", NULL);
+	if (!intspec)
+		panic("no interrupts property for timer");
+
+	mcs814x_timer_irq.irq = be32_to_cpup(intspec);
+}
+
+static void __init mcs814x_timer_init(void)
+{
+	struct clk *clk;
+
+	clk = clk_get_sys("timer0", NULL);
+	if (IS_ERR_OR_NULL(clk))
+		panic("unable to get timer0 clock");
+
+	clock_rate = clk_get_rate(clk);
+	clk_put(clk);
+
+	mcs814x_of_timer_init();
+
+	pr_info("Timer frequency: %d (kHz)\n", clock_rate / 1000);
+
+	timer_reload_value = 0xffffffff - (clock_rate / HZ);
+
+	/* disable timer */
+	__raw_writel(0, mcs814x_timer_base + TIMER_CTL);
+	__raw_writel(timer_reload_value, mcs814x_timer_base + TIMER_VAL);
+	last_reload = timer_reload_value;

Hello Florian,

Le Sun, 15 Jul 2012 16:49:07 +0200,
Florian Fainelli <[email protected]> a ?crit :

arch/arm/mach-

Quoted 1 line

arch/arm/mach- renamed from arch/arm/mach-mcs814x/Makefile.boot +0 -0

arch/arm/mach-mcs814x/Makefile.boot line 3

Quoted 8 lines

new file mode 100644
index 0000000..414db8b

arch/arm/mach-mcs814x/Makefile.boot new file +3 -0

@@ -0,0 +1,3 @@
+    zreladdr-y := 0x00008000
+ params_phys-y := 0x00000008
+ initrd_phys-y := 0x00400000

params_phys-y and initrd_phys-y are useless for DT-based platforms, if I'm correct.

Quoted 6 lines

arch/arm/mach-mcs814x/clock.c new file +0 -0

new file mode 100644
index 0000000..eb30ae2

You should rather use the new clock framework instead of providing your own implementation of the clk_*() API. And therefore your clock driver should be in drivers/clk/ instead.

Quoted 40 lines

arch/arm/mach-mcs814x/clock.c new file +39 -0

+struct cpu_mode {
+	const char *name;
+	int gpio_start;
+	int gpio_end;
+};
+
+static const struct cpu_mode cpu_modes[] = {
+	{
+		.name		= "I2S",
+		.gpio_start	= 4,
+		.gpio_end	= 8,
+	},
+	{
+		.name		= "UART",
+		.gpio_start	= 4,
+		.gpio_end	= 9,
+	},
+	{
+		.name		= "External MII",
+		.gpio_start	= 0,
+		.gpio_end	= 16,
+	},
+	{
+		.name		= "Normal",
+		.gpio_start	= -1,
+		.gpio_end	= -1,
+	},
+};
+
+void __init mcs814x_init_machine(void)
+{
+	u32 bs2, cpu_mode;
+	int gpio;
+
+	bs2 = __raw_readl(mcs814x_sysdbg_base + SYSDBG_BS2);
+	cpu_mode = (bs2 >> CPU_MODE_SHIFT) & CPU_MODE_MASK;
+
+	pr_info("CPU mode: %s\n", cpu_modes[cpu_mode].name);
+

arch/arm/mach-mcs814x/clock.c

Quoted 5 lines

arch/arm/mach-mcs814x/clock.c new file +5 -0

+	/* request the gpios since the pins are muxed for functionnality */
+	for (gpio = cpu_modes[cpu_mode].gpio_start;
+		gpio == cpu_modes[cpu_mode].gpio_end; gpio++) {
+		if (gpio != -1)
+			gpio_request(gpio, cpu_modes[cpu_mode].name);

I am not sure here, but shouldn't this be done with the pinctrl subsystem instead?

arch/arm/mach-mcs814x/clock.c

Quoted 9 lines

arch/arm/mach-mcs814x/clock.c new file +8 -0

+void __init mcs814x_map_io(void)
+{
+	iotable_init(mcs814x_io_desc, ARRAY_SIZE(mcs814x_io_desc));
+
+	mcs814x_sysdbg_base = ioremap(MCS814X_IO_START + MCS814X_SYSDBG,
+					MCS814X_SYSDBG_SIZE);
+	if (!mcs814x_sysdbg_base)
+		panic("unable to remap sysdbg base");

Any reason to have a static mapping initialized with iotable_init() and then a dynamic mapping initialized with ioremap() ? I thought that ioremap() wasn't ready at the ->map_io() time, and it was therefore the reason we had static mappings.

Review

Quoted 2 lines

(quoted diff) +0 -0

Quoted 11 lines

new file mode 100644
index 0000000..cbad566

arch/arm/mach-mcs814x/include/mach/entry-macro.S new file +6 -0

@@ -0,0 +1,29 @@
+#include <mach/mcs814x.h>
+                .macro  disable_fiq
+                .endm
+
+		.macro arch_ret_to_user, tmp1, tmp2
+		.endm

arch/arm/mach-mcs814x/include/mach/entry-macro.S

Quoted 8 lines

arch/arm/mach-mcs814x/include/mach/entry-macro.S new file +8 -0

+
+		.macro  get_irqnr_preamble, base, tmp
+		ldr	\base, =mcs814x_intc_base@ base virtual address of INTC
+		ldr	\base, [\base]
+		.endm
+
+		.macro  get_irqnr_and_base, irqnr, irqstat, base, tmp
+		mov	\tmp, #MCS814X_IRQ_STS0	 @ load tmp with STS0 register 

arch/arm/mach-mcs814x/include/mach/entry-macro.S

Quoted 3 lines

arch/arm/mach-mcs814x/include/mach/entry-macro.S new file +3 -0

+		ldr	\irqstat, [\base, \tmp]	 @ load value at base + tmp
+		tst	\irqstat, \irqstat       @ test if no active IRQ's
+		beq	1002f                    @ if no active irqs return with 

arch/arm/mach-mcs814x/include/mach/entry-macro.S

Quoted 10 lines

arch/arm/mach-mcs814x/include/mach/entry-macro.S new file +10 -0

+		mov	\irqnr, #0               @ start from irq zero
+		mov	\tmp,   #1               @ the mask initially 1
+1001:
+		tst     \irqstat, \tmp           @ and with mask
+		addeq   \irqnr, \irqnr, #1       @ if  zero then add one to nr
+		moveq   \tmp,   \tmp, lsl #1     @ shift mask one to left
+		beq     1001b                    @ if  zero then loop again
+		mov     \irqstat, \tmp           @ save the return mask
+		mov	\tmp, #MCS814X_IRQ_STS0  @ load tmp with ICR offset
+		str     \irqstat,  [\base, \tmp] @ clear irq with selected 

arch/arm/mach-mcs814x/include/mach/entry-macro.S line 29

Quoted 2 lines

arch/arm/mach-mcs814x/include/mach/entry-macro.S new file +2 -0

+1002:
+                .endm

Hum, you should instead use the MULTI_IRQ_HANDLER feature so that this can be written in C.

arch/arm/mach-

Quoted 2 lines

arch/arm/mach- renamed from arch/arm/mach-mcs814x/include/mach/hardware.h +0 -0

Quoted 11 lines

new file mode 100644
index 0000000..529f648

arch/arm/mach-mcs814x/include/mach/hardware.h new file +6 -0

@@ -0,0 +1,16 @@
+/*
+ * Copyright (C) 2003 Artec Design Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.

arch/arm/mach-mcs814x/include/mach/hardware.h line 11

Quoted 5 lines

arch/arm/mach-mcs814x/include/mach/hardware.h new file +5 -0

+ *
+ */
+
+#ifndef __ASM_ARCH_HARDWARE_H
+#define __ASM_ARCH_HARDWARE_H

This #define name no longer looks consistent with where the file is located.

arch/arm/mach-

Quoted 6 lines

arch/arm/mach-mcs814x/include/mach/hardware.h new file +4 -0

+#include "mcs814x.h"
+
+#endif
+

arch/arm/mach- renamed from arch/arm/mach-mcs814x/include/mach/irqs.h +0 -0

Quoted 10 lines

new file mode 100644
index 0000000..78021d1

arch/arm/mach-mcs814x/include/mach/irqs.h new file +5 -0

@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2003 Artec Design Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as

arch/arm/mach-mcs814x/include/mach/irqs.h line 15

Quoted 10 lines

arch/arm/mach-mcs814x/include/mach/irqs.h new file +10 -0

+ * published by the Free Software Foundation.
+ *
+ */
+
+#ifndef __ASM_ARCH_IRQS_H
+#define __ASM_ARCH_IRQS_H
+
+#define FIQ_START	0
+
+#define NR_IRQS		32

I think this shouldn't be needed if you use SPARSE_IRQ support.

arch/arm/mach-mcs814x/include/mach/irqs.h line 20

Quoted 4 lines

arch/arm/mach-mcs814x/include/mach/irqs.h new file +4 -0

+#define IRQ_PCI_INTA            22
+#define IRQ_PCI_INTB            23
+#define IRQ_PCI_INTC            24
+#define IRQ_PCI_INTD            26

And these probably belong to the DT somehow?

Quoted 4 lines

arch/arm/mach-mcs814x/timer.c new file +0 -0

new file mode 100644
index 0000000..e8408e4

Quoted 28 lines

arch/arm/mach-mcs814x/timer.c new file +28 -0

+
+static void __init mcs814x_timer_init(void)
+{
+	struct clk *clk;
+
+	clk = clk_get_sys("timer0", NULL);
+	if (IS_ERR_OR_NULL(clk))
+		panic("unable to get timer0 clock");
+
+	clock_rate = clk_get_rate(clk);
+	clk_put(clk);
+
+	mcs814x_of_timer_init();
+
+	pr_info("Timer frequency: %d (kHz)\n", clock_rate / 1000);
+
+	timer_reload_value = 0xffffffff - (clock_rate / HZ);
+
+	/* disable timer */
+	__raw_writel(0, mcs814x_timer_base + TIMER_CTL);
+	__raw_writel(timer_reload_value, mcs814x_timer_base + TIMER_VAL);
+	last_reload = timer_reload_value;
+
+	setup_irq(mcs814x_timer_irq.irq, &mcs814x_timer_irq);
+	/* enable timer, stop timer in debug mode */
+	__raw_writel(0x03, mcs814x_timer_base + TIMER_CTL);
+}
+

arch/arm/mach-mcs814x/timer.c line 133

Quoted 4 lines

arch/arm/mach-mcs814x/timer.c new file +4 -0

+struct sys_timer mcs814x_timer = {
+	.init	= mcs814x_timer_init,
+	.offset	= mcs814x_gettimeoffset,
+};

I am surprised that this doesn't use the clocksource and clockevents infrastructure. It probably should, and be implemented in drivers/clocksource/.

Quoted 7 lines

arch/arm/mach-mcs814x/include/mach/irqs.h line 20

Quoted 5 lines

arch/arm/mach-mcs814x/include/mach/irqs.h new file +4 -0

+#define IRQ_PCI_INTA            22
+#define IRQ_PCI_INTB            23
+#define IRQ_PCI_INTC            24
+#define IRQ_PCI_INTD            26

And these probably belong to the DT somehow?

This would need a complete PCI irq mapping representation, so far we have the DT properties, but not the corresponding code to parse them. I would not qualify this as a major blocker.

Hi Thomas,

Quoted 6 lines · On Monday 16 July 2012 14:29:41 Thomas Petazzoni wrote:

Hello Florian,

Le Sun, 15 Jul 2012 16:49:07 +0200,
Florian Fainelli <[email protected]> a ?crit :

arch/arm/mach-

Quoted 1 line

arch/arm/mach- renamed from arch/arm/mach-mcs814x/Makefile.boot +0 -0

mcs814x/Makefile.boot

Quoted 11 lines

arch/arm/mach-mcs814x/Makefile.boot line 3

Quoted 8 lines

new file mode 100644
index 0000000..414db8b

arch/arm/mach-mcs814x/Makefile.boot new file +3 -0

@@ -0,0 +1,3 @@
+    zreladdr-y := 0x00008000
+ params_phys-y := 0x00000008
+ initrd_phys-y := 0x00400000

params_phys-y and initrd_phys-y are useless for DT-based platforms, if I'm correct.

Indeed, only zreladdr is actually required.

Hello Florian,

Quoted 47 lines

arch/arm/mach-mcs814x/timer.c new file +47 -0

+static void __init mcs814x_of_timer_init(void)
+{
+	struct device_node *np;
+	const unsigned int *intspec;
+
+	np = of_find_matching_node(NULL, mcs814x_timer_ids);
+	if (!np)
+		panic("unable to find compatible timer node in dtb");
+
+	mcs814x_timer_base = of_iomap(np, 0);
+	if (!mcs814x_timer_base)
+		panic("unable to remap timer cpu registers");
+
+	intspec = of_get_property(np, "interrupts", NULL);
+	if (!intspec)
+		panic("no interrupts property for timer");
+
+	mcs814x_timer_irq.irq = be32_to_cpup(intspec);
+}
+
+static void __init mcs814x_timer_init(void)
+{
+	struct clk *clk;
+
+	clk = clk_get_sys("timer0", NULL);
+	if (IS_ERR_OR_NULL(clk))
+		panic("unable to get timer0 clock");
+
+	clock_rate = clk_get_rate(clk);
+	clk_put(clk);
+
+	mcs814x_of_timer_init();
+
+	pr_info("Timer frequency: %d (kHz)\n", clock_rate / 1000);
+
+	timer_reload_value = 0xffffffff - (clock_rate / HZ);
+
+	/* disable timer */
+	__raw_writel(0, mcs814x_timer_base + TIMER_CTL);
+	__raw_writel(timer_reload_value, mcs814x_timer_base + TIMER_VAL);
+	last_reload = timer_reload_value;
+
+	setup_irq(mcs814x_timer_irq.irq, &mcs814x_timer_irq);
+	/* enable timer, stop timer in debug mode */
+	__raw_writel(0x03, mcs814x_timer_base + TIMER_CTL);
+}
+

arch/arm/mach-mcs814x/timer.c line 133

Quoted 4 lines

arch/arm/mach-mcs814x/timer.c new file +4 -0

+struct sys_timer mcs814x_timer = {
+	.init	= mcs814x_timer_init,
+	.offset	= mcs814x_gettimeoffset,
+};

I am surprised that this doesn't use the clocksource and clockevents infrastructure. It probably should, and be implemented in drivers/clocksource/.

[Thomas P]

Quoted 3 lines

I am surprised that this doesn't use the clocksource and clockevents infrastructure. It probably should, and be implemented in drivers/clocksource/.

This may sound like a stupid question, but the big constraint with this SoC is that the timers do not reload themselves, and so far, I did not find out how I can handle this properly with a clocksource. I suppose that it should not be a big problem for the clockevent device however.

Has anyone any example of such particular case?

arch/arm/mach-mcs814x/Kconfig new file +1 -0

new file mode 100644
index 0000000..c89422f
@@ -0,0 +1,11 @@
+if ARCH_MCS814X

+void __iomem *mcs814x_sysdbg_base;

arch/arm/mach-mcs814x/Kconfig new file +37 -0

+
+struct cpu_mode {
+	const char *name;
+	int gpio_start;
+	int gpio_end;
+};
+
+static const struct cpu_mode cpu_modes[] = {
+	{
+		.name		= "I2S",
+		.gpio_start	= 4,
+		.gpio_end	= 8,
+	},
+	{
+		.name		= "UART",
+		.gpio_start	= 4,
+		.gpio_end	= 9,
+	},
+	{
+		.name		= "External MII",
+		.gpio_start	= 0,
+		.gpio_end	= 16,
+	},
+	{
+		.name		= "Normal",
+		.gpio_start	= -1,
+		.gpio_end	= -1,
+	},
+};
+
+void __init mcs814x_init_machine(void)
+{
+	u32 bs2, cpu_mode;
+	int gpio;
+
+	bs2 = __raw_readl(mcs814x_sysdbg_base + SYSDBG_BS2);
+	cpu_mode = (bs2 >> CPU_MODE_SHIFT) & CPU_MODE_MASK;

arch/arm/mach-mcs814x/Kconfig new file +4 -0

+void __init mcs814x_map_io(void)
+{
+	iotable_init(mcs814x_io_desc, ARRAY_SIZE(mcs814x_io_desc));
+

arch/arm/mach-mcs814x/common.h new file +5 -0

new file mode 100644
index 0000000..e523abe
@@ -0,0 +1,15 @@
+#ifndef __ARCH_MCS814X_COMMON_H
+#define __ARCH_MCS814X_COMMON_H
+
+#include <asm/mach/time.h>
+

arch/arm/mach-mcs814x/include/mach/cpu.h new file +6 -0

new file mode 100644
index 0000000..1ef3c4a
@@ -0,0 +1,16 @@
+#ifndef __ASM_ARCH_CPU_H__
+#define __ASM_ARCH_CPU_H__
+
+#include <asm/cputype.h>
+
+#define MCS8140_ID	0x41069260	/* ARM926EJ-S */

arch/arm/mach-mcs814x/include/mach/hardware.h new file +5 -0

new file mode 100644
index 0000000..529f648
@@ -0,0 +1,16 @@
+/*
+ * Copyright (C) 2003 Artec Design Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as

arch/arm/mach-mcs814x/include/mach/irqs.h new file +3 -0

new file mode 100644
index 0000000..78021d1
@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2003 Artec Design Ltd.
+ *

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +5 -0

new file mode 100644
index 0000000..a4a369e
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2003 Artec Design Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +18 -0

+#define MCS814X_PHYS_BASE	0x40000000
+#define MCS814X_VIRT_BASE	MCS814X_IO_BASE
+
+#define MCS814X_UART		0x000DC000
+#define MCS814X_DBGLED		0x000EC000
+#define MCS814X_SYSDBG		0x000F8000
+#define MCS814X_SYSDBG_SIZE	0x50
+
+/* System configuration and bootstrap registers */
+#define SYSDBG_BS1		0x00
+#define  CPU_FREQ_SHIFT		27
+#define  CPU_FREQ_MASK		0x0F
+#define  SDRAM_FREQ_BIT		(1 << 22)
+
+#define SYSDBG_BS2		0x04
+#define  LED_CFG_MASK		0x03
+#define  CPU_MODE_SHIFT		23
+#define  CPU_MODE_MASK		0x03

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +13 -0

+#define MCS8140_PCI_HOST_BASE           0x80000000
+#define MCS8140_PCI_IOMISC_BASE         0x00000000
+#define MCS8140_PCI_PRE_BASE            0x10000000
+#define MCS8140_PCI_NONPRE_BASE         0x30000000
+
+#define MCS8140_PCI_CFG_BASE		(MCS8140_PCI_HOST_BASE + 0x04000000)
+#define MCS8140_PCI_IO_BASE		(MCS8140_PCI_HOST_BASE)
+
+#define MCS8140_PCI_IO_VIRT_BASE	(MCS814X_IO_BASE - \
+					 MCS8140_PCI_CONFIG_SIZE - \
+					 MCS8140_PCI_IOMISC_SIZE)
+#define MCS8140_PCI_CFG_VIRT_BASE	(MCS814X_IO_BASE - \
+					 MCS8140_PCI_CONFIG_SIZE)
 

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +13 -0

+static int mcs8140_pci_read_config(struct pci_bus *bus,
+					unsigned int devfn, int where,
+					int size, u32 *val)
+{
+	unsigned long v = 0xFFFFFFFF;
+	unsigned long addr = __pci_addr(bus, devfn, where);
+
+	if (addr != 0) {
+		switch (size) {
+		case 1:
+			v = __raw_readb(addr);
+			break;
+		case 2:

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +7 -0

+static int __init mcs8140_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
+{
+	int line = IRQ_PCI_INTA;
+
+	if (pin != 0) {
+		/* IRQ_PCIA - 22 */
+		if (pin == PCI_INTD)

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +4 -0

+static struct map_desc mcs8140_pci_io_desc[] __initdata = {
+	{
+		.virtual	= MCS8140_PCI_CFG_VIRT_BASE,
+		.pfn		= __phys_to_pfn(MCS8140_PCI_CFG_BASE),

Quoted 7 lines · Florian Fainelli

arch/arm/mach-mcs814x/Kconfig new file +1 -0

new file mode 100644
index 0000000..c89422f
@@ -0,0 +1,11 @@
+if ARCH_MCS814X

arch/arm/mach-mcs814x/Kconfig line 11

Quoted 10 lines

arch/arm/mach-mcs814x/Kconfig new file +10 -0

+
+config MCS8140
+     select CPU_ARM926T
+     bool
+
+menu "Moschip MCS8140 boards"
+
+endmenu
+
+endif

What values does the Kconfig file actually provide? I see that you are adding the boards here later, but I think it would be nice to just make the new platform a single Kconfig option with no individual board selection.

arch/arm/mach-mcs814x/Kconfig

Quoted 9 lines

arch/arm/mach-mcs814x/Kconfig new file +9 -0

+struct clk {
+     struct clk *parent;             /* parent clk */
+     unsigned long rate;             /* clock rate in Hz */
+     unsigned long divider;          /* clock divider */
+     u32 usecount;                   /* reference count */
+     struct clk_ops *ops;            /* clock operation */
+     u32 enable_reg;                 /* clock enable register */
+     u32 enable_mask;                /* clock enable mask */
+};

Platforms are now converting to the common clock framework in drivers/clk. Mike Turquette as the subsystem maintainer can probably judge better whether we should still be allowing new platforms with their own implementation of clk, but my feeling is that you should use the subsystem instead and add a driver in a subdirectory of drivers/clk instead of in the platform.

On Mon, Jul 16, 2012 at 8:54 AM, Arnd Bergmann <[email protected]> wrote:

Quoted 14 lines

board selection.

arch/arm/mach-mcs814x/clock.c line 39

Quoted 9 lines

arch/arm/mach-mcs814x/clock.c new file +9 -0

+struct clk {
+     struct clk *parent;             /* parent clk */
+     unsigned long rate;             /* clock rate in Hz */
+     unsigned long divider;          /* clock divider */
+     u32 usecount;                   /* reference count */
+     struct clk_ops *ops;            /* clock operation */
+     u32 enable_reg;                 /* clock enable register */
+     u32 enable_mask;                /* clock enable mask */
+};

Platforms are now converting to the common clock framework in drivers/clk. Mike Turquette as the subsystem maintainer can probably judge better

Quoted 4 lines

we should still be allowing new platforms with their own implementation of clk, but my feeling is that you should use the subsystem instead and add a driver in a subdirectory of drivers/clk instead of in the platform.

Hi Florian & Arnd,

Adopting the new clock framework is highly preferable, especially if MCS814x support is being delayed until 3.7. There is also a push to put clock drivers into drivers/clk and I would like to continue that trend, but I'm less strict on that measure if it proves very difficult for your platform immediately (e.g. duplicating headers across platform and generic code, etc). Migrating code from arch/arm to drivers/clk can always be done in a future series.

This platform also seems to be making use of DT so it would be very nice if we use MCS814x to push the state of clk DT bindings and adoption forward a bit. There are not a lot of folks using the patches from Rob/Grant today. Hopefully delaying these patches by another merge cycle means that these requests are not asking too much :-)

On Mon, Jul 16, 2012 at 8:54 AM, Arnd Bergmann <[email protected]> wrote:

Quoted 14 lines

board selection.

arch/arm/mach-mcs814x/clock.c line 39

Quoted 9 lines

arch/arm/mach-mcs814x/clock.c new file +9 -0

+struct clk {
+     struct clk *parent;             /* parent clk */
+     unsigned long rate;             /* clock rate in Hz */
+     unsigned long divider;          /* clock divider */
+     u32 usecount;                   /* reference count */
+     struct clk_ops *ops;            /* clock operation */
+     u32 enable_reg;                 /* clock enable register */
+     u32 enable_mask;                /* clock enable mask */
+};

Platforms are now converting to the common clock framework in drivers/clk. Mike Turquette as the subsystem maintainer can probably judge better

Quoted 4 lines

we should still be allowing new platforms with their own implementation of clk, but my feeling is that you should use the subsystem instead and add a driver in a subdirectory of drivers/clk instead of in the platform.

Hi Florian & Arnd,

Adopting the new clock framework is highly preferable, especially if MCS814x support is being delayed until 3.7. There is also a push to put clock drivers into drivers/clk and I would like to continue that trend, but I'm less strict on that measure if it proves very difficult for your platform immediately (e.g. duplicating headers across platform and generic code, etc). Migrating code from arch/arm to drivers/clk can always be done in a future series.

This platform also seems to be making use of DT so it would be very nice if we use MCS814x to push the state of clk DT bindings and adoption forward a bit. There are not a lot of folks using the patches from Rob/Grant today. Hopefully delaying these patches by another merge cycle means that these requests are not asking too much :-)

Quoted 37 lines

arch/arm/mach-mcs814x/common.c new file +37 -0

+static const struct cpu_mode cpu_modes[] = {
+     {
+             .name           = "I2S",
+             .gpio_start     = 4,
+             .gpio_end       = 8,
+     },
+     {
+             .name           = "UART",
+             .gpio_start     = 4,
+             .gpio_end       = 9,
+     },
+     {
+             .name           = "External MII",
+             .gpio_start     = 0,
+             .gpio_end       = 16,
+     },
+     {
+             .name           = "Normal",
+             .gpio_start     = -1,
+             .gpio_end       = -1,
+     },
+};
+
+void __init mcs814x_init_machine(void)
+{
+     u32 bs2, cpu_mode;
+     int gpio;
+
+     bs2 = __raw_readl(mcs814x_sysdbg_base + SYSDBG_BS2);
+     cpu_mode = (bs2 >> CPU_MODE_SHIFT) & CPU_MODE_MASK;
+
+     pr_info("CPU mode: %s\n", cpu_modes[cpu_mode].name);
+
+     /* request the gpios since the pins are muxed for functionnality */
+     for (gpio = cpu_modes[cpu_mode].gpio_start;
+             gpio == cpu_modes[cpu_mode].gpio_end; gpio++) {
+             if (gpio != -1)

arch/arm/mach-mcs814x/common.c line 82

Quoted 3 lines

arch/arm/mach-mcs814x/common.c new file +3 -0

+                     gpio_request(gpio, cpu_modes[cpu_mode].name);
+     }
+}

This looks like a very simple instance of a pinmux driver. I wonder if it's worth doing an actual pinctrl driver that knows about these modes and about the gpio handling of the platform. Maybe Linus Walleij can comment on that.

Quoted 37 lines · Arnd Bergmann

Quoted 37 lines

arch/arm/mach-mcs814x/common.c new file +37 -0

+static const struct cpu_mode cpu_modes[] = {
+     {
+             .name           = "I2S",
+             .gpio_start     = 4,
+             .gpio_end       = 8,
+     },
+     {
+             .name           = "UART",
+             .gpio_start     = 4,
+             .gpio_end       = 9,
+     },
+     {
+             .name           = "External MII",
+             .gpio_start     = 0,
+             .gpio_end       = 16,
+     },
+     {
+             .name           = "Normal",
+             .gpio_start     = -1,
+             .gpio_end       = -1,
+     },
+};
+
+void __init mcs814x_init_machine(void)
+{
+     u32 bs2, cpu_mode;
+     int gpio;
+
+     bs2 = __raw_readl(mcs814x_sysdbg_base + SYSDBG_BS2);
+     cpu_mode = (bs2 >> CPU_MODE_SHIFT) & CPU_MODE_MASK;
+
+     pr_info("CPU mode: %s\n", cpu_modes[cpu_mode].name);
+
+     /* request the gpios since the pins are muxed for functionnality */
+     for (gpio = cpu_modes[cpu_mode].gpio_start;
+             gpio == cpu_modes[cpu_mode].gpio_end; gpio++) {
+             if (gpio != -1)

Quoted 8 lines

arch/arm/mach-mcs814x/common.c line 82

Quoted 3 lines

arch/arm/mach-mcs814x/common.c new file +3 -0

+                     gpio_request(gpio, cpu_modes[cpu_mode].name);
+     }
+}

This looks like a very simple instance of a pinmux driver. I wonder if it's worth doing an actual pinctrl driver that knows about these modes and about the gpio handling of the platform. Maybe Linus Walleij can comment on that.

It is worth, because usually it is not simple at all but these are, as I highly suspect, the first few entries to get the platform up and running. Then it's going to need to add another one, and then another one, and then break it into a separate file, and then it's a custom pin controller implementation all over again

Nothing is forbidding a simple pin controller implementation in drivers/pinctrl, just a few tens of lines is perfectly OK with me.

Documentation/pinctrl.txt is the friend at all times. arch/arm/mach-u300/core.c shows how a simple set-up at boot time can be achieved using so-called pinctrl hogs.

Yours,
Linus Walleij

Quoted 14 lines · Florian Fainelli

arch/arm/mach-mcs814x/Kconfig new file +8 -0

new file mode 100644
index 0000000..c89422f
@@ -0,0 +1,11 @@
+if ARCH_MCS814X
+
+config MCS8140
+	select CPU_ARM926T
+	bool
+
+menu "Moschip MCS8140 boards"
+

arch/arm/mach-mcs814x/Kconfig line 11

Quoted 3 lines

arch/arm/mach-mcs814x/Kconfig new file +3 -0

+endmenu
+
+endif

What values does the Kconfig file actually provide? I see that you are adding the boards here later, but I think it would be nice to just make the new platform a single Kconfig option with no individual board selection.

arch/arm/mach-mcs814x/Kconfig

Quoted 10 lines

arch/arm/mach-mcs814x/Kconfig new file +9 -0

+struct clk {
+	struct clk *parent;		/* parent clk */
+	unsigned long rate;		/* clock rate in Hz */
+	unsigned long divider;		/* clock divider */
+	u32 usecount;			/* reference count */
+	struct clk_ops *ops;		/* clock operation */
+	u32 enable_reg;			/* clock enable register */
+	u32 enable_mask;		/* clock enable mask */
+};

Platforms are now converting to the common clock framework in drivers/clk. Mike Turquette as the subsystem maintainer can probably judge better whether we should still be allowing new platforms with their own implementation of clk, but my feeling is that you should use the subsystem instead and add a driver in a subdirectory of drivers/clk instead of in the platform.

Quoted 1 line

+void __iomem *mcs814x_sysdbg_base;

Does this have to be globally visible? I would recommend making it static.

Quoted 38 lines

arch/arm/mach-mcs814x/Kconfig new file +37 -0

+
+struct cpu_mode {
+	const char *name;
+	int gpio_start;
+	int gpio_end;
+};
+
+static const struct cpu_mode cpu_modes[] = {
+	{
+		.name		= "I2S",
+		.gpio_start	= 4,
+		.gpio_end	= 8,
+	},
+	{
+		.name		= "UART",
+		.gpio_start	= 4,
+		.gpio_end	= 9,
+	},
+	{
+		.name		= "External MII",
+		.gpio_start	= 0,
+		.gpio_end	= 16,
+	},
+	{
+		.name		= "Normal",
+		.gpio_start	= -1,
+		.gpio_end	= -1,
+	},
+};
+
+void __init mcs814x_init_machine(void)
+{
+	u32 bs2, cpu_mode;
+	int gpio;
+
+	bs2 = __raw_readl(mcs814x_sysdbg_base + SYSDBG_BS2);
+	cpu_mode = (bs2 >> CPU_MODE_SHIFT) & CPU_MODE_MASK;

arch/arm/mach-mcs814x/Kconfig

Quoted 10 lines

arch/arm/mach-mcs814x/Kconfig new file +10 -0

+
+	pr_info("CPU mode: %s\n", cpu_modes[cpu_mode].name);
+
+	/* request the gpios since the pins are muxed for functionnality */
+	for (gpio = cpu_modes[cpu_mode].gpio_start;
+		gpio == cpu_modes[cpu_mode].gpio_end; gpio++) {
+		if (gpio != -1)
+			gpio_request(gpio, cpu_modes[cpu_mode].name);
+	}
+}

This looks like a very simple instance of a pinmux driver. I wonder if it's worth doing an actual pinctrl driver that knows about these modes and about the gpio handling of the platform. Maybe Linus Walleij can comment on that.

How is the cpu mode managed? Is it hardwired through something like strapping pins, or could you have a system that sets it dynamically based on what hardware is being plugged in?

Quoted 10 lines

arch/arm/mach-mcs814x/Kconfig new file +9 -0

+void __init mcs814x_map_io(void)
+{
+	iotable_init(mcs814x_io_desc, ARRAY_SIZE(mcs814x_io_desc));
+
+	mcs814x_sysdbg_base = ioremap(MCS814X_IO_START + MCS814X_SYSDBG,
+					MCS814X_SYSDBG_SIZE);
+	if (!mcs814x_sysdbg_base)
+		panic("unable to remap sysdbg base");
+}

arch/arm/mach-mcs814x/Kconfig

Quoted 5 lines

arch/arm/mach-mcs814x/Kconfig new file +5 -0

+
+void mcs814x_restart(char mode, const char *cmd)
+{
+	__raw_writel(~(1 << 31), mcs814x_sysdbg_base);
+}

You should generally avoid using __raw_readl etc. and instead use readl/write or readl_relaxed/writel_relaxed.

arch/arm/mach-

Quoted 2 lines

arch/arm/mach- renamed from arch/arm/mach-mcs814x/common.h +0 -0

Quoted 10 lines

new file mode 100644
index 0000000..e523abe

arch/arm/mach-mcs814x/common.h new file +5 -0

@@ -0,0 +1,15 @@
+#ifndef __ARCH_MCS814X_COMMON_H
+#define __ARCH_MCS814X_COMMON_H
+
+#include <asm/mach/time.h>
+

arch/arm/mach-mcs814x/common.h line 15

Quoted 10 lines

arch/arm/mach-mcs814x/common.h new file +10 -0

+void mcs814x_map_io(void);
+void mcs814x_clk_init(void);
+void mcs814x_of_irq_init(void);
+void mcs814x_init_machine(void);
+void mcs814x_handle_irq(struct pt_regs *regs);
+void mcs814x_restart(char mode, const char *cmd);
+extern struct sys_timer mcs814x_timer;
+extern void __iomem *mcs814x_sysdbg_base;
+
+#endif /* __ARCH_MCS814X_COMMON_H */

I think a lot of these don't actually need to be global if you combine some of the files.

arch/arm/mach-

Quoted 1 line

arch/arm/mach- renamed from arch/arm/mach-mcs814x/include/mach/cpu.h +0 -0

Quoted 16 lines

new file mode 100644
index 0000000..1ef3c4a

arch/arm/mach-mcs814x/include/mach/cpu.h new file +11 -0

@@ -0,0 +1,16 @@
+#ifndef __ASM_ARCH_CPU_H__
+#define __ASM_ARCH_CPU_H__
+
+#include <asm/cputype.h>
+
+#define MCS8140_ID	0x41069260	/* ARM926EJ-S */
+#define MCS814X_MASK	0xff0ffff0
+
+#ifdef CONFIG_MCS8140
+/* Moschip MCS8140 is based on an ARM926EJ-S core */
+#define soc_is_mcs8140()	((read_cpuid_id() & MCS814X_MASK) == MCS8140_ID)

arch/arm/mach-mcs814x/include/mach/cpu.h line 16

Quoted 5 lines

arch/arm/mach-mcs814x/include/mach/cpu.h new file +5 -0

+#else
+#define soc_is_mcs8140()	(0)
+#endif /* !CONFIG_MCS8140 */
+
+#endif /* __ASM_ARCH_CPU_H__ */

Can you explain what this is used for? Are there other SoCs in the same platform that will get added later?

arch/arm/mach-

Quoted 2 lines

arch/arm/mach- renamed from arch/arm/mach-mcs814x/include/mach/hardware.h +0 -0

Quoted 15 lines

new file mode 100644
index 0000000..529f648

arch/arm/mach-mcs814x/include/mach/hardware.h new file +10 -0

@@ -0,0 +1,16 @@
+/*
+ * Copyright (C) 2003 Artec Design Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#ifndef __ASM_ARCH_HARDWARE_H

arch/arm/mach-mcs814x/include/mach/hardware.h line 15

Quoted 5 lines

arch/arm/mach-mcs814x/include/mach/hardware.h new file +5 -0

+#define __ASM_ARCH_HARDWARE_H
+
+#include "mcs814x.h"
+
+#endif

I'd just make this an empty file, like gpio.h. That will let us kill off these files more easily in the future.

arch/arm/mach-

Quoted 2 lines

arch/arm/mach- renamed from arch/arm/mach-mcs814x/include/mach/irqs.h +0 -0

Quoted 12 lines

new file mode 100644
index 0000000..78021d1

arch/arm/mach-mcs814x/include/mach/irqs.h new file +7 -0

@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2003 Artec Design Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *

arch/arm/mach-mcs814x/include/mach/irqs.h line 13

Quoted 6 lines

arch/arm/mach-mcs814x/include/mach/irqs.h new file +6 -0

+ */
+
+#ifndef __ASM_ARCH_IRQS_H
+#define __ASM_ARCH_IRQS_H
+
+#define FIQ_START	0

Why do you need the FIQ_START macro here?

arch/arm/mach-mcs814x/include/mach/irqs.h line 20

Quoted 7 lines

arch/arm/mach-mcs814x/include/mach/irqs.h new file +6 -0

+#define NR_IRQS		32
+
+#define IRQ_PCI_INTA            22
+#define IRQ_PCI_INTB            23
+#define IRQ_PCI_INTC            24
+#define IRQ_PCI_INTD            26

The PCI interrupts should come from the device tree.

You should also select CONFIG_SPARSE_IRQ unconditionally and use IRQ domains so that you no longer need to set NR_IRQS.

arch/arm/mach-

Quoted 2 lines

arch/arm/mach- renamed from arch/arm/mach-mcs814x/include/mach/mcs814x.h +0 -0

Quoted 15 lines

new file mode 100644
index 0000000..a4a369e

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +10 -0

@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2003 Artec Design Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#ifndef __ASM_ARCH_MCS814X_H

arch/arm/mach-mcs814x/include/mach/mcs814x.h line 15

Quoted 5 lines

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +5 -0

+#define __ASM_ARCH_MCS814X_H
+
+#define MCS814X_IO_BASE		0xF0000000
+#define MCS814X_IO_START	0x40000000
+#define MCS814X_IO_SIZE		0x00100000

What are these required for in the global space? Can you move them into a single .c file?

arch/arm/mach-mcs814x/include/mach/mcs814x.h line 21

Quoted 6 lines

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +5 -0

+/* IRQ controller register offset */
+#define MCS814X_IRQ_ICR		0x00
+#define MCS814X_IRQ_ISR		0x04
+#define MCS814X_IRQ_MASK	0x20
+#define MCS814X_IRQ_STS0	0x40

I'm pretty sure these can be in irq.c

Quoted 19 lines

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +18 -0

+#define MCS814X_PHYS_BASE	0x40000000
+#define MCS814X_VIRT_BASE	MCS814X_IO_BASE
+
+#define MCS814X_UART		0x000DC000
+#define MCS814X_DBGLED		0x000EC000
+#define MCS814X_SYSDBG		0x000F8000
+#define MCS814X_SYSDBG_SIZE	0x50
+
+/* System configuration and bootstrap registers */
+#define SYSDBG_BS1		0x00
+#define  CPU_FREQ_SHIFT		27
+#define  CPU_FREQ_MASK		0x0F
+#define  SDRAM_FREQ_BIT		(1 << 22)
+
+#define SYSDBG_BS2		0x04
+#define  LED_CFG_MASK		0x03
+#define  CPU_MODE_SHIFT		23
+#define  CPU_MODE_MASK		0x03

arch/arm/mach-mcs814x/include/mach/mcs814x.h line 50

Quoted 10 lines

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +10 -0

+
+#define SYSDBG_SYSCTL_MAC	0x1d
+#define  BUF_SHIFT_BIT		(1 << 0)
+
+#define SYSDBG_SYSCTL		0x08
+#define  SYSCTL_EMAC		(1 << 0)
+#define  SYSCTL_EPHY		(1 << 0) /* active low */
+#define  SYSCTL_CIPHER		(1 << 16)
+
+#define SYSDBG_PLL_CTL		0x3C

The sysdbg stuff can probably go into a file that deals with the sysdbg registers and exports a high-level interface.

arch/arm/mach-mcs814x/include/mach/mcs814x.h line 50

Quoted 2 lines

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +2 -0

+
+void __iomem *mcs814x_intc_base;

static?

arch/arm/mach-mcs814x/include/mach/mcs814x.h

Quoted 7 lines

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +6 -0

+#define MCS8140_PCI_HOST_BASE           0x80000000
+#define MCS8140_PCI_IOMISC_BASE         0x00000000
+#define MCS8140_PCI_PRE_BASE            0x10000000
+#define MCS8140_PCI_NONPRE_BASE         0x30000000
+
+#define MCS8140_PCI_CFG_BASE		(MCS8140_PCI_HOST_BASE + 

Quoted 8 lines

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +7 -0

+#define MCS8140_PCI_IO_BASE		(MCS8140_PCI_HOST_BASE)
+
+#define MCS8140_PCI_IO_VIRT_BASE	(MCS814X_IO_BASE - \
+					 MCS8140_PCI_CONFIG_SIZE - \
+					 MCS8140_PCI_IOMISC_SIZE)
+#define MCS8140_PCI_CFG_VIRT_BASE	(MCS814X_IO_BASE - \
+					 MCS8140_PCI_CONFIG_SIZE)
 

arch/arm/mach-mcs814x/include/mach/mcs814x.h

Quoted 10 lines

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +9 -0

 
+static unsigned long __pci_addr(struct pci_bus *bus,
+				unsigned int devfn, int offset)
+{
+	unsigned int busnr = bus->number;
+	unsigned int slot;
+
+	/* we only support bus 0 */
+	if (busnr != 0)
+		return 0;

Why? A lot of devices have bridges on them and they should just work if you take out this check.

arch/arm/mach-mcs814x/include/mach/mcs814x.h

Quoted 4 lines

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +4 -0

+	/*
+	 * Trap out illegal values
+	 */
+	BUG_ON(devfn > 255 || busnr > 255 || devfn > 255);

You're checking devfn twice...

arch/arm/mach-mcs814x/include/mach/mcs814x.h

Quoted 8 lines

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +8 -0

+	/* Scan 3 slots */
+	slot = PCI_SLOT(devfn);
+	switch (slot) {
+	case 1:
+	case 2:
+	case 3:
+		if (PCI_FUNC(devfn) >= 4)
+			return 0;

This also looks bogus. Why limit it to three devices?

Quoted 17 lines

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +17 -0

+static int mcs8140_pci_read_config(struct pci_bus *bus,
+					unsigned int devfn, int where,
+					int size, u32 *val)
+{
+	unsigned long v = 0xFFFFFFFF;
+	unsigned long addr = __pci_addr(bus, devfn, where);
+
+	if (addr != 0) {
+		switch (size) {
+		case 1:
+			v = __raw_readb(addr);
+			break;
+		case 2:
+			addr &= ~1;
+			v = __raw_readw(addr);
+			break;
+		default:

arch/arm/mach-mcs814x/include/mach/mcs814x.h

Quoted 6 lines

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +6 -0

+			addr &= ~3;
+			v = __raw_readl(addr);
+			break;
+		}
+	} else
+		v = 0xffffffff;

This is just mmconfig, right? Don't we have a generic implementation for

arch/arm/mach-mcs814x/include/mach/mcs814x.h

Quoted 8 lines

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +7 -0

+
+static struct resource io_mem = {
+	.name	= "PCI I/O space",
+	.start	= MCS8140_PCI_HOST_BASE + MCS8140_PCI_IOMISC_BASE,
+	.end	= MCS8140_PCI_HOST_BASE + MCS8140_PCI_IOMISC_BASE + SZ_64M,
+	.flags	= IORESOURCE_IO,
+};

This is wrong: MCS8140_PCI_HOST_BASE is an address in IORESOURCE_MEM space. You probably also mean SZ_64K instead of SZ_64M.

arch/arm/mach-mcs814x/include/mach/mcs814x.h

Quoted 10 lines

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +9 -0

+int __init pci_mcs8140_setup(int nr, struct pci_sys_data *sys)
+{
+	int ret = 0;
+	u32 val;
+
+	if (nr > 0)
+		return 0;
+
+	sys->mem_offset = MCS8140_PCI_IO_VIRT_BASE - MCS8140_PCI_IO_BASE;

Your address spaces are very confusing (or confused). So you have defined

#define MCS8140_PCI_IO_VIRT_BASE       (MCS814X_IO_BASE - \
                                        MCS8140_PCI_CONFIG_SIZE - \
                                        MCS8140_PCI_IOMISC_SIZE)
#define MCS8140_PCI_CFG_VIRT_BASE      (MCS814X_IO_BASE - \
                                        MCS8140_PCI_CONFIG_SIZE)

which means you mem_offset is (MCS814X_IO_BASE - MCS8140_PCI_CONFIG_SIZE - MCS8140_PCI_IOMISC_SIZE) - (MCS814X_IO_BASE - MCS8140_PCI_CONFIG_SIZE) which is (-MCS8140_PCI_IOMISC_SIZE), which in turn evaluates to -SZ_64M or 0xfc000000. If that is the correct value, I'm sure it's just coincidence ;-)

Quoted 1 line

+static int __init mcs8140_map_irq(const struct pci_dev *dev, u8 slot, u8

Quoted 6 lines

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +6 -0

+{
+	int line = IRQ_PCI_INTA;
+
+	if (pin != 0) {
+		/* IRQ_PCIA - 22 */
+		if (pin == PCI_INTD)

arch/arm/mach-mcs814x/include/mach/mcs814x.h

Quoted 10 lines

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +10 -0

+			line = IRQ_PCI_INTA + pin; /* IRQ_PCIA - 22 */
+		else
+			line = IRQ_PCI_INTA + pin - 1; /* IRQ_PCIA - 22 */
+	}
+
+	pr_info("PCI: Map interrupt slot 0x%02x pin 0x%02x line 0x%02x\n",
+		slot, pin, line);
+
+	return line;
+}

This looks like a very unusual mapping. Maybe it's just wrong and that explains why you don't support child buses or additional slots?

In any case, you should use an "interrupt-map" in the device tree that describes how the IRQs are mapped to the slots. I think we have generic code to deal with that already.

Quoted 4 lines

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +4 -0

+static struct map_desc mcs8140_pci_io_desc[] __initdata = {
+	{
+		.virtual	= MCS8140_PCI_CFG_VIRT_BASE,
+		.pfn		= __phys_to_pfn(MCS8140_PCI_CFG_BASE),

arch/arm/mach-mcs814x/include/mach/mcs814x.h

Quoted 10 lines

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +10 -0

+		.length		= MCS8140_PCI_CONFIG_SIZE,
+		.type		= MT_DEVICE
+	},
+	{
+		.virtual	= MCS8140_PCI_IO_VIRT_BASE,
+		.pfn		= __phys_to_pfn(MCS8140_PCI_IO_BASE),
+		.length		= MCS8140_PCI_IOMISC_SIZE,
+		.type		= MT_DEVICE
+	},
+};

Please have a look at the latest patches from Rob Herring about mapping the I/O space. I don't think you need to map the config space early because you don't rely on a fixed location for that. Just use of_iomap() to find that and store the pointer to the config space somewhere.

arch/arm/mach-mcs814x/include/mach/mcs814x.h

Quoted 1 line

arch/arm/mach-mcs814x/include/mach/mcs814x.h new file +1 -0

+	pcibios_min_io = MCS8140_PCI_HOST_BASE;

Leave this one at the default of 0x1000. MCS8140_PCI_HOST_BASE is the address from the CPU's point of view, not from the bus. You just need to stay out of the range of possible legacy ISA devices.

It's probably a good idea to keep the PCI support as a separate patch for reviewing purposes.

Arnd

Quoted 16 lines · On Monday 16 July 2012 15:54:04 Arnd Bergmann wrote:

Quoted 14 lines · Florian Fainelli

arch/arm/mach-mcs814x/Kconfig new file +8 -0

new file mode 100644
index 0000000..c89422f
@@ -0,0 +1,11 @@
+if ARCH_MCS814X
+
+config MCS8140
+	select CPU_ARM926T
+	bool
+
+menu "Moschip MCS8140 boards"
+

Quoted 8 lines

arch/arm/mach-mcs814x/Kconfig line 11

Quoted 3 lines

arch/arm/mach-mcs814x/Kconfig new file +3 -0

+endmenu
+
+endif

What values does the Kconfig file actually provide? I see that you are adding the boards here later, but I think it would be nice to just make the new platform a single Kconfig option with no individual board selection.

I plan on adding support for MCS8142 later on, thus I thought it would be nice to be able to selectively compile board support for MCS8140 or some later chip.

Quoted 7 lines

Platforms are now converting to the common clock framework in drivers/clk. Mike Turquette as the subsystem maintainer can probably judge better whether we should still be allowing new platforms with their own implementation of clk, but my feeling is that you should use the subsystem instead and add a driver in a subdirectory of drivers/clk instead of in the platform.

Quoted 3 lines

Quoted 1 line

+void __iomem *mcs814x_sysdbg_base;

Does this have to be globally visible? I would recommend making it static.

Yes, since it is used both by clock.c and common.c

Quoted 9 lines

Quoted 9 lines

arch/arm/mach-mcs814x/Kconfig new file +9 -0

+void __init mcs814x_map_io(void)
+{
+	iotable_init(mcs814x_io_desc, ARRAY_SIZE(mcs814x_io_desc));
+
+	mcs814x_sysdbg_base = ioremap(MCS814X_IO_START + MCS814X_SYSDBG,
+					MCS814X_SYSDBG_SIZE);
+	if (!mcs814x_sysdbg_base)
+		panic("unable to remap sysdbg base");
+}

Quoted 8 lines

arch/arm/mach-mcs814x/Kconfig

Quoted 5 lines

arch/arm/mach-mcs814x/Kconfig new file +5 -0

+
+void mcs814x_restart(char mode, const char *cmd)
+{
+	__raw_writel(~(1 << 31), mcs814x_sysdbg_base);
+}

You should generally avoid using __raw_readl etc. and instead use readl/write or readl_relaxed/writel_relaxed.

Allright, even on an ARM926-EJS-based system?

Quoted 1 line

arch/arm/mach-

Quoted 1 line

arch/arm/mach- renamed from arch/arm/mach-mcs814x/include/mach/cpu.h +0 -0

mcs814x/include/mach/cpu.h

Quoted 16 lines

Quoted 16 lines

new file mode 100644
index 0000000..1ef3c4a

arch/arm/mach-mcs814x/include/mach/cpu.h new file +11 -0

@@ -0,0 +1,16 @@
+#ifndef __ASM_ARCH_CPU_H__
+#define __ASM_ARCH_CPU_H__
+
+#include <asm/cputype.h>
+
+#define MCS8140_ID	0x41069260	/* ARM926EJ-S */
+#define MCS814X_MASK	0xff0ffff0
+
+#ifdef CONFIG_MCS8140
+/* Moschip MCS8140 is based on an ARM926EJ-S core */
+#define soc_is_mcs8140()	((read_cpuid_id() & MCS814X_MASK) == MCS8140_ID)

Quoted 8 lines

arch/arm/mach-mcs814x/include/mach/cpu.h line 16

Quoted 5 lines

arch/arm/mach-mcs814x/include/mach/cpu.h new file +5 -0

+#else
+#define soc_is_mcs8140()	(0)
+#endif /* !CONFIG_MCS8140 */
+
+#endif /* __ASM_ARCH_CPU_H__ */

Can you explain what this is used for? Are there other SoCs in the same platform that will get added later?

Yes, that's the plan.

Quoted 7 lines

arch/arm/mach-mcs814x/include/mach/cpu.h

Quoted 5 lines

arch/arm/mach-mcs814x/include/mach/cpu.h new file +5 -0

+/* IRQ controller register offset */
+#define MCS814X_IRQ_ICR		0x00
+#define MCS814X_IRQ_ISR		0x04
+#define MCS814X_IRQ_MASK	0x20
+#define MCS814X_IRQ_STS0	0x40

I'm pretty sure these can be in irq.c

Not unless I switch to MULTI_IRQ_HANDLER.

Quoted 10 lines

arch/arm/mach-mcs814x/include/mach/cpu.h

Quoted 7 lines

arch/arm/mach-mcs814x/include/mach/cpu.h new file +7 -0

+
+static struct resource io_mem = {
+	.name	= "PCI I/O space",
+	.start	= MCS8140_PCI_HOST_BASE + MCS8140_PCI_IOMISC_BASE,
+	.end	= MCS8140_PCI_HOST_BASE + MCS8140_PCI_IOMISC_BASE + SZ_64M,
+	.flags	= IORESOURCE_IO,
+};

This is wrong: MCS8140_PCI_HOST_BASE is an address in IORESOURCE_MEM space. You probably also mean SZ_64K instead of SZ_64M.

I will double check with the hardware manual, but I really think it is 64MBytes.

Quoted 5 lines

Leave this one at the default of 0x1000. MCS8140_PCI_HOST_BASE is the address from the CPU's point of view, not from the bus. You just need to stay out of the range of possible legacy ISA devices.

Quoted 2 lines

It's probably a good idea to keep the PCI support as a separate patch for reviewing purposes.

Good idea, I will do that. Note that none of the boards I have actually have PCI, so the code is just there in case someone needs it. I could also go along and remove it, since I cannot test it, which explains why there are so many inconsistencies with it.

Quoted 4 lines · Florian Fainelli

Quoted 3 lines · On Monday 16 July 2012 15:54:04 Arnd Bergmann wrote:

arch/arm/mach-

Quoted 1 line · Florian Fainelli

arch/arm/mach- renamed from arch/arm/mach-mcs814x/Kconfig +0 -0

Quoted 22 lines

Quoted 21 lines

Quoted 6 lines

new file mode 100644
index 0000000..c89422f

arch/arm/mach-mcs814x/Kconfig new file +1 -0

@@ -0,0 +1,11 @@
+if ARCH_MCS814X

arch/arm/mach-mcs814x/Kconfig line 11

Quoted 10 lines

arch/arm/mach-mcs814x/Kconfig new file +10 -0

+
+config MCS8140
+	select CPU_ARM926T
+	bool
+
+menu "Moschip MCS8140 boards"
+
+endmenu
+
+endif

What values does the Kconfig file actually provide? I see that you are adding the boards here later, but I think it would be nice to just make the new platform a single Kconfig option with no individual board selection.

Quoted 1 line

I plan on adding support for MCS8142 later on, thus I thought it would be

Quoted 2 lines

to be able to selectively compile board support for MCS8140 or some later chip.

But is there much difference between the two? How much code would actually get built for one chip but not the other? If all the differences can be run-time detected from the device tree, I think it's better to not have different compile-time options at all.

Quoted 2 lines

Quoted 2 lines

Platforms are now converting to the common clock framework in

Quoted 1 line

Quoted 1 line

Mike Turquette as the subsystem maintainer can probably judge better

Quoted 8 lines

Quoted 8 lines

we should still be allowing new platforms with their own implementation of clk, but my feeling is that you should use the subsystem instead and add a driver in a subdirectory of drivers/clk instead of in the platform.

Quoted 1 line

+void __iomem *mcs814x_sysdbg_base;

Does this have to be globally visible? I would recommend making it

Quoted 2 lines

Yes, since it is used both by clock.c and common.c

Ok. How about converting this to an global function in common.c that is called from clk_local_onoff_enable instead?

Quoted 19 lines

Quoted 10 lines

Quoted 10 lines

arch/arm/mach-mcs814x/Kconfig new file +9 -0

+void __init mcs814x_map_io(void)
+{
+	iotable_init(mcs814x_io_desc, ARRAY_SIZE(mcs814x_io_desc));
+
+	mcs814x_sysdbg_base = ioremap(MCS814X_IO_START + MCS814X_SYSDBG,
+					MCS814X_SYSDBG_SIZE);
+	if (!mcs814x_sysdbg_base)
+		panic("unable to remap sysdbg base");
+}

Quoted 8 lines

arch/arm/mach-mcs814x/Kconfig

Quoted 5 lines

arch/arm/mach-mcs814x/Kconfig new file +5 -0

+
+void mcs814x_restart(char mode, const char *cmd)
+{
+	__raw_writel(~(1 << 31), mcs814x_sysdbg_base);
+}

You should generally avoid using __raw_readl etc. and instead use readl/write or readl_relaxed/writel_relaxed.

Quoted 1 line

Allright, even on an ARM926-EJS-based system?

Yes, there is no practical difference in the way they are defined at the moment, but the __raw_ versions are still wrong, you should really never be using them from driver code.

Quoted 2 lines

Quoted 1 line

arch/arm/mach-

Quoted 1 line

arch/arm/mach- renamed from arch/arm/mach-mcs814x/include/mach/cpu.h +0 -0

mcs814x/include/mach/cpu.h

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

new file mode 100644
index 0000000..1ef3c4a

arch/arm/mach-mcs814x/include/mach/cpu.h new file +3 -0

@@ -0,0 +1,16 @@
+#ifndef __ASM_ARCH_CPU_H__
+#define __ASM_ARCH_CPU_H__
+

Quoted 8 lines

Quoted 8 lines

arch/arm/mach-mcs814x/include/mach/cpu.h

Quoted 8 lines

arch/arm/mach-mcs814x/include/mach/cpu.h new file +8 -0

+#include <asm/cputype.h>
+
+#define MCS8140_ID	0x41069260	/* ARM926EJ-S */
+#define MCS814X_MASK	0xff0ffff0
+
+#ifdef CONFIG_MCS8140
+/* Moschip MCS8140 is based on an ARM926EJ-S core */
+#define soc_is_mcs8140()	((read_cpuid_id() & MCS814X_MASK) == 

Quoted 10 lines

Quoted 8 lines

arch/arm/mach-mcs814x/include/mach/cpu.h line 16

Quoted 5 lines

arch/arm/mach-mcs814x/include/mach/cpu.h new file +5 -0

+#else
+#define soc_is_mcs8140()	(0)
+#endif /* !CONFIG_MCS8140 */
+
+#endif /* __ASM_ARCH_CPU_H__ */

Can you explain what this is used for? Are there other SoCs in the same platform that will get added later?

Yes, that's the plan.

Ok. But why do you need to know the soc type? If it's only for the uncompress.h code, it should probably stay local to that file.

Quoted 8 lines

Quoted 7 lines

arch/arm/mach-mcs814x/include/mach/cpu.h

Quoted 5 lines

arch/arm/mach-mcs814x/include/mach/cpu.h new file +5 -0

+/* IRQ controller register offset */
+#define MCS814X_IRQ_ICR		0x00
+#define MCS814X_IRQ_ISR		0x04
+#define MCS814X_IRQ_MASK	0x20
+#define MCS814X_IRQ_STS0	0x40

I'm pretty sure these can be in irq.c

Quoted 1 line

Not unless I switch to MULTI_IRQ_HANDLER.

I think you should do that, too. Forgot to mention it.

Quoted 9 lines

Quoted 9 lines

arch/arm/mach-mcs814x/include/mach/cpu.h

Quoted 7 lines

arch/arm/mach-mcs814x/include/mach/cpu.h new file +7 -0

+
+static struct resource io_mem = {
+	.name	= "PCI I/O space",
+	.start	= MCS8140_PCI_HOST_BASE + MCS8140_PCI_IOMISC_BASE,
+	.end	= MCS8140_PCI_HOST_BASE + MCS8140_PCI_IOMISC_BASE + SZ_64M,
+	.flags	= IORESOURCE_IO,
+};

This is wrong: MCS8140_PCI_HOST_BASE is an address in IORESOURCE_MEM

Quoted 4 lines

Quoted 1 line

You probably also mean SZ_64K instead of SZ_64M.

I will double check with the hardware manual, but I really think it is 64MBytes.

Are there any devices hardwired to your PCI bus that require more than a few bytes of I/O space? If not, I think it should be 64KB. No known add-on cards require more than that, and it would just waste virtual address space, and prevents us from using the new common PCI I/O space helpers.

Quoted 10 lines

Quoted 8 lines

Leave this one at the default of 0x1000. MCS8140_PCI_HOST_BASE is the address from the CPU's point of view, not from the bus. You just need to stay out of the range of possible legacy ISA devices.

It's probably a good idea to keep the PCI support as a separate patch for reviewing purposes.

Good idea, I will do that. Note that none of the boards I have actually

Quoted 1 line

PCI, so the code is just there in case someone needs it. I could also go

Quoted 1 line

and remove it, since I cannot test it, which explains why there are so

Quoted 1 line

inconsistencies with it.

Fair enough. Best drop it for now then. The PCI code is currently undergoing some changes, and when someone wants to get this to work, they should can probably better start from scratch with an independent bus driver that will end up looking much simpler than what you have today.

Arnd

Quoted 5 lines · On Tuesday 17 July 2012 13:07:23 Arnd Bergmann wrote:

But is there much difference between the two? How much code would actually get built for one chip but not the other? If all the differences can be run-time detected from the device tree, I think it's better to not have different compile-time options at all.

The big difference between the two is actually the CPU core, for MCS8140 they used a genuine ARM926EJS and for MCS8142 they actually use a FA526 core.

arch/arm/mach-mcs814x/common.c line 97

Quoted 4 lines

arch/arm/mach-mcs814x/common.c new file +4 -0

+void mcs814x_restart(char mode, const char *cmd)
+{
+	__raw_writel(~(1 << 31), mcs814x_sysdbg_base);
+}

You should generally avoid using __raw_readl etc. and instead use readl/write or readl_relaxed/writel_relaxed.

Le Mon, 16 Jul 2012 15:54:04 +0000,
Arnd Bergmann <[email protected]> a ?crit :

Quoted 7 lines

arch/arm/mach-mcs814x/common.c line 97

Quoted 4 lines

arch/arm/mach-mcs814x/common.c new file +4 -0

+void mcs814x_restart(char mode, const char *cmd)
+{
+   __raw_writel(~(1 << 31), mcs814x_sysdbg_base);
+}

You should generally avoid using __raw_readl etc. and instead use readl/write or readl_relaxed/writel_relaxed.

I thought that readl/writel and readl_relaxed/writel_relaxed were used for PCI-style access, for which these macros/functions do automatically a conversion from the CPU-endianness to the PCI-endianness (little-endian). Here of course, this the SoC is used little-endian, it will just work because readl/writel will not do any endianness conversion, but if we were on a big-endian machine?

My understanding until now was that __raw_readl/__raw_writel should be used for accesses with native endianness, but apparently, it's more subtle than this. Would you mind expanding a bit on this?

Quoted 3 lines

My understanding until now was that __raw_readl/__raw_writel should be used for accesses with native endianness, but apparently, it's more subtle than this. Would you mind expanding a bit on this?

The __raw_* versions are basically only valid if you access a memory buffer, such as a video framebuffer. They do the conversion of an __iomem pointer into something that can be accessed by the compiler.

The non-raw versions have fixed endianess, guarantee that the access is done atomically (could be byte-wise otherwise) and that all the necessary barriers are used to synchronize against DMA and out-of-order execution.

We don't actually have accessors that are CPU-endian and guarantee that you can access an MMIO register properly.

On powerpc, the convention is that readl/writel should only be used to access PCI, but that is for the enhanced error handling, not for endianess.

And then, on PowerPC, which accessors do you use to access SoC-peripherals that are not on the PCI bus?

For example, drivers/tty/serial/mpsc.c uses writel/readl, so a conversion to/from little-endian is done when writing/reading registers. Are those memory-mapped devices little-endian even though they are used on big-endian PowerPCs?

I apologize for the silly questions, but I'm trying to make some sense out of these numerous I/O and memory accessors.

arch/arm/mach-mcs814x/common.c +7 -0

+void __init mcs814x_init_machine(void)
+{
+       u32 bs2, cpu_mode;
+       int gpio;
+
+       bs2 = __raw_readl(mcs814x_sysdbg_base + SYSDBG_BS2);
+       cpu_mode = (bs2 >> CPU_MODE_SHIFT) & CPU_MODE_MASK;

arch/arm/mach-mcs814x/irq.c new file +0 -0

new file mode 100644
index 0000000..089937b

arch/arm/mach-mcs814x/include/mach/entry-macro.S new file +19 -0

new file mode 100644
index 0000000..cbad566
@@ -0,0 +1,29 @@
+#include <mach/mcs814x.h>
+                .macro  disable_fiq
+                .endm
+
+               .macro arch_ret_to_user, tmp1, tmp2
+               .endm
+
+               .macro  get_irqnr_preamble, base, tmp
+               ldr     \base, =mcs814x_intc_base@ base virtual address of INTC
+               ldr     \base, [\base]
+               .endm
+
+               .macro  get_irqnr_and_base, irqnr, irqstat, base, tmp
+               mov     \tmp, #MCS814X_IRQ_STS0  @ load tmp with STS0 register offset
+               ldr     \irqstat, [\base, \tmp]  @ load value at base + tmp
+               tst     \irqstat, \irqstat       @ test if no active IRQ's
+               beq     1002f                    @ if no active irqs return with status 0
+               mov     \irqnr, #0               @ start from irq zero
+               mov     \tmp,   #1               @ the mask initially 1

arch/arm/mach-mcs814x/timer.c +8 -0

+static inline unsigned long ticks2usecs(u32 x)
+{
+       return x / (clock_rate / 1000000);
+}
+
+/*
+ * Returns number of ms since last clock interrupt.  Note that interrupts
+ * will have been disabled by do_gettimeoffset()

arch/arm/mach-mcs814x/timer.c +23 -0

+
+       return IRQ_HANDLED;
+}
+
+static struct irqaction mcs814x_timer_irq = {
+       .name           = "mcs814x-timer",
+       .flags          = IRQF_DISABLED | IRQF_TIMER | IRQF_IRQPOLL,
+       .handler        = mcs814x_timer_interrupt,
+};
+
+static struct of_device_id mcs814x_timer_ids[] = {
+       { .compatible = "moschip,mcs814x-timer" },
+       { /* sentinel */ },
+};
+
+static void __init mcs814x_of_timer_init(void)
+{
+       struct device_node *np;
+       const unsigned int *intspec;
+
+       np = of_find_matching_node(NULL, mcs814x_timer_ids);
+       if (!np)
+               panic("unable to find compatible timer node in dtb");

arch/arm/mach-mcs814x/timer.c +11 -0

+
+       mcs814x_of_timer_init();
+
+       pr_info("Timer frequency: %d (kHz)\n", clock_rate / 1000);
+
+       timer_reload_value = 0xffffffff - (clock_rate / HZ);
+
+       /* disable timer */
+       __raw_writel(0, mcs814x_timer_base + TIMER_CTL);
+       __raw_writel(timer_reload_value, mcs814x_timer_base + TIMER_VAL);
+       last_reload = timer_reload_value;