linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] Proper arch timer support for Exynos5433-based TM2(e) boards
       [not found] <CGME20181017134204eucas1p10d92a91f2b945afdc7b39ce6e7813a8e@eucas1p1.samsung.com>
@ 2018-10-17 13:41 ` Marek Szyprowski
       [not found]   ` <CGME20181017134205eucas1p148faab76ac153e9afbb8a519e6d8d1e2@eucas1p1.samsung.com>
                     ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Marek Szyprowski @ 2018-10-17 13:41 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Marek Szyprowski, Will Deacon, Catalin Marinas, Marc Zyngier,
	Thomas Gleixner, Daniel Lezcano, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Inki Dae

Dear All,

This patchset is an attempt to submit the last piece of missing code
to have proper support for Exynos5433 SoCs based TM2(e) boards. It
performs a cleanup of timer configuration, which so far needed various
out-of-tree workarounds. The fixes provided by this patchset are also
needed for add proper support for system suspend/resume.

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Changelog:

v3:
- added patch, which splits resources and interrupts allocation
- simplified arch timer cooperation mode
- dropped CPU hotplug priority change patch, it is not really needed
- removed more non-dt dead code

v2:
- dropped arch timer patch, it will be discussed separately
- fixed issues pointed by Krzysztof Kozlowski

v1: https://patchwork.kernel.org/project/linux-samsung-soc/list/?series=27965&state=*&archive=both
- initial version


Patch summary:

Marek Szyprowski (6):
  clocksource: exynos_mct: Remove dead code
  clocksource: exynos_mct: Fix error path in timer resources
    initialization
  clocksource: exynos_mct: Refactor resources allocation
  clocksource: exynos_mct: Add arch_timer cooperation mode for ARM64
  arm64: dts: exynos: Move arch-timer node to right place
  arm64: platform: Add enable Exynos Multi-Core Timer driver

 arch/arm64/Kconfig.platforms               |  1 +
 arch/arm64/boot/dts/exynos/exynos5433.dtsi | 23 +++---
 drivers/clocksource/exynos_mct.c           | 81 ++++++++++++++--------
 3 files changed, 67 insertions(+), 38 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/6] clocksource: exynos_mct: Remove dead code
       [not found]   ` <CGME20181017134205eucas1p148faab76ac153e9afbb8a519e6d8d1e2@eucas1p1.samsung.com>
@ 2018-10-17 13:41     ` Marek Szyprowski
  0 siblings, 0 replies; 11+ messages in thread
From: Marek Szyprowski @ 2018-10-17 13:41 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Marek Szyprowski, Will Deacon, Catalin Marinas, Marc Zyngier,
	Thomas Gleixner, Daniel Lezcano, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Inki Dae

Exynos Multi-Core Timer driver is used only on device-tree based
systems, so remove non-dt related code. In case of !CONFIG_OF
the code is anyway equal because of_irq_count() has a stub
returning 0. Device node pointer is always provided when driver
has been probed from device tree.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/clocksource/exynos_mct.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 7a244b681876..ef18bbf8d20c 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -507,13 +507,12 @@ static int __init exynos4_timer_resources(struct device_node *np, void __iomem *
 	int err, cpu;
 	struct clk *mct_clk, *tick_clk;
 
-	tick_clk = np ? of_clk_get_by_name(np, "fin_pll") :
-				clk_get(NULL, "fin_pll");
+	tick_clk = of_clk_get_by_name(np, "fin_pll");
 	if (IS_ERR(tick_clk))
 		panic("%s: unable to determine tick clock rate\n", __func__);
 	clk_rate = clk_get_rate(tick_clk);
 
-	mct_clk = np ? of_clk_get_by_name(np, "mct") : clk_get(NULL, "mct");
+	mct_clk = of_clk_get_by_name(np, "mct");
 	if (IS_ERR(mct_clk))
 		panic("%s: unable to retrieve mct clock instance\n", __func__);
 	clk_prepare_enable(mct_clk);
@@ -581,11 +580,7 @@ static int __init mct_init_dt(struct device_node *np, unsigned int int_type)
 	 * timer irqs are specified after the four global timer
 	 * irqs are specified.
 	 */
-#ifdef CONFIG_OF
 	nr_irqs = of_irq_count(np);
-#else
-	nr_irqs = 0;
-#endif
 	for (i = MCT_L0_IRQ; i < nr_irqs; i++)
 		mct_irqs[i] = irq_of_parse_and_map(np, i);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/6] clocksource: exynos_mct: Fix error path in timer resources initialization
       [not found]   ` <CGME20181017134206eucas1p1caf7cdec20ec620a5bf52fb9e060a7de@eucas1p1.samsung.com>
@ 2018-10-17 13:41     ` Marek Szyprowski
  0 siblings, 0 replies; 11+ messages in thread
From: Marek Szyprowski @ 2018-10-17 13:41 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Marek Szyprowski, Will Deacon, Catalin Marinas, Marc Zyngier,
	Thomas Gleixner, Daniel Lezcano, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Inki Dae

While freeing interrupt handlers in error path, don't assume that all
requested interrupts are per-processor interrupts and properly release
standard interrupts too.

Reported-by: Krzysztof Kozlowski <krzk@kernel.org>
Fixes: 56a94f13919c ("clocksource: exynos_mct: Avoid blocking calls in the cpu hotplug notifier")
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/clocksource/exynos_mct.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index ef18bbf8d20c..49413900b24c 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -561,7 +561,19 @@ static int __init exynos4_timer_resources(struct device_node *np, void __iomem *
 	return 0;
 
 out_irq:
-	free_percpu_irq(mct_irqs[MCT_L0_IRQ], &percpu_mct_tick);
+	if (mct_int_type == MCT_INT_PPI) {
+		free_percpu_irq(mct_irqs[MCT_L0_IRQ], &percpu_mct_tick);
+	} else {
+		for_each_possible_cpu(cpu) {
+			struct mct_clock_event_device *pcpu_mevt =
+				per_cpu_ptr(&percpu_mct_tick, cpu);
+
+			if (pcpu_mevt->evt.irq != -1) {
+				free_irq(pcpu_mevt->evt.irq, pcpu_mevt);
+				pcpu_mevt->evt.irq = -1;
+			}
+		}
+	}
 	return err;
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 3/6] clocksource: exynos_mct: Refactor resources allocation
       [not found]   ` <CGME20181017134206eucas1p1ebc76569f706675d41f2d2183349f1f3@eucas1p1.samsung.com>
@ 2018-10-17 13:41     ` Marek Szyprowski
  2018-10-17 14:29       ` Krzysztof Kozlowski
  2018-10-18  3:50       ` Chanwoo Choi
  0 siblings, 2 replies; 11+ messages in thread
From: Marek Szyprowski @ 2018-10-17 13:41 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Marek Szyprowski, Will Deacon, Catalin Marinas, Marc Zyngier,
	Thomas Gleixner, Daniel Lezcano, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Inki Dae

Move interrupts allocation from exynos4_timer_resources() into separate
function together with the interrupt number parsing code from
mct_init_dt(), so the code for managing interrupts is kept together.
While touching exynos4_timer_resources() function, move of_iomap() to it.
No functional changes.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/clocksource/exynos_mct.c | 49 +++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 49413900b24c..02ad55db390b 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -502,11 +502,14 @@ static int exynos4_mct_dying_cpu(unsigned int cpu)
 	return 0;
 }
 
-static int __init exynos4_timer_resources(struct device_node *np, void __iomem *base)
+static int __init exynos4_timer_resources(struct device_node *np)
 {
-	int err, cpu;
 	struct clk *mct_clk, *tick_clk;
 
+	reg_base = of_iomap(np, 0);
+	if (!reg_base)
+		panic("%s: unable to ioremap mct address space\n", __func__);
+
 	tick_clk = of_clk_get_by_name(np, "fin_pll");
 	if (IS_ERR(tick_clk))
 		panic("%s: unable to determine tick clock rate\n", __func__);
@@ -517,9 +520,27 @@ static int __init exynos4_timer_resources(struct device_node *np, void __iomem *
 		panic("%s: unable to retrieve mct clock instance\n", __func__);
 	clk_prepare_enable(mct_clk);
 
-	reg_base = base;
-	if (!reg_base)
-		panic("%s: unable to ioremap mct address space\n", __func__);
+	return 0;
+}
+
+static int __init exynos4_timer_interrupts(struct device_node *np,
+					   unsigned int int_type)
+{
+	int i, err, cpu;
+
+	mct_int_type = int_type;
+
+	/* This driver uses only one global timer interrupt */
+	mct_irqs[MCT_G0_IRQ] = irq_of_parse_and_map(np, MCT_G0_IRQ);
+
+	/*
+	 * Find out the number of local irqs specified. The local
+	 * timer irqs are specified after the four global timer
+	 * irqs are specified.
+	 */
+	nr_irqs = of_irq_count(np);
+	for (i = MCT_L0_IRQ; i < nr_irqs; i++)
+		mct_irqs[i] = irq_of_parse_and_map(np, i);
 
 	if (mct_int_type == MCT_INT_PPI) {
 
@@ -579,24 +600,14 @@ static int __init exynos4_timer_resources(struct device_node *np, void __iomem *
 
 static int __init mct_init_dt(struct device_node *np, unsigned int int_type)
 {
-	u32 nr_irqs, i;
 	int ret;
 
-	mct_int_type = int_type;
-
-	/* This driver uses only one global timer interrupt */
-	mct_irqs[MCT_G0_IRQ] = irq_of_parse_and_map(np, MCT_G0_IRQ);
+	ret = exynos4_timer_resources(np);
+	if (ret)
+		return ret;
 
-	/*
-	 * Find out the number of local irqs specified. The local
-	 * timer irqs are specified after the four global timer
-	 * irqs are specified.
-	 */
-	nr_irqs = of_irq_count(np);
-	for (i = MCT_L0_IRQ; i < nr_irqs; i++)
-		mct_irqs[i] = irq_of_parse_and_map(np, i);
 
-	ret = exynos4_timer_resources(np, of_iomap(np, 0));
+	ret = exynos4_timer_interrupts(np, int_type);
 	if (ret)
 		return ret;
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 4/6] clocksource: exynos_mct: Add arch_timer cooperation mode for ARM64
       [not found]   ` <CGME20181017134207eucas1p1b938eefe31fc47baaf538c9ebafc1a7e@eucas1p1.samsung.com>
@ 2018-10-17 13:41     ` Marek Szyprowski
  2018-10-18  3:51       ` Chanwoo Choi
  0 siblings, 1 reply; 11+ messages in thread
From: Marek Szyprowski @ 2018-10-17 13:41 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Marek Szyprowski, Will Deacon, Catalin Marinas, Marc Zyngier,
	Thomas Gleixner, Daniel Lezcano, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Inki Dae

To get ARM Architected Timers working on Samsung Exynos SoCs, one has to
first configure and enable Exynos Multi-Core Timer, because they both
share some common hardware blocks (global system counter). This patch
adds a mode of cooperation with arch_timer driver, so kernel can use
CP15 based timer interface via arch_timer driver, which is mandatory
on ARM64. In such mode MCT driver only enables its clocks and starts
global timer. Everything else will be handled by arch_timer driver.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/clocksource/exynos_mct.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 02ad55db390b..1b19a4f03929 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -606,6 +606,15 @@ static int __init mct_init_dt(struct device_node *np, unsigned int int_type)
 	if (ret)
 		return ret;
 
+	if (IS_ENABLED(CONFIG_ARM64) && IS_ENABLED(CONFIG_ARM_ARCH_TIMER)) {
+		struct device_node *np = of_find_compatible_node(NULL, NULL,
+							     "arm,armv8-timer");
+		if (np) {
+			of_node_put(np);
+			exynos4_mct_frc_start();
+			return 0;
+		}
+	}
 
 	ret = exynos4_timer_interrupts(np, int_type);
 	if (ret)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 5/6] arm64: dts: exynos: Move arch-timer node to right place
       [not found]   ` <CGME20181017134207eucas1p18188ff6debdbabbbf8b01cc4dc1d4b13@eucas1p1.samsung.com>
@ 2018-10-17 13:41     ` Marek Szyprowski
  0 siblings, 0 replies; 11+ messages in thread
From: Marek Szyprowski @ 2018-10-17 13:41 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Marek Szyprowski, Will Deacon, Catalin Marinas, Marc Zyngier,
	Thomas Gleixner, Daniel Lezcano, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Inki Dae

Move ARM architected timer device-tree node to the beginning of 'soc'
node, to group it together with other ARM CPU core devices (like PMU).

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos5433.dtsi | 23 +++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index 2131f12364cb..fa20eb3495b3 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -255,6 +255,18 @@
 			interrupt-affinity = <&cpu4>, <&cpu5>, <&cpu6>, <&cpu7>;
 		};
 
+		timer: timer {
+			compatible = "arm,armv8-timer";
+			interrupts = <GIC_PPI 13
+					(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>,
+				<GIC_PPI 14
+					(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>,
+				<GIC_PPI 11
+					(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>,
+				<GIC_PPI 10
+					(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
+		};
+
 		chipid@10000000 {
 			compatible = "samsung,exynos4210-chipid";
 			reg = <0x10000000 0x100>;
@@ -1750,17 +1762,6 @@
 		};
 	};
 
-	timer: timer {
-		compatible = "arm,armv8-timer";
-		interrupts = <GIC_PPI 13
-				(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>,
-			<GIC_PPI 14
-				(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>,
-			<GIC_PPI 11
-				(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>,
-			<GIC_PPI 10
-				(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
-	};
 };
 
 #include "exynos5433-bus.dtsi"
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 6/6] arm64: platform: Add enable Exynos Multi-Core Timer driver
       [not found]   ` <CGME20181017134208eucas1p19c81d1ce9050be1b78e386d8a349f441@eucas1p1.samsung.com>
@ 2018-10-17 13:41     ` Marek Szyprowski
  0 siblings, 0 replies; 11+ messages in thread
From: Marek Szyprowski @ 2018-10-17 13:41 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Marek Szyprowski, Will Deacon, Catalin Marinas, Marc Zyngier,
	Thomas Gleixner, Daniel Lezcano, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Inki Dae

On Exynos SoCs enabling MCT driver is required even if ARM Architected
Timer driver is used to for managing timer hardware and clock source
events.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 arch/arm64/Kconfig.platforms | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 51bc479334a4..7cc687580fad 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -62,6 +62,7 @@ config ARCH_BRCMSTB
 config ARCH_EXYNOS
 	bool "ARMv8 based Samsung Exynos SoC family"
 	select COMMON_CLK_SAMSUNG
+	select CLKSRC_EXYNOS_MCT
 	select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS
 	select EXYNOS_PMU
 	select HAVE_S3C2410_WATCHDOG if WATCHDOG
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 3/6] clocksource: exynos_mct: Refactor resources allocation
  2018-10-17 13:41     ` [PATCH v3 3/6] clocksource: exynos_mct: Refactor resources allocation Marek Szyprowski
@ 2018-10-17 14:29       ` Krzysztof Kozlowski
  2018-10-18  9:40         ` Marek Szyprowski
  2018-10-18  3:50       ` Chanwoo Choi
  1 sibling, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2018-10-17 14:29 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: linux-samsung-soc, linux-arm-kernel, linux-kernel, will.deacon,
	catalin.marinas, marc.zyngier, tglx, daniel.lezcano,
	Chanwoo Choi, Bartłomiej Żołnierkiewicz, Inki Dae

On Wed, 17 Oct 2018 at 15:42, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Move interrupts allocation from exynos4_timer_resources() into separate
> function together with the interrupt number parsing code from
> mct_init_dt(), so the code for managing interrupts is kept together.
> While touching exynos4_timer_resources() function, move of_iomap() to it.
> No functional changes.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/clocksource/exynos_mct.c | 49 +++++++++++++++++++-------------
>  1 file changed, 30 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
> index 49413900b24c..02ad55db390b 100644
> --- a/drivers/clocksource/exynos_mct.c
> +++ b/drivers/clocksource/exynos_mct.c
> @@ -502,11 +502,14 @@ static int exynos4_mct_dying_cpu(unsigned int cpu)
>         return 0;
>  }
>
> -static int __init exynos4_timer_resources(struct device_node *np, void __iomem *base)
> +static int __init exynos4_timer_resources(struct device_node *np)
>  {
> -       int err, cpu;
>         struct clk *mct_clk, *tick_clk;
>
> +       reg_base = of_iomap(np, 0);
> +       if (!reg_base)
> +               panic("%s: unable to ioremap mct address space\n", __func__);
> +
>         tick_clk = of_clk_get_by_name(np, "fin_pll");
>         if (IS_ERR(tick_clk))
>                 panic("%s: unable to determine tick clock rate\n", __func__);
> @@ -517,9 +520,27 @@ static int __init exynos4_timer_resources(struct device_node *np, void __iomem *
>                 panic("%s: unable to retrieve mct clock instance\n", __func__);
>         clk_prepare_enable(mct_clk);
>
> -       reg_base = base;
> -       if (!reg_base)
> -               panic("%s: unable to ioremap mct address space\n", __func__);
> +       return 0;
> +}
> +
> +static int __init exynos4_timer_interrupts(struct device_node *np,
> +                                          unsigned int int_type)
> +{
> +       int i, err, cpu;
> +
> +       mct_int_type = int_type;

This does not look good. Before, assignment was done before call to
exynos4_timer_resources(). Now, if I follow the path correctly, it
will be after. Therefore the exynos4_timer_resources() will use wrong
value. This should pop up during tests.

Best regards,
Krzysztof

> +
> +       /* This driver uses only one global timer interrupt */
> +       mct_irqs[MCT_G0_IRQ] = irq_of_parse_and_map(np, MCT_G0_IRQ);
> +
> +       /*
> +        * Find out the number of local irqs specified. The local
> +        * timer irqs are specified after the four global timer
> +        * irqs are specified.
> +        */
> +       nr_irqs = of_irq_count(np);
> +       for (i = MCT_L0_IRQ; i < nr_irqs; i++)
> +               mct_irqs[i] = irq_of_parse_and_map(np, i);
>
>         if (mct_int_type == MCT_INT_PPI) {
>
> @@ -579,24 +600,14 @@ static int __init exynos4_timer_resources(struct device_node *np, void __iomem *
>
>  static int __init mct_init_dt(struct device_node *np, unsigned int int_type)
>  {
> -       u32 nr_irqs, i;
>         int ret;
>
> -       mct_int_type = int_type;
> -
> -       /* This driver uses only one global timer interrupt */
> -       mct_irqs[MCT_G0_IRQ] = irq_of_parse_and_map(np, MCT_G0_IRQ);
> +       ret = exynos4_timer_resources(np);
> +       if (ret)
> +               return ret;
>
> -       /*
> -        * Find out the number of local irqs specified. The local
> -        * timer irqs are specified after the four global timer
> -        * irqs are specified.
> -        */
> -       nr_irqs = of_irq_count(np);
> -       for (i = MCT_L0_IRQ; i < nr_irqs; i++)
> -               mct_irqs[i] = irq_of_parse_and_map(np, i);
>
> -       ret = exynos4_timer_resources(np, of_iomap(np, 0));
> +       ret = exynos4_timer_interrupts(np, int_type);
>         if (ret)
>                 return ret;
>
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 3/6] clocksource: exynos_mct: Refactor resources allocation
  2018-10-17 13:41     ` [PATCH v3 3/6] clocksource: exynos_mct: Refactor resources allocation Marek Szyprowski
  2018-10-17 14:29       ` Krzysztof Kozlowski
@ 2018-10-18  3:50       ` Chanwoo Choi
  1 sibling, 0 replies; 11+ messages in thread
From: Chanwoo Choi @ 2018-10-18  3:50 UTC (permalink / raw)
  To: Marek Szyprowski, linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Will Deacon, Catalin Marinas, Marc Zyngier, Thomas Gleixner,
	Daniel Lezcano, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Inki Dae

Hi Marek,

I tested it for both kernel booting and cpu hotplugging
on Exynos5433-based TM2 board.

Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
Tested-by: Chanwoo Choi <cw00.choi@samsung.com>

On 2018년 10월 17일 22:41, Marek Szyprowski wrote:
> Move interrupts allocation from exynos4_timer_resources() into separate
> function together with the interrupt number parsing code from
> mct_init_dt(), so the code for managing interrupts is kept together.
> While touching exynos4_timer_resources() function, move of_iomap() to it.
> No functional changes.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/clocksource/exynos_mct.c | 49 +++++++++++++++++++-------------
>  1 file changed, 30 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
> index 49413900b24c..02ad55db390b 100644
> --- a/drivers/clocksource/exynos_mct.c
> +++ b/drivers/clocksource/exynos_mct.c
> @@ -502,11 +502,14 @@ static int exynos4_mct_dying_cpu(unsigned int cpu)
>  	return 0;
>  }
>  
> -static int __init exynos4_timer_resources(struct device_node *np, void __iomem *base)
> +static int __init exynos4_timer_resources(struct device_node *np)
>  {
> -	int err, cpu;
>  	struct clk *mct_clk, *tick_clk;
>  
> +	reg_base = of_iomap(np, 0);
> +	if (!reg_base)
> +		panic("%s: unable to ioremap mct address space\n", __func__);
> +
>  	tick_clk = of_clk_get_by_name(np, "fin_pll");
>  	if (IS_ERR(tick_clk))
>  		panic("%s: unable to determine tick clock rate\n", __func__);
> @@ -517,9 +520,27 @@ static int __init exynos4_timer_resources(struct device_node *np, void __iomem *
>  		panic("%s: unable to retrieve mct clock instance\n", __func__);
>  	clk_prepare_enable(mct_clk);
>  
> -	reg_base = base;
> -	if (!reg_base)
> -		panic("%s: unable to ioremap mct address space\n", __func__);
> +	return 0;
> +}
> +
> +static int __init exynos4_timer_interrupts(struct device_node *np,
> +					   unsigned int int_type)
> +{
> +	int i, err, cpu;
> +
> +	mct_int_type = int_type;
> +
> +	/* This driver uses only one global timer interrupt */
> +	mct_irqs[MCT_G0_IRQ] = irq_of_parse_and_map(np, MCT_G0_IRQ);
> +
> +	/*
> +	 * Find out the number of local irqs specified. The local
> +	 * timer irqs are specified after the four global timer
> +	 * irqs are specified.
> +	 */
> +	nr_irqs = of_irq_count(np);
> +	for (i = MCT_L0_IRQ; i < nr_irqs; i++)
> +		mct_irqs[i] = irq_of_parse_and_map(np, i);
>  
>  	if (mct_int_type == MCT_INT_PPI) {
>  
> @@ -579,24 +600,14 @@ static int __init exynos4_timer_resources(struct device_node *np, void __iomem *
>  
>  static int __init mct_init_dt(struct device_node *np, unsigned int int_type)
>  {
> -	u32 nr_irqs, i;
>  	int ret;
>  
> -	mct_int_type = int_type;
> -
> -	/* This driver uses only one global timer interrupt */
> -	mct_irqs[MCT_G0_IRQ] = irq_of_parse_and_map(np, MCT_G0_IRQ);
> +	ret = exynos4_timer_resources(np);
> +	if (ret)
> +		return ret;
>  
> -	/*
> -	 * Find out the number of local irqs specified. The local
> -	 * timer irqs are specified after the four global timer
> -	 * irqs are specified.
> -	 */
> -	nr_irqs = of_irq_count(np);
> -	for (i = MCT_L0_IRQ; i < nr_irqs; i++)
> -		mct_irqs[i] = irq_of_parse_and_map(np, i);
>  
> -	ret = exynos4_timer_resources(np, of_iomap(np, 0));
> +	ret = exynos4_timer_interrupts(np, int_type);
>  	if (ret)
>  		return ret;
>  
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 4/6] clocksource: exynos_mct: Add arch_timer cooperation mode for ARM64
  2018-10-17 13:41     ` [PATCH v3 4/6] clocksource: exynos_mct: Add arch_timer cooperation mode for ARM64 Marek Szyprowski
@ 2018-10-18  3:51       ` Chanwoo Choi
  0 siblings, 0 replies; 11+ messages in thread
From: Chanwoo Choi @ 2018-10-18  3:51 UTC (permalink / raw)
  To: Marek Szyprowski, linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Will Deacon, Catalin Marinas, Marc Zyngier, Thomas Gleixner,
	Daniel Lezcano, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Inki Dae

Hi Marek,

On 2018년 10월 17일 22:41, Marek Szyprowski wrote:
> To get ARM Architected Timers working on Samsung Exynos SoCs, one has to
> first configure and enable Exynos Multi-Core Timer, because they both
> share some common hardware blocks (global system counter). This patch
> adds a mode of cooperation with arch_timer driver, so kernel can use
> CP15 based timer interface via arch_timer driver, which is mandatory
> on ARM64. In such mode MCT driver only enables its clocks and starts
> global timer. Everything else will be handled by arch_timer driver.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/clocksource/exynos_mct.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
> index 02ad55db390b..1b19a4f03929 100644
> --- a/drivers/clocksource/exynos_mct.c
> +++ b/drivers/clocksource/exynos_mct.c
> @@ -606,6 +606,15 @@ static int __init mct_init_dt(struct device_node *np, unsigned int int_type)
>  	if (ret)
>  		return ret;
>  
> +	if (IS_ENABLED(CONFIG_ARM64) && IS_ENABLED(CONFIG_ARM_ARCH_TIMER)) {
> +		struct device_node *np = of_find_compatible_node(NULL, NULL,
> +							     "arm,armv8-timer");
> +		if (np) {
> +			of_node_put(np);
> +			exynos4_mct_frc_start();
> +			return 0;
> +		}
> +	}
>  
>  	ret = exynos4_timer_interrupts(np, int_type);
>  	if (ret)
> 

I tested it on Exynos5433-based TM2 board.

Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
Tested-by: Chanwoo Choi <cw00.choi@samsung.com>


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 3/6] clocksource: exynos_mct: Refactor resources allocation
  2018-10-17 14:29       ` Krzysztof Kozlowski
@ 2018-10-18  9:40         ` Marek Szyprowski
  0 siblings, 0 replies; 11+ messages in thread
From: Marek Szyprowski @ 2018-10-18  9:40 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-samsung-soc, linux-arm-kernel, linux-kernel, will.deacon,
	catalin.marinas, marc.zyngier, tglx, daniel.lezcano,
	Chanwoo Choi, Bartłomiej Żołnierkiewicz, Inki Dae

Hi Krzysztof,

On 2018-10-17 16:29, Krzysztof Kozlowski wrote:
> On Wed, 17 Oct 2018 at 15:42, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> Move interrupts allocation from exynos4_timer_resources() into separate
>> function together with the interrupt number parsing code from
>> mct_init_dt(), so the code for managing interrupts is kept together.
>> While touching exynos4_timer_resources() function, move of_iomap() to it.
>> No functional changes.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>  drivers/clocksource/exynos_mct.c | 49 +++++++++++++++++++-------------
>>  1 file changed, 30 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
>> index 49413900b24c..02ad55db390b 100644
>> --- a/drivers/clocksource/exynos_mct.c
>> +++ b/drivers/clocksource/exynos_mct.c
>> @@ -502,11 +502,14 @@ static int exynos4_mct_dying_cpu(unsigned int cpu)
>>         return 0;
>>  }
>>
>> -static int __init exynos4_timer_resources(struct device_node *np, void __iomem *base)
>> +static int __init exynos4_timer_resources(struct device_node *np)
>>  {
>> -       int err, cpu;
>>         struct clk *mct_clk, *tick_clk;
>>
>> +       reg_base = of_iomap(np, 0);
>> +       if (!reg_base)
>> +               panic("%s: unable to ioremap mct address space\n", __func__);
>> +
>>         tick_clk = of_clk_get_by_name(np, "fin_pll");
>>         if (IS_ERR(tick_clk))
>>                 panic("%s: unable to determine tick clock rate\n", __func__);
>> @@ -517,9 +520,27 @@ static int __init exynos4_timer_resources(struct device_node *np, void __iomem *
>>                 panic("%s: unable to retrieve mct clock instance\n", __func__);
>>         clk_prepare_enable(mct_clk);
>>
>> -       reg_base = base;
>> -       if (!reg_base)
>> -               panic("%s: unable to ioremap mct address space\n", __func__);
>> +       return 0;
>> +}
>> +
>> +static int __init exynos4_timer_interrupts(struct device_node *np,
>> +                                          unsigned int int_type)
>> +{
>> +       int i, err, cpu;
>> +
>> +       mct_int_type = int_type;
> This does not look good. Before, assignment was done before call to
> exynos4_timer_resources(). Now, if I follow the path correctly, it
> will be after. Therefore the exynos4_timer_resources() will use wrong
> value. This should pop up during tests.

After refactoring exynos4_timer_resource() doesn't use int_type, but you
are right, there is one item missing: nr_irqs as local variable, what
causes the driver to mess with the global one, what is fatal to the
system. I will send a fixed version in a few minutes.

> ...

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-10-18  9:40 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20181017134204eucas1p10d92a91f2b945afdc7b39ce6e7813a8e@eucas1p1.samsung.com>
2018-10-17 13:41 ` [PATCH v3 0/6] Proper arch timer support for Exynos5433-based TM2(e) boards Marek Szyprowski
     [not found]   ` <CGME20181017134205eucas1p148faab76ac153e9afbb8a519e6d8d1e2@eucas1p1.samsung.com>
2018-10-17 13:41     ` [PATCH v3 1/6] clocksource: exynos_mct: Remove dead code Marek Szyprowski
     [not found]   ` <CGME20181017134206eucas1p1caf7cdec20ec620a5bf52fb9e060a7de@eucas1p1.samsung.com>
2018-10-17 13:41     ` [PATCH v3 2/6] clocksource: exynos_mct: Fix error path in timer resources initialization Marek Szyprowski
     [not found]   ` <CGME20181017134206eucas1p1ebc76569f706675d41f2d2183349f1f3@eucas1p1.samsung.com>
2018-10-17 13:41     ` [PATCH v3 3/6] clocksource: exynos_mct: Refactor resources allocation Marek Szyprowski
2018-10-17 14:29       ` Krzysztof Kozlowski
2018-10-18  9:40         ` Marek Szyprowski
2018-10-18  3:50       ` Chanwoo Choi
     [not found]   ` <CGME20181017134207eucas1p1b938eefe31fc47baaf538c9ebafc1a7e@eucas1p1.samsung.com>
2018-10-17 13:41     ` [PATCH v3 4/6] clocksource: exynos_mct: Add arch_timer cooperation mode for ARM64 Marek Szyprowski
2018-10-18  3:51       ` Chanwoo Choi
     [not found]   ` <CGME20181017134207eucas1p18188ff6debdbabbbf8b01cc4dc1d4b13@eucas1p1.samsung.com>
2018-10-17 13:41     ` [PATCH v3 5/6] arm64: dts: exynos: Move arch-timer node to right place Marek Szyprowski
     [not found]   ` <CGME20181017134208eucas1p19c81d1ce9050be1b78e386d8a349f441@eucas1p1.samsung.com>
2018-10-17 13:41     ` [PATCH v3 6/6] arm64: platform: Add enable Exynos Multi-Core Timer driver Marek Szyprowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).