Skip to contentSkip to conversation

No subject

PATCH New Haojian Zhuang 6 participants 32 messages Jul 26, 2013 – Aug 24, 2013

Subsystems (7)
lore
Share & tools
lore
#0 lore raw

Changelog:
v6:
1. Merge clk apbc & apbcp together.
2. Add DT binding document of clock driver.
3. Remove marvell string in properties of clock driver.

v5:
1. Use clk mux table.

v4:
1. Remove .init_irq in mmp & mmp2 DT machine descriptor.
2. Use an argument to replace TIMER_PHYS_BASE. Now transfer virtual address directly.
3. Merge 10th & 11th patches together. Remove the redundant changes on drivers/clocksource/Kconfig & drivers/clocksource/Makefile.

v3:
1. Don't use include/linux/irqchip/mmp.h since we don't need to move <mach/irqs.h> to <include/linux/irqchip/mmp.h>.
2. Move timer-mmp driver into clocksource directory & support clocksource.
3. Support clksrc in mmp & parse all clock from DTS.

v2:
1. Avoid to include <mach/irqs.h>. Move the head file into include/linux/irqchip directory.
2. Avoid to include <mach/pm-pxa910.h> & <mach/pm-mmp2.h>.

#1 lore raw

PATCH v6 1/11 irqchip: move mmp irq driver

Move irq-mmp driver from mach-mmp directory into irqchip directory.

---
 arch/arm/mach-mmp/Makefile                           | 2 +-
 arch/arm/mach-mmp/common.h                           | 1 -
 arch/arm/mach-mmp/include/mach/pxa168.h              | 1 +
 arch/arm/mach-mmp/include/mach/pxa910.h              | 1 +
 arch/arm/mach-mmp/pxa910.c                           | 1 +
 drivers/irqchip/Makefile                             | 1 +
 arch/arm/mach-mmp/irq.c => drivers/irqchip/irq-mmp.c | 2 --
 7 files changed, 5 insertions(+), 4 deletions(-)
 rename arch/arm/mach-mmp/irq.c => drivers/irqchip/irq-mmp.c (99%)
arch/arm/mach-mmp/Makefile +1 -1
index 095c155..9b702a1 100644
@@ -2,7 +2,7 @@
 # Makefile for Marvell's PXA168 processors line
 #
 
-obj-y				+= common.o devices.o time.o irq.o
+obj-y				+= common.o devices.o time.o
 
 # SoC support
 obj-$(CONFIG_CPU_PXA168)	+= pxa168.o
arch/arm/mach-mmp/common.h +0 -1
index 991d7e9..cf445ba 100644
@@ -3,7 +3,6 @@
 
 extern void timer_init(int irq);
 
-extern void __init icu_init_irq(void);
 extern void __init mmp_map_io(void);
 extern void mmp_restart(enum reboot_mode, const char *);
 extern void __init pxa168_clk_init(void);
arch/arm/mach-mmp/include/mach/pxa168.h +1 -0
index 459c2d0..a83ba7c 100644
@@ -4,6 +4,7 @@
 #include <linux/reboot.h>
 
 extern void pxa168_timer_init(void);
+extern void __init icu_init_irq(void);
 extern void __init pxa168_init_irq(void);
 extern void pxa168_restart(enum reboot_mode, const char *);
 extern void pxa168_clear_keypad_wakeup(void);
arch/arm/mach-mmp/include/mach/pxa910.h +1 -0
index b914afa..9225320 100644
@@ -2,6 +2,7 @@
 #define __ASM_MACH_PXA910_H
 
 extern void pxa910_timer_init(void);
+extern void __init icu_init_irq(void);
 extern void __init pxa910_init_irq(void);
 
 #include <linux/i2c.h>
arch/arm/mach-mmp/pxa910.c +1 -0
index ce6393a..a586742 100644
@@ -23,6 +23,7 @@
 #include <mach/dma.h>
 #include <mach/mfp.h>
 #include <mach/devices.h>
+#include <mach/pxa910.h>
 
 #include "common.h"
 
drivers/irqchip/Makefile +1 -0
index e65c41a..c452943 100644
@@ -2,6 +2,7 @@ obj-$(CONFIG_IRQCHIP)			+= irqchip.o
 
 obj-$(CONFIG_ARCH_BCM2835)		+= irq-bcm2835.o
 obj-$(CONFIG_ARCH_EXYNOS)		+= exynos-combiner.o
+obj-$(CONFIG_ARCH_MMP)			+= irq-mmp.o
 obj-$(CONFIG_ARCH_MVEBU)		+= irq-armada-370-xp.o
 obj-$(CONFIG_ARCH_MXS)			+= irq-mxs.o
 obj-$(CONFIG_ARCH_S3C24XX)		+= irq-s3c24xx.o
drivers/irqchip/irq-mmp.c renamed from arch/arm/mach-mmp/irq.c +0 -2
similarity index 99%
rename from arch/arm/mach-mmp/irq.c
rename to drivers/irqchip/irq-mmp.c
index 3c71246..dab6def 100644
@@ -30,8 +30,6 @@
 #include <mach/pm-pxa910.h>
 #endif
 
-#include "common.h"
-
 #define MAX_ICU_NR		16
 
 struct icu_chip_data {
Signature · 1 line
1.8.1.2
#2 lore raw

PATCH v6 1/11 irqchip: move mmp irq driver

This patch series affects 3 different subsystems: irqchip, clocksource, clk. It's not clear from your submission who you are expecting to review/merge this. Unless there is some prior agreement, maybe it would be easier and clearer to divide this up by subsystem. There doesn't seem to be any overlap/dependencies between the 3 parts.

Quoted 1 line · Haojian Zhuang

Move irq-mmp driver from mach-mmp directory into irqchip directory.

Might be worth a 1-line commit message, explaining the motivation
here: moving hardware support from arch/arm to drivers/

Quoted 1 line
#3 lore raw

PATCH v6 1/11 irqchip: move mmp irq driver

Hi Haojian,

Quoted 5 lines · Daniel Drake

This patch series affects 3 different subsystems: irqchip, clocksource, clk. It's not clear from your submission who you are expecting to review/merge this. Unless there is some prior agreement, maybe it would be easier and clearer to divide this up by subsystem. There doesn't seem to be any overlap/dependencies between the 3 parts.

I haven't heard back from you on these patches. Is there anything I can do to help?

I know these patches are blocking the 5-or-so patches I am trying to submit for OLPC XO support. Only one of those patches is perhaps calling for discussion (how to put the clocks in DT) and it is a bit frustrating that after two months we have still not progressed.

To start with, maybe I could resend patches 1 to 5 in this series to the irqchip maintainers, with the review comments incorporated, adding the following 2 patches on at the end?http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/176643.html
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/176643.html

Or any other suggestions for how I can help things move along?

Thanks
Daniel

#4 lore raw

PATCH v6 1/11 irqchip: move mmp irq driver

Quoted 23 lines · Daniel Drake

Hi Haojian,

Quoted 5 lines · Daniel Drake

This patch series affects 3 different subsystems: irqchip, clocksource, clk. It's not clear from your submission who you are expecting to review/merge this. Unless there is some prior agreement, maybe it would be easier and clearer to divide this up by subsystem. There doesn't seem to be any overlap/dependencies between the 3 parts.

I haven't heard back from you on these patches. Is there anything I can do to help?

I know these patches are blocking the 5-or-so patches I am trying to submit for OLPC XO support. Only one of those patches is perhaps calling for discussion (how to put the clocks in DT) and it is a bit frustrating that after two months we have still not progressed.

To start with, maybe I could resend patches 1 to 5 in this series to the irqchip maintainers, with the review comments incorporated, adding the following 2 patches on at the end?http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/176643.html
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/176643.html

Quoted 4 lines

Or any other suggestions for how I can help things move along?

Thanks
Daniel

I'll update the clock patches in this series. Since I was very busy, I didn't have time
to handle it. I'll submit them again in this week.

IRQ & clocksource patches will go through ARM SoC. clock patches will go through clock tree. The clocksource patches are depended on clock patches.

Regards
Haojian

#5 lore raw

PATCH v6 2/11 irqchip: mmp: support irqchip

In reply to Haojian Zhuang

Support IRQCHIP on irq-mmp driver.

---
 arch/arm/mach-mmp/mmp-dt.c  |   8 +-
 arch/arm/mach-mmp/mmp2-dt.c |   8 +-
 drivers/irqchip/irq-mmp.c   | 238 +++++++++++++++++++++++---------------------
 3 files changed, 126 insertions(+), 128 deletions(-)
arch/arm/mach-mmp/mmp-dt.c +1 -7
index b37915d..cca529c 100644
@@ -9,17 +9,13 @@
  *  publishhed by the Free Software Foundation.
  */
 
-#include <linux/irq.h>
-#include <linux/irqdomain.h>
-#include <linux/of_irq.h>
+#include <linux/irqchip.h>
 #include <linux/of_platform.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/time.h>
-#include <mach/irqs.h>
 
 #include "common.h"
 
-extern void __init mmp_dt_irq_init(void);
 extern void __init mmp_dt_init_timer(void);
 
 static const struct of_dev_auxdata pxa168_auxdata_lookup[] __initconst = {
@@ -64,7 +60,6 @@ static const char *mmp_dt_board_compat[] __initdata = {
 
 DT_MACHINE_START(PXA168_DT, "Marvell PXA168 (Device Tree Support)")
 	.map_io		= mmp_map_io,
-	.init_irq	= mmp_dt_irq_init,
 	.init_time	= mmp_dt_init_timer,
 	.init_machine	= pxa168_dt_init,
 	.dt_compat	= mmp_dt_board_compat,
@@ -72,7 +67,6 @@ MACHINE_END
 
 DT_MACHINE_START(PXA910_DT, "Marvell PXA910 (Device Tree Support)")
 	.map_io		= mmp_map_io,
-	.init_irq	= mmp_dt_irq_init,
 	.init_time	= mmp_dt_init_timer,
 	.init_machine	= pxa910_dt_init,
 	.dt_compat	= mmp_dt_board_compat,
arch/arm/mach-mmp/mmp2-dt.c +1 -7
index 4ac2567..023cb45 100644
@@ -10,18 +10,13 @@
  */
 
 #include <linux/io.h>
-#include <linux/irq.h>
-#include <linux/irqdomain.h>
-#include <linux/of_irq.h>
+#include <linux/irqchip.h>
 #include <linux/of_platform.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/time.h>
-#include <mach/irqs.h>
-#include <mach/regs-apbc.h>
 
 #include "common.h"
 
-extern void __init mmp_dt_irq_init(void);
 extern void __init mmp_dt_init_timer(void);
 
 static const struct of_dev_auxdata mmp2_auxdata_lookup[] __initconst = {
@@ -49,7 +44,6 @@ static const char *mmp2_dt_board_compat[] __initdata = {
 
 DT_MACHINE_START(MMP2_DT, "Marvell MMP2 (Device Tree Support)")
 	.map_io		= mmp_map_io,
-	.init_irq	= mmp_dt_irq_init,
 	.init_time	= mmp_dt_init_timer,
 	.init_machine	= mmp2_dt_init,
 	.dt_compat	= mmp2_dt_board_compat,
drivers/irqchip/irq-mmp.c +124 -114

268 lines not on this page · raw message

Signature · 1 line
1.8.1.2
#6 lore raw

PATCH v6 2/11 irqchip: mmp: support irqchip

Quoted 8 lines · Haojian Zhuang

Support IRQCHIP on irq-mmp driver.

---
 arch/arm/mach-mmp/mmp-dt.c  |   8 +-
 arch/arm/mach-mmp/mmp2-dt.c |   8 +-
 drivers/irqchip/irq-mmp.c   | 238 +++++++++++++++++++++++---------------------
 3 files changed, 126 insertions(+), 128 deletions(-)

This patch causes boot to fail, tested on OLPC XO-1.75 (MMP2).

Calibrating delay loop... irq 13, desc: db804740, depth: 1, count: 0,
unhandled: 0
->handle_irq(): c00704f4, handle_bad_irq+0x0/0x214 ->irq_data.chip(): c05aa2e8, 0xc05aa2e8
->action(): (null)
IRQ_NOPROBE set
IRQ_NOREQUEST set
irq 13, desc: db804740, depth: 1, count: 0, unhandled: 0 ->handle_irq(): c00704f4, handle_bad_irq+0x0/0x214 ->irq_data.chip(): c05aa2e8, 0xc05aa2e8
->action(): (null)
IRQ_NOPROBE set
IRQ_NOREQUEST set
irq 13, desc: db804740, depth: 1, count: 0, unhandled: 0 ->handle_irq(): c00704f4, handle_bad_irq+0x0/0x214 ->irq_data.chip(): c05aa2e8, 0xc05aa2e8
->action(): (null)
IRQ_NOPROBE set
IRQ_NOREQUEST set

continues as infinite loop.

I had a poke, trying to figure out what would have caused this, without much luck. I tried to go back to irq_domain_add_legacy() and sprinkled some icu_mask_irq() calls around, nothing changed. Any ideas?

Thanks
Daniel

#7 lore raw

PATCH v6 2/11 irqchip: mmp: support irqchip

Quoted 10 lines · Daniel Drake

This patch causes boot to fail, tested on OLPC XO-1.75 (MMP2).

Calibrating delay loop... irq 13, desc: db804740, depth: 1, count: 0,
unhandled: 0
->handle_irq(): c00704f4, handle_bad_irq+0x0/0x214 ->irq_data.chip(): c05aa2e8, 0xc05aa2e8
->action(): (null)
IRQ_NOPROBE set
IRQ_NOREQUEST set

Quoted 6 lines

continues as infinite loop.

I had a poke, trying to figure out what would have caused this, without much luck. I tried to go back to irq_domain_add_legacy() and sprinkled some icu_mask_irq() calls around, nothing changed. Any ideas?

In my above experiments I was assuming things were crashing during the IRQ setup path (e.g mmp_setup_bases). That was wrong - I now realise that the setup code moved around by this patch exits OK, then the timer driver loads and requests + unmasks interrupt 13, and these messages start appearing upon the first interrupt which happens right after.

It is not happy because no IRQ handler was registered. This is normally done by mmp_irq_domain_map(), previously called immediately from irq_domain_add_legacy().
Now that we moved to irq_domain_add_linear() in this patch, the map function is not called.
I fixed this by calling irq_set_chip_and_handler() and set_irq_flags() in the loop immediately after where irq_create_mapping() is used, as is done by irq-bcm2835.c.

Now I get a new crash, still handling the first interrupt. In icu_mask_ack_irq(), d->domain is NULL. We dereference it, causing a crash. The domain field should have been set by irq_create_mapping().

What actually happens is that early_irq_init (kernel/irqdesc.c) allocates 16 interrupt descriptors even before we've looked at the device tree. Are those "legacy" interrupts? I can't figure out what that is supposed to mean. Then the interrupts registered by irq-mmp.c start at 16 (e.g. hwirq 0 becomes virq 16).

When the hardware interrupt 13 comes in, the call chain is:
handle_IRQ()
generic_handle_irq()
handle_level_irq()
icu_mask_ack_irq()

generic_handle_irq() above looks up the descriptor for IRQ 13, but it does it without considering the mapping. So it picks up the descriptor for "legacy" interrupt 13 and passes that to icu_mask_ack_irq() - rather than passing the descriptor for the hwirq 13 that irq-mmp.c registered and was expecting (i.e. no translation was performed).

How is this supposed to work?

Thanks
Daniel

#8 lore raw

PATCH v6 2/11 irqchip: mmp: support irqchip

Quoted 7 lines · Daniel Drake

generic_handle_irq() above looks up the descriptor for IRQ 13, but it does it without considering the mapping. So it picks up the descriptor for "legacy" interrupt 13 and passes that to icu_mask_ack_irq() - rather than passing the descriptor for the hwirq 13 that irq-mmp.c registered and was expecting (i.e. no translation was performed).

How is this supposed to work?

All the users of irq_domain_add_linear() that I have looked at supply a custom IRQ handler routine (via set_handle_irq) that does the translation.

So this patch needs to be merged with patch 3 in the series (which does just that), or the conversion from legacy to linear needs to happen in patch 3, not this one. Personally I would just merge the 2 patches.

With patch 3 also applied, there is no more crash.

Thanks,
Daniel

#9 lore raw

PATCH v6 3/11 irqchip: mmp: support MULTI_IRQ_HANDLER

In reply to Haojian Zhuang

Support CONFIG_MULTI_IRQ_HANDLER in ARCH_MMP. So remove entry-macro.S.

---
 arch/arm/Kconfig                             |  1 +
 arch/arm/mach-mmp/include/mach/entry-macro.S | 26 -----------------
 drivers/irqchip/irq-mmp.c                    | 42 +++++++++++++++++++++++++++-
 3 files changed, 42 insertions(+), 27 deletions(-)
 delete mode 100644 arch/arm/mach-mmp/include/mach/entry-macro.S
arch/arm/Kconfig +1 -0
index ba412e0..76793a6 100644
@@ -557,6 +557,7 @@ config ARCH_MMP
 	select GENERIC_CLOCKEVENTS
 	select GPIO_PXA
 	select IRQ_DOMAIN
+	select MULTI_IRQ_HANDLER
 	select NEED_MACH_GPIO_H
 	select PINCTRL
 	select PLAT_PXA
arch/arm/mach-mmp/include/mach/entry-macro.S deleted +0 -26
deleted file mode 100644
index bd152e2..0000000
@@ -1,26 +0,0 @@
-/*
- * linux/arch/arm/mach-mmp/include/mach/entry-macro.S
- *
- * 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.
- */
-
-#include <asm/irq.h>
-#include <mach/regs-icu.h>
-
-	.macro	get_irqnr_preamble, base, tmp
-	mrc	p15, 0, \tmp, c0, c0, 0		@ CPUID
-	and	\tmp, \tmp, #0xff00
-	cmp	\tmp, #0x5800
-	ldr	\base, =mmp_icu_base
-	ldr	\base, [\base, #0]
-	addne	\base, \base, #0x10c		@ PJ1 AP INT SEL register
-	addeq	\base, \base, #0x104		@ PJ4 IRQ SEL register
-	.endm
-
-	.macro	get_irqnr_and_base, irqnr, irqstat, base, tmp
-	ldr	\tmp, [\base, #0]
-	and	\irqnr, \tmp, #0x3f
-	tst	\tmp, #(1 << 6)
-	.endm
drivers/irqchip/irq-mmp.c +41 -1

90 lines not on this page · raw message

Signature · 1 line
1.8.1.2
#10 lore raw

PATCH v6 4/11 ARM: mmp: avoid to include head file in mach-mmp

In reply to Haojian Zhuang

pxa910_set_wake() & mmp2_set_wake() are both declared in head files of arch/arm/mach-mmp/include/mach directory. If we include these head files in irq-mmp driver, it blocks the multiplatform build. So adjust the code.

---
 arch/arm/mach-mmp/mmp2.c    |  4 ++++
 arch/arm/mach-mmp/pxa910.c  |  4 ++++
 drivers/irqchip/irq-mmp.c   | 15 +--------------
 include/linux/irqchip/mmp.h |  6 ++++++
 4 files changed, 15 insertions(+), 14 deletions(-)
 create mode 100644 include/linux/irqchip/mmp.h
arch/arm/mach-mmp/mmp2.c +4 -0
index c7592f1..09fbe7d 100644
@@ -13,6 +13,8 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqchip/mmp.h>
 #include <linux/platform_device.h>
 
 #include <asm/hardware/cache-tauros2.h>
@@ -26,6 +28,7 @@
 #include <mach/mfp.h>
 #include <mach/devices.h>
 #include <mach/mmp2.h>
+#include <mach/pm-mmp2.h>
 
 #include "common.h"
 
@@ -94,6 +97,7 @@ void mmp2_clear_pmic_int(void)
 void __init mmp2_init_irq(void)
 {
 	mmp2_init_icu();
+	icu_irq_chip.irq_set_wake = mmp2_set_wake;
 }
 
 static int __init mmp2_init(void)
arch/arm/mach-mmp/pxa910.c +4 -0
index a586742..96e125b 100644
@@ -12,6 +12,8 @@
 #include <linux/init.h>
 #include <linux/list.h>
 #include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqchip/mmp.h>
 #include <linux/platform_device.h>
 
 #include <asm/hardware/cache-tauros2.h>
@@ -23,6 +25,7 @@
 #include <mach/dma.h>
 #include <mach/mfp.h>
 #include <mach/devices.h>
+#include <mach/pm-pxa910.h>
 #include <mach/pxa910.h>
 
 #include "common.h"
@@ -80,6 +83,7 @@ static struct mfp_addr_map pxa910_mfp_addr_map[] __initdata =
 void __init pxa910_init_irq(void)
 {
 	icu_init_irq();
+	icu_irq_chip.irq_set_wake = pxa910_set_wake;
 }
 
 static int __init pxa910_init(void)
drivers/irqchip/irq-mmp.c +1 -14
index 84d51ff..1f81432 100644
@@ -26,13 +26,6 @@
 
 #include <mach/irqs.h>
 
-#ifdef CONFIG_CPU_MMP2
-#include <mach/pm-mmp2.h>
-#endif
-#ifdef CONFIG_CPU_PXA910
-#include <mach/pm-pxa910.h>
-#endif
-
 #include "irqchip.h"
 
 #define MAX_ICU_NR		16
@@ -132,7 +125,7 @@ static void icu_unmask_irq(struct irq_data *d)
 	}
 }
 
-static struct irq_chip icu_irq_chip = {
+struct irq_chip icu_irq_chip = {
 	.name		= "icu_irq",
 	.irq_mask	= icu_mask_irq,
 	.irq_mask_ack	= icu_mask_ack_irq,
@@ -251,9 +244,6 @@ void __init icu_init_irq(void)
 	}
 	irq_set_default_host(icu_data[0].domain);
 	set_handle_irq(mmp_handle_irq);
-#ifdef CONFIG_CPU_PXA910
-	icu_irq_chip.irq_set_wake = pxa910_set_wake;
-#endif
 }
 
 /* MMP2 (ARMv7) */
@@ -358,9 +348,6 @@ void __init mmp2_init_icu(void)
 	}
 	irq_set_default_host(icu_data[0].domain);
 	set_handle_irq(mmp2_handle_irq);
-#ifdef CONFIG_CPU_MMP2
-	icu_irq_chip.irq_set_wake = mmp2_set_wake;
-#endif
 }
 
 #ifdef CONFIG_OF
include/linux/irqchip/mmp.h new file +6 -0
new file mode 100644
index 0000000..c78a892
@@ -0,0 +1,6 @@
+#ifndef	__IRQCHIP_MMP_H
+#define	__IRQCHIP_MMP_H
+
+extern struct irq_chip icu_irq_chip;
+
+#endif	/* __IRQCHIP_MMP_H */
Signature · 1 line
1.8.1.2
#11 lore raw

PATCH v6 4/11 ARM: mmp: avoid to include head file in mach-mmp

Quoted 6 lines · Haojian Zhuang

pxa910_set_wake() & mmp2_set_wake() are both declared in head files of arch/arm/mach-mmp/include/mach directory. If we include these head files in irq-mmp driver, it blocks the multiplatform build. So adjust the code.

This fails to build with CONFIG_PM=n, since mmp2_set_wake() is not defined anywhere.
This is not a regression introduced by this patch, but now seems like a good time to fix that minor problem.

Could this line

       icu_irq_chip.irq_set_wake = mmp2_set_wake;

be moved to mmp2_pm_init() in pm-mmp2.c (or is that too late)? If so you could even make the mmp2_set_wake function static.

Thanks
Daniel

#12 lore raw

PATCH v6 4/11 ARM: mmp: avoid to include head file in mach-mmp

Quoted 14 lines · Daniel Drake
Quoted 6 lines · Haojian Zhuang

pxa910_set_wake() & mmp2_set_wake() are both declared in head files of arch/arm/mach-mmp/include/mach directory. If we include these head files in irq-mmp driver, it blocks the multiplatform build. So adjust the code.

This fails to build with CONFIG_PM=n, since mmp2_set_wake() is not defined anywhere.
This is not a regression introduced by this patch, but now seems like a good time to fix that minor problem.

Quoted 7 lines

Could this line

       icu_irq_chip.irq_set_wake = mmp2_set_wake;

be moved to mmp2_pm_init() in pm-mmp2.c (or is that too late)? If so you could even make the mmp2_set_wake function static.

Thanks
Daniel

Fixed.

Regards
Haojian

#13 lore raw

PATCH v6 5/11 irqchip: mmp: avoid to include irqs head file

In reply to Haojian Zhuang

Since <mach/irqs.h> in irq-mmp.c blocks the multiplatform build, remove it instead.

---
 drivers/irqchip/irq-mmp.c | 45 +++++++++++++++++++++------------------------
 1 file changed, 21 insertions(+), 24 deletions(-)
drivers/irqchip/irq-mmp.c +21 -24
index 1f81432..2cb7cd0 100644
@@ -24,8 +24,6 @@
 #include <asm/exception.h>
 #include <asm/mach/irq.h>
 
-#include <mach/irqs.h>
-
 #include "irqchip.h"
 
 #define MAX_ICU_NR		16
@@ -249,7 +247,7 @@ void __init icu_init_irq(void)
 /* MMP2 (ARMv7) */
 void __init mmp2_init_icu(void)
 {
-	int irq;
+	int irq, end;
 
 	max_icu_nr = 8;
 	mmp_icu_base = ioremap(0xd4282000, 0x1000);
@@ -263,11 +261,12 @@ void __init mmp2_init_icu(void)
 						   &icu_data[0]);
 	icu_data[1].reg_status = mmp_icu_base + 0x150;
 	icu_data[1].reg_mask = mmp_icu_base + 0x168;
-	icu_data[1].clr_mfp_irq_base = IRQ_MMP2_PMIC_BASE;
-	icu_data[1].clr_mfp_hwirq = IRQ_MMP2_PMIC - IRQ_MMP2_PMIC_BASE;
+	icu_data[1].clr_mfp_irq_base = icu_data[0].virq_base +
+				icu_data[0].nr_irqs;
+	icu_data[1].clr_mfp_hwirq = 1;		/* offset to IRQ_MMP2_PMIC_BASE */
 	icu_data[1].nr_irqs = 2;
 	icu_data[1].cascade_irq = 4;
-	icu_data[1].virq_base = IRQ_MMP2_PMIC_BASE;
+	icu_data[1].virq_base = icu_data[0].virq_base + icu_data[0].nr_irqs;
 	icu_data[1].domain = irq_domain_add_legacy(NULL, icu_data[1].nr_irqs,
 						   icu_data[1].virq_base, 0,
 						   &irq_domain_simple_ops,
@@ -276,7 +275,7 @@ void __init mmp2_init_icu(void)
 	icu_data[2].reg_mask = mmp_icu_base + 0x16c;
 	icu_data[2].nr_irqs = 2;
 	icu_data[2].cascade_irq = 5;
-	icu_data[2].virq_base = IRQ_MMP2_RTC_BASE;
+	icu_data[2].virq_base = icu_data[1].virq_base + icu_data[1].nr_irqs;
 	icu_data[2].domain = irq_domain_add_legacy(NULL, icu_data[2].nr_irqs,
 						   icu_data[2].virq_base, 0,
 						   &irq_domain_simple_ops,
@@ -285,7 +284,7 @@ void __init mmp2_init_icu(void)
 	icu_data[3].reg_mask = mmp_icu_base + 0x17c;
 	icu_data[3].nr_irqs = 3;
 	icu_data[3].cascade_irq = 9;
-	icu_data[3].virq_base = IRQ_MMP2_KEYPAD_BASE;
+	icu_data[3].virq_base = icu_data[2].virq_base + icu_data[2].nr_irqs;
 	icu_data[3].domain = irq_domain_add_legacy(NULL, icu_data[3].nr_irqs,
 						   icu_data[3].virq_base, 0,
 						   &irq_domain_simple_ops,
@@ -294,7 +293,7 @@ void __init mmp2_init_icu(void)
 	icu_data[4].reg_mask = mmp_icu_base + 0x170;
 	icu_data[4].nr_irqs = 5;
 	icu_data[4].cascade_irq = 17;
-	icu_data[4].virq_base = IRQ_MMP2_TWSI_BASE;
+	icu_data[4].virq_base = icu_data[3].virq_base + icu_data[3].nr_irqs;
 	icu_data[4].domain = irq_domain_add_legacy(NULL, icu_data[4].nr_irqs,
 						   icu_data[4].virq_base, 0,
 						   &irq_domain_simple_ops,
@@ -303,7 +302,7 @@ void __init mmp2_init_icu(void)
 	icu_data[5].reg_mask = mmp_icu_base + 0x174;
 	icu_data[5].nr_irqs = 15;
 	icu_data[5].cascade_irq = 35;
-	icu_data[5].virq_base = IRQ_MMP2_MISC_BASE;
+	icu_data[5].virq_base = icu_data[4].virq_base + icu_data[4].nr_irqs;
 	icu_data[5].domain = irq_domain_add_legacy(NULL, icu_data[5].nr_irqs,
 						   icu_data[5].virq_base, 0,
 						   &irq_domain_simple_ops,
@@ -312,7 +311,7 @@ void __init mmp2_init_icu(void)
 	icu_data[6].reg_mask = mmp_icu_base + 0x178;
 	icu_data[6].nr_irqs = 2;
 	icu_data[6].cascade_irq = 51;
-	icu_data[6].virq_base = IRQ_MMP2_MIPI_HSI1_BASE;
+	icu_data[6].virq_base = icu_data[5].virq_base + icu_data[5].nr_irqs;
 	icu_data[6].domain = irq_domain_add_legacy(NULL, icu_data[6].nr_irqs,
 						   icu_data[6].virq_base, 0,
 						   &irq_domain_simple_ops,
@@ -321,28 +320,26 @@ void __init mmp2_init_icu(void)
 	icu_data[7].reg_mask = mmp_icu_base + 0x184;
 	icu_data[7].nr_irqs = 2;
 	icu_data[7].cascade_irq = 55;
-	icu_data[7].virq_base = IRQ_MMP2_MIPI_HSI0_BASE;
+	icu_data[7].virq_base = icu_data[6].virq_base + icu_data[6].nr_irqs;
 	icu_data[7].domain = irq_domain_add_legacy(NULL, icu_data[7].nr_irqs,
 						   icu_data[7].virq_base, 0,
 						   &irq_domain_simple_ops,
 						   &icu_data[7]);
-	for (irq = 0; irq < IRQ_MMP2_MUX_END; irq++) {
+	end = icu_data[7].virq_base + icu_data[7].nr_irqs;
+	for (irq = 0; irq < end; irq++) {
 		icu_mask_irq(irq_get_irq_data(irq));
-		switch (irq) {
-		case IRQ_MMP2_PMIC_MUX:
-		case IRQ_MMP2_RTC_MUX:
-		case IRQ_MMP2_KEYPAD_MUX:
-		case IRQ_MMP2_TWSI_MUX:
-		case IRQ_MMP2_MISC_MUX:
-		case IRQ_MMP2_MIPI_HSI1_MUX:
-		case IRQ_MMP2_MIPI_HSI0_MUX:
+		if (irq == icu_data[1].cascade_irq ||
+		    irq == icu_data[2].cascade_irq ||
+		    irq == icu_data[3].cascade_irq ||
+		    irq == icu_data[4].cascade_irq ||
+		    irq == icu_data[5].cascade_irq ||
+		    irq == icu_data[6].cascade_irq ||
+		    irq == icu_data[7].cascade_irq) {
 			irq_set_chip(irq, &icu_irq_chip);
 			irq_set_chained_handler(irq, icu_mux_irq_demux);
-			break;
-		default:
+		} else {
 			irq_set_chip_and_handler(irq, &icu_irq_chip,
 						 handle_level_irq);
-			break;
 		}
 		set_irq_flags(irq, IRQF_VALID);
 	}
Signature · 1 line
1.8.1.2
#14 lore raw

PATCH v6 5/11 irqchip: mmp: avoid to include irqs head file

Quoted 7 lines · Haojian Zhuang

Since <mach/irqs.h> in irq-mmp.c blocks the multiplatform build, remove it instead.

---
 drivers/irqchip/irq-mmp.c | 45 +++++++++++++++++++++------------------------
 1 file changed, 21 insertions(+), 24 deletions(-)

The intention behind this patch is good, but I think the implementation is bad, since the platform still relies on specific hardcoded values for the base irq numbers. As long as this is the case, you should use the constants.

Quoted 15 lines
drivers/irqchip/irq-mmp.c +1 -3
@@ -249,7 +247,7 @@ void __init icu_init_irq(void)
 /* MMP2 (ARMv7) */
 void __init mmp2_init_icu(void)
 {
-	int irq;
+	int irq, end;
 
 	max_icu_nr = 8;
 	mmp_icu_base = ioremap(0xd4282000, 0x1000);
@@ -263,11 +261,12 @@ void __init (void)
 						   &icu_data[0]);
 	icu_data[1].reg_status = mmp_icu_base + 0x150;
 	icu_data[1].reg_mask = mmp_icu_base + 0x168;
-	icu_data[1].clr_mfp_irq_base = IRQ_MMP2_PMIC_BASE;
-	icu_data[1].clr_mfp_hwirq = IRQ_MMP2_PMIC - IRQ_MMP2_PMIC_BASE;

drivers/irqchip/irq-mmp.c void __init (void) · line 272

Quoted 10 lines
drivers/irqchip/irq-mmp.c +4 -1
+	icu_data[1].clr_mfp_irq_base = icu_data[0].virq_base +
+				icu_data[0].nr_irqs;
+	icu_data[1].clr_mfp_hwirq = 1;		/* offset to IRQ_MMP2_PMIC_BASE */
 	icu_data[1].nr_irqs = 2;
 	icu_data[1].cascade_irq = 4;
-	icu_data[1].virq_base = IRQ_MMP2_PMIC_BASE;
+	icu_data[1].virq_base = icu_data[0].virq_base + icu_data[0].nr_irqs;
 	icu_data[1].domain = irq_domain_add_legacy(NULL, icu_data[1].nr_irqs,
 						   icu_data[1].virq_base, 0,
 						   &irq_domain_simple_ops,

I'd suggest passing IRQ_MMP2_PMIC_BASE and the other constants from the caller and changing the prototype accordingly so you can do

	mmp2_init_icu(0, IRQ_MMP2_PMIC_BASE, IRQ_MMP2_RTC_BASE, IRQ_MMP2_KEYPAD_BASE,
			 IRQ_MMP2_TWSI_BASE, IRQ_MMP2_MISC_BASE, IRQ_MMP2_MIPI_HSI1_BASE,
			 IRQ_MMP2_MIPI_HSI0_BASE);

	Arnd
#15 lore raw

PATCH v6 6/11 clocksource: mmp: move mmp timer driver

In reply to Haojian Zhuang

Move timer-mmp driver from mach-mmp to clocksource directory.

---
 arch/arm/mach-mmp/Makefile                                  | 2 +-
 drivers/clocksource/Makefile                                | 1 +
 arch/arm/mach-mmp/time.c => drivers/clocksource/timer-mmp.c | 2 --
 3 files changed, 2 insertions(+), 3 deletions(-)
 rename arch/arm/mach-mmp/time.c => drivers/clocksource/timer-mmp.c (99%)
arch/arm/mach-mmp/Makefile +1 -1
index 9b702a1..5d4ccc6 100644
@@ -2,7 +2,7 @@
 # Makefile for Marvell's PXA168 processors line
 #
 
-obj-y				+= common.o devices.o time.o
+obj-y				+= common.o devices.o
 
 # SoC support
 obj-$(CONFIG_CPU_PXA168)	+= pxa168.o
drivers/clocksource/Makefile +1 -0
index 8b00c5c..7dda633 100644
@@ -18,6 +18,7 @@ obj-$(CONFIG_ARMADA_370_XP_TIMER)	+= time-armada-370-xp.o
 obj-$(CONFIG_ORION_TIMER)	+= time-orion.o
 obj-$(CONFIG_ARCH_BCM2835)	+= bcm2835_timer.o
 obj-$(CONFIG_ARCH_MARCO)	+= timer-marco.o
+obj-$(CONFIG_ARCH_MMP)		+= timer-mmp.o
 obj-$(CONFIG_ARCH_MXS)		+= mxs_timer.o
 obj-$(CONFIG_ARCH_PRIMA2)	+= timer-prima2.o
 obj-$(CONFIG_SUN4I_TIMER)	+= sun4i_timer.o
drivers/clocksource/timer-mmp.c renamed from arch/arm/mach-mmp/time.c +0 -2
similarity index 99%
rename from arch/arm/mach-mmp/time.c
rename to drivers/clocksource/timer-mmp.c
index 7ac41e8..ddc1b15 100644
@@ -37,8 +37,6 @@
 #include <mach/cputype.h>
 #include <asm/mach/time.h>
 
-#include "clock.h"
-
 #define TIMERS_VIRT_BASE	TIMERS1_VIRT_BASE
 
 #define MAX_DELTA		(0xfffffffe)
Signature · 1 line
1.8.1.2
#17 lore raw

PATCH v6 7/11 ARM: mmp: move timer registers into driver

In reply to Haojian Zhuang

Move the definition of timer registers into timer-mmp driver. And map timer registers in driver.

---
 arch/arm/mach-mmp/common.h                   |  2 +-
 arch/arm/mach-mmp/include/mach/addr-map.h    |  3 ++
 arch/arm/mach-mmp/include/mach/regs-timers.h | 44 ----------------------------
 arch/arm/mach-mmp/mmp2.c                     |  2 +-
 arch/arm/mach-mmp/pxa168.c                   |  2 +-
 arch/arm/mach-mmp/pxa910.c                   |  2 +-
 drivers/clocksource/timer-mmp.c              | 37 ++++++++++++++++++-----
 7 files changed, 37 insertions(+), 55 deletions(-)
 delete mode 100644 arch/arm/mach-mmp/include/mach/regs-timers.h
arch/arm/mach-mmp/common.h +1 -1
index cf445ba..22e67c6 100644
@@ -1,7 +1,7 @@
 #include <linux/reboot.h>
 #define ARRAY_AND_SIZE(x)	(x), ARRAY_SIZE(x)
 
-extern void timer_init(int irq);
+extern void timer_init(void __iomem *base, int irq);
 
 extern void __init mmp_map_io(void);
 extern void mmp_restart(enum reboot_mode, const char *);
arch/arm/mach-mmp/include/mach/addr-map.h +3 -0
index f88a44c..169c280 100644
@@ -43,4 +43,7 @@
 #define CIU_VIRT_BASE		(AXI_VIRT_BASE + 0x82c00)
 #define CIU_REG(x)		(CIU_VIRT_BASE + (x))
 
+#define TIMER1_VIRT_BASE	(APB_VIRT_BASE + 0x14000)
+#define TIMER2_VIRT_BASE	(APB_VIRT_BASE + 0x16000)
+
 #endif /* __ASM_MACH_ADDR_MAP_H */
arch/arm/mach-mmp/include/mach/regs-timers.h deleted +0 -44

44 lines not on this page · raw message

arch/arm/mach-mmp/mmp2.c +1 -1
index 09fbe7d..f26ea9d 100644
@@ -131,7 +131,7 @@ void __init mmp2_timer_init(void)
 	clk_rst = APBC_APBCLK | APBC_FNCLK | APBC_FNCLKSEL(1);
 	__raw_writel(clk_rst, APBC_TIMERS);
 
-	timer_init(IRQ_MMP2_TIMER1);
+	timer_init(TIMER1_VIRT_BASE, IRQ_MMP2_TIMER1);
 }
 
 /* on-chip devices */
arch/arm/mach-mmp/pxa168.c +1 -1
index 144e997..b6b3d5b 100644
@@ -78,7 +78,7 @@ void __init pxa168_timer_init(void)
 	/* 3.25MHz, bus/functional clock enabled, release reset */
 	__raw_writel(TIMER_CLK_RST, APBC_TIMERS);
 
-	timer_init(IRQ_PXA168_TIMER1);
+	timer_init(TIMER1_VIRT_BASE, IRQ_PXA168_TIMER1);
 }
 
 void pxa168_clear_keypad_wakeup(void)
arch/arm/mach-mmp/pxa910.c +1 -1
index 96e125b..df379c2 100644
@@ -112,7 +112,7 @@ void __init pxa910_timer_init(void)
 	__raw_writel(APBC_APBCLK | APBC_RST, APBC_TIMERS);
 	__raw_writel(TIMER_CLK_RST, APBC_TIMERS);
 
-	timer_init(IRQ_PXA910_AP1_TIMER1);
+	timer_init(TIMER1_VIRT_BASE, IRQ_PXA910_AP1_TIMER1);
 }
 
 /* on-chip devices */
drivers/clocksource/timer-mmp.c +30 -7
index ddc1b15..937d51e 100644
@@ -30,19 +30,39 @@
 #include <linux/of_irq.h>
 #include <linux/sched_clock.h>
 
-#include <mach/addr-map.h>
-#include <mach/regs-timers.h>
-#include <mach/regs-apbc.h>
 #include <mach/irqs.h>
 #include <mach/cputype.h>
 #include <asm/mach/time.h>
 
-#define TIMERS_VIRT_BASE	TIMERS1_VIRT_BASE
+#define TMR_CCR		(0x0000)
+#define TMR_TN_MM(n, m)	(0x0004 + ((n) << 3) + (((n) + (m)) << 2))
+#define TMR_CR(n)	(0x0028 + ((n) << 2))
+#define TMR_SR(n)	(0x0034 + ((n) << 2))
+#define TMR_IER(n)	(0x0040 + ((n) << 2))
+#define TMR_PLVR(n)	(0x004c + ((n) << 2))
+#define TMR_PLCR(n)	(0x0058 + ((n) << 2))
+#define TMR_WMER	(0x0064)
+#define TMR_WMR		(0x0068)
+#define TMR_WVR		(0x006c)
+#define TMR_WSR		(0x0070)
+#define TMR_ICR(n)	(0x0074 + ((n) << 2))
+#define TMR_WICR	(0x0080)
+#define TMR_CER		(0x0084)
+#define TMR_CMR		(0x0088)
+#define TMR_ILR(n)	(0x008c + ((n) << 2))
+#define TMR_WCR		(0x0098)
+#define TMR_WFAR	(0x009c)
+#define TMR_WSAR	(0x00A0)
+#define TMR_CVWR(n)	(0x00A4 + ((n) << 2))
+
+#define TMR_CCR_CS_0(x)	(((x) & 0x3) << 0)
+#define TMR_CCR_CS_1(x)	(((x) & 0x7) << 2)
+#define TMR_CCR_CS_2(x)	(((x) & 0x3) << 5)
 
 #define MAX_DELTA		(0xfffffffe)
 #define MIN_DELTA		(16)
 
-static void __iomem *mmp_timer_base = TIMERS_VIRT_BASE;
+static void __iomem *mmp_timer_base;
 
 /*
  * FIXME: the timer needs some delay to stablize the counter capture
@@ -189,8 +209,11 @@ static struct irqaction timer_irq = {
 	.dev_id		= &ckevt,
 };
 
-void __init timer_init(int irq)
+void __init timer_init(void __iomem *base, int irq)
 {
+	BUG_ON(!base);
+	mmp_timer_base = base;
+
 	timer_config();
 
 	setup_sched_clock(mmp_read_sched_clock, 32, CLOCK_TICK_RATE);
@@ -231,7 +254,7 @@ void __init mmp_dt_init_timer(void)
 		ret = -ENOMEM;
 		goto out;
 	}
-	timer_init(irq);
+	timer_init(mmp_timer_base, irq);
 	return;
 out:
 	pr_err("Failed to get timer from device tree with error:%d\n", ret);
Signature · 1 line
1.8.1.2
#18 lore raw

PATCH v6 7/11 ARM: mmp: move timer registers into driver

Quoted 4 lines · Haojian Zhuang

Move the definition of timer registers into timer-mmp driver. And map timer registers in driver.

Again, the commit message could better explain the "why". I guess this is to reduce drivers/* independence on mach/ includes, which must be eliminated for multiplatform build support.

Daniel

#19 lore raw

PATCH v6 8/11 ARM: pxa: init dma debugfs in late level

In reply to Haojian Zhuang

Remove pxa_init_dma() from core_initcall level since it's unncecssary for DT mode. But dma debugfs is also included in pxa_init_dma(). So only initiliaze dma debugfs in late_initcall level.

---
 arch/arm/mach-mmp/mmp2.c   |  2 +-
 arch/arm/mach-mmp/pxa168.c |  2 +-
 arch/arm/mach-mmp/pxa910.c |  2 +-
 arch/arm/plat-pxa/dma.c    | 30 ++++++++++++++++++++----------
 4 files changed, 23 insertions(+), 13 deletions(-)
arch/arm/mach-mmp/mmp2.c +1 -1
index f26ea9d..a0abfd7 100644
@@ -114,7 +114,6 @@ static int __init mmp2_init(void)
 
 	return 0;
 }
-postcore_initcall(mmp2_init);
 
 #define APBC_TIMERS	APBC_REG(0x024)
 
@@ -122,6 +121,7 @@ void __init mmp2_timer_init(void)
 {
 	unsigned long clk_rst;
 
+	mmp2_init();
 	__raw_writel(APBC_APBCLK | APBC_RST, APBC_TIMERS);
 
 	/*
arch/arm/mach-mmp/pxa168.c +1 -1
index b6b3d5b..a332d51 100644
@@ -61,7 +61,6 @@ static int __init pxa168_init(void)
 
 	return 0;
 }
-postcore_initcall(pxa168_init);
 
 /* system timer - clock enabled, 3.25MHz */
 #define TIMER_CLK_RST	(APBC_APBCLK | APBC_FNCLK | APBC_FNCLKSEL(3))
@@ -69,6 +68,7 @@ postcore_initcall(pxa168_init);
 
 void __init pxa168_timer_init(void)
 {
+	pxa168_init();
 	/* this is early, we have to initialize the CCU registers by
 	 * ourselves instead of using clk_* API. Clock rate is defined
 	 * by APBC_TIMERS_CLK_RST (3.25MHz) and enabled free-running
arch/arm/mach-mmp/pxa910.c +1 -1
index df379c2..1a84800 100644
@@ -100,7 +100,6 @@ static int __init pxa910_init(void)
 
 	return 0;
 }
-postcore_initcall(pxa910_init);
 
 /* system timer - clock enabled, 3.25MHz */
 #define TIMER_CLK_RST	(APBC_APBCLK | APBC_FNCLK | APBC_FNCLKSEL(3))
@@ -108,6 +107,7 @@ postcore_initcall(pxa910_init);
 
 void __init pxa910_timer_init(void)
 {
+	pxa910_init();
 	/* reset and configure */
 	__raw_writel(APBC_APBCLK | APBC_RST, APBC_TIMERS);
 	__raw_writel(TIMER_CLK_RST, APBC_TIMERS);
arch/arm/plat-pxa/dma.c +20 -10
index 79ef102..6c5472b 100644
@@ -228,36 +228,46 @@ err_state:
 	return NULL;
 }
 
-static void pxa_dma_init_debugfs(void)
+static int __init pxa_dma_init_debugfs(void)
 {
-	int i;
+	int i, ret;
 	struct dentry *chandir;
 
 	dbgfs_root = debugfs_create_dir(DMA_DEBUG_NAME, NULL);
-	if (IS_ERR(dbgfs_root) || !dbgfs_root)
+	if (IS_ERR(dbgfs_root) || !dbgfs_root) {
+		ret = -EINVAL;
 		goto err_root;
+	}
 
 	dbgfs_state = debugfs_create_file("state", 0400, dbgfs_root, NULL,
 					  &dbg_fops_state);
-	if (!dbgfs_state)
+	if (!dbgfs_state) {
+		ret = -EINVAL;
 		goto err_state;
+	}
 
 	dbgfs_chan = kmalloc(sizeof(*dbgfs_state) * num_dma_channels,
 			     GFP_KERNEL);
-	if (!dbgfs_chan)
+	if (!dbgfs_chan) {
+		ret = -ENOMEM;
 		goto err_alloc;
+	}
 
 	chandir = debugfs_create_dir("channels", dbgfs_root);
-	if (!chandir)
+	if (!chandir) {
+		ret = -EINVAL;
 		goto err_chandir;
+	}
 
 	for (i = 0; i < num_dma_channels; i++) {
 		dbgfs_chan[i] = pxa_dma_dbg_alloc_chan(i, chandir);
-		if (!dbgfs_chan[i])
+		if (!dbgfs_chan[i]) {
+			ret = -EINVAL;
 			goto err_chans;
+		}
 	}
 
-	return;
+	return 0;
 err_chans:
 err_chandir:
 	kfree(dbgfs_chan);
@@ -266,7 +276,9 @@ err_state:
 	debugfs_remove_recursive(dbgfs_root);
 err_root:
 	pr_err("pxa_dma: debugfs is not available\n");
+	return ret;
 }
+late_initcall(pxa_dma_init_debugfs);
 
 static void __exit pxa_dma_cleanup_debugfs(void)
 {
@@ -385,7 +397,5 @@ int __init pxa_init_dma(int irq, int num_ch)
 	}
 	num_dma_channels = num_ch;
 
-	pxa_dma_init_debugfs();
-
 	return 0;
 }
Signature · 1 line
1.8.1.2
#20 lore raw

PATCH v6 8/11 ARM: pxa: init dma debugfs in late level

Hi Haojian,

Quoted 11 lines · Haojian Zhuang

Remove pxa_init_dma() from core_initcall level since it's unncecssary for DT mode. But dma debugfs is also included in pxa_init_dma(). So only initiliaze dma debugfs in late_initcall level.

---
 arch/arm/mach-mmp/mmp2.c   |  2 +-
 arch/arm/mach-mmp/pxa168.c |  2 +-
 arch/arm/mach-mmp/pxa910.c |  2 +-
 arch/arm/plat-pxa/dma.c    | 30 ++++++++++++++++++++----------
 4 files changed, 23 insertions(+), 13 deletions(-)

This will collide with my dma rework series, which removes arch/arm/plat-pxa/dma.c eventually. Do you think it's possible to just drop that patch here and wait for my series to settle?

Daniel

#21 lore raw

PATCH v6 8/11 ARM: pxa: init dma debugfs in late level

Quoted 15 lines · Daniel Mack

Hi Haojian,

Quoted 11 lines · Haojian Zhuang

Remove pxa_init_dma() from core_initcall level since it's unncecssary for DT mode. But dma debugfs is also included in pxa_init_dma(). So only initiliaze dma debugfs in late_initcall level.

---
 arch/arm/mach-mmp/mmp2.c   |  2 +-
 arch/arm/mach-mmp/pxa168.c |  2 +-
 arch/arm/mach-mmp/pxa910.c |  2 +-
 arch/arm/plat-pxa/dma.c    | 30 ++++++++++++++++++++----------
 4 files changed, 23 insertions(+), 13 deletions(-)
Quoted 7 lines

This will collide with my dma rework series, which removes arch/arm/plat-pxa/dma.c eventually. Do you think it's possible to just drop that patch here and wait for my series to settle?

Daniel

Sure. I'll drop it from my patch list.

Best Regards
Haojian

#22 lore raw

PATCH v6 9/11 clk: mmp: parse clock from dts

In reply to Haojian Zhuang

Parse clock information from DTS file for mach-mmp.

Changelog:
v6:
1. Remove marvell string from properties in clock driver.
2. Append document for clock binding device tree.
3. Use apbc to replace apbcp. Since the main difference is register base.

v5:
1. Replace clk_register_mux() by clk_register_mux_table().

---
 .../devicetree/bindings/clock/mmp-clock.txt        |  51 ++++
 arch/arm/mach-mmp/mmp-dt.c                         |   2 +
 drivers/clk/mmp/Makefile                           |   2 +-
 drivers/clk/mmp/clk-mmp.c                          | 300 +++++++++++++++++++++
 4 files changed, 354 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/clock/mmp-clock.txt
 create mode 100644 drivers/clk/mmp/clk-mmp.c
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +51 -0
new file mode 100644
index 0000000..5fe52a2
@@ -0,0 +1,51 @@
+Device Tree Clock bindings for arch-mmp
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties for fixed rate clocks:
+  - compatible : should be "marvell,mmp-fixed-clkrate".
+  - clocks : should be the input parent clock phandle for the clock. This
+	should be the reference clock.
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.
+  - clock-frequency : frequency of the fixed rate clock
+
+Required properties for fixed factor clocks:
+  - compatible : should be "marvell,mmp-fixed-clkfactor".
+  - clocks : should be the input parent clock phandle for the clock. This
+	should be the reference clock.
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.
+
+Required properties for mux clocks:
+  - compatible : should be "marvell,mmp-clkmux".
+  - clocks : should be the input parent clock phandle for the clock. This
+	should be the reference clock.
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.
+  - mmp-clk-reg : mux register offset & mask bits.
+  - mmp-clk-sel : array of mux select bits
+
+Required properties for divider clocks:
+  - compatible : should be "marvell,mmp-apmu-clkdiv".
+  - clocks : should be the input parent clock phandle for the clock. This
+	should be the reference clock.
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.
+  - mmp-clk-reg : divider register offset & mask bits.
+
+Required properties for gate clocks:
+  - compatible : should be "marvell,mmp-apbc-clk".
+  - clocks : should be the input parent clock phandle for the clock. This
+	should be the reference clock.
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.
+  - mmp-clk-reg : gate register offset & mask bits.
+
+Optional properties for gate clocks:
+  - mmp-clk-delay : delay between enabling function clock and apb clock
+  - mmp-apbc-power-ctrl : enable apbc power control bit
+
+For example:
arch/arm/mach-mmp/mmp-dt.c +2 -0
index cca529c..f109bf6 100644
@@ -9,6 +9,7 @@
  *  publishhed by the Free Software Foundation.
  */
 
+#include <linux/clk-provider.h>
 #include <linux/irqchip.h>
 #include <linux/of_platform.h>
 #include <asm/mach/arch.h>
@@ -48,6 +49,7 @@ static void __init pxa168_dt_init(void)
 
 static void __init pxa910_dt_init(void)
 {
+	of_clk_init(NULL);
 	of_platform_populate(NULL, of_default_bus_match_table,
 			     pxa910_auxdata_lookup, NULL);
 }
drivers/clk/mmp/Makefile +1 -1
index 392d780..83182e5 100644
@@ -2,7 +2,7 @@
 # Makefile for mmp specific clk
 #
 
-obj-y += clk-apbc.o clk-apmu.o clk-frac.o
+obj-y += clk-apbc.o clk-apmu.o clk-frac.o clk-mmp.o
 
 obj-$(CONFIG_CPU_PXA168) += clk-pxa168.o
 obj-$(CONFIG_CPU_PXA910) += clk-pxa910.o
drivers/clk/mmp/clk-mmp.c new file +300 -0

300 lines not on this page · raw message

Signature · 1 line
1.8.1.2
#23 lore raw

PATCH v6 9/11 clk: mmp: parse clock from dts

Quoted 28 lines · Haojian Zhuang

Parse clock information from DTS file for mach-mmp.

Changelog:
v6:
1. Remove marvell string from properties in clock driver.
2. Append document for clock binding device tree.
3. Use apbc to replace apbcp. Since the main difference is register base.

v5:
1. Replace clk_register_mux() by clk_register_mux_table().

---
 .../devicetree/bindings/clock/mmp-clock.txt        |  51 ++++
 arch/arm/mach-mmp/mmp-dt.c                         |   2 +
 drivers/clk/mmp/Makefile                           |   2 +-
 drivers/clk/mmp/clk-mmp.c                          | 300 +++++++++++++++++++++
 4 files changed, 354 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/clock/mmp-clock.txt
 create mode 100644 drivers/clk/mmp/clk-mmp.c
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +0 -0
new file mode 100644
index 0000000..5fe52a2

Documentation/devicetree/bindings/clock/mmp-clock.txt line 6

Quoted 6 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +6 -0
@@ -0,0 +1,51 @@
+Device Tree Clock bindings for arch-mmp
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+

Are these used somewhere already?

Documentation/devicetree/bindings/clock/mmp-clock.txt line 13

Quoted 7 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +7 -0
+Required properties for fixed rate clocks:
+  - compatible : should be "marvell,mmp-fixed-clkrate".
+  - clocks : should be the input parent clock phandle for the clock. This
+       should be the reference clock.
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.
+  - clock-frequency : frequency of the fixed rate clock

Why is "fixed-clock" not good enough?

This doesn't seem to describe anything more, and there's nothing additional in the driver...

Documentation/devicetree/bindings/clock/mmp-clock.txt line 20

Quoted 7 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +7 -0
+
+Required properties for fixed factor clocks:
+  - compatible : should be "marvell,mmp-fixed-clkfactor".
+  - clocks : should be the input parent clock phandle for the clock. This
+       should be the reference clock.
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.

The code seems to use an additional property not described here (mmp-fixed-factor). What does that represent?

Why not "fixed-factor-clock"?

Documentation/devicetree/bindings/clock/mmp-clock.txt line 28

Quoted 8 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +8 -0
+
+Required properties for mux clocks:
+  - compatible : should be "marvell,mmp-clkmux".
+  - clocks : should be the input parent clock phandle for the clock. This
+       should be the reference clock.
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.
+  - mmp-clk-reg : mux register offset & mask bits.

Offset from where? There's no reg, no container's described and there's no example.

Documentation/devicetree/bindings/clock/mmp-clock.txt line 37

Quoted 9 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +9 -0
+  - mmp-clk-sel : array of mux select bits
+
+Required properties for divider clocks:
+  - compatible : should be "marvell,mmp-apmu-clkdiv".
+  - clocks : should be the input parent clock phandle for the clock. This
+       should be the reference clock.
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.
+  - mmp-clk-reg : divider register offset & mask bits.

Similar comments to those above. This is also a fixed-factor-clock, no?

Documentation/devicetree/bindings/clock/mmp-clock.txt line 45

Quoted 8 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +8 -0
+
+Required properties for gate clocks:
+  - compatible : should be "marvell,mmp-apbc-clk".
+  - clocks : should be the input parent clock phandle for the clock. This
+       should be the reference clock.
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.
+  - mmp-clk-reg : gate register offset & mask bits.

Offset from where?

Documentation/devicetree/bindings/clock/mmp-clock.txt line 48

Quoted 3 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +3 -0
+
+Optional properties for gate clocks:
+  - mmp-clk-delay : delay between enabling function clock and apb clock

Units?

Documentation/devicetree/bindings/clock/mmp-clock.txt line 49

Quoted 1 line
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +1 -0
+  - mmp-apbc-power-ctrl : enable apbc power control bit

Is this a boolean or the offset of the bit offset?

Documentation/devicetree/bindings/clock/mmp-clock.txt line 51

Quoted 2 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +2 -0
+
+For example:

Missing example?

[...]

Documentation/devicetree/bindings/clock/mmp-clock.txt

Quoted 7 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +7 -0
+static const struct of_device_id mmp_of_match[] = {
+       { .compatible = "marvell,mmp-mpmu", .data = (void *)MMP_MPMU, },
+       { .compatible = "marvell,mmp-apmu", .data = (void *)MMP_APMU, },
+       { .compatible = "marvell,mmp-apbc", .data = (void *)MMP_APBC, },
+       { .compatible = "marvell,mmp-apbcp", .data = (void *)MMP_APBCP, },
+       { .compatible = "mrvl,apb-bus", .data = (void *)MMP_APB, },
+};

Not documented?

Thanks,
Mark.

#24 lore raw

PATCH v6 9/11 clk: mmp: parse clock from dts

Hi Mark, Haojian,

Quoted 26 lines · On Friday 09 of August 2013 17:04:01 Mark Rutland wrote:
Quoted 25 lines · Haojian Zhuang

Parse clock information from DTS file for mach-mmp.

Changelog:
v6:
1. Remove marvell string from properties in clock driver.
2. Append document for clock binding device tree.
3. Use apbc to replace apbcp. Since the main difference is register base.

v5:
1. Replace clk_register_mux() by clk_register_mux_table().

---

 .../devicetree/bindings/clock/mmp-clock.txt        |  51 ++++
 arch/arm/mach-mmp/mmp-dt.c                         |   2 +
 drivers/clk/mmp/Makefile                           |   2 +-
 drivers/clk/mmp/clk-mmp.c                          | 300
 +++++++++++++++++++++ 4 files changed, 354 insertions(+), 1
 deletion(-)
 create mode 100644

Documentation/devicetree/bindings/clock/mmp-clock.txt create mode 100644 drivers/clk/mmp/clk-mmp.c

Quoted 15 lines

Documentation/devicetree/bindings/clock/mmp-clock.txt line 6

Quoted 13 lines
(quoted diff) +0 -0

b/Documentation/devicetree/bindings/clock/mmp-clock.txt new file mode 100644
index 0000000..5fe52a2

Documentation/devicetree/bindings/clock/mmp-clock.txt new file +6 -0
@@ -0,0 +1,51 @@
+Device Tree Clock bindings for arch-mmp
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+

Are these used somewhere already?

Hmm? Are you asking about the generic clock bindings or I didn't get your question right? They are supposed to be used (and they are used) whenever clocks are involved.

Quoted 8 lines

Documentation/devicetree/bindings/clock/mmp-clock.txt line 13

Quoted 7 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +3 -0
+Required properties for fixed rate clocks:
+  - compatible : should be "marvell,mmp-fixed-clkrate".
+  - clocks : should be the input parent clock phandle for the clock.

This + should be the reference clock.

Documentation/devicetree/bindings/clock/mmp-clock.txt new file +3 -0
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.
+  - clock-frequency : frequency of the fixed rate clock
Quoted 4 lines

Why is "fixed-clock" not good enough?

This doesn't seem to describe anything more, and there's nothing additional in the driver...

+1

Quoted 11 lines

Documentation/devicetree/bindings/clock/mmp-clock.txt

Quoted 7 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +4 -0
+
+Required properties for fixed factor clocks:
+  - compatible : should be "marvell,mmp-fixed-clkfactor".
+  - clocks : should be the input parent clock phandle for the clock.

This + should be the reference clock.

Documentation/devicetree/bindings/clock/mmp-clock.txt new file +2 -0
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.

The code seems to use an additional property not described here (mmp-fixed-factor). What does that represent?

Quoted 1 line

Why not "fixed-factor-clock"?

+1

Quoted 11 lines

Documentation/devicetree/bindings/clock/mmp-clock.txt line 28

Quoted 8 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +4 -0
+
+Required properties for mux clocks:
+  - compatible : should be "marvell,mmp-clkmux".
+  - clocks : should be the input parent clock phandle for the clock.

This + should be the reference clock.

Documentation/devicetree/bindings/clock/mmp-clock.txt new file +3 -0
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.
+  - mmp-clk-reg : mux register offset & mask bits.

Offset from where? There's no reg, no container's described and there's no example.

+1

Quoted 11 lines

Documentation/devicetree/bindings/clock/mmp-clock.txt line 37

Quoted 9 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +5 -0
+  - mmp-clk-sel : array of mux select bits
+
+Required properties for divider clocks:
+  - compatible : should be "marvell,mmp-apmu-clkdiv".
+  - clocks : should be the input parent clock phandle for the clock.

This + should be the reference clock.

Documentation/devicetree/bindings/clock/mmp-clock.txt new file +3 -0
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.
+  - mmp-clk-reg : divider register offset & mask bits.

Similar comments to those above. This is also a fixed-factor-clock, no?

This looks like a clock with configurable divisor for me, but again, why not use the standard divider clock here?

Best regards,
Tomasz

Quoted 45 lines

Documentation/devicetree/bindings/clock/mmp-clock.txt line 45

Quoted 8 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +4 -0
+
+Required properties for gate clocks:
+  - compatible : should be "marvell,mmp-apbc-clk".
+  - clocks : should be the input parent clock phandle for the clock.

This + should be the reference clock.

Documentation/devicetree/bindings/clock/mmp-clock.txt new file +3 -0
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.
+  - mmp-clk-reg : gate register offset & mask bits.

Offset from where?

Quoted 4 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +3 -0
+
+Optional properties for gate clocks:
+  - mmp-clk-delay : delay between enabling function clock and apb

clock

Units?

Quoted 1 line

+ - mmp-apbc-power-ctrl : enable apbc power control bit

Is this a boolean or the offset of the bit offset?

Documentation/devicetree/bindings/clock/mmp-clock.txt line 51

Quoted 3 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +2 -0
+
 
+For example:

Missing example?

[...]

Quoted 7 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +2 -0
+static const struct of_device_id mmp_of_match[] = {
+       { .compatible = "marvell,mmp-mpmu", .data = (void *)MMP_MPMU,

}, + { .compatible = "marvell,mmp-apmu", .data = (void
*)MMP_APMU, }, + { .compatible = "marvell,mmp-apbc", .data =
(void *)MMP_APBC, }, + { .compatible = "marvell,mmp-apbcp",
.data = (void *)MMP_APBCP, }, + { .compatible = "mrvl,apb-bus",
.data = (void *)MMP_APB, }, +};

Not documented?

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#25 lore raw

PATCH v6 9/11 clk: mmp: parse clock from dts

Quoted 45 lines · Tomasz Figa

Hi Mark, Haojian,

Quoted 26 lines · On Friday 09 of August 2013 17:04:01 Mark Rutland wrote:
Quoted 25 lines · Haojian Zhuang

Parse clock information from DTS file for mach-mmp.

Changelog:
v6:
1. Remove marvell string from properties in clock driver.
2. Append document for clock binding device tree.
3. Use apbc to replace apbcp. Since the main difference is register base.

v5:
1. Replace clk_register_mux() by clk_register_mux_table().

---

 .../devicetree/bindings/clock/mmp-clock.txt        |  51 ++++
 arch/arm/mach-mmp/mmp-dt.c                         |   2 +
 drivers/clk/mmp/Makefile                           |   2 +-
 drivers/clk/mmp/clk-mmp.c                          | 300
 +++++++++++++++++++++ 4 files changed, 354 insertions(+), 1
 deletion(-)
 create mode 100644

Documentation/devicetree/bindings/clock/mmp-clock.txt create mode 100644 drivers/clk/mmp/clk-mmp.c

Quoted 15 lines

Documentation/devicetree/bindings/clock/mmp-clock.txt line 6

Quoted 13 lines
(quoted diff) +0 -0

b/Documentation/devicetree/bindings/clock/mmp-clock.txt new file mode 100644
index 0000000..5fe52a2

Documentation/devicetree/bindings/clock/mmp-clock.txt new file +6 -0
@@ -0,0 +1,51 @@
+Device Tree Clock bindings for arch-mmp
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+

Are these used somewhere already?

Quoted 3 lines

Hmm? Are you asking about the generic clock bindings or I didn't get your question right? They are supposed to be used (and they are used) whenever clocks are involved.

Sorry, I meant are there any dts out there using these specific bindings? I wasn't sure how far upstream the series was, and wanted to make sure this hadn't gone upstream before I managed to reply.

Thanks,
Mark

#26 lore raw

PATCH v6 9/11 clk: mmp: parse clock from dts

Quoted 48 lines · On Saturday 10 of August 2013 13:31:23 Mark Rutland wrote:
Quoted 47 lines · Tomasz Figa

Hi Mark, Haojian,

Quoted 43 lines · On Friday 09 of August 2013 17:04:01 Mark Rutland wrote:
Quoted 34 lines · Haojian Zhuang

Parse clock information from DTS file for mach-mmp.

Changelog:
v6:
1. Remove marvell string from properties in clock driver.
2. Append document for clock binding device tree.
3. Use apbc to replace apbcp. Since the main difference is register
base.

v5:
1. Replace clk_register_mux() by clk_register_mux_table().

---

 .../devicetree/bindings/clock/mmp-clock.txt        |  51 ++++
 arch/arm/mach-mmp/mmp-dt.c                         |   2 +
 drivers/clk/mmp/Makefile                           |   2 +-
 drivers/clk/mmp/clk-mmp.c                          | 300
 +++++++++++++++++++++ 4 files changed, 354 insertions(+), 1
 deletion(-)
 create mode 100644

Documentation/devicetree/bindings/clock/mmp-clock.txt create mode 100644 drivers/clk/mmp/clk-mmp.c

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/clock/mmp-clock.txt new file mode
100644
index 0000000..5fe52a2

Documentation/devicetree/bindings/clock/mmp-clock.txt line 6

Quoted 6 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +6 -0
@@ -0,0 +1,51 @@
+Device Tree Clock bindings for arch-mmp
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+

Are these used somewhere already?

Quoted 7 lines
Quoted 3 lines

Hmm? Are you asking about the generic clock bindings or I didn't get your question right? They are supposed to be used (and they are used) whenever clocks are involved.

Sorry, I meant are there any dts out there using these specific bindings? I wasn't sure how far upstream the series was, and wanted to make sure this hadn't gone upstream before I managed to reply.

OK, sorry for the noise then.

Best regards,
Tomasz

#27 lore raw

PATCH v6 9/11 clk: mmp: parse clock from dts

In reply to Haojian Zhuang

Hi Haojian,

I just stumbed upon this mail... I see that I have done some similar work in my recent patches. Maybe you could CC me in future when we are duplicating efforts.

Quoted 8 lines · Haojian Zhuang

Parse clock information from DTS file for mach-mmp.

Changelog:
v6:
1. Remove marvell string from properties in clock driver.
2. Append document for clock binding device tree.
3. Use apbc to replace apbcp. Since the main difference is register base.

If one replaces the other then why are both still present in the patch? I think all these clock type acronyms deserve a quick comment explaining what they are.

Documentation/devicetree/bindings/clock/mmp-clock.txt line 37

Quoted 7 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +7 -0
+Required properties for divider clocks:
+  - compatible : should be "marvell,mmp-apmu-clkdiv".
+  - clocks : should be the input parent clock phandle for the clock. This
+       should be the reference clock.
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.
+  - mmp-clk-reg : divider register offset & mask bits.

I think you should use the standard 'reg' property for providing register offets, and then other properties (with better names) for other info that needs to be supplied e.g. mask.

I see that your implementation here moves away from using the existing code in clk-apmu.c and instead just uses clk-divider directly. That might make sense, but your new DTS does not reimplement many of the static APMU clocks that were previously implemented (e.g. ccic, sdh in clk-mmp2.c). Why is this?

Documentation/devicetree/bindings/clock/mmp-clock.txt line 45

Quoted 7 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +7 -0
+Required properties for gate clocks:
+  - compatible : should be "marvell,mmp-apbc-clk".
+  - clocks : should be the input parent clock phandle for the clock. This
+       should be the reference clock.
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.
+  - mmp-clk-reg : gate register offset & mask bits.

mmp-clk-reg contains two elements, I guess. But the second element is not used in your driver for mmp-apbc-clk.

Thanks
Daniel

#28 lore raw

PATCH v6 9/11 clk: mmp: parse clock from dts

Quoted 5 lines · Daniel Drake

Hi Haojian,

I just stumbed upon this mail... I see that I have done some similar work in my recent patches. Maybe you could CC me in future when we are duplicating efforts.

Yes, I'll cc you for the next patches.

Quoted 12 lines
Quoted 8 lines · Haojian Zhuang

Parse clock information from DTS file for mach-mmp.

Changelog:
v6:
1. Remove marvell string from properties in clock driver.
2. Append document for clock binding device tree.
3. Use apbc to replace apbcp. Since the main difference is register base.

Quoted 3 lines

If one replaces the other then why are both still present in the patch? I think all these clock type acronyms deserve a quick comment explaining what they are.

Maybe I didn't explain it well. I still use the apbcp node, but I replace all apbcp properties by apbc properties.

The apbc registers are similar to apbcp registers. But the base registers are different. So I use different apbc & apbcp node.

Quoted 9 lines

Documentation/devicetree/bindings/clock/mmp-clock.txt line 37

Quoted 8 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +7 -0
+Required properties for divider clocks:
+  - compatible : should be "marvell,mmp-apmu-clkdiv".
+  - clocks : should be the input parent clock phandle for the clock. This
+       should be the reference clock.
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.
+  - mmp-clk-reg : divider register offset & mask bits.
Quoted 3 lines

I think you should use the standard 'reg' property for providing register offets, and then other properties (with better names) for other info that needs to be supplied e.g. mask.

It seems that reg property is better. I'll change it.

Quoted 7 lines

I see that your implementation here moves away from using the existing code in clk-apmu.c and instead just uses clk-divider directly. That might make sense, but your new DTS does not reimplement many of the static APMU clocks that were previously implemented (e.g. ccic, sdh in clk-mmp2.c). Why is this?

I'll append them later.

Quoted 10 lines

Documentation/devicetree/bindings/clock/mmp-clock.txt line 45

Quoted 7 lines
Documentation/devicetree/bindings/clock/mmp-clock.txt new file +7 -0
+Required properties for gate clocks:
+  - compatible : should be "marvell,mmp-apbc-clk".
+  - clocks : should be the input parent clock phandle for the clock. This
+       should be the reference clock.
+  - clock-output-names : should be reference name.
+  - #clock-cells : from common clock binding; should be set to 0.
+  - mmp-clk-reg : gate register offset & mask bits.

mmp-clk-reg contains two elements, I guess. But the second element is not used in your driver for mmp-apbc-clk.

Thanks. I'll fix it.

Quoted 3 lines

Thanks
Daniel

#29 lore raw

PATCH v6 9/11 clk: mmp: parse clock from dts

Quoted 7 lines · Haojian Zhuang
Quoted 6 lines

I see that your implementation here moves away from using the existing code in clk-apmu.c and instead just uses clk-divider directly. That might make sense, but your new DTS does not reimplement many of the static APMU clocks that were previously implemented (e.g. ccic, sdh in clk-mmp2.c). Why is this?

I'll append them later.

Looking closer I see that you only implemented this for pxa910 - and (as noted above) you only reimplemented a subset of the clocks.

The original static clock code is still in place and being called (pxa910_init calls pxa910_clk_init). This will result in some of the
clocks being duplicated (defined once in DT and again in static code).
I think you should either add a mechanism to avoid doing both (e.g.
see the end of http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/177153.html)
or simply remove support for non-DT clocks (depending on user base,
not sure if this is possible).

I think adding support for all the clocks in DT now is quite important as I suspect it will change your DT design. Your patch treats APMU clocks always as simple dividers or muxes, always enabled, however I don't think it is possible to represent the SDH/display APMU clocks in this way, at least I have not found a way to do it. These clocks are in reset by default and certain bits must be written to enable them.

Finally there is something confusing in mmp_apbc_setup()

+ if (of_property_read_string(np, "clock-names", &clk_name))
+ clkdev = 1;
+ else {
+ of_property_read_string(np, "clock-output-names", &clk_name);
+ clkdev = 0;
+ }
+ if (clkdev)
+ clk_register_clkdev(clk, clk_name, NULL);

Can you explain what this logic is trying to do?
I think it is wrong. If I provide the name for the clock I want to output from my provider, I do not get registered? And in order for me to get my clock provider registered, I have to provide a clock-names property which is then used to name the device?? But clock-names is usually used for the input clock name. And I don't see why providing a name is necessary, it could just use node->name.

Thanks
Daniel

#30 lore raw

PATCH v6 10/11 ARM: dts: support common clock in arch mmp

In reply to Haojian Zhuang

Support common clock in DTS file for arch mmp.

---
 arch/arm/boot/dts/pxa168-aspenite.dts |   3 +
 arch/arm/boot/dts/pxa168-clk.dtsi     | 304 ++++++++++++++++++
 arch/arm/boot/dts/pxa168.dtsi         |  10 +
 arch/arm/boot/dts/pxa910-clk.dtsi     | 569 ++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/pxa910-dkb.dts      |  11 +
 arch/arm/boot/dts/pxa910.dtsi         |  12 +-
 6 files changed, 908 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/pxa168-clk.dtsi
 create mode 100644 arch/arm/boot/dts/pxa910-clk.dtsi
arch/arm/boot/dts/pxa168-aspenite.dts +3 -0
index e762fac..2597e98 100644
@@ -24,6 +24,9 @@
 
 	soc {
 		apb at d4000000 {
+			timer0: timer at d4014000 {
+				status = "okay";
+			};
 			uart1: uart at d4017000 {
 				status = "okay";
 			};
arch/arm/boot/dts/pxa168-clk.dtsi new file +304 -0

304 lines not on this page · raw message

arch/arm/boot/dts/pxa168.dtsi +10 -0
index 975dad2..250bf2c 100644
@@ -8,6 +8,7 @@
  */
 
 /include/ "skeleton.dtsi"
+/include/ "pxa910-clk.dtsi"
 
 / {
 	aliases {
@@ -53,12 +54,15 @@
 				compatible = "mrvl,mmp-timer";
 				reg = <0xd4014000 0x100>;
 				interrupts = <13>;
+				clocks = <&apbc_timer0_clk>;
+				status = "disabled";
 			};
 
 			uart1: uart at d4017000 {
 				compatible = "mrvl,mmp-uart";
 				reg = <0xd4017000 0x1000>;
 				interrupts = <27>;
+				clocks = <&apbc_uart1_clk>;
 				status = "disabled";
 			};
 
@@ -66,6 +70,7 @@
 				compatible = "mrvl,mmp-uart";
 				reg = <0xd4018000 0x1000>;
 				interrupts = <28>;
+				clocks = <&apbc_uart2_clk>;
 				status = "disabled";
 			};
 
@@ -73,6 +78,7 @@
 				compatible = "mrvl,mmp-uart";
 				reg = <0xd4026000 0x1000>;
 				interrupts = <29>;
+				clocks = <&apbc_uart3_clk>;
 				status = "disabled";
 			};
 
@@ -87,6 +93,7 @@
 				interrupt-names = "gpio_mux";
 				interrupt-controller;
 				#interrupt-cells = <1>;
+				clocks = <&apbc_gpio_clk>;
 				ranges;
 
 				gcb0: gpio at d4019000 {
@@ -111,6 +118,7 @@
 				reg = <0xd4011000 0x1000>;
 				interrupts = <7>;
 				mrvl,i2c-fast-mode;
+				clocks = <&apbc_twsi1_clk>;
 				status = "disabled";
 			};
 
@@ -118,6 +126,7 @@
 				compatible = "mrvl,mmp-twsi";
 				reg = <0xd4025000 0x1000>;
 				interrupts = <58>;
+				clocks = <&apbc_twsi2_clk>;
 				status = "disabled";
 			};
 
@@ -126,6 +135,7 @@
 				reg = <0xd4010000 0x1000>;
 				interrupts = <5 6>;
 				interrupt-names = "rtc 1Hz", "rtc alarm";
+				clocks = <&apbc_rtc_clk>;
 				status = "disabled";
 			};
 		};
arch/arm/boot/dts/pxa910-clk.dtsi new file +569 -0

569 lines not on this page · raw message

arch/arm/boot/dts/pxa910-dkb.dts +11 -0
index 595492a..b892ebe 100644
@@ -24,6 +24,17 @@
 
 	soc {
 		apb at d4000000 {
+			pll2: refclk1001mhz {
+				/* Reference clock for internal PLL2 */
+				compatible = "marvell,mmp-fixed-clkrate";
+				#clock-cells = <0>;
+				clocks = <&osc_26m>;
+				clock-frequency = <1001000000>;
+				clock-output-names = "refclk1001mhz";
+			};
+			timer0: timer at d4014000 {
+				status = "okay";
+			};
 			uart1: uart at d4017000 {
 				status = "okay";
 			};
arch/arm/boot/dts/pxa910.dtsi +11 -1
index 0247c62..cd888d3 100644
@@ -8,6 +8,7 @@
  */
 
 /include/ "skeleton.dtsi"
+/include/ "pxa910-clk.dtsi"
 
 / {
 	aliases {
@@ -44,7 +45,6 @@
 				reg = <0xd4282000 0x1000>;
 				mrvl,intc-nr-irqs = <64>;
 			};
-
 		};
 
 		apb at d4000000 {	/* APB */
@@ -58,12 +58,15 @@
 				compatible = "mrvl,mmp-timer";
 				reg = <0xd4014000 0x100>;
 				interrupts = <13>;
+				clocks = <&apbc_timer0_clk>;
+				status = "disabled";
 			};
 
 			timer1: timer at d4016000 {
 				compatible = "mrvl,mmp-timer";
 				reg = <0xd4016000 0x100>;
 				interrupts = <29>;
+				clocks = <&apbc_timer1_clk>;
 				status = "disabled";
 			};
 
@@ -71,6 +74,7 @@
 				compatible = "mrvl,mmp-uart";
 				reg = <0xd4017000 0x1000>;
 				interrupts = <27>;
+				clocks = <&apbc_uart1_clk>;
 				status = "disabled";
 			};
 
@@ -78,6 +82,7 @@
 				compatible = "mrvl,mmp-uart";
 				reg = <0xd4018000 0x1000>;
 				interrupts = <28>;
+				clocks = <&apbc_uart2_clk>;
 				status = "disabled";
 			};
 
@@ -85,6 +90,7 @@
 				compatible = "mrvl,mmp-uart";
 				reg = <0xd4036000 0x1000>;
 				interrupts = <59>;
+				clocks = <&apbcp_uart3_clk>;
 				status = "disabled";
 			};
 
@@ -99,6 +105,7 @@
 				interrupt-names = "gpio_mux";
 				interrupt-controller;
 				#interrupt-cells = <1>;
+				clocks = <&apbc_gpio_clk>;
 				ranges;
 
 				gcb0: gpio at d4019000 {
@@ -124,6 +131,7 @@
 				#size-cells = <0>;
 				reg = <0xd4011000 0x1000>;
 				interrupts = <7>;
+				clocks = <&apbc_twsi1_clk>;
 				mrvl,i2c-fast-mode;
 				status = "disabled";
 			};
@@ -134,6 +142,7 @@
 				#size-cells = <0>;
 				reg = <0xd4037000 0x1000>;
 				interrupts = <54>;
+				clocks = <&apbcp_twsi2_clk>;
 				status = "disabled";
 			};
 
@@ -142,6 +151,7 @@
 				reg = <0xd4010000 0x1000>;
 				interrupts = <5 6>;
 				interrupt-names = "rtc 1Hz", "rtc alarm";
+				clocks = <&apbc_rtc_clk>;
 				status = "disabled";
 			};
 		};
Signature · 1 line
1.8.1.2
#31 lore raw

PATCH v6 11/11 ARM: mmp: avoid to use cpu_is_xxx in timer

In reply to Haojian Zhuang

Avoid to use cpu_is_xxx() in timer-mmp driver, since it blocks the multiplatform build. Now add mmp2_mode variable for legacy mode. In DT mode, set the right clock rate in DTS file.

---
 arch/arm/Kconfig                      |  1 +
 arch/arm/boot/dts/mmp2-brownstone.dts |  4 ++
 arch/arm/boot/dts/pxa168-aspenite.dts |  1 +
 arch/arm/boot/dts/pxa910-dkb.dts      |  1 +
 arch/arm/mach-mmp/common.h            |  2 +-
 arch/arm/mach-mmp/mmp-dt.c            | 14 ++++---
 arch/arm/mach-mmp/mmp2-dt.c           | 12 ++++--
 arch/arm/mach-mmp/mmp2.c              |  2 +-
 arch/arm/mach-mmp/pxa168.c            |  2 +-
 arch/arm/mach-mmp/pxa910.c            |  2 +-
 drivers/clocksource/timer-mmp.c       | 72 +++++++++++++++++++++--------------
 11 files changed, 72 insertions(+), 41 deletions(-)
arch/arm/Kconfig +1 -0
index 76793a6..eed4919 100644
@@ -553,6 +553,7 @@ config ARCH_MMP
 	depends on MMU
 	select ARCH_REQUIRE_GPIOLIB
 	select CLKDEV_LOOKUP
+	select CLKSRC_OF if OF
 	select GENERIC_ALLOCATOR
 	select GENERIC_CLOCKEVENTS
 	select GPIO_PXA
arch/arm/boot/dts/mmp2-brownstone.dts +4 -0
index 7f70a39..29c7dc6 100644
@@ -24,6 +24,10 @@
 
 	soc {
 		apb at d4000000 {
+			timer0: timer at d4014000 {
+				clock-frequency = <6500000>;
+				status = "okay";
+			};
 			uart3: uart at d4018000 {
 				status = "okay";
 			};
arch/arm/boot/dts/pxa168-aspenite.dts +1 -0
index 2597e98..9b3d6d9 100644
@@ -25,6 +25,7 @@
 	soc {
 		apb at d4000000 {
 			timer0: timer at d4014000 {
+				clock-frequency = <3250000>;
 				status = "okay";
 			};
 			uart1: uart at d4017000 {
arch/arm/boot/dts/pxa910-dkb.dts +1 -0
index b892ebe..3cd1a85 100644
@@ -33,6 +33,7 @@
 				clock-output-names = "refclk1001mhz";
 			};
 			timer0: timer at d4014000 {
+				clock-frequency = <3250000>;
 				status = "okay";
 			};
 			uart1: uart at d4017000 {
arch/arm/mach-mmp/common.h +1 -1
index 22e67c6..2415282 100644
@@ -1,7 +1,7 @@
 #include <linux/reboot.h>
 #define ARRAY_AND_SIZE(x)	(x), ARRAY_SIZE(x)
 
-extern void timer_init(void __iomem *base, int irq);
+extern void timer_init(void __iomem *base, int irq, int mmp2_mode);
 
 extern void __init mmp_map_io(void);
 extern void mmp_restart(enum reboot_mode, const char *);
arch/arm/mach-mmp/mmp-dt.c +9 -5
index f109bf6..8d0253b 100644
@@ -10,6 +10,7 @@
  */
 
 #include <linux/clk-provider.h>
+#include <linux/clocksource.h>
 #include <linux/irqchip.h>
 #include <linux/of_platform.h>
 #include <asm/mach/arch.h>
@@ -17,8 +18,6 @@
 
 #include "common.h"
 
-extern void __init mmp_dt_init_timer(void);
-
 static const struct of_dev_auxdata pxa168_auxdata_lookup[] __initconst = {
 	OF_DEV_AUXDATA("mrvl,mmp-uart", 0xd4017000, "pxa2xx-uart.0", NULL),
 	OF_DEV_AUXDATA("mrvl,mmp-uart", 0xd4018000, "pxa2xx-uart.1", NULL),
@@ -49,7 +48,6 @@ static void __init pxa168_dt_init(void)
 
 static void __init pxa910_dt_init(void)
 {
-	of_clk_init(NULL);
 	of_platform_populate(NULL, of_default_bus_match_table,
 			     pxa910_auxdata_lookup, NULL);
 }
@@ -60,16 +58,22 @@ static const char *mmp_dt_board_compat[] __initdata = {
 	NULL,
 };
 
+static void __init mmp_init_timer(void)
+{
+	of_clk_init(NULL);
+	clocksource_of_init();
+}
+
 DT_MACHINE_START(PXA168_DT, "Marvell PXA168 (Device Tree Support)")
 	.map_io		= mmp_map_io,
-	.init_time	= mmp_dt_init_timer,
+	.init_time	= mmp_init_timer,
 	.init_machine	= pxa168_dt_init,
 	.dt_compat	= mmp_dt_board_compat,
 MACHINE_END
 
 DT_MACHINE_START(PXA910_DT, "Marvell PXA910 (Device Tree Support)")
 	.map_io		= mmp_map_io,
-	.init_time	= mmp_dt_init_timer,
+	.init_time	= mmp_init_timer,
 	.init_machine	= pxa910_dt_init,
 	.dt_compat	= mmp_dt_board_compat,
 MACHINE_END
arch/arm/mach-mmp/mmp2-dt.c +9 -3
index 023cb45..2229d1d 100644
@@ -9,6 +9,8 @@
  *  publishhed by the Free Software Foundation.
  */
 
+#include <linux/clk-provider.h>
+#include <linux/clocksource.h>
 #include <linux/io.h>
 #include <linux/irqchip.h>
 #include <linux/of_platform.h>
@@ -17,8 +19,6 @@
 
 #include "common.h"
 
-extern void __init mmp_dt_init_timer(void);
-
 static const struct of_dev_auxdata mmp2_auxdata_lookup[] __initconst = {
 	OF_DEV_AUXDATA("mrvl,mmp-uart", 0xd4030000, "pxa2xx-uart.0", NULL),
 	OF_DEV_AUXDATA("mrvl,mmp-uart", 0xd4017000, "pxa2xx-uart.1", NULL),
@@ -37,6 +37,12 @@ static void __init mmp2_dt_init(void)
 			     mmp2_auxdata_lookup, NULL);
 }
 
+static void __init mmp2_init_timer(void)
+{
+	of_clk_init(NULL);
+	clocksource_of_init();
+}
+
 static const char *mmp2_dt_board_compat[] __initdata = {
 	"mrvl,mmp2-brownstone",
 	NULL,
@@ -44,7 +50,7 @@ static const char *mmp2_dt_board_compat[] __initdata = {
 
 DT_MACHINE_START(MMP2_DT, "Marvell MMP2 (Device Tree Support)")
 	.map_io		= mmp_map_io,
-	.init_time	= mmp_dt_init_timer,
+	.init_time	= mmp2_init_timer,
 	.init_machine	= mmp2_dt_init,
 	.dt_compat	= mmp2_dt_board_compat,
 MACHINE_END
arch/arm/mach-mmp/mmp2.c +1 -1
index a0abfd7..d00c0fe 100644
@@ -131,7 +131,7 @@ void __init mmp2_timer_init(void)
 	clk_rst = APBC_APBCLK | APBC_FNCLK | APBC_FNCLKSEL(1);
 	__raw_writel(clk_rst, APBC_TIMERS);
 
-	timer_init(TIMER1_VIRT_BASE, IRQ_MMP2_TIMER1);
+	timer_init(TIMER1_VIRT_BASE, IRQ_MMP2_TIMER1, 1);
 }
 
 /* on-chip devices */
arch/arm/mach-mmp/pxa168.c +1 -1
index a332d51..28fba2b 100644
@@ -78,7 +78,7 @@ void __init pxa168_timer_init(void)
 	/* 3.25MHz, bus/functional clock enabled, release reset */
 	__raw_writel(TIMER_CLK_RST, APBC_TIMERS);
 
-	timer_init(TIMER1_VIRT_BASE, IRQ_PXA168_TIMER1);
+	timer_init(TIMER1_VIRT_BASE, IRQ_PXA168_TIMER1, 0);
 }
 
 void pxa168_clear_keypad_wakeup(void)
arch/arm/mach-mmp/pxa910.c +1 -1
index 1a84800..0e98bf3 100644
@@ -112,7 +112,7 @@ void __init pxa910_timer_init(void)
 	__raw_writel(APBC_APBCLK | APBC_RST, APBC_TIMERS);
 	__raw_writel(TIMER_CLK_RST, APBC_TIMERS);
 
-	timer_init(TIMER1_VIRT_BASE, IRQ_PXA910_AP1_TIMER1);
+	timer_init(TIMER1_VIRT_BASE, IRQ_PXA910_AP1_TIMER1, 0);
 }
 
 /* on-chip devices */
drivers/clocksource/timer-mmp.c +43 -29

120 lines not on this page · raw message

Signature · 1 line
1.8.1.2