[PATCH v9 0/3] arm: Add basic support for Mediatek Cortex-A7 SoCs

[PATCH v9 0/3] arm: Add basic support for Mediatek Cortex-A7 SoCs

[Matthias Brugger]

This patch series includes just the patches for the timer driver.
They are rebased against tip/timers/core from the tip repository [0].

The only change applied are line shifts in the OF patch.

I will send the rest of the series as a pull-request.

[0] git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git


Matthias Brugger (3):
  of: Provide function to request and map memory
  clocksource: Add support for the Mediatek SoCs
  dt-bindings: add mtk-timer bindings

 .../bindings/timer/mediatek,mtk-timer.txt          |   18 ++
 drivers/clocksource/Kconfig                        |    5 +
 drivers/clocksource/Makefile                       |    1 +
 drivers/clocksource/mtk_timer.c                    |  261 ++++++++++++++++++++
 drivers/of/address.c                               |   29 +++
 include/linux/of_address.h                         |    8 +
 6 files changed, 322 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
 create mode 100644 drivers/clocksource/mtk_timer.c

-- 
1.7.9.5

[PATCH v9 1/3] of: Provide function to request and map memory

[Matthias Brugger]

From: Matthias Brugger <matthias.brugger-K3U4GQvHnyU@public.gmane.org>

A call to of_iomap does not request the memory region.
This patch adds the function of_io_request_and_map which requests
the memory region before mapping it.

Signed-off-by: Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Suggested-by: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
Suggested-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Acked-by: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/of/address.c       |   29 +++++++++++++++++++++++++++++
 include/linux/of_address.h |    8 ++++++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index cb4242a..09074ba 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -721,3 +721,32 @@ void __iomem *of_iomap(struct device_node *np, int index)
 	return ioremap(res.start, resource_size(&res));
 }
 EXPORT_SYMBOL(of_iomap);
+
+/*
+ * of_io_request_and_map - Requests a resource and maps the memory mapped IO
+ *			   for a given device_node
+ * @device:	the device whose io range will be mapped
+ * @index:	index of the io range
+ * @name:	name of the resource
+ *
+ * Returns a pointer to the requested and mapped memory
+ */
+void __iomem *of_io_request_and_map(struct device_node *np, int index,
+					char *name)
+{
+	struct resource res;
+	void __iomem *mem;
+
+	if (of_address_to_resource(np, index, &res))
+		return NULL;
+
+	if (!request_mem_region(res.start, resource_size(&res), name))
+		return NULL;
+
+	mem = ioremap(res.start, resource_size(&res));
+	if (!mem)
+		release_mem_region(res.start, resource_size(&res));
+
+	return mem;
+}
+EXPORT_SYMBOL(of_io_request_and_map);
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 5f6ed6b..725b875 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -96,6 +96,8 @@ static inline struct of_pci_range *of_pci_range_parser_one(
 extern int of_address_to_resource(struct device_node *dev, int index,
 				  struct resource *r);
 void __iomem *of_iomap(struct device_node *node, int index);
+void __iomem *of_io_request_and_map(struct device_node *device,
+					int index, char *name);
 #else
 static inline int of_address_to_resource(struct device_node *dev, int index,
 					 struct resource *r)
@@ -107,6 +109,12 @@ static inline void __iomem *of_iomap(struct device_node *device, int index)
 {
 	return NULL;
 }
+
+static inline void __iomem *of_io_request_and_map(struct device_node *device,
+					int index, char *name)
+{
+	return NULL;
+}
 #endif
 
 #if defined(CONFIG_OF_ADDRESS) && defined(CONFIG_PCI)
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v9.1] of: Provide function to request and map memory

[Matthias Brugger]

A call to of_iomap does not request the memory region.
This patch adds the function of_io_request_and_map which requests
the memory region before mapping it.

Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Suggested-by: Rob Herring <robh@kernel.org>
Acked-by: Grant Likely <grant.likely@linaro.org>
---
Changes for v9.1:
- fixed the Author email address

 drivers/of/address.c       |   29 +++++++++++++++++++++++++++++
 include/linux/of_address.h |    8 ++++++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index cb4242a..09074ba 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -721,3 +721,32 @@ void __iomem *of_iomap(struct device_node *np, int index)
 	return ioremap(res.start, resource_size(&res));
 }
 EXPORT_SYMBOL(of_iomap);
+
+/*
+ * of_io_request_and_map - Requests a resource and maps the memory mapped IO
+ *			   for a given device_node
+ * @device:	the device whose io range will be mapped
+ * @index:	index of the io range
+ * @name:	name of the resource
+ *
+ * Returns a pointer to the requested and mapped memory
+ */
+void __iomem *of_io_request_and_map(struct device_node *np, int index,
+					char *name)
+{
+	struct resource res;
+	void __iomem *mem;
+
+	if (of_address_to_resource(np, index, &res))
+		return NULL;
+
+	if (!request_mem_region(res.start, resource_size(&res), name))
+		return NULL;
+
+	mem = ioremap(res.start, resource_size(&res));
+	if (!mem)
+		release_mem_region(res.start, resource_size(&res));
+
+	return mem;
+}
+EXPORT_SYMBOL(of_io_request_and_map);
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 5f6ed6b..725b875 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -96,6 +96,8 @@ static inline struct of_pci_range *of_pci_range_parser_one(
 extern int of_address_to_resource(struct device_node *dev, int index,
 				  struct resource *r);
 void __iomem *of_iomap(struct device_node *node, int index);
+void __iomem *of_io_request_and_map(struct device_node *device,
+					int index, char *name);
 #else
 static inline int of_address_to_resource(struct device_node *dev, int index,
 					 struct resource *r)
@@ -107,6 +109,12 @@ static inline void __iomem *of_iomap(struct device_node *device, int index)
 {
 	return NULL;
 }
+
+static inline void __iomem *of_io_request_and_map(struct device_node *device,
+					int index, char *name)
+{
+	return NULL;
+}
 #endif
 
 #if defined(CONFIG_OF_ADDRESS) && defined(CONFIG_PCI)
-- 
1.7.9.5

Re: [PATCH v9.1] of: Provide function to request and map memory

[Russell King - ARM Linux]

On Fri, Jun 20, 2014 at 02:28:48PM +0200, Matthias Brugger wrote:
> +/*
> + * of_io_request_and_map - Requests a resource and maps the memory mapped IO
> + *			   for a given device_node
> + * @device:	the device whose io range will be mapped
> + * @index:	index of the io range
> + * @name:	name of the resource
> + *
> + * Returns a pointer to the requested and mapped memory
> + */
> +void __iomem *of_io_request_and_map(struct device_node *np, int index,
> +					char *name)
> +{
> +	struct resource res;
> +	void __iomem *mem;
> +
> +	if (of_address_to_resource(np, index, &res))
> +		return NULL;
> +
> +	if (!request_mem_region(res.start, resource_size(&res), name))
> +		return NULL;
> +
> +	mem = ioremap(res.start, resource_size(&res));
> +	if (!mem)
> +		release_mem_region(res.start, resource_size(&res));

We did this "return NULL" approach for devm_request_and_ioremap(), and
ended up not liking the number of drivers returning random error codes
on failure (despite the proper return code being documented.)  It's
worth reading the commit text for the commit introducing
devm_ioremap_resource() which replaces devm_request_and_ioremap():

commit 75096579c3ac39ddc2f8b0d9a8924eba31f4d920
Author: Thierry Reding <thierry.reding@avionic-design.de>
Date:   Mon Jan 21 11:08:54 2013 +0100

    lib: devres: Introduce devm_ioremap_resource()

which may help avoid repeating the same mistakes here.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH v9 2/3] clocksource: Add support for the Mediatek SoCs

[Matthias Brugger]

This patch adds a clock source and clock event for the timer found
on the Mediatek SoCs.

The Mediatek General Purpose Timer block provides five 32 bit timers and
one 64 bit timer.

Two 32 bit timers are used by this driver:
TIMER1: clock events supporting periodic and oneshot events
TIMER2: clock source configured as a free running counter

The General Purpose Timer block can be run with two clocks. A 13 MHz system
clock and the RTC clock running at 32 KHz. This implementation uses the system
clock with no clock source divider.

The interrupts are shared between the different timers and have to be read back
from a register. We just enable one interrupt for the clock event. The clock
event timer is used by all cores.

Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/clocksource/Kconfig     |    5 +
 drivers/clocksource/Makefile    |    1 +
 drivers/clocksource/mtk_timer.c |  261 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 267 insertions(+)
 create mode 100644 drivers/clocksource/mtk_timer.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 0437767..c1ec81a 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -149,6 +149,11 @@ config VF_PIT_TIMER
 config SYS_SUPPORTS_SH_CMT
         bool
 
+config MTK_TIMER
+	select CLKSRC_OF
+	select CLKSRC_MMIO
+	bool
+
 config SYS_SUPPORTS_SH_MTU2
         bool
 
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 0770916..4256ab7 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_CLKSRC_SAMSUNG_PWM)	+= samsung_pwm_timer.o
 obj-$(CONFIG_FSL_FTM_TIMER)	+= fsl_ftm_timer.o
 obj-$(CONFIG_VF_PIT_TIMER)	+= vf_pit_timer.o
 obj-$(CONFIG_CLKSRC_QCOM)	+= qcom-timer.o
+obj-$(CONFIG_MTK_TIMER)		+= mtk_timer.o
 
 obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
 obj-$(CONFIG_ARM_GLOBAL_TIMER)		+= arm_global_timer.o
diff --git a/drivers/clocksource/mtk_timer.c b/drivers/clocksource/mtk_timer.c
new file mode 100644
index 0000000..5de3bf5
--- /dev/null
+++ b/drivers/clocksource/mtk_timer.c
@@ -0,0 +1,261 @@
+/*
+ * Mediatek SoCs General-Purpose Timer handling.
+ *
+ * Copyright (C) 2014 Matthias Brugger
+ *
+ * Matthias Brugger <matthias.bgg@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqreturn.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+
+#define GPT_IRQ_EN_REG		0x00
+#define GPT_IRQ_ENABLE(val)	BIT((val) - 1)
+#define GPT_IRQ_ACK_REG		0x08
+#define GPT_IRQ_ACK(val)	BIT((val) - 1)
+
+#define TIMER_CTRL_REG(val)	(0x10 * (val))
+#define TIMER_CTRL_OP(val)	(((val) & 0x3) << 4)
+#define TIMER_CTRL_OP_ONESHOT	(0)
+#define TIMER_CTRL_OP_REPEAT	(1)
+#define TIMER_CTRL_OP_FREERUN	(3)
+#define TIMER_CTRL_CLEAR	(2)
+#define TIMER_CTRL_ENABLE	(1)
+#define TIMER_CTRL_DISABLE	(0)
+
+#define TIMER_CLK_REG(val)	(0x04 + (0x10 * (val)))
+#define TIMER_CLK_SRC(val)	(((val) & 0x1) << 4)
+#define TIMER_CLK_SRC_SYS13M	(0)
+#define TIMER_CLK_SRC_RTC32K	(1)
+#define TIMER_CLK_DIV1		(0x0)
+#define TIMER_CLK_DIV2		(0x1)
+
+#define TIMER_CNT_REG(val)	(0x08 + (0x10 * (val)))
+#define TIMER_CMP_REG(val)	(0x0C + (0x10 * (val)))
+
+#define GPT_CLK_EVT	1
+#define GPT_CLK_SRC	2
+
+struct mtk_clock_event_device {
+	void __iomem *gpt_base;
+	u32 ticks_per_jiffy;
+	struct clock_event_device dev;
+};
+
+static inline struct mtk_clock_event_device *to_mtk_clk(
+				struct clock_event_device *c)
+{
+	return container_of(c, struct mtk_clock_event_device, dev);
+}
+
+static void mtk_clkevt_time_stop(struct mtk_clock_event_device *evt, u8 timer)
+{
+	u32 val;
+
+	val = readl(evt->gpt_base + TIMER_CTRL_REG(timer));
+	writel(val & ~TIMER_CTRL_ENABLE, evt->gpt_base +
+			TIMER_CTRL_REG(timer));
+}
+
+static void mtk_clkevt_time_setup(struct mtk_clock_event_device *evt,
+				unsigned long delay, u8 timer)
+{
+	writel(delay, evt->gpt_base + TIMER_CMP_REG(timer));
+}
+
+static void mtk_clkevt_time_start(struct mtk_clock_event_device *evt,
+		bool periodic, u8 timer)
+{
+	u32 val;
+
+	/* Acknowledge interrupt */
+	writel(GPT_IRQ_ACK(timer), evt->gpt_base + GPT_IRQ_ACK_REG);
+
+	val = readl(evt->gpt_base + TIMER_CTRL_REG(timer));
+
+	/* Clear 2 bit timer operation mode field */
+	val &= ~TIMER_CTRL_OP(0x3);
+
+	if (periodic)
+		val |= TIMER_CTRL_OP(TIMER_CTRL_OP_REPEAT);
+	else
+		val |= TIMER_CTRL_OP(TIMER_CTRL_OP_ONESHOT);
+
+	writel(val | TIMER_CTRL_ENABLE | TIMER_CTRL_CLEAR,
+	       evt->gpt_base + TIMER_CTRL_REG(timer));
+}
+
+static void mtk_clkevt_mode(enum clock_event_mode mode,
+				struct clock_event_device *clk)
+{
+	struct mtk_clock_event_device *evt = to_mtk_clk(clk);
+
+	mtk_clkevt_time_stop(evt, GPT_CLK_EVT);
+
+	switch (mode) {
+	case CLOCK_EVT_MODE_PERIODIC:
+		mtk_clkevt_time_setup(evt, evt->ticks_per_jiffy, GPT_CLK_EVT);
+		mtk_clkevt_time_start(evt, true, GPT_CLK_EVT);
+		break;
+	case CLOCK_EVT_MODE_ONESHOT:
+		/* Timer is enabled in set_next_event */
+		break;
+	case CLOCK_EVT_MODE_UNUSED:
+	case CLOCK_EVT_MODE_SHUTDOWN:
+	default:
+		/* No more interrupts will occur as source is disabled */
+		break;
+	}
+}
+
+static int mtk_clkevt_next_event(unsigned long event,
+				   struct clock_event_device *clk)
+{
+	struct mtk_clock_event_device *evt = to_mtk_clk(clk);
+
+	mtk_clkevt_time_stop(evt, GPT_CLK_EVT);
+	mtk_clkevt_time_setup(evt, event, GPT_CLK_EVT);
+	mtk_clkevt_time_start(evt, false, GPT_CLK_EVT);
+
+	return 0;
+}
+
+static irqreturn_t mtk_timer_interrupt(int irq, void *dev_id)
+{
+	struct mtk_clock_event_device *evt = dev_id;
+
+	/* Acknowledge timer0 irq */
+	writel(GPT_IRQ_ACK(GPT_CLK_EVT), evt->gpt_base + GPT_IRQ_ACK_REG);
+	evt->dev.event_handler(&evt->dev);
+
+	return IRQ_HANDLED;
+}
+
+static void mtk_timer_global_reset(struct mtk_clock_event_device *evt)
+{
+	/* Disable all interrupts */
+	writel(0x0, evt->gpt_base + GPT_IRQ_EN_REG);
+	/* Acknowledge all interrupts */
+	writel(0x3f, evt->gpt_base + GPT_IRQ_ACK_REG);
+}
+
+static void
+mtk_timer_setup(struct mtk_clock_event_device *evt, u8 timer, u8 option)
+{
+	writel(TIMER_CTRL_CLEAR | TIMER_CTRL_DISABLE,
+		evt->gpt_base + TIMER_CTRL_REG(timer));
+
+	writel(TIMER_CLK_SRC(TIMER_CLK_SRC_SYS13M) | TIMER_CLK_DIV1,
+			evt->gpt_base + TIMER_CLK_REG(timer));
+
+	writel(0x0, evt->gpt_base + TIMER_CMP_REG(timer));
+
+	writel(TIMER_CTRL_OP(option) | TIMER_CTRL_ENABLE,
+			evt->gpt_base + TIMER_CTRL_REG(timer));
+}
+
+static void mtk_timer_enable_irq(struct mtk_clock_event_device *evt, u8 timer)
+{
+	u32 val;
+
+	val = readl(evt->gpt_base + GPT_IRQ_EN_REG);
+	writel(val | GPT_IRQ_ENABLE(timer),
+			evt->gpt_base + GPT_IRQ_EN_REG);
+}
+
+static void __init mtk_timer_init(struct device_node *node)
+{
+	struct mtk_clock_event_device *evt;
+	struct resource res;
+	unsigned long rate = 0;
+	struct clk *clk;
+
+	evt = kzalloc(sizeof(*evt), GFP_KERNEL);
+	if (!evt) {
+		pr_warn("Can't allocate mtk clock event driver struct");
+		return;
+	}
+
+	evt->dev.name = "mtk_tick";
+	evt->dev.rating = 300;
+	evt->dev.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
+	evt->dev.set_mode = mtk_clkevt_mode;
+	evt->dev.set_next_event = mtk_clkevt_next_event;
+	evt->dev.cpumask = cpu_possible_mask;
+
+	evt->gpt_base = of_io_request_and_map(node, 0, "mtk-timer");
+	if (!evt->gpt_base) {
+		pr_warn("Can't get resource\n");
+		return;
+	}
+
+	evt->dev.irq = irq_of_parse_and_map(node, 0);
+	if (evt->dev.irq <= 0) {
+		pr_warn("Can't parse IRQ");
+		goto err_mem;
+	}
+
+	clk = of_clk_get(node, 0);
+	if (IS_ERR(clk)) {
+		pr_warn("Can't get timer clock");
+		goto err_irq;
+	}
+
+	if (clk_prepare_enable(clk)) {
+		pr_warn("Can't prepare clock");
+		goto err_clk_put;
+	}
+	rate = clk_get_rate(clk);
+
+	if (request_irq(evt->dev.irq, mtk_timer_interrupt,
+			IRQF_TIMER | IRQF_IRQPOLL, "mtk_timer", evt)) {
+		pr_warn("failed to setup irq %d\n", evt->dev.irq);
+		goto err_clk_disable;
+	}
+
+	evt->ticks_per_jiffy = DIV_ROUND_UP(rate, HZ);
+
+	mtk_timer_global_reset(evt);
+
+	/* Configure clock source */
+	mtk_timer_setup(evt, GPT_CLK_SRC, TIMER_CTRL_OP_FREERUN);
+	clocksource_mmio_init(evt->gpt_base + TIMER_CNT_REG(GPT_CLK_SRC),
+			node->name, rate, 300, 32, clocksource_mmio_readl_up);
+
+	/* Configure clock event */
+	mtk_timer_setup(evt, GPT_CLK_EVT, TIMER_CTRL_OP_REPEAT);
+	mtk_timer_enable_irq(evt, GPT_CLK_EVT);
+
+	clockevents_config_and_register(&evt->dev, rate, 0x3,
+					0xffffffff);
+	return;
+
+err_clk_disable:
+	clk_disable_unprepare(clk);
+err_clk_put:
+	clk_put(clk);
+err_irq:
+	irq_dispose_mapping(evt->dev.irq);
+err_mem:
+	iounmap(evt->gpt_base);
+	of_address_to_resource(node, 0, &res);
+	release_mem_region(res.start, resource_size(&res));
+}
+CLOCKSOURCE_OF_DECLARE(mtk_mt6577, "mediatek,mt6577-timer", mtk_timer_init);
-- 
1.7.9.5

[PATCH v9 3/3] dt-bindings: add mtk-timer bindings

[Matthias Brugger]

Add binding documentation for the General Porpose Timer driver of
the Mediatek SoCs.

Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
Acked-by: Sören Brinkmann <soren.brinkmann@xilinx.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../bindings/timer/mediatek,mtk-timer.txt          |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt

diff --git a/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
new file mode 100644
index 0000000..10b7e09
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
@@ -0,0 +1,18 @@
+Mediatek MT6577, MT6572 and MT6589 Timers
+---------------------------------------
+
+Required properties:
+- compatible: Should be "mediatek,mt6577-timer"
+- reg: Should contain location and length for timers register.
+- clocks: Clocks driving the timer hardware. This list should include two
+	clocks. The order is system clock and as second clock the RTC clock.
+
+Examples:
+
+	timer@10008000 {
+		compatible = "mediatek,mt6577-timer";
+		reg = <0x10008000 0x80>;
+		interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
+		clocks = <&system_clk>, <&rtc_clk>;
+		clock-names = "system-clk", "rtc-clk";
+	};
-- 
1.7.9.5

Re: [PATCH v9 3/3] dt-bindings: add mtk-timer bindings

[Mark Rutland]

On Fri, Jun 20, 2014 at 12:48:49PM +0100, Matthias Brugger wrote:
> Add binding documentation for the General Porpose Timer driver of
> the Mediatek SoCs.
> 
> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
> Acked-by: Sören Brinkmann <soren.brinkmann@xilinx.com>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/timer/mediatek,mtk-timer.txt          |   18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
> 
> diff --git a/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
> new file mode 100644
> index 0000000..10b7e09
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
> @@ -0,0 +1,18 @@
> +Mediatek MT6577, MT6572 and MT6589 Timers
> +---------------------------------------
> +
> +Required properties:
> +- compatible: Should be "mediatek,mt6577-timer"
> +- reg: Should contain location and length for timers register.
> +- clocks: Clocks driving the timer hardware. This list should include two
> +	clocks. The order is system clock and as second clock the RTC clock.
> +
> +Examples:
> +
> +	timer@10008000 {
> +		compatible = "mediatek,mt6577-timer";
> +		reg = <0x10008000 0x80>;
> +		interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&system_clk>, <&rtc_clk>;
> +		clock-names = "system-clk", "rtc-clk";

These names should be mentioned above, or removed.

This looks fine otherwise.

Mark.

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

Re: [PATCH v9 3/3] dt-bindings: add mtk-timer bindings

[Matthias Brugger]

2014-06-20 14:36 GMT+02:00 Mark Rutland <mark.rutland@arm.com>:
> On Fri, Jun 20, 2014 at 12:48:49PM +0100, Matthias Brugger wrote:
>> Add binding documentation for the General Porpose Timer driver of
>> the Mediatek SoCs.
>>
>> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
>> Acked-by: Sören Brinkmann <soren.brinkmann@xilinx.com>
>> Acked-by: Rob Herring <robh@kernel.org>
>> ---
>>  .../bindings/timer/mediatek,mtk-timer.txt          |   18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
>>
>> diff --git a/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
>> new file mode 100644
>> index 0000000..10b7e09
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
>> @@ -0,0 +1,18 @@
>> +Mediatek MT6577, MT6572 and MT6589 Timers
>> +---------------------------------------
>> +
>> +Required properties:
>> +- compatible: Should be "mediatek,mt6577-timer"
>> +- reg: Should contain location and length for timers register.
>> +- clocks: Clocks driving the timer hardware. This list should include two
>> +     clocks. The order is system clock and as second clock the RTC clock.
>> +
>> +Examples:
>> +
>> +     timer@10008000 {
>> +             compatible = "mediatek,mt6577-timer";
>> +             reg = <0x10008000 0x80>;
>> +             interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
>> +             clocks = <&system_clk>, <&rtc_clk>;
>> +             clock-names = "system-clk", "rtc-clk";
>
> These names should be mentioned above, or removed.

I added the clock-names to make it the clock order clearer, as it is
done in the arm sp,804 [1].
But I think it is clear enough through the phandle names and the
description of the clocks property.
So I will delete the clock-names in the v10 round.

Cheers,
Matthias

[1] http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/timer/arm,sp804.txt

>
> This looks fine otherwise.
>
> Mark.
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
motzblog.wordpress.com

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

Re: [PATCH v9 3/3] dt-bindings: add mtk-timer bindings

[Sören Brinkmann]

On Tue, 2014-07-01 at 07:46PM +0200, Matthias Brugger wrote:
> 2014-06-20 14:36 GMT+02:00 Mark Rutland <mark.rutland@arm.com>:
> > On Fri, Jun 20, 2014 at 12:48:49PM +0100, Matthias Brugger wrote:
> >> Add binding documentation for the General Porpose Timer driver of
> >> the Mediatek SoCs.
> >>
> >> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
> >> Acked-by: Sören Brinkmann <soren.brinkmann@xilinx.com>
> >> Acked-by: Rob Herring <robh@kernel.org>
> >> ---
> >>  .../bindings/timer/mediatek,mtk-timer.txt          |   18 ++++++++++++++++++
> >>  1 file changed, 18 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
> >> new file mode 100644
> >> index 0000000..10b7e09
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
> >> @@ -0,0 +1,18 @@
> >> +Mediatek MT6577, MT6572 and MT6589 Timers
> >> +---------------------------------------
> >> +
> >> +Required properties:
> >> +- compatible: Should be "mediatek,mt6577-timer"
> >> +- reg: Should contain location and length for timers register.
> >> +- clocks: Clocks driving the timer hardware. This list should include two
> >> +     clocks. The order is system clock and as second clock the RTC clock.
> >> +
> >> +Examples:
> >> +
> >> +     timer@10008000 {
> >> +             compatible = "mediatek,mt6577-timer";
> >> +             reg = <0x10008000 0x80>;
> >> +             interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
> >> +             clocks = <&system_clk>, <&rtc_clk>;
> >> +             clock-names = "system-clk", "rtc-clk";
> >
> > These names should be mentioned above, or removed.
> 
> I added the clock-names to make it the clock order clearer, as it is
> done in the arm sp,804 [1].
> But I think it is clear enough through the phandle names and the
> description of the clocks property.
> So I will delete the clock-names in the v10 round.

I think I originally advocated for adding those names. And my argument
was that the names in the dts are probably not clearly identifying the
timer IP's clock inputs since they refer to SOC names for the clocks. We
had this argument for a while now and it doesn't seem to get better
than this. But to explain why I try to get IP vs. SOC clock names in
here is, that I had such a case with Zynq's Ethernet core. That IP has
quite a few clock inputs and luckily the mainline driver for that IP
used the proper names from the IP data sheet for the clocks instead
of the SOC names. That made it relatively easy to match things with the
Zynq integration of that IP. Had that driver used SOC clock names,
things would have been more difficult.

	Sören

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

Re: [PATCH v9 3/3] dt-bindings: add mtk-timer bindings

[Matthias Brugger]

2014-07-01 20:00 GMT+02:00 Sören Brinkmann <soren.brinkmann@xilinx.com>:
> On Tue, 2014-07-01 at 07:46PM +0200, Matthias Brugger wrote:
>> 2014-06-20 14:36 GMT+02:00 Mark Rutland <mark.rutland@arm.com>:
>> > On Fri, Jun 20, 2014 at 12:48:49PM +0100, Matthias Brugger wrote:
>> >> Add binding documentation for the General Porpose Timer driver of
>> >> the Mediatek SoCs.
>> >>
>> >> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
>> >> Acked-by: Sören Brinkmann <soren.brinkmann@xilinx.com>
>> >> Acked-by: Rob Herring <robh@kernel.org>
>> >> ---
>> >>  .../bindings/timer/mediatek,mtk-timer.txt          |   18 ++++++++++++++++++
>> >>  1 file changed, 18 insertions(+)
>> >>  create mode 100644 Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
>> >> new file mode 100644
>> >> index 0000000..10b7e09
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
>> >> @@ -0,0 +1,18 @@
>> >> +Mediatek MT6577, MT6572 and MT6589 Timers
>> >> +---------------------------------------
>> >> +
>> >> +Required properties:
>> >> +- compatible: Should be "mediatek,mt6577-timer"
>> >> +- reg: Should contain location and length for timers register.
>> >> +- clocks: Clocks driving the timer hardware. This list should include two
>> >> +     clocks. The order is system clock and as second clock the RTC clock.
>> >> +
>> >> +Examples:
>> >> +
>> >> +     timer@10008000 {
>> >> +             compatible = "mediatek,mt6577-timer";
>> >> +             reg = <0x10008000 0x80>;
>> >> +             interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
>> >> +             clocks = <&system_clk>, <&rtc_clk>;
>> >> +             clock-names = "system-clk", "rtc-clk";
>> >
>> > These names should be mentioned above, or removed.
>>
>> I added the clock-names to make it the clock order clearer, as it is
>> done in the arm sp,804 [1].
>> But I think it is clear enough through the phandle names and the
>> description of the clocks property.
>> So I will delete the clock-names in the v10 round.
>
> I think I originally advocated for adding those names. And my argument
> was that the names in the dts are probably not clearly identifying the
> timer IP's clock inputs since they refer to SOC names for the clocks. We
> had this argument for a while now and it doesn't seem to get better
> than this. But to explain why I try to get IP vs. SOC clock names in
> here is, that I had such a case with Zynq's Ethernet core. That IP has
> quite a few clock inputs and luckily the mainline driver for that IP
> used the proper names from the IP data sheet for the clocks instead
> of the SOC names. That made it relatively easy to match things with the
> Zynq integration of that IP. Had that driver used SOC clock names,
> things would have been more difficult.

Sören, I get the point. The problem is, that we don't have any
documentation other then the SoC datasheet for the timer.
Anyway the clock-names property does not help us in this at all. As I
already mentioned, the clock-names property was to illustrate the
clock order, but as I think, with the comment for the clocks property,
the order is clear enough.

Shall I delete your ack, when I resend the patch without the
clock-names property?

Regards,
Matthias

>
>         Sören



-- 
motzblog.wordpress.com

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

Re: [PATCH v9 3/3] dt-bindings: add mtk-timer bindings

[Sören Brinkmann]

On Sat, 2014-07-05 at 02:28AM +0200, Matthias Brugger wrote:
> 2014-07-01 20:00 GMT+02:00 Sören Brinkmann <soren.brinkmann@xilinx.com>:
> > On Tue, 2014-07-01 at 07:46PM +0200, Matthias Brugger wrote:
> >> 2014-06-20 14:36 GMT+02:00 Mark Rutland <mark.rutland@arm.com>:
> >> > On Fri, Jun 20, 2014 at 12:48:49PM +0100, Matthias Brugger wrote:
> >> >> Add binding documentation for the General Porpose Timer driver of
> >> >> the Mediatek SoCs.
> >> >>
> >> >> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
> >> >> Acked-by: Sören Brinkmann <soren.brinkmann@xilinx.com>
> >> >> Acked-by: Rob Herring <robh@kernel.org>
> >> >> ---
> >> >>  .../bindings/timer/mediatek,mtk-timer.txt          |   18 ++++++++++++++++++
> >> >>  1 file changed, 18 insertions(+)
> >> >>  create mode 100644 Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
> >> >> new file mode 100644
> >> >> index 0000000..10b7e09
> >> >> --- /dev/null
> >> >> +++ b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
> >> >> @@ -0,0 +1,18 @@
> >> >> +Mediatek MT6577, MT6572 and MT6589 Timers
> >> >> +---------------------------------------
> >> >> +
> >> >> +Required properties:
> >> >> +- compatible: Should be "mediatek,mt6577-timer"
> >> >> +- reg: Should contain location and length for timers register.
> >> >> +- clocks: Clocks driving the timer hardware. This list should include two
> >> >> +     clocks. The order is system clock and as second clock the RTC clock.
> >> >> +
> >> >> +Examples:
> >> >> +
> >> >> +     timer@10008000 {
> >> >> +             compatible = "mediatek,mt6577-timer";
> >> >> +             reg = <0x10008000 0x80>;
> >> >> +             interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
> >> >> +             clocks = <&system_clk>, <&rtc_clk>;
> >> >> +             clock-names = "system-clk", "rtc-clk";
> >> >
> >> > These names should be mentioned above, or removed.
> >>
> >> I added the clock-names to make it the clock order clearer, as it is
> >> done in the arm sp,804 [1].
> >> But I think it is clear enough through the phandle names and the
> >> description of the clocks property.
> >> So I will delete the clock-names in the v10 round.
> >
> > I think I originally advocated for adding those names. And my argument
> > was that the names in the dts are probably not clearly identifying the
> > timer IP's clock inputs since they refer to SOC names for the clocks. We
> > had this argument for a while now and it doesn't seem to get better
> > than this. But to explain why I try to get IP vs. SOC clock names in
> > here is, that I had such a case with Zynq's Ethernet core. That IP has
> > quite a few clock inputs and luckily the mainline driver for that IP
> > used the proper names from the IP data sheet for the clocks instead
> > of the SOC names. That made it relatively easy to match things with the
> > Zynq integration of that IP. Had that driver used SOC clock names,
> > things would have been more difficult.
> 
> Sören, I get the point. The problem is, that we don't have any
> documentation other then the SoC datasheet for the timer.
> Anyway the clock-names property does not help us in this at all. As I
> already mentioned, the clock-names property was to illustrate the
> clock order, but as I think, with the comment for the clocks property,
> the order is clear enough.
> 
> Shall I delete your ack, when I resend the patch without the
> clock-names property?

No, it's fine.

	Sören

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