[PATCH v6 0/2] Add support for the OST in Ingenic X1000.

[PATCH v6 0/2] Add support for the OST in Ingenic X1000.

[周琰杰 (Zhou Yanjie)]

v5->v6:
1.Drop "oneOf" and the blank line in "ingenic,sysost.yaml".
2.Add "additionalProperties: false" in "ingenic,sysost.yaml".

周琰杰 (Zhou Yanjie) (2):
  dt-bindings: timer: Add Ingenic X1000 OST bindings.
  clocksource: Ingenic: Add support for the Ingenic X1000 OST.

 .../devicetree/bindings/timer/ingenic,sysost.yaml  |  63 +++
 drivers/clocksource/Kconfig                        |  11 +
 drivers/clocksource/Makefile                       |   1 +
 drivers/clocksource/ingenic-sysost.c               | 539 +++++++++++++++++++++
 include/dt-bindings/clock/ingenic,sysost.h         |  12 +
 5 files changed, 626 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/ingenic,sysost.yaml
 create mode 100644 drivers/clocksource/ingenic-sysost.c
 create mode 100644 include/dt-bindings/clock/ingenic,sysost.h

-- 
2.11.0

[PATCH v6 1/2] dt-bindings: timer: Add Ingenic X1000 OST bindings.

[周琰杰 (Zhou Yanjie)]

Add the OST bindings for the X10000 SoC from Ingenic.

Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
Reviewed-by: Paul Cercueil <paul@crapouillou.net>
---

Notes:
    v1->v2:
    No change.
    
    v2->v3:
    Fix wrong parameters in "clocks".
    
    v3->v4:
    1.Rename "ingenic,ost.yaml" to "ingenic,sysost.yaml".
    2.Rename "ingenic,ost.h" to "ingenic,sysost.h".
    3.Modify the description in "ingenic,sysost.yaml".
    
    v4->v5:
    No change.
    
    v5->v6:
    1.Drop "oneOf" and the blank line.
    2.Add "additionalProperties: false".

 .../devicetree/bindings/timer/ingenic,sysost.yaml  | 63 ++++++++++++++++++++++
 include/dt-bindings/clock/ingenic,sysost.h         | 12 +++++
 2 files changed, 75 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/ingenic,sysost.yaml
 create mode 100644 include/dt-bindings/clock/ingenic,sysost.h

diff --git a/Documentation/devicetree/bindings/timer/ingenic,sysost.yaml b/Documentation/devicetree/bindings/timer/ingenic,sysost.yaml
new file mode 100644
index 000000000000..1dae2e538725
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/ingenic,sysost.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/timer/ingenic,sysost.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bindings for SYSOST in Ingenic XBurst family SoCs
+
+maintainers:
+  - 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
+
+description:
+  The SYSOST in an Ingenic SoC provides one 64bit timer for clocksource
+  and one or more 32bit timers for clockevent.
+
+properties:
+  "#clock-cells":
+    const: 1
+
+  compatible:
+    enum:
+      - ingenic,x1000-ost
+      - ingenic,x2000-ost
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: ost
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - "#clock-cells"
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/x1000-cgu.h>
+
+    ost: timer@12000000 {
+    		compatible = "ingenic,x1000-ost";
+    		reg = <0x12000000 0x3c>;
+
+    		#clock-cells = <1>;
+
+    		clocks = <&cgu X1000_CLK_OST>;
+    		clock-names = "ost";
+
+    		interrupt-parent = <&cpuintc>;
+    		interrupts = <3>;
+    	};
+...
diff --git a/include/dt-bindings/clock/ingenic,sysost.h b/include/dt-bindings/clock/ingenic,sysost.h
new file mode 100644
index 000000000000..9ac88e90babf
--- /dev/null
+++ b/include/dt-bindings/clock/ingenic,sysost.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * This header provides clock numbers for the ingenic,tcu DT binding.
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_INGENIC_OST_H__
+#define __DT_BINDINGS_CLOCK_INGENIC_OST_H__
+
+#define OST_CLK_PERCPU_TIMER	0
+#define OST_CLK_GLOBAL_TIMER	1
+
+#endif /* __DT_BINDINGS_CLOCK_INGENIC_OST_H__ */
-- 
2.11.0

Re: [PATCH v6 1/2] dt-bindings: timer: Add Ingenic X1000 OST bindings.

[Rob Herring]

On Sat, 11 Jul 2020 01:02:58 +0800, 周琰杰 (Zhou Yanjie) wrote:
> Add the OST bindings for the X10000 SoC from Ingenic.
> 
> Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> Reviewed-by: Paul Cercueil <paul@crapouillou.net>
> ---
> 
> Notes:
>     v1->v2:
>     No change.
> 
>     v2->v3:
>     Fix wrong parameters in "clocks".
> 
>     v3->v4:
>     1.Rename "ingenic,ost.yaml" to "ingenic,sysost.yaml".
>     2.Rename "ingenic,ost.h" to "ingenic,sysost.h".
>     3.Modify the description in "ingenic,sysost.yaml".
> 
>     v4->v5:
>     No change.
> 
>     v5->v6:
>     1.Drop "oneOf" and the blank line.
>     2.Add "additionalProperties: false".
> 
>  .../devicetree/bindings/timer/ingenic,sysost.yaml  | 63 ++++++++++++++++++++++
>  include/dt-bindings/clock/ingenic,sysost.h         | 12 +++++
>  2 files changed, 75 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/timer/ingenic,sysost.yaml
>  create mode 100644 include/dt-bindings/clock/ingenic,sysost.h
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v6 2/2] clocksource: Ingenic: Add support for the Ingenic X1000 OST.

[周琰杰 (Zhou Yanjie)]

X1000 and SoCs after X1000 (such as X1500 and X1830) had a separate
OST, it no longer belongs to TCU. This driver will register both a
clocksource and a sched_clock to the system.

Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
Reviewed-by: Paul Cercueil <paul@crapouillou.net>
---

Notes:
    v1->v2:
    Fix compile warnings.
    Reported-by: kernel test robot <lkp@intel.com>
    
    v2->v3:
    No change.
    
    v3->v4:
    1.Rename "ost" to "sysost"
    1.Remove unrelated changes.
    2.Remove ost_clock_parent enum.
    3.Remove ost->percpu_timer_channel/ost->global_timer_channel.
    4.Set up independent .recalc_rate/.set_rate for percpu/global timer.
    5.No longer call functions in variable declarations.
    
    v4->v5:
    Use "of_io_request_and_map()" instead "of_iomap()".
    Suggested-by: Paul Cercueil <paul@crapouillou.net>
    
    v5->v6:
    No change.

 drivers/clocksource/Kconfig          |  11 +
 drivers/clocksource/Makefile         |   1 +
 drivers/clocksource/ingenic-sysost.c | 539 +++++++++++++++++++++++++++++++++++
 3 files changed, 551 insertions(+)
 create mode 100644 drivers/clocksource/ingenic-sysost.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 91418381fcd4..1bca8b8fb30f 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -696,6 +696,17 @@ config INGENIC_TIMER
 	help
 	  Support for the timer/counter unit of the Ingenic JZ SoCs.
 
+config INGENIC_SYSOST
+	bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"
+	default MACH_INGENIC
+	depends on MIPS || COMPILE_TEST
+	depends on COMMON_CLK
+	select MFD_SYSCON
+	select TIMER_OF
+	select IRQ_DOMAIN
+	help
+	  Support for the SYSOST of the Ingenic X Series SoCs.
+
 config INGENIC_OST
 	bool "Clocksource for Ingenic OS Timer"
 	depends on MIPS || COMPILE_TEST
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index bdda1a2e4097..3994e221e262 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_H8300_TMR8)		+= h8300_timer8.o
 obj-$(CONFIG_H8300_TMR16)		+= h8300_timer16.o
 obj-$(CONFIG_H8300_TPU)			+= h8300_tpu.o
 obj-$(CONFIG_INGENIC_OST)		+= ingenic-ost.o
+obj-$(CONFIG_INGENIC_SYSOST)	+= ingenic-sysost.o
 obj-$(CONFIG_INGENIC_TIMER)		+= ingenic-timer.o
 obj-$(CONFIG_CLKSRC_ST_LPC)		+= clksrc_st_lpc.o
 obj-$(CONFIG_X86_NUMACHIP)		+= numachip.o
diff --git a/drivers/clocksource/ingenic-sysost.c b/drivers/clocksource/ingenic-sysost.c
new file mode 100644
index 000000000000..e77d58449005
--- /dev/null
+++ b/drivers/clocksource/ingenic-sysost.c
@@ -0,0 +1,539 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Ingenic XBurst SoCs SYSOST clocks driver
+ * Copyright (c) 2020 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/clockchips.h>
+#include <linux/clocksource.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/sched_clock.h>
+#include <linux/slab.h>
+#include <linux/syscore_ops.h>
+
+#include <dt-bindings/clock/ingenic,sysost.h>
+
+/* OST register offsets */
+#define OST_REG_OSTCCR			0x00
+#define OST_REG_OSTCR			0x08
+#define OST_REG_OSTFR			0x0c
+#define OST_REG_OSTMR			0x10
+#define OST_REG_OST1DFR			0x14
+#define OST_REG_OST1CNT			0x18
+#define OST_REG_OST2CNTL		0x20
+#define OST_REG_OSTCNT2HBUF		0x24
+#define OST_REG_OSTESR			0x34
+#define OST_REG_OSTECR			0x38
+
+/* bits within the OSTCCR register */
+#define OSTCCR_PRESCALE1_MASK	0x3
+#define OSTCCR_PRESCALE2_MASK	0xc
+#define OSTCCR_PRESCALE1_LSB	0
+#define OSTCCR_PRESCALE2_LSB	2
+
+/* bits within the OSTCR register */
+#define OSTCR_OST1CLR			BIT(0)
+#define OSTCR_OST2CLR			BIT(1)
+
+/* bits within the OSTFR register */
+#define OSTFR_FFLAG				BIT(0)
+
+/* bits within the OSTMR register */
+#define OSTMR_FMASK				BIT(0)
+
+/* bits within the OSTESR register */
+#define OSTESR_OST1ENS			BIT(0)
+#define OSTESR_OST2ENS			BIT(1)
+
+/* bits within the OSTECR register */
+#define OSTECR_OST1ENC			BIT(0)
+#define OSTECR_OST2ENC			BIT(1)
+
+struct ingenic_soc_info {
+	unsigned int num_channels;
+};
+
+struct ingenic_ost_clk_info {
+	struct clk_init_data init_data;
+	u8 ostccr_reg;
+};
+
+struct ingenic_ost_clk {
+	struct clk_hw hw;
+	unsigned int idx;
+	struct ingenic_ost *ost;
+	const struct ingenic_ost_clk_info *info;
+};
+
+struct ingenic_ost {
+	void __iomem *base;
+	const struct ingenic_soc_info *soc_info;
+	struct clk *clk, *percpu_timer_clk, *global_timer_clk;
+	struct clock_event_device cevt;
+	struct clocksource cs;
+	char name[20];
+
+	struct clk_hw_onecell_data *clocks;
+};
+
+static struct ingenic_ost *ingenic_ost;
+
+static inline struct ingenic_ost_clk *to_ost_clk(struct clk_hw *hw)
+{
+	return container_of(hw, struct ingenic_ost_clk, hw);
+}
+
+static unsigned long ingenic_ost_percpu_timer_recalc_rate(struct clk_hw *hw,
+		unsigned long parent_rate)
+{
+	struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
+	const struct ingenic_ost_clk_info *info = ost_clk->info;
+	unsigned int prescale;
+
+	prescale = readl(ost_clk->ost->base + info->ostccr_reg);
+
+	prescale = (prescale & OSTCCR_PRESCALE1_MASK) >> OSTCCR_PRESCALE1_LSB;
+
+	return parent_rate >> (prescale * 2);
+}
+
+static unsigned long ingenic_ost_global_timer_recalc_rate(struct clk_hw *hw,
+		unsigned long parent_rate)
+{
+	struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
+	const struct ingenic_ost_clk_info *info = ost_clk->info;
+	unsigned int prescale;
+
+	prescale = readl(ost_clk->ost->base + info->ostccr_reg);
+
+	prescale = (prescale & OSTCCR_PRESCALE2_MASK) >> OSTCCR_PRESCALE2_LSB;
+
+	return parent_rate >> (prescale * 2);
+}
+
+static u8 ingenic_ost_get_prescale(unsigned long rate, unsigned long req_rate)
+{
+	u8 prescale;
+
+	for (prescale = 0; prescale < 2; prescale++)
+		if ((rate >> (prescale * 2)) <= req_rate)
+			return prescale;
+
+	return 2; /* /16 divider */
+}
+
+static long ingenic_ost_round_rate(struct clk_hw *hw, unsigned long req_rate,
+		unsigned long *parent_rate)
+{
+	unsigned long rate = *parent_rate;
+	u8 prescale;
+
+	if (req_rate > rate)
+		return rate;
+
+	prescale = ingenic_ost_get_prescale(rate, req_rate);
+
+	return rate >> (prescale * 2);
+}
+
+static int ingenic_ost_percpu_timer_set_rate(struct clk_hw *hw, unsigned long req_rate,
+		unsigned long parent_rate)
+{
+	struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
+	const struct ingenic_ost_clk_info *info = ost_clk->info;
+	u8 prescale = ingenic_ost_get_prescale(parent_rate, req_rate);
+	int val;
+
+	val = readl(ost_clk->ost->base + info->ostccr_reg);
+	val = (val & ~OSTCCR_PRESCALE1_MASK) | (prescale << OSTCCR_PRESCALE1_LSB);
+	writel(val, ost_clk->ost->base + info->ostccr_reg);
+
+	return 0;
+}
+
+static int ingenic_ost_global_timer_set_rate(struct clk_hw *hw, unsigned long req_rate,
+		unsigned long parent_rate)
+{
+	struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
+	const struct ingenic_ost_clk_info *info = ost_clk->info;
+	u8 prescale = ingenic_ost_get_prescale(parent_rate, req_rate);
+	int val;
+
+	val = readl(ost_clk->ost->base + info->ostccr_reg);
+	val = (val & ~OSTCCR_PRESCALE2_MASK) | (prescale << OSTCCR_PRESCALE2_LSB);
+	writel(val, ost_clk->ost->base + info->ostccr_reg);
+
+	return 0;
+}
+
+static const struct clk_ops ingenic_ost_percpu_timer_ops = {
+	.recalc_rate	= ingenic_ost_percpu_timer_recalc_rate,
+	.round_rate		= ingenic_ost_round_rate,
+	.set_rate		= ingenic_ost_percpu_timer_set_rate,
+};
+
+static const struct clk_ops ingenic_ost_global_timer_ops = {
+	.recalc_rate	= ingenic_ost_global_timer_recalc_rate,
+	.round_rate		= ingenic_ost_round_rate,
+	.set_rate		= ingenic_ost_global_timer_set_rate,
+};
+
+static const char * const ingenic_ost_clk_parents[] = { "ext" };
+
+static const struct ingenic_ost_clk_info ingenic_ost_clk_info[] = {
+	[OST_CLK_PERCPU_TIMER] = {
+		.init_data = {
+			.name = "percpu timer",
+			.parent_names = ingenic_ost_clk_parents,
+			.num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
+			.ops = &ingenic_ost_percpu_timer_ops,
+			.flags = CLK_SET_RATE_UNGATE,
+		},
+		.ostccr_reg = OST_REG_OSTCCR,
+	},
+
+	[OST_CLK_GLOBAL_TIMER] = {
+		.init_data = {
+			.name = "global timer",
+			.parent_names = ingenic_ost_clk_parents,
+			.num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
+			.ops = &ingenic_ost_global_timer_ops,
+			.flags = CLK_SET_RATE_UNGATE,
+		},
+		.ostccr_reg = OST_REG_OSTCCR,
+	},
+};
+
+static u64 notrace ingenic_ost_global_timer_read_cntl(void)
+{
+	struct ingenic_ost *ost = ingenic_ost;
+	unsigned int count;
+
+	count = readl(ost->base + OST_REG_OST2CNTL);
+
+	return count;
+}
+
+static u64 notrace ingenic_ost_clocksource_read(struct clocksource *cs)
+{
+	return ingenic_ost_global_timer_read_cntl();
+}
+
+static inline struct ingenic_ost *to_ingenic_ost(struct clock_event_device *evt)
+{
+	return container_of(evt, struct ingenic_ost, cevt);
+}
+
+static int ingenic_ost_cevt_set_state_shutdown(struct clock_event_device *evt)
+{
+	struct ingenic_ost *ost = to_ingenic_ost(evt);
+
+	writel(OSTECR_OST1ENC, ost->base + OST_REG_OSTECR);
+
+	return 0;
+}
+
+static int ingenic_ost_cevt_set_next(unsigned long next,
+				     struct clock_event_device *evt)
+{
+	struct ingenic_ost *ost = to_ingenic_ost(evt);
+
+	writel((u32)~OSTFR_FFLAG, ost->base + OST_REG_OSTFR);
+	writel(next, ost->base + OST_REG_OST1DFR);
+	writel(OSTCR_OST1CLR, ost->base + OST_REG_OSTCR);
+	writel(OSTESR_OST1ENS, ost->base + OST_REG_OSTESR);
+	writel((u32)~OSTMR_FMASK, ost->base + OST_REG_OSTMR);
+
+	return 0;
+}
+
+static irqreturn_t ingenic_ost_cevt_cb(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = dev_id;
+	struct ingenic_ost *ost = to_ingenic_ost(evt);
+
+	writel(OSTECR_OST1ENC, ost->base + OST_REG_OSTECR);
+
+	if (evt->event_handler)
+		evt->event_handler(evt);
+
+	return IRQ_HANDLED;
+}
+
+static int __init ingenic_ost_register_clock(struct ingenic_ost *ost,
+			unsigned int idx, const struct ingenic_ost_clk_info *info,
+			struct clk_hw_onecell_data *clocks)
+{
+	struct ingenic_ost_clk *ost_clk;
+	int val, err;
+
+	ost_clk = kzalloc(sizeof(*ost_clk), GFP_KERNEL);
+	if (!ost_clk)
+		return -ENOMEM;
+
+	ost_clk->hw.init = &info->init_data;
+	ost_clk->idx = idx;
+	ost_clk->info = info;
+	ost_clk->ost = ost;
+
+	/* Reset clock divider */
+	val = readl(ost->base + info->ostccr_reg);
+	val &= ~(OSTCCR_PRESCALE1_MASK | OSTCCR_PRESCALE2_MASK);
+	writel(val, ost->base + info->ostccr_reg);
+
+	err = clk_hw_register(NULL, &ost_clk->hw);
+	if (err) {
+		kfree(ost_clk);
+		return err;
+	}
+
+	clocks->hws[idx] = &ost_clk->hw;
+
+	return 0;
+}
+
+static struct clk * __init ingenic_ost_get_clock(struct device_node *np, int id)
+{
+	struct of_phandle_args args;
+
+	args.np = np;
+	args.args_count = 1;
+	args.args[0] = id;
+
+	return of_clk_get_from_provider(&args);
+}
+
+static int __init ingenic_ost_percpu_timer_init(struct device_node *np,
+					 struct ingenic_ost *ost)
+{
+	unsigned int timer_virq, channel = OST_CLK_PERCPU_TIMER;
+	unsigned long rate;
+	int err;
+
+	ost->percpu_timer_clk = ingenic_ost_get_clock(np, channel);
+	if (IS_ERR(ost->percpu_timer_clk))
+		return PTR_ERR(ost->percpu_timer_clk);
+
+	err = clk_prepare_enable(ost->percpu_timer_clk);
+	if (err)
+		goto err_clk_put;
+
+	rate = clk_get_rate(ost->percpu_timer_clk);
+	if (!rate) {
+		err = -EINVAL;
+		goto err_clk_disable;
+	}
+
+	timer_virq = of_irq_get(np, 0);
+	if (!timer_virq) {
+		err = -EINVAL;
+		goto err_clk_disable;
+	}
+
+	snprintf(ost->name, sizeof(ost->name), "OST percpu timer");
+
+	err = request_irq(timer_virq, ingenic_ost_cevt_cb, IRQF_TIMER,
+			  ost->name, &ost->cevt);
+	if (err)
+		goto err_irq_dispose_mapping;
+
+	ost->cevt.cpumask = cpumask_of(smp_processor_id());
+	ost->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
+	ost->cevt.name = ost->name;
+	ost->cevt.rating = 400;
+	ost->cevt.set_state_shutdown = ingenic_ost_cevt_set_state_shutdown;
+	ost->cevt.set_next_event = ingenic_ost_cevt_set_next;
+
+	clockevents_config_and_register(&ost->cevt, rate, 4, 0xffffffff);
+
+	return 0;
+
+err_irq_dispose_mapping:
+	irq_dispose_mapping(timer_virq);
+err_clk_disable:
+	clk_disable_unprepare(ost->percpu_timer_clk);
+err_clk_put:
+	clk_put(ost->percpu_timer_clk);
+	return err;
+}
+
+static int __init ingenic_ost_global_timer_init(struct device_node *np,
+					       struct ingenic_ost *ost)
+{
+	unsigned int channel = OST_CLK_GLOBAL_TIMER;
+	struct clocksource *cs = &ost->cs;
+	unsigned long rate;
+	int err;
+
+	ost->global_timer_clk = ingenic_ost_get_clock(np, channel);
+	if (IS_ERR(ost->global_timer_clk))
+		return PTR_ERR(ost->global_timer_clk);
+
+	err = clk_prepare_enable(ost->global_timer_clk);
+	if (err)
+		goto err_clk_put;
+
+	rate = clk_get_rate(ost->global_timer_clk);
+	if (!rate) {
+		err = -EINVAL;
+		goto err_clk_disable;
+	}
+
+	/* Clear counter CNT registers */
+	writel(OSTCR_OST2CLR, ost->base + OST_REG_OSTCR);
+
+	/* Enable OST channel */
+	writel(OSTESR_OST2ENS, ost->base + OST_REG_OSTESR);
+
+	cs->name = "ingenic-ost";
+	cs->rating = 400;
+	cs->flags = CLOCK_SOURCE_IS_CONTINUOUS;
+	cs->mask = CLOCKSOURCE_MASK(32);
+	cs->read = ingenic_ost_clocksource_read;
+
+	err = clocksource_register_hz(cs, rate);
+	if (err)
+		goto err_clk_disable;
+
+	return 0;
+
+err_clk_disable:
+	clk_disable_unprepare(ost->global_timer_clk);
+err_clk_put:
+	clk_put(ost->global_timer_clk);
+	return err;
+}
+
+static const struct ingenic_soc_info x1000_soc_info = {
+	.num_channels = 2,
+};
+
+static const struct of_device_id __maybe_unused ingenic_ost_of_match[] __initconst = {
+	{ .compatible = "ingenic,x1000-ost", .data = &x1000_soc_info, },
+	{ /* sentinel */ }
+};
+
+static int __init ingenic_ost_probe(struct device_node *np)
+{
+	const struct of_device_id *id = of_match_node(ingenic_ost_of_match, np);
+	struct ingenic_ost *ost;
+	unsigned int i;
+	int ret;
+
+	ost = kzalloc(sizeof(*ost), GFP_KERNEL);
+	if (!ost)
+		return -ENOMEM;
+
+	ost->base = of_io_request_and_map(np, 0, of_node_full_name(np));
+	if (IS_ERR(ost->base)) {
+		pr_err("%s: Failed to map OST registers\n", __func__);
+		ret = PTR_ERR(ost->base);
+		goto err_free_ost;
+	}
+
+	ost->clk = of_clk_get_by_name(np, "ost");
+	if (IS_ERR(ost->clk)) {
+		ret = PTR_ERR(ost->clk);
+		pr_crit("%s: Cannot get OST clock\n", __func__);
+		goto err_free_ost;
+	}
+
+	ret = clk_prepare_enable(ost->clk);
+	if (ret) {
+		pr_crit("%s: Unable to enable OST clock\n", __func__);
+		goto err_put_clk;
+	}
+
+	ost->soc_info = id->data;
+
+	ost->clocks = kzalloc(struct_size(ost->clocks, hws, ost->soc_info->num_channels),
+			      GFP_KERNEL);
+	if (!ost->clocks) {
+		ret = -ENOMEM;
+		goto err_clk_disable;
+	}
+
+	ost->clocks->num = ost->soc_info->num_channels;
+
+	for (i = 0; i < ost->clocks->num; i++) {
+		ret = ingenic_ost_register_clock(ost, i, &ingenic_ost_clk_info[i], ost->clocks);
+		if (ret) {
+			pr_crit("%s: Cannot register clock %d\n", __func__, i);
+			goto err_unregister_ost_clocks;
+		}
+	}
+
+	ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, ost->clocks);
+	if (ret) {
+		pr_crit("%s: Cannot add OF clock provider\n", __func__);
+		goto err_unregister_ost_clocks;
+	}
+
+	ingenic_ost = ost;
+
+	return 0;
+
+err_unregister_ost_clocks:
+	for (i = 0; i < ost->clocks->num; i++)
+		if (ost->clocks->hws[i])
+			clk_hw_unregister(ost->clocks->hws[i]);
+	kfree(ost->clocks);
+err_clk_disable:
+	clk_disable_unprepare(ost->clk);
+err_put_clk:
+	clk_put(ost->clk);
+err_free_ost:
+	kfree(ost);
+	return ret;
+}
+
+static int __init ingenic_ost_init(struct device_node *np)
+{
+	struct ingenic_ost *ost;
+	unsigned long rate;
+	int ret;
+
+	ret = ingenic_ost_probe(np);
+	if (ret) {
+		pr_crit("%s: Failed to initialize OST clocks: %d\n", __func__, ret);
+		return ret;
+	}
+
+	of_node_clear_flag(np, OF_POPULATED);
+
+	ost = ingenic_ost;
+	if (IS_ERR(ost))
+		return PTR_ERR(ost);
+
+	ret = ingenic_ost_global_timer_init(np, ost);
+	if (ret) {
+		pr_crit("%s: Unable to init global timer: %x\n", __func__, ret);
+		goto err_free_ingenic_ost;
+	}
+
+	ret = ingenic_ost_percpu_timer_init(np, ost);
+	if (ret)
+		goto err_ost_global_timer_cleanup;
+
+	/* Register the sched_clock at the end as there's no way to undo it */
+	rate = clk_get_rate(ost->global_timer_clk);
+	sched_clock_register(ingenic_ost_global_timer_read_cntl, 32, rate);
+
+	return 0;
+
+err_ost_global_timer_cleanup:
+	clocksource_unregister(&ost->cs);
+	clk_disable_unprepare(ost->global_timer_clk);
+	clk_put(ost->global_timer_clk);
+err_free_ingenic_ost:
+	kfree(ost);
+	return ret;
+}
+
+TIMER_OF_DECLARE(x1000_ost,  "ingenic,x1000-ost",  ingenic_ost_init);
-- 
2.11.0

Re: [PATCH v6 2/2] clocksource: Ingenic: Add support for the Ingenic X1000 OST.

[Daniel Lezcano]

On 10/07/2020 19:02, 周琰杰 (Zhou Yanjie) wrote:
> X1000 and SoCs after X1000 (such as X1500 and X1830) had a separate
> OST, it no longer belongs to TCU. This driver will register both a
> clocksource and a sched_clock to the system.
> 
> Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
> Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
> Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> Reviewed-by: Paul Cercueil <paul@crapouillou.net>
> ---
> 
> Notes:
>     v1->v2:
>     Fix compile warnings.
>     Reported-by: kernel test robot <lkp@intel.com>
>     
>     v2->v3:
>     No change.
>     
>     v3->v4:
>     1.Rename "ost" to "sysost"
>     1.Remove unrelated changes.
>     2.Remove ost_clock_parent enum.
>     3.Remove ost->percpu_timer_channel/ost->global_timer_channel.
>     4.Set up independent .recalc_rate/.set_rate for percpu/global timer.
>     5.No longer call functions in variable declarations.
>     
>     v4->v5:
>     Use "of_io_request_and_map()" instead "of_iomap()".
>     Suggested-by: Paul Cercueil <paul@crapouillou.net>
>     
>     v5->v6:
>     No change.
> 
>  drivers/clocksource/Kconfig          |  11 +
>  drivers/clocksource/Makefile         |   1 +
>  drivers/clocksource/ingenic-sysost.c | 539 +++++++++++++++++++++++++++++++++++
>  3 files changed, 551 insertions(+)
>  create mode 100644 drivers/clocksource/ingenic-sysost.c
> 
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 91418381fcd4..1bca8b8fb30f 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -696,6 +696,17 @@ config INGENIC_TIMER
>  	help
>  	  Support for the timer/counter unit of the Ingenic JZ SoCs.
>  
> +config INGENIC_SYSOST
> +	bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"

We usually use silent options and let the platform's Kconfig enable it.
We show up the option only when COMPILE_TEST is enabled.

Is there a reason to do it differently?

> +	default MACH_INGENIC
> +	depends on MIPS || COMPILE_TEST
> +	depends on COMMON_CLK
> +	select MFD_SYSCON
> +	select TIMER_OF
> +	select IRQ_DOMAIN
> +	help
> +	  Support for the SYSOST of the Ingenic X Series SoCs.
> +

[ ... ]


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v6 2/2] clocksource: Ingenic: Add support for the Ingenic X1000 OST.

[Zhou Yanjie]

Hi Daniel,

在 2020/7/17 下午12:20, Daniel Lezcano 写道:
> On 10/07/2020 19:02, 周琰杰 (Zhou Yanjie) wrote:
>> X1000 and SoCs after X1000 (such as X1500 and X1830) had a separate
>> OST, it no longer belongs to TCU. This driver will register both a
>> clocksource and a sched_clock to the system.
>>
>> Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
>> Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>> Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>> Reviewed-by: Paul Cercueil <paul@crapouillou.net>
>> ---
>>
>> Notes:
>>      v1->v2:
>>      Fix compile warnings.
>>      Reported-by: kernel test robot <lkp@intel.com>
>>      
>>      v2->v3:
>>      No change.
>>      
>>      v3->v4:
>>      1.Rename "ost" to "sysost"
>>      1.Remove unrelated changes.
>>      2.Remove ost_clock_parent enum.
>>      3.Remove ost->percpu_timer_channel/ost->global_timer_channel.
>>      4.Set up independent .recalc_rate/.set_rate for percpu/global timer.
>>      5.No longer call functions in variable declarations.
>>      
>>      v4->v5:
>>      Use "of_io_request_and_map()" instead "of_iomap()".
>>      Suggested-by: Paul Cercueil <paul@crapouillou.net>
>>      
>>      v5->v6:
>>      No change.
>>
>>   drivers/clocksource/Kconfig          |  11 +
>>   drivers/clocksource/Makefile         |   1 +
>>   drivers/clocksource/ingenic-sysost.c | 539 +++++++++++++++++++++++++++++++++++
>>   3 files changed, 551 insertions(+)
>>   create mode 100644 drivers/clocksource/ingenic-sysost.c
>>
>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>> index 91418381fcd4..1bca8b8fb30f 100644
>> --- a/drivers/clocksource/Kconfig
>> +++ b/drivers/clocksource/Kconfig
>> @@ -696,6 +696,17 @@ config INGENIC_TIMER
>>   	help
>>   	  Support for the timer/counter unit of the Ingenic JZ SoCs.
>>   
>> +config INGENIC_SYSOST
>> +	bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"
> We usually use silent options and let the platform's Kconfig enable it.
> We show up the option only when COMPILE_TEST is enabled.
>
> Is there a reason to do it differently?


Do you mean

bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"

or

default MACH_INGENIC ?

This driver has some origins from "INGENIC_TIMER" driver and "INGENIC_OST" driver.
Early Ingenic processors used TCU (timer/counter unit, has 6 or 8 generic timer channels) to provide clocksource and clockevent (both with only 16bit precision). This part of the processor can only use "INGENIC_TIMER" driver.

Later processors provide an independent 32bit or 64bit timer channel (still under TCU, known as ost channel, this channel can not generate interrupt) to provid higher precision clocksource. The "INGENIC_OST" driver is for this channel. These processors can use "INGENIC_TIMER" driver, but using "INGENIC_OST" driver to provide higher precision clocksource would be a better choice (clockevent still needs to be provided by generic timer channel of TCU, and still 16bit precision).

And the recent processors provide a SYSOST components, it is independent from TCU, including a 64bit timer channel for clocksource and a 32bit timer channel for clockevent. Although these processors can also use "INGENIC_TIMER" driver, but the better choice is completely independent use of "INGENIC_SYSOST" driver to provide higher precision clocksource and clockevent.

You may have already noticed that this independent SYSOST component is like an upgraded and streamlined TCU, which only retains one generic timer channel that can generate interrupts, upgrade it from 16bit to 32bit, and then retain the 64bit ost channel. so the driver code and Kconfig code of this patch is largely referenced
"INGENIC_TIMER" driver and "INGENIC_OST" driver.

Thanks and best regards!

>> +	default MACH_INGENIC
>> +	depends on MIPS || COMPILE_TEST
>> +	depends on COMMON_CLK
>> +	select MFD_SYSCON
>> +	select TIMER_OF
>> +	select IRQ_DOMAIN
>> +	help
>> +	  Support for the SYSOST of the Ingenic X Series SoCs.
>> +
> [ ... ]
>
>

Re: [PATCH v6 2/2] clocksource: Ingenic: Add support for the Ingenic X1000 OST.

[Daniel Lezcano]

On 17/07/2020 08:13, Zhou Yanjie wrote:
> Hi Daniel,
> 
> 在 2020/7/17 下午12:20, Daniel Lezcano 写道:
>> On 10/07/2020 19:02, 周琰杰 (Zhou Yanjie) wrote:
>>> X1000 and SoCs after X1000 (such as X1500 and X1830) had a separate
>>> OST, it no longer belongs to TCU. This driver will register both a
>>> clocksource and a sched_clock to the system.
>>>
>>> Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
>>> Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>>> Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>>> Reviewed-by: Paul Cercueil <paul@crapouillou.net>
>>> ---
>>>
>>> Notes:
>>>      v1->v2:
>>>      Fix compile warnings.
>>>      Reported-by: kernel test robot <lkp@intel.com>
>>>           v2->v3:
>>>      No change.
>>>           v3->v4:
>>>      1.Rename "ost" to "sysost"
>>>      1.Remove unrelated changes.
>>>      2.Remove ost_clock_parent enum.
>>>      3.Remove ost->percpu_timer_channel/ost->global_timer_channel.
>>>      4.Set up independent .recalc_rate/.set_rate for percpu/global
>>> timer.
>>>      5.No longer call functions in variable declarations.
>>>           v4->v5:
>>>      Use "of_io_request_and_map()" instead "of_iomap()".
>>>      Suggested-by: Paul Cercueil <paul@crapouillou.net>
>>>           v5->v6:
>>>      No change.
>>>
>>>   drivers/clocksource/Kconfig          |  11 +
>>>   drivers/clocksource/Makefile         |   1 +
>>>   drivers/clocksource/ingenic-sysost.c | 539
>>> +++++++++++++++++++++++++++++++++++
>>>   3 files changed, 551 insertions(+)
>>>   create mode 100644 drivers/clocksource/ingenic-sysost.c
>>>
>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>> index 91418381fcd4..1bca8b8fb30f 100644
>>> --- a/drivers/clocksource/Kconfig
>>> +++ b/drivers/clocksource/Kconfig
>>> @@ -696,6 +696,17 @@ config INGENIC_TIMER
>>>       help
>>>         Support for the timer/counter unit of the Ingenic JZ SoCs.
>>>   +config INGENIC_SYSOST
>>> +    bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"
>> We usually use silent options and let the platform's Kconfig enable it.
>> We show up the option only when COMPILE_TEST is enabled.
>>
>> Is there a reason to do it differently?
> 
> 
> Do you mean
> 
> bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"
> 
> or
> 
> default MACH_INGENIC ?

Both, no default here.

eg.

bool "Clocksource/timer using the SYSOST in Ingenic X SoCs" if COMPILE_TEST

and

in arch/mips/Kconfig in the config MACH_INGENIC section :

...
select INGENIC_SYSOST
...

> This driver has some origins from "INGENIC_TIMER" driver and
> "INGENIC_OST" driver.
> Early Ingenic processors used TCU (timer/counter unit, has 6 or 8
> generic timer channels) to provide clocksource and clockevent (both with
> only 16bit precision). This part of the processor can only use
> "INGENIC_TIMER" driver.
> 
> Later processors provide an independent 32bit or 64bit timer channel
> (still under TCU, known as ost channel, this channel can not generate
> interrupt) to provid higher precision clocksource. The "INGENIC_OST"
> driver is for this channel. These processors can use "INGENIC_TIMER"
> driver, but using "INGENIC_OST" driver to provide higher precision
> clocksource would be a better choice (clockevent still needs to be
> provided by generic timer channel of TCU, and still 16bit precision).
> 
> And the recent processors provide a SYSOST components, it is independent
> from TCU, including a 64bit timer channel for clocksource and a 32bit
> timer channel for clockevent. Although these processors can also use
> "INGENIC_TIMER" driver, but the better choice is completely independent
> use of "INGENIC_SYSOST" driver to provide higher precision clocksource
> and clockevent.

Ok, the rating should do the job then.

Thanks for the explanation.

> You may have already noticed that this independent SYSOST component is
> like an upgraded and streamlined TCU, which only retains one generic
> timer channel that can generate interrupts, upgrade it from 16bit to
> 32bit, and then retain the 64bit ost channel. so the driver code and
> Kconfig code of this patch is largely referenced
> "INGENIC_TIMER" driver and "INGENIC_OST" driver.
> 
> Thanks and best regards!
> 
>>> +    default MACH_INGENIC
>>> +    depends on MIPS || COMPILE_TEST
>>> +    depends on COMMON_CLK
>>> +    select MFD_SYSCON
>>> +    select TIMER_OF
>>> +    select IRQ_DOMAIN
>>> +    help
>>> +      Support for the SYSOST of the Ingenic X Series SoCs.
>>> +
>> [ ... ]
>>
>>


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v6 2/2] clocksource: Ingenic: Add support for the Ingenic X1000 OST.

[Zhou Yanjie]

Hi Daniel,

在 2020/7/17 下午4:02, Daniel Lezcano 写道:
> On 17/07/2020 08:13, Zhou Yanjie wrote:
>> Hi Daniel,
>>
>> 在 2020/7/17 下午12:20, Daniel Lezcano 写道:
>>> On 10/07/2020 19:02, 周琰杰 (Zhou Yanjie) wrote:
>>>> X1000 and SoCs after X1000 (such as X1500 and X1830) had a separate
>>>> OST, it no longer belongs to TCU. This driver will register both a
>>>> clocksource and a sched_clock to the system.
>>>>
>>>> Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
>>>> Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>>>> Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>>>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>>>> Reviewed-by: Paul Cercueil <paul@crapouillou.net>
>>>> ---
>>>>
>>>> Notes:
>>>>       v1->v2:
>>>>       Fix compile warnings.
>>>>       Reported-by: kernel test robot <lkp@intel.com>
>>>>            v2->v3:
>>>>       No change.
>>>>            v3->v4:
>>>>       1.Rename "ost" to "sysost"
>>>>       1.Remove unrelated changes.
>>>>       2.Remove ost_clock_parent enum.
>>>>       3.Remove ost->percpu_timer_channel/ost->global_timer_channel.
>>>>       4.Set up independent .recalc_rate/.set_rate for percpu/global
>>>> timer.
>>>>       5.No longer call functions in variable declarations.
>>>>            v4->v5:
>>>>       Use "of_io_request_and_map()" instead "of_iomap()".
>>>>       Suggested-by: Paul Cercueil <paul@crapouillou.net>
>>>>            v5->v6:
>>>>       No change.
>>>>
>>>>    drivers/clocksource/Kconfig          |  11 +
>>>>    drivers/clocksource/Makefile         |   1 +
>>>>    drivers/clocksource/ingenic-sysost.c | 539
>>>> +++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 551 insertions(+)
>>>>    create mode 100644 drivers/clocksource/ingenic-sysost.c
>>>>
>>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>>> index 91418381fcd4..1bca8b8fb30f 100644
>>>> --- a/drivers/clocksource/Kconfig
>>>> +++ b/drivers/clocksource/Kconfig
>>>> @@ -696,6 +696,17 @@ config INGENIC_TIMER
>>>>        help
>>>>          Support for the timer/counter unit of the Ingenic JZ SoCs.
>>>>    +config INGENIC_SYSOST
>>>> +    bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"
>>> We usually use silent options and let the platform's Kconfig enable it.
>>> We show up the option only when COMPILE_TEST is enabled.
>>>
>>> Is there a reason to do it differently?
>>
>> Do you mean
>>
>> bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"
>>
>> or
>>
>> default MACH_INGENIC ?
> Both, no default here.
>
> eg.
>
> bool "Clocksource/timer using the SYSOST in Ingenic X SoCs" if COMPILE_TEST
>
> and
>
> in arch/mips/Kconfig in the config MACH_INGENIC section :
>
> ...
> select INGENIC_SYSOST
> ...


Sure, I will do it in the next version.


Thanks and best regards!


>> This driver has some origins from "INGENIC_TIMER" driver and
>> "INGENIC_OST" driver.
>> Early Ingenic processors used TCU (timer/counter unit, has 6 or 8
>> generic timer channels) to provide clocksource and clockevent (both with
>> only 16bit precision). This part of the processor can only use
>> "INGENIC_TIMER" driver.
>>
>> Later processors provide an independent 32bit or 64bit timer channel
>> (still under TCU, known as ost channel, this channel can not generate
>> interrupt) to provid higher precision clocksource. The "INGENIC_OST"
>> driver is for this channel. These processors can use "INGENIC_TIMER"
>> driver, but using "INGENIC_OST" driver to provide higher precision
>> clocksource would be a better choice (clockevent still needs to be
>> provided by generic timer channel of TCU, and still 16bit precision).
>>
>> And the recent processors provide a SYSOST components, it is independent
>> from TCU, including a 64bit timer channel for clocksource and a 32bit
>> timer channel for clockevent. Although these processors can also use
>> "INGENIC_TIMER" driver, but the better choice is completely independent
>> use of "INGENIC_SYSOST" driver to provide higher precision clocksource
>> and clockevent.
> Ok, the rating should do the job then.
>
> Thanks for the explanation.
>
>> You may have already noticed that this independent SYSOST component is
>> like an upgraded and streamlined TCU, which only retains one generic
>> timer channel that can generate interrupts, upgrade it from 16bit to
>> 32bit, and then retain the 64bit ost channel. so the driver code and
>> Kconfig code of this patch is largely referenced
>> "INGENIC_TIMER" driver and "INGENIC_OST" driver.
>>
>> Thanks and best regards!
>>
>>>> +    default MACH_INGENIC
>>>> +    depends on MIPS || COMPILE_TEST
>>>> +    depends on COMMON_CLK
>>>> +    select MFD_SYSCON
>>>> +    select TIMER_OF
>>>> +    select IRQ_DOMAIN
>>>> +    help
>>>> +      Support for the SYSOST of the Ingenic X Series SoCs.
>>>> +
>>> [ ... ]
>>>
>>>
>

Re: [PATCH v6 2/2] clocksource: Ingenic: Add support for the Ingenic X1000 OST.

[Paul Cercueil]

Hi Daniel,

Le ven. 17 juil. 2020 à 10:02, Daniel Lezcano 
<daniel.lezcano@linaro.org> a écrit :
> On 17/07/2020 08:13, Zhou Yanjie wrote:
>>  Hi Daniel,
>> 
>>  在 2020/7/17 下午12:20, Daniel Lezcano 写道:
>>>  On 10/07/2020 19:02, 周琰杰 (Zhou Yanjie) wrote:
>>>>  X1000 and SoCs after X1000 (such as X1500 and X1830) had a 
>>>> separate
>>>>  OST, it no longer belongs to TCU. This driver will register both a
>>>>  clocksource and a sched_clock to the system.
>>>> 
>>>>  Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
>>>>  Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>>>>  Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>>>>  Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>>>>  Reviewed-by: Paul Cercueil <paul@crapouillou.net>
>>>>  ---
>>>> 
>>>>  Notes:
>>>>       v1->v2:
>>>>       Fix compile warnings.
>>>>       Reported-by: kernel test robot <lkp@intel.com>
>>>>            v2->v3:
>>>>       No change.
>>>>            v3->v4:
>>>>       1.Rename "ost" to "sysost"
>>>>       1.Remove unrelated changes.
>>>>       2.Remove ost_clock_parent enum.
>>>>       3.Remove ost->percpu_timer_channel/ost->global_timer_channel.
>>>>       4.Set up independent .recalc_rate/.set_rate for percpu/global
>>>>  timer.
>>>>       5.No longer call functions in variable declarations.
>>>>            v4->v5:
>>>>       Use "of_io_request_and_map()" instead "of_iomap()".
>>>>       Suggested-by: Paul Cercueil <paul@crapouillou.net>
>>>>            v5->v6:
>>>>       No change.
>>>> 
>>>>    drivers/clocksource/Kconfig          |  11 +
>>>>    drivers/clocksource/Makefile         |   1 +
>>>>    drivers/clocksource/ingenic-sysost.c | 539
>>>>  +++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 551 insertions(+)
>>>>    create mode 100644 drivers/clocksource/ingenic-sysost.c
>>>> 
>>>>  diff --git a/drivers/clocksource/Kconfig 
>>>> b/drivers/clocksource/Kconfig
>>>>  index 91418381fcd4..1bca8b8fb30f 100644
>>>>  --- a/drivers/clocksource/Kconfig
>>>>  +++ b/drivers/clocksource/Kconfig
>>>>  @@ -696,6 +696,17 @@ config INGENIC_TIMER
>>>>        help
>>>>          Support for the timer/counter unit of the Ingenic JZ SoCs.
>>>>    +config INGENIC_SYSOST
>>>>  +    bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"
>>>  We usually use silent options and let the platform's Kconfig 
>>> enable it.
>>>  We show up the option only when COMPILE_TEST is enabled.
>>> 
>>>  Is there a reason to do it differently?
>> 
>> 
>>  Do you mean
>> 
>>  bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"
>> 
>>  or
>> 
>>  default MACH_INGENIC ?
> 
> Both, no default here.
> 
> eg.
> 
> bool "Clocksource/timer using the SYSOST in Ingenic X SoCs" if 
> COMPILE_TEST
> 
> and
> 
> in arch/mips/Kconfig in the config MACH_INGENIC section :
> 
> ...
> select INGENIC_SYSOST
> ...

Disagreed. That's not how we do things on MIPS. Selecting MACH_INGENIC 
means "this kernel will support Ingenic SoCs", but not that it will 
only support these. Hence the depends on MIPS / default MACH_INGENIC.

As for the select INGENIC_SYSOST, this driver only applies to a few 
SoCs, I certainly don't want it to be force-enabled. I don't even wait 
it to be force-enabled on X1000, since it is optional there too.

Cheers,
-Paul

> 
>>  This driver has some origins from "INGENIC_TIMER" driver and
>>  "INGENIC_OST" driver.
>>  Early Ingenic processors used TCU (timer/counter unit, has 6 or 8
>>  generic timer channels) to provide clocksource and clockevent (both 
>> with
>>  only 16bit precision). This part of the processor can only use
>>  "INGENIC_TIMER" driver.
>> 
>>  Later processors provide an independent 32bit or 64bit timer channel
>>  (still under TCU, known as ost channel, this channel can not 
>> generate
>>  interrupt) to provid higher precision clocksource. The "INGENIC_OST"
>>  driver is for this channel. These processors can use "INGENIC_TIMER"
>>  driver, but using "INGENIC_OST" driver to provide higher precision
>>  clocksource would be a better choice (clockevent still needs to be
>>  provided by generic timer channel of TCU, and still 16bit 
>> precision).
>> 
>>  And the recent processors provide a SYSOST components, it is 
>> independent
>>  from TCU, including a 64bit timer channel for clocksource and a 
>> 32bit
>>  timer channel for clockevent. Although these processors can also use
>>  "INGENIC_TIMER" driver, but the better choice is completely 
>> independent
>>  use of "INGENIC_SYSOST" driver to provide higher precision 
>> clocksource
>>  and clockevent.
> 
> Ok, the rating should do the job then.
> 
> Thanks for the explanation.
> 
>>  You may have already noticed that this independent SYSOST component 
>> is
>>  like an upgraded and streamlined TCU, which only retains one generic
>>  timer channel that can generate interrupts, upgrade it from 16bit to
>>  32bit, and then retain the 64bit ost channel. so the driver code and
>>  Kconfig code of this patch is largely referenced
>>  "INGENIC_TIMER" driver and "INGENIC_OST" driver.
>> 
>>  Thanks and best regards!
>> 
>>>>  +    default MACH_INGENIC
>>>>  +    depends on MIPS || COMPILE_TEST
>>>>  +    depends on COMMON_CLK
>>>>  +    select MFD_SYSCON
>>>>  +    select TIMER_OF
>>>>  +    select IRQ_DOMAIN
>>>>  +    help
>>>>  +      Support for the SYSOST of the Ingenic X Series SoCs.
>>>>  +
>>>  [ ... ]
>>> 
>>> 
> 
> 
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM 
> SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v6 2/2] clocksource: Ingenic: Add support for the Ingenic X1000 OST.

[Zhou Yanjie]

Hello Paul and Daniel,

在 2020/7/18 下午9:12, Paul Cercueil 写道:
> Hi Daniel,
>
> Le ven. 17 juil. 2020 à 10:02, Daniel Lezcano 
> <daniel.lezcano@linaro.org> a écrit :
>> On 17/07/2020 08:13, Zhou Yanjie wrote:
>>>  Hi Daniel,
>>>
>>>  在 2020/7/17 下午12:20, Daniel Lezcano 写道:
>>>>  On 10/07/2020 19:02, 周琰杰 (Zhou Yanjie) wrote:
>>>>>  X1000 and SoCs after X1000 (such as X1500 and X1830) had a separate
>>>>>  OST, it no longer belongs to TCU. This driver will register both a
>>>>>  clocksource and a sched_clock to the system.
>>>>>
>>>>>  Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
>>>>>  Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>>>>>  Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>>>>>  Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>>>>>  Reviewed-by: Paul Cercueil <paul@crapouillou.net>
>>>>>  ---
>>>>>
>>>>>  Notes:
>>>>>       v1->v2:
>>>>>       Fix compile warnings.
>>>>>       Reported-by: kernel test robot <lkp@intel.com>
>>>>>            v2->v3:
>>>>>       No change.
>>>>>            v3->v4:
>>>>>       1.Rename "ost" to "sysost"
>>>>>       1.Remove unrelated changes.
>>>>>       2.Remove ost_clock_parent enum.
>>>>>       3.Remove ost->percpu_timer_channel/ost->global_timer_channel.
>>>>>       4.Set up independent .recalc_rate/.set_rate for percpu/global
>>>>>  timer.
>>>>>       5.No longer call functions in variable declarations.
>>>>>            v4->v5:
>>>>>       Use "of_io_request_and_map()" instead "of_iomap()".
>>>>>       Suggested-by: Paul Cercueil <paul@crapouillou.net>
>>>>>            v5->v6:
>>>>>       No change.
>>>>>
>>>>>    drivers/clocksource/Kconfig          |  11 +
>>>>>    drivers/clocksource/Makefile         |   1 +
>>>>>    drivers/clocksource/ingenic-sysost.c | 539
>>>>>  +++++++++++++++++++++++++++++++++++
>>>>>    3 files changed, 551 insertions(+)
>>>>>    create mode 100644 drivers/clocksource/ingenic-sysost.c
>>>>>
>>>>>  diff --git a/drivers/clocksource/Kconfig 
>>>>> b/drivers/clocksource/Kconfig
>>>>>  index 91418381fcd4..1bca8b8fb30f 100644
>>>>>  --- a/drivers/clocksource/Kconfig
>>>>>  +++ b/drivers/clocksource/Kconfig
>>>>>  @@ -696,6 +696,17 @@ config INGENIC_TIMER
>>>>>        help
>>>>>          Support for the timer/counter unit of the Ingenic JZ SoCs.
>>>>>    +config INGENIC_SYSOST
>>>>>  +    bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"
>>>>  We usually use silent options and let the platform's Kconfig 
>>>> enable it.
>>>>  We show up the option only when COMPILE_TEST is enabled.
>>>>
>>>>  Is there a reason to do it differently?
>>>
>>>
>>>  Do you mean
>>>
>>>  bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"
>>>
>>>  or
>>>
>>>  default MACH_INGENIC ?
>>
>> Both, no default here.
>>
>> eg.
>>
>> bool "Clocksource/timer using the SYSOST in Ingenic X SoCs" if 
>> COMPILE_TEST
>>
>> and
>>
>> in arch/mips/Kconfig in the config MACH_INGENIC section :
>>
>> ...
>> select INGENIC_SYSOST
>> ...
>
> Disagreed. That's not how we do things on MIPS. Selecting MACH_INGENIC 
> means "this kernel will support Ingenic SoCs", but not that it will 
> only support these. Hence the depends on MIPS / default MACH_INGENIC.
>
> As for the select INGENIC_SYSOST, this driver only applies to a few 
> SoCs, I certainly don't want it to be force-enabled. I don't even wait 
> it to be force-enabled on X1000, since it is optional there too.
>
> Cheers,
> -Paul


If we still need to keep the "default MACH_INGENIC", then Daniel can 
directly apply the v6 version.

If we need to use the silent options, maybe we can enable them 
separately according to 
MACH_JZ4740/MACH_JZ4770/MACH_JZ4780/MACH_X1000/MACH_X1830.

In fact, I think X1000 and X1830 need to enable this driver in most 
cases, because the current test has found that use TCU to provide 
clocksource and clockevent will cause data loss/error when transmitting 
data through spi or ethernet. And these errors no longer appear after 
using OST.

Thanks and best regards!


>
>>
>>>  This driver has some origins from "INGENIC_TIMER" driver and
>>>  "INGENIC_OST" driver.
>>>  Early Ingenic processors used TCU (timer/counter unit, has 6 or 8
>>>  generic timer channels) to provide clocksource and clockevent (both 
>>> with
>>>  only 16bit precision). This part of the processor can only use
>>>  "INGENIC_TIMER" driver.
>>>
>>>  Later processors provide an independent 32bit or 64bit timer channel
>>>  (still under TCU, known as ost channel, this channel can not generate
>>>  interrupt) to provid higher precision clocksource. The "INGENIC_OST"
>>>  driver is for this channel. These processors can use "INGENIC_TIMER"
>>>  driver, but using "INGENIC_OST" driver to provide higher precision
>>>  clocksource would be a better choice (clockevent still needs to be
>>>  provided by generic timer channel of TCU, and still 16bit precision).
>>>
>>>  And the recent processors provide a SYSOST components, it is 
>>> independent
>>>  from TCU, including a 64bit timer channel for clocksource and a 32bit
>>>  timer channel for clockevent. Although these processors can also use
>>>  "INGENIC_TIMER" driver, but the better choice is completely 
>>> independent
>>>  use of "INGENIC_SYSOST" driver to provide higher precision clocksource
>>>  and clockevent.
>>
>> Ok, the rating should do the job then.
>>
>> Thanks for the explanation.
>>
>>>  You may have already noticed that this independent SYSOST component is
>>>  like an upgraded and streamlined TCU, which only retains one generic
>>>  timer channel that can generate interrupts, upgrade it from 16bit to
>>>  32bit, and then retain the 64bit ost channel. so the driver code and
>>>  Kconfig code of this patch is largely referenced
>>>  "INGENIC_TIMER" driver and "INGENIC_OST" driver.
>>>
>>>  Thanks and best regards!
>>>
>>>>>  +    default MACH_INGENIC
>>>>>  +    depends on MIPS || COMPILE_TEST
>>>>>  +    depends on COMMON_CLK
>>>>>  +    select MFD_SYSCON
>>>>>  +    select TIMER_OF
>>>>>  +    select IRQ_DOMAIN
>>>>>  +    help
>>>>>  +      Support for the SYSOST of the Ingenic X Series SoCs.
>>>>>  +
>>>>  [ ... ]
>>>>
>>>>
>>
>>
>> -- 
>> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>>
>> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
>> <http://twitter.com/#!/linaroorg> Twitter |
>> <http://www.linaro.org/linaro-blog/> Blog
>

Re: [PATCH v6 2/2] clocksource: Ingenic: Add support for the Ingenic X1000 OST.

[Paul Cercueil]

Hi Zhou,

Le sam. 18 juil. 2020 à 21:42, Zhou Yanjie <zhouyanjie@wanyeetech.com> 
a écrit :
> Hello Paul and Daniel,
> 
> 在 2020/7/18 下午9:12, Paul Cercueil 写道:
>> Hi Daniel,
>> 
>> Le ven. 17 juil. 2020 à 10:02, Daniel Lezcano 
>> \x7f<daniel.lezcano@linaro.org> a écrit :
>>> On 17/07/2020 08:13, Zhou Yanjie wrote:
>>>>  Hi Daniel,
>>>> 
>>>>  在 2020/7/17 下午12:20, Daniel Lezcano 写道:
>>>>>  On 10/07/2020 19:02, 周琰杰 (Zhou Yanjie) wrote:
>>>>>>  X1000 and SoCs after X1000 (such as X1500 and X1830) had a 
>>>>>> separate
>>>>>>  OST, it no longer belongs to TCU. This driver will register 
>>>>>> both a
>>>>>>  clocksource and a sched_clock to the system.
>>>>>> 
>>>>>>  Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
>>>>>>  Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>>>>>>  Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>>>>>>  Signed-off-by: 周琰杰 (Zhou Yanjie) 
>>>>>> <zhouyanjie@wanyeetech.com>
>>>>>>  Reviewed-by: Paul Cercueil <paul@crapouillou.net>
>>>>>>  ---
>>>>>> 
>>>>>>  Notes:
>>>>>>       v1->v2:
>>>>>>       Fix compile warnings.
>>>>>>       Reported-by: kernel test robot <lkp@intel.com>
>>>>>>            v2->v3:
>>>>>>       No change.
>>>>>>            v3->v4:
>>>>>>       1.Rename "ost" to "sysost"
>>>>>>       1.Remove unrelated changes.
>>>>>>       2.Remove ost_clock_parent enum.
>>>>>>       3.Remove 
>>>>>> ost->percpu_timer_channel/ost->global_timer_channel.
>>>>>>       4.Set up independent .recalc_rate/.set_rate for 
>>>>>> percpu/global
>>>>>>  timer.
>>>>>>       5.No longer call functions in variable declarations.
>>>>>>            v4->v5:
>>>>>>       Use "of_io_request_and_map()" instead "of_iomap()".
>>>>>>       Suggested-by: Paul Cercueil <paul@crapouillou.net>
>>>>>>            v5->v6:
>>>>>>       No change.
>>>>>> 
>>>>>>    drivers/clocksource/Kconfig          |  11 +
>>>>>>    drivers/clocksource/Makefile         |   1 +
>>>>>>    drivers/clocksource/ingenic-sysost.c | 539
>>>>>>  +++++++++++++++++++++++++++++++++++
>>>>>>    3 files changed, 551 insertions(+)
>>>>>>    create mode 100644 drivers/clocksource/ingenic-sysost.c
>>>>>> 
>>>>>>  diff --git a/drivers/clocksource/Kconfig 
>>>>>> \x7f\x7f\x7f\x7f\x7fb/drivers/clocksource/Kconfig
>>>>>>  index 91418381fcd4..1bca8b8fb30f 100644
>>>>>>  --- a/drivers/clocksource/Kconfig
>>>>>>  +++ b/drivers/clocksource/Kconfig
>>>>>>  @@ -696,6 +696,17 @@ config INGENIC_TIMER
>>>>>>        help
>>>>>>          Support for the timer/counter unit of the Ingenic JZ 
>>>>>> SoCs.
>>>>>>    +config INGENIC_SYSOST
>>>>>>  +    bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"
>>>>>  We usually use silent options and let the platform's Kconfig 
>>>>> \x7f\x7f\x7f\x7fenable it.
>>>>>  We show up the option only when COMPILE_TEST is enabled.
>>>>> 
>>>>>  Is there a reason to do it differently?
>>>> 
>>>> 
>>>>  Do you mean
>>>> 
>>>>  bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"
>>>> 
>>>>  or
>>>> 
>>>>  default MACH_INGENIC ?
>>> 
>>> Both, no default here.
>>> 
>>> eg.
>>> 
>>> bool "Clocksource/timer using the SYSOST in Ingenic X SoCs" if 
>>> \x7f\x7fCOMPILE_TEST
>>> 
>>> and
>>> 
>>> in arch/mips/Kconfig in the config MACH_INGENIC section :
>>> 
>>> ...
>>> select INGENIC_SYSOST
>>> ...
>> 
>> Disagreed. That's not how we do things on MIPS. Selecting 
>> MACH_INGENIC \x7fmeans "this kernel will support Ingenic SoCs", but not 
>> that it will \x7fonly support these. Hence the depends on MIPS / 
>> default MACH_INGENIC.
>> 
>> As for the select INGENIC_SYSOST, this driver only applies to a few 
>> \x7fSoCs, I certainly don't want it to be force-enabled. I don't even 
>> wait \x7fit to be force-enabled on X1000, since it is optional there 
>> too.
>> 
>> Cheers,
>> -Paul
> 
> 
> If we still need to keep the "default MACH_INGENIC", then Daniel can 
> directly apply the v6 version.
> 
> If we need to use the silent options, maybe we can enable them 
> separately according to 
> MACH_JZ4740/MACH_JZ4770/MACH_JZ4780/MACH_X1000/MACH_X1830.
> 
> In fact, I think X1000 and X1830 need to enable this driver in most 
> cases, because the current test has found that use TCU to provide 
> clocksource and clockevent will cause data loss/error when 
> transmitting data through spi or ethernet. And these errors no longer 
> appear after using OST.

This is likely because the clock is too fast, try to reduce it by a 
factor 2 or 4, it should behave better.

If it turns out OST is really needed, then it should be selected from 
MACH_X1000/X1830, not MACH_INGENIC.

Cheers,
-Paul

> Thanks and best regards!
> 
> 
>> 
>>> 
>>>>  This driver has some origins from "INGENIC_TIMER" driver and
>>>>  "INGENIC_OST" driver.
>>>>  Early Ingenic processors used TCU (timer/counter unit, has 6 or 8
>>>>  generic timer channels) to provide clocksource and clockevent 
>>>> (both \x7f\x7f\x7fwith
>>>>  only 16bit precision). This part of the processor can only use
>>>>  "INGENIC_TIMER" driver.
>>>> 
>>>>  Later processors provide an independent 32bit or 64bit timer 
>>>> channel
>>>>  (still under TCU, known as ost channel, this channel can not 
>>>> generate
>>>>  interrupt) to provid higher precision clocksource. The 
>>>> "INGENIC_OST"
>>>>  driver is for this channel. These processors can use 
>>>> "INGENIC_TIMER"
>>>>  driver, but using "INGENIC_OST" driver to provide higher precision
>>>>  clocksource would be a better choice (clockevent still needs to be
>>>>  provided by generic timer channel of TCU, and still 16bit 
>>>> precision).
>>>> 
>>>>  And the recent processors provide a SYSOST components, it is 
>>>> \x7f\x7f\x7findependent
>>>>  from TCU, including a 64bit timer channel for clocksource and a 
>>>> 32bit
>>>>  timer channel for clockevent. Although these processors can also 
>>>> use
>>>>  "INGENIC_TIMER" driver, but the better choice is completely 
>>>> \x7f\x7f\x7findependent
>>>>  use of "INGENIC_SYSOST" driver to provide higher precision 
>>>> clocksource
>>>>  and clockevent.
>>> 
>>> Ok, the rating should do the job then.
>>> 
>>> Thanks for the explanation.
>>> 
>>>>  You may have already noticed that this independent SYSOST 
>>>> component is
>>>>  like an upgraded and streamlined TCU, which only retains one 
>>>> generic
>>>>  timer channel that can generate interrupts, upgrade it from 16bit 
>>>> to
>>>>  32bit, and then retain the 64bit ost channel. so the driver code 
>>>> and
>>>>  Kconfig code of this patch is largely referenced
>>>>  "INGENIC_TIMER" driver and "INGENIC_OST" driver.
>>>> 
>>>>  Thanks and best regards!
>>>> 
>>>>>>  +    default MACH_INGENIC
>>>>>>  +    depends on MIPS || COMPILE_TEST
>>>>>>  +    depends on COMMON_CLK
>>>>>>  +    select MFD_SYSCON
>>>>>>  +    select TIMER_OF
>>>>>>  +    select IRQ_DOMAIN
>>>>>>  +    help
>>>>>>  +      Support for the SYSOST of the Ingenic X Series SoCs.
>>>>>>  +
>>>>>  [ ... ]
>>>>> 
>>>>> 
>>> 
>>> 
>>> --
>>> <http://www.linaro.org/> Linaro.org │ Open source software for 
>>> ARM SoCs
>>> 
>>> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
>>> <http://twitter.com/#!/linaroorg> Twitter |
>>> <http://www.linaro.org/linaro-blog/> Blog
>> 

Re: [PATCH v6 2/2] clocksource: Ingenic: Add support for the Ingenic X1000 OST.

[Zhou Yanjie]

Hi Paul,

在 2020/7/18 下午11:44, Paul Cercueil 写道:
> Hi Zhou,
>
> Le sam. 18 juil. 2020 à 21:42, Zhou Yanjie <zhouyanjie@wanyeetech.com> 
> a écrit :
>> Hello Paul and Daniel,
>>
>> 在 2020/7/18 下午9:12, Paul Cercueil 写道:
>>> Hi Daniel,
>>>
>>> Le ven. 17 juil. 2020 à 10:02, Daniel Lezcano 
>>> \x7f<daniel.lezcano@linaro.org> a écrit :
>>>> On 17/07/2020 08:13, Zhou Yanjie wrote:
>>>>>  Hi Daniel,
>>>>>
>>>>>  在 2020/7/17 下午12:20, Daniel Lezcano 写道:
>>>>>>  On 10/07/2020 19:02, 周琰杰 (Zhou Yanjie) wrote:
>>>>>>>  X1000 and SoCs after X1000 (such as X1500 and X1830) had a 
>>>>>>> separate
>>>>>>>  OST, it no longer belongs to TCU. This driver will register both a
>>>>>>>  clocksource and a sched_clock to the system.
>>>>>>>
>>>>>>>  Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
>>>>>>>  Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>>>>>>>  Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>>>>>>>  Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>>>>>>>  Reviewed-by: Paul Cercueil <paul@crapouillou.net>
>>>>>>>  ---
>>>>>>>
>>>>>>>  Notes:
>>>>>>>       v1->v2:
>>>>>>>       Fix compile warnings.
>>>>>>>       Reported-by: kernel test robot <lkp@intel.com>
>>>>>>>            v2->v3:
>>>>>>>       No change.
>>>>>>>            v3->v4:
>>>>>>>       1.Rename "ost" to "sysost"
>>>>>>>       1.Remove unrelated changes.
>>>>>>>       2.Remove ost_clock_parent enum.
>>>>>>>       3.Remove ost->percpu_timer_channel/ost->global_timer_channel.
>>>>>>>       4.Set up independent .recalc_rate/.set_rate for percpu/global
>>>>>>>  timer.
>>>>>>>       5.No longer call functions in variable declarations.
>>>>>>>            v4->v5:
>>>>>>>       Use "of_io_request_and_map()" instead "of_iomap()".
>>>>>>>       Suggested-by: Paul Cercueil <paul@crapouillou.net>
>>>>>>>            v5->v6:
>>>>>>>       No change.
>>>>>>>
>>>>>>>    drivers/clocksource/Kconfig          |  11 +
>>>>>>>    drivers/clocksource/Makefile         |   1 +
>>>>>>>    drivers/clocksource/ingenic-sysost.c | 539
>>>>>>>  +++++++++++++++++++++++++++++++++++
>>>>>>>    3 files changed, 551 insertions(+)
>>>>>>>    create mode 100644 drivers/clocksource/ingenic-sysost.c
>>>>>>>
>>>>>>>  diff --git a/drivers/clocksource/Kconfig 
>>>>>>> \x7f\x7f\x7f\x7f\x7fb/drivers/clocksource/Kconfig
>>>>>>>  index 91418381fcd4..1bca8b8fb30f 100644
>>>>>>>  --- a/drivers/clocksource/Kconfig
>>>>>>>  +++ b/drivers/clocksource/Kconfig
>>>>>>>  @@ -696,6 +696,17 @@ config INGENIC_TIMER
>>>>>>>        help
>>>>>>>          Support for the timer/counter unit of the Ingenic JZ SoCs.
>>>>>>>    +config INGENIC_SYSOST
>>>>>>>  +    bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"
>>>>>>  We usually use silent options and let the platform's Kconfig 
>>>>>> \x7f\x7f\x7f\x7fenable it.
>>>>>>  We show up the option only when COMPILE_TEST is enabled.
>>>>>>
>>>>>>  Is there a reason to do it differently?
>>>>>
>>>>>
>>>>>  Do you mean
>>>>>
>>>>>  bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"
>>>>>
>>>>>  or
>>>>>
>>>>>  default MACH_INGENIC ?
>>>>
>>>> Both, no default here.
>>>>
>>>> eg.
>>>>
>>>> bool "Clocksource/timer using the SYSOST in Ingenic X SoCs" if 
>>>> \x7f\x7fCOMPILE_TEST
>>>>
>>>> and
>>>>
>>>> in arch/mips/Kconfig in the config MACH_INGENIC section :
>>>>
>>>> ...
>>>> select INGENIC_SYSOST
>>>> ...
>>>
>>> Disagreed. That's not how we do things on MIPS. Selecting 
>>> MACH_INGENIC \x7fmeans "this kernel will support Ingenic SoCs", but not 
>>> that it will \x7fonly support these. Hence the depends on MIPS / 
>>> default MACH_INGENIC.
>>>
>>> As for the select INGENIC_SYSOST, this driver only applies to a few 
>>> \x7fSoCs, I certainly don't want it to be force-enabled. I don't even 
>>> wait \x7fit to be force-enabled on X1000, since it is optional there too.
>>>
>>> Cheers,
>>> -Paul
>>
>>
>> If we still need to keep the "default MACH_INGENIC", then Daniel can 
>> directly apply the v6 version.
>>
>> If we need to use the silent options, maybe we can enable them 
>> separately according to 
>> MACH_JZ4740/MACH_JZ4770/MACH_JZ4780/MACH_X1000/MACH_X1830.
>>
>> In fact, I think X1000 and X1830 need to enable this driver in most 
>> cases, because the current test has found that use TCU to provide 
>> clocksource and clockevent will cause data loss/error when 
>> transmitting data through spi or ethernet. And these errors no longer 
>> appear after using OST.
>
> This is likely because the clock is too fast, try to reduce it by a 
> factor 2 or 4, it should behave better.
>

Yes, it is indeed because the clock is too fast, especially the TCU only 
has 16-bit, which makes this problem more serious. I even reduced the 
timing clock to the lowest possible 23kHz (divided by 1024). The problem 
has been alleviated, but it cannot completely solved, and finally there 
is no problem after opening the OST.


> If it turns out OST is really needed, then it should be selected from 
> MACH_X1000/X1830, not MACH_INGENIC.
>

Sure, I will do it in v8.

Thanks and best regards!


> Cheers,
> -Paul
>
>> Thanks and best regards!
>>
>>
>>>
>>>>
>>>>>  This driver has some origins from "INGENIC_TIMER" driver and
>>>>>  "INGENIC_OST" driver.
>>>>>  Early Ingenic processors used TCU (timer/counter unit, has 6 or 8
>>>>>  generic timer channels) to provide clocksource and clockevent 
>>>>> (both \x7f\x7f\x7fwith
>>>>>  only 16bit precision). This part of the processor can only use
>>>>>  "INGENIC_TIMER" driver.
>>>>>
>>>>>  Later processors provide an independent 32bit or 64bit timer channel
>>>>>  (still under TCU, known as ost channel, this channel can not 
>>>>> generate
>>>>>  interrupt) to provid higher precision clocksource. The "INGENIC_OST"
>>>>>  driver is for this channel. These processors can use "INGENIC_TIMER"
>>>>>  driver, but using "INGENIC_OST" driver to provide higher precision
>>>>>  clocksource would be a better choice (clockevent still needs to be
>>>>>  provided by generic timer channel of TCU, and still 16bit 
>>>>> precision).
>>>>>
>>>>>  And the recent processors provide a SYSOST components, it is 
>>>>> \x7f\x7f\x7findependent
>>>>>  from TCU, including a 64bit timer channel for clocksource and a 
>>>>> 32bit
>>>>>  timer channel for clockevent. Although these processors can also use
>>>>>  "INGENIC_TIMER" driver, but the better choice is completely 
>>>>> \x7f\x7f\x7findependent
>>>>>  use of "INGENIC_SYSOST" driver to provide higher precision 
>>>>> clocksource
>>>>>  and clockevent.
>>>>
>>>> Ok, the rating should do the job then.
>>>>
>>>> Thanks for the explanation.
>>>>
>>>>>  You may have already noticed that this independent SYSOST 
>>>>> component is
>>>>>  like an upgraded and streamlined TCU, which only retains one generic
>>>>>  timer channel that can generate interrupts, upgrade it from 16bit to
>>>>>  32bit, and then retain the 64bit ost channel. so the driver code and
>>>>>  Kconfig code of this patch is largely referenced
>>>>>  "INGENIC_TIMER" driver and "INGENIC_OST" driver.
>>>>>
>>>>>  Thanks and best regards!
>>>>>
>>>>>>>  +    default MACH_INGENIC
>>>>>>>  +    depends on MIPS || COMPILE_TEST
>>>>>>>  +    depends on COMMON_CLK
>>>>>>>  +    select MFD_SYSCON
>>>>>>>  +    select TIMER_OF
>>>>>>>  +    select IRQ_DOMAIN
>>>>>>>  +    help
>>>>>>>  +      Support for the SYSOST of the Ingenic X Series SoCs.
>>>>>>>  +
>>>>>>  [ ... ]
>>>>>>
>>>>>>
>>>>
>>>>
>>>> -- 
>>>> <http://www.linaro.org/> Linaro.org │ Open source software for ARM 
>>>> SoCs
>>>>
>>>> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
>>>> <http://twitter.com/#!/linaroorg> Twitter |
>>>> <http://www.linaro.org/linaro-blog/> Blog
>>>
>

Re: [PATCH v6 2/2] clocksource: Ingenic: Add support for the Ingenic X1000 OST.

[Paul Cercueil]

Le sam. 18 juil. 2020 à 23:55, Zhou Yanjie <zhouyanjie@wanyeetech.com> 
a écrit :
> Hi Paul,
> 
> 在 2020/7/18 下午11:44, Paul Cercueil 写道:
>> Hi Zhou,
>> 
>> Le sam. 18 juil. 2020 à 21:42, Zhou Yanjie 
>> <zhouyanjie@wanyeetech.com> \x7fa écrit :
>>> Hello Paul and Daniel,
>>> 
>>> 在 2020/7/18 下午9:12, Paul Cercueil 写道:
>>>> Hi Daniel,
>>>> 
>>>> Le ven. 17 juil. 2020 à 10:02, Daniel Lezcano 
>>>> \x7f\x7f\x7f\x7f<daniel.lezcano@linaro.org> a écrit :
>>>>> On 17/07/2020 08:13, Zhou Yanjie wrote:
>>>>>>  Hi Daniel,
>>>>>> 
>>>>>>  在 2020/7/17 下午12:20, Daniel Lezcano 写道:
>>>>>>>  On 10/07/2020 19:02, 周琰杰 (Zhou Yanjie) wrote:
>>>>>>>>  X1000 and SoCs after X1000 (such as X1500 and X1830) had a 
>>>>>>>> \x7f\x7f\x7f\x7f\x7f\x7f\x7fseparate
>>>>>>>>  OST, it no longer belongs to TCU. This driver will register 
>>>>>>>> both a
>>>>>>>>  clocksource and a sched_clock to the system.
>>>>>>>> 
>>>>>>>>  Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
>>>>>>>>  Co-developed-by: 漆鹏振 (Qi Pengzhen) 
>>>>>>>> <aric.pzqi@ingenic.com>
>>>>>>>>  Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>>>>>>>>  Signed-off-by: 周琰杰 (Zhou Yanjie) 
>>>>>>>> <zhouyanjie@wanyeetech.com>
>>>>>>>>  Reviewed-by: Paul Cercueil <paul@crapouillou.net>
>>>>>>>>  ---
>>>>>>>> 
>>>>>>>>  Notes:
>>>>>>>>       v1->v2:
>>>>>>>>       Fix compile warnings.
>>>>>>>>       Reported-by: kernel test robot <lkp@intel.com>
>>>>>>>>            v2->v3:
>>>>>>>>       No change.
>>>>>>>>            v3->v4:
>>>>>>>>       1.Rename "ost" to "sysost"
>>>>>>>>       1.Remove unrelated changes.
>>>>>>>>       2.Remove ost_clock_parent enum.
>>>>>>>>       3.Remove 
>>>>>>>> ost->percpu_timer_channel/ost->global_timer_channel.
>>>>>>>>       4.Set up independent .recalc_rate/.set_rate for 
>>>>>>>> percpu/global
>>>>>>>>  timer.
>>>>>>>>       5.No longer call functions in variable declarations.
>>>>>>>>            v4->v5:
>>>>>>>>       Use "of_io_request_and_map()" instead "of_iomap()".
>>>>>>>>       Suggested-by: Paul Cercueil <paul@crapouillou.net>
>>>>>>>>            v5->v6:
>>>>>>>>       No change.
>>>>>>>> 
>>>>>>>>    drivers/clocksource/Kconfig          |  11 +
>>>>>>>>    drivers/clocksource/Makefile         |   1 +
>>>>>>>>    drivers/clocksource/ingenic-sysost.c | 539
>>>>>>>>  +++++++++++++++++++++++++++++++++++
>>>>>>>>    3 files changed, 551 insertions(+)
>>>>>>>>    create mode 100644 drivers/clocksource/ingenic-sysost.c
>>>>>>>> 
>>>>>>>>  diff --git a/drivers/clocksource/Kconfig 
>>>>>>>> \x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7fb/drivers/clocksource/Kconfig
>>>>>>>>  index 91418381fcd4..1bca8b8fb30f 100644
>>>>>>>>  --- a/drivers/clocksource/Kconfig
>>>>>>>>  +++ b/drivers/clocksource/Kconfig
>>>>>>>>  @@ -696,6 +696,17 @@ config INGENIC_TIMER
>>>>>>>>        help
>>>>>>>>          Support for the timer/counter unit of the Ingenic JZ 
>>>>>>>> SoCs.
>>>>>>>>    +config INGENIC_SYSOST
>>>>>>>>  +    bool "Clocksource/timer using the SYSOST in Ingenic X 
>>>>>>>> SoCs"
>>>>>>>  We usually use silent options and let the platform's Kconfig 
>>>>>>> \x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7fenable it.
>>>>>>>  We show up the option only when COMPILE_TEST is enabled.
>>>>>>> 
>>>>>>>  Is there a reason to do it differently?
>>>>>> 
>>>>>> 
>>>>>>  Do you mean
>>>>>> 
>>>>>>  bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"
>>>>>> 
>>>>>>  or
>>>>>> 
>>>>>>  default MACH_INGENIC ?
>>>>> 
>>>>> Both, no default here.
>>>>> 
>>>>> eg.
>>>>> 
>>>>> bool "Clocksource/timer using the SYSOST in Ingenic X SoCs" if 
>>>>> \x7f\x7f\x7f\x7f\x7f\x7fCOMPILE_TEST
>>>>> 
>>>>> and
>>>>> 
>>>>> in arch/mips/Kconfig in the config MACH_INGENIC section :
>>>>> 
>>>>> ...
>>>>> select INGENIC_SYSOST
>>>>> ...
>>>> 
>>>> Disagreed. That's not how we do things on MIPS. Selecting 
>>>> \x7f\x7f\x7fMACH_INGENIC \x7fmeans "this kernel will support Ingenic SoCs", 
>>>> but not \x7f\x7f\x7fthat it will \x7fonly support these. Hence the depends on 
>>>> MIPS / \x7f\x7f\x7fdefault MACH_INGENIC.
>>>> 
>>>> As for the select INGENIC_SYSOST, this driver only applies to a 
>>>> few \x7f\x7f\x7f\x7fSoCs, I certainly don't want it to be force-enabled. I 
>>>> don't even \x7f\x7f\x7fwait \x7fit to be force-enabled on X1000, since it is 
>>>> optional there too.
>>>> 
>>>> Cheers,
>>>> -Paul
>>> 
>>> 
>>> If we still need to keep the "default MACH_INGENIC", then Daniel 
>>> can \x7f\x7fdirectly apply the v6 version.
>>> 
>>> If we need to use the silent options, maybe we can enable them 
>>> \x7f\x7fseparately according to 
>>> \x7f\x7fMACH_JZ4740/MACH_JZ4770/MACH_JZ4780/MACH_X1000/MACH_X1830.
>>> 
>>> In fact, I think X1000 and X1830 need to enable this driver in most 
>>> \x7f\x7fcases, because the current test has found that use TCU to provide 
>>> \x7f\x7fclocksource and clockevent will cause data loss/error when 
>>> \x7f\x7ftransmitting data through spi or ethernet. And these errors no 
>>> longer \x7f\x7fappear after using OST.
>> 
>> This is likely because the clock is too fast, try to reduce it by a 
>> \x7ffactor 2 or 4, it should behave better.
>> 
> 
> Yes, it is indeed because the clock is too fast, especially the TCU 
> only has 16-bit, which makes this problem more serious. I even 
> reduced the timing clock to the lowest possible 23kHz (divided by 
> 1024). The problem has been alleviated, but it cannot completely 
> solved, and finally there is no problem after opening the OST.
> 
> 
>> If it turns out OST is really needed, then it should be selected 
>> from \x7fMACH_X1000/X1830, not MACH_INGENIC.
>> 
> 
> Sure, I will do it in v8.
> 
> Thanks and best regards!

Please do it in a separate patch then (but in the same patchset).

Cheers,
-Paul

> 
>> Cheers,
>> -Paul
>> 
>>> Thanks and best regards!
>>> 
>>> 
>>>> 
>>>>> 
>>>>>>  This driver has some origins from "INGENIC_TIMER" driver and
>>>>>>  "INGENIC_OST" driver.
>>>>>>  Early Ingenic processors used TCU (timer/counter unit, has 6 or 
>>>>>> 8
>>>>>>  generic timer channels) to provide clocksource and clockevent 
>>>>>> \x7f\x7f\x7f\x7f\x7f(both \x7f\x7f\x7fwith
>>>>>>  only 16bit precision). This part of the processor can only use
>>>>>>  "INGENIC_TIMER" driver.
>>>>>> 
>>>>>>  Later processors provide an independent 32bit or 64bit timer 
>>>>>> channel
>>>>>>  (still under TCU, known as ost channel, this channel can not 
>>>>>> \x7f\x7f\x7f\x7f\x7fgenerate
>>>>>>  interrupt) to provid higher precision clocksource. The 
>>>>>> "INGENIC_OST"
>>>>>>  driver is for this channel. These processors can use 
>>>>>> "INGENIC_TIMER"
>>>>>>  driver, but using "INGENIC_OST" driver to provide higher 
>>>>>> precision
>>>>>>  clocksource would be a better choice (clockevent still needs to 
>>>>>> be
>>>>>>  provided by generic timer channel of TCU, and still 16bit 
>>>>>> \x7f\x7f\x7f\x7f\x7fprecision).
>>>>>> 
>>>>>>  And the recent processors provide a SYSOST components, it is 
>>>>>> \x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7findependent
>>>>>>  from TCU, including a 64bit timer channel for clocksource and a 
>>>>>> \x7f\x7f\x7f\x7f\x7f32bit
>>>>>>  timer channel for clockevent. Although these processors can 
>>>>>> also use
>>>>>>  "INGENIC_TIMER" driver, but the better choice is completely 
>>>>>> \x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7findependent
>>>>>>  use of "INGENIC_SYSOST" driver to provide higher precision 
>>>>>> \x7f\x7f\x7f\x7f\x7fclocksource
>>>>>>  and clockevent.
>>>>> 
>>>>> Ok, the rating should do the job then.
>>>>> 
>>>>> Thanks for the explanation.
>>>>> 
>>>>>>  You may have already noticed that this independent SYSOST 
>>>>>> \x7f\x7f\x7f\x7f\x7fcomponent is
>>>>>>  like an upgraded and streamlined TCU, which only retains one 
>>>>>> generic
>>>>>>  timer channel that can generate interrupts, upgrade it from 
>>>>>> 16bit to
>>>>>>  32bit, and then retain the 64bit ost channel. so the driver 
>>>>>> code and
>>>>>>  Kconfig code of this patch is largely referenced
>>>>>>  "INGENIC_TIMER" driver and "INGENIC_OST" driver.
>>>>>> 
>>>>>>  Thanks and best regards!
>>>>>> 
>>>>>>>>  +    default MACH_INGENIC
>>>>>>>>  +    depends on MIPS || COMPILE_TEST
>>>>>>>>  +    depends on COMMON_CLK
>>>>>>>>  +    select MFD_SYSCON
>>>>>>>>  +    select TIMER_OF
>>>>>>>>  +    select IRQ_DOMAIN
>>>>>>>>  +    help
>>>>>>>>  +      Support for the SYSOST of the Ingenic X Series SoCs.
>>>>>>>>  +
>>>>>>>  [ ... ]
>>>>>>> 
>>>>>>> 
>>>>> 
>>>>> 
>>>>> --
>>>>> <http://www.linaro.org/> Linaro.org │ Open source software for 
>>>>> ARM \x7f\x7f\x7f\x7fSoCs
>>>>> 
>>>>> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
>>>>> <http://twitter.com/#!/linaroorg> Twitter |
>>>>> <http://www.linaro.org/linaro-blog/> Blog
>>>> 
>> 

Re: [PATCH v6 2/2] clocksource: Ingenic: Add support for the Ingenic X1000 OST.

[Zhou Yanjie]

在 2020/7/19 上午12:11, Paul Cercueil 写道:
>
>
> Le sam. 18 juil. 2020 à 23:55, Zhou Yanjie <zhouyanjie@wanyeetech.com> 
> a écrit :
>> Hi Paul,
>>
>> 在 2020/7/18 下午11:44, Paul Cercueil 写道:
>>> Hi Zhou,
>>>
>>> Le sam. 18 juil. 2020 à 21:42, Zhou Yanjie 
>>> <zhouyanjie@wanyeetech.com> \x7fa écrit :
>>>> Hello Paul and Daniel,
>>>>
>>>> 在 2020/7/18 下午9:12, Paul Cercueil 写道:
>>>>> Hi Daniel,
>>>>>
>>>>> Le ven. 17 juil. 2020 à 10:02, Daniel Lezcano 
>>>>> \x7f\x7f\x7f\x7f<daniel.lezcano@linaro.org> a écrit :
>>>>>> On 17/07/2020 08:13, Zhou Yanjie wrote:
>>>>>>>  Hi Daniel,
>>>>>>>
>>>>>>>  在 2020/7/17 下午12:20, Daniel Lezcano 写道:
>>>>>>>>  On 10/07/2020 19:02, 周琰杰 (Zhou Yanjie) wrote:
>>>>>>>>>  X1000 and SoCs after X1000 (such as X1500 and X1830) had a 
>>>>>>>>> \x7f\x7f\x7f\x7f\x7f\x7f\x7fseparate
>>>>>>>>>  OST, it no longer belongs to TCU. This driver will register 
>>>>>>>>> both a
>>>>>>>>>  clocksource and a sched_clock to the system.
>>>>>>>>>
>>>>>>>>>  Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
>>>>>>>>>  Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>>>>>>>>>  Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>
>>>>>>>>>  Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>>>>>>>>>  Reviewed-by: Paul Cercueil <paul@crapouillou.net>
>>>>>>>>>  ---
>>>>>>>>>
>>>>>>>>>  Notes:
>>>>>>>>>       v1->v2:
>>>>>>>>>       Fix compile warnings.
>>>>>>>>>       Reported-by: kernel test robot <lkp@intel.com>
>>>>>>>>>            v2->v3:
>>>>>>>>>       No change.
>>>>>>>>>            v3->v4:
>>>>>>>>>       1.Rename "ost" to "sysost"
>>>>>>>>>       1.Remove unrelated changes.
>>>>>>>>>       2.Remove ost_clock_parent enum.
>>>>>>>>>       3.Remove 
>>>>>>>>> ost->percpu_timer_channel/ost->global_timer_channel.
>>>>>>>>>       4.Set up independent .recalc_rate/.set_rate for 
>>>>>>>>> percpu/global
>>>>>>>>>  timer.
>>>>>>>>>       5.No longer call functions in variable declarations.
>>>>>>>>>            v4->v5:
>>>>>>>>>       Use "of_io_request_and_map()" instead "of_iomap()".
>>>>>>>>>       Suggested-by: Paul Cercueil <paul@crapouillou.net>
>>>>>>>>>            v5->v6:
>>>>>>>>>       No change.
>>>>>>>>>
>>>>>>>>>    drivers/clocksource/Kconfig          |  11 +
>>>>>>>>>    drivers/clocksource/Makefile         |   1 +
>>>>>>>>>    drivers/clocksource/ingenic-sysost.c | 539
>>>>>>>>>  +++++++++++++++++++++++++++++++++++
>>>>>>>>>    3 files changed, 551 insertions(+)
>>>>>>>>>    create mode 100644 drivers/clocksource/ingenic-sysost.c
>>>>>>>>>
>>>>>>>>>  diff --git a/drivers/clocksource/Kconfig 
>>>>>>>>> \x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7fb/drivers/clocksource/Kconfig
>>>>>>>>>  index 91418381fcd4..1bca8b8fb30f 100644
>>>>>>>>>  --- a/drivers/clocksource/Kconfig
>>>>>>>>>  +++ b/drivers/clocksource/Kconfig
>>>>>>>>>  @@ -696,6 +696,17 @@ config INGENIC_TIMER
>>>>>>>>>        help
>>>>>>>>>          Support for the timer/counter unit of the Ingenic JZ 
>>>>>>>>> SoCs.
>>>>>>>>>    +config INGENIC_SYSOST
>>>>>>>>>  +    bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"
>>>>>>>>  We usually use silent options and let the platform's Kconfig 
>>>>>>>> \x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7fenable it.
>>>>>>>>  We show up the option only when COMPILE_TEST is enabled.
>>>>>>>>
>>>>>>>>  Is there a reason to do it differently?
>>>>>>>
>>>>>>>
>>>>>>>  Do you mean
>>>>>>>
>>>>>>>  bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"
>>>>>>>
>>>>>>>  or
>>>>>>>
>>>>>>>  default MACH_INGENIC ?
>>>>>>
>>>>>> Both, no default here.
>>>>>>
>>>>>> eg.
>>>>>>
>>>>>> bool "Clocksource/timer using the SYSOST in Ingenic X SoCs" if 
>>>>>> \x7f\x7f\x7f\x7f\x7f\x7fCOMPILE_TEST
>>>>>>
>>>>>> and
>>>>>>
>>>>>> in arch/mips/Kconfig in the config MACH_INGENIC section :
>>>>>>
>>>>>> ...
>>>>>> select INGENIC_SYSOST
>>>>>> ...
>>>>>
>>>>> Disagreed. That's not how we do things on MIPS. Selecting 
>>>>> \x7f\x7f\x7fMACH_INGENIC \x7fmeans "this kernel will support Ingenic SoCs", 
>>>>> but not \x7f\x7f\x7fthat it will \x7fonly support these. Hence the depends on 
>>>>> MIPS / \x7f\x7f\x7fdefault MACH_INGENIC.
>>>>>
>>>>> As for the select INGENIC_SYSOST, this driver only applies to a 
>>>>> few \x7f\x7f\x7f\x7fSoCs, I certainly don't want it to be force-enabled. I 
>>>>> don't even \x7f\x7f\x7fwait \x7fit to be force-enabled on X1000, since it is 
>>>>> optional there too.
>>>>>
>>>>> Cheers,
>>>>> -Paul
>>>>
>>>>
>>>> If we still need to keep the "default MACH_INGENIC", then Daniel 
>>>> can \x7f\x7fdirectly apply the v6 version.
>>>>
>>>> If we need to use the silent options, maybe we can enable them 
>>>> \x7f\x7fseparately according to 
>>>> \x7f\x7fMACH_JZ4740/MACH_JZ4770/MACH_JZ4780/MACH_X1000/MACH_X1830.
>>>>
>>>> In fact, I think X1000 and X1830 need to enable this driver in most 
>>>> \x7f\x7fcases, because the current test has found that use TCU to provide 
>>>> \x7f\x7fclocksource and clockevent will cause data loss/error when 
>>>> \x7f\x7ftransmitting data through spi or ethernet. And these errors no 
>>>> longer \x7f\x7fappear after using OST.
>>>
>>> This is likely because the clock is too fast, try to reduce it by a 
>>> \x7ffactor 2 or 4, it should behave better.
>>>
>>
>> Yes, it is indeed because the clock is too fast, especially the TCU 
>> only has 16-bit, which makes this problem more serious. I even 
>> reduced the timing clock to the lowest possible 23kHz (divided by 
>> 1024). The problem has been alleviated, but it cannot completely 
>> solved, and finally there is no problem after opening the OST.
>>
>>
>>> If it turns out OST is really needed, then it should be selected 
>>> from \x7fMACH_X1000/X1830, not MACH_INGENIC.
>>>
>>
>> Sure, I will do it in v8.
>>
>> Thanks and best regards!
>
> Please do it in a separate patch then (but in the same patchset).
>

Okay, I will remove "default MACH_INGENIC" first, and then enable OST later.


> Cheers,
> -Paul
>
>>
>>> Cheers,
>>> -Paul
>>>
>>>> Thanks and best regards!
>>>>
>>>>
>>>>>
>>>>>>
>>>>>>>  This driver has some origins from "INGENIC_TIMER" driver and
>>>>>>>  "INGENIC_OST" driver.
>>>>>>>  Early Ingenic processors used TCU (timer/counter unit, has 6 or 8
>>>>>>>  generic timer channels) to provide clocksource and clockevent 
>>>>>>> \x7f\x7f\x7f\x7f\x7f(both \x7f\x7f\x7fwith
>>>>>>>  only 16bit precision). This part of the processor can only use
>>>>>>>  "INGENIC_TIMER" driver.
>>>>>>>
>>>>>>>  Later processors provide an independent 32bit or 64bit timer 
>>>>>>> channel
>>>>>>>  (still under TCU, known as ost channel, this channel can not 
>>>>>>> \x7f\x7f\x7f\x7f\x7fgenerate
>>>>>>>  interrupt) to provid higher precision clocksource. The 
>>>>>>> "INGENIC_OST"
>>>>>>>  driver is for this channel. These processors can use 
>>>>>>> "INGENIC_TIMER"
>>>>>>>  driver, but using "INGENIC_OST" driver to provide higher precision
>>>>>>>  clocksource would be a better choice (clockevent still needs to be
>>>>>>>  provided by generic timer channel of TCU, and still 16bit 
>>>>>>> \x7f\x7f\x7f\x7f\x7fprecision).
>>>>>>>
>>>>>>>  And the recent processors provide a SYSOST components, it is 
>>>>>>> \x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7findependent
>>>>>>>  from TCU, including a 64bit timer channel for clocksource and a 
>>>>>>> \x7f\x7f\x7f\x7f\x7f32bit
>>>>>>>  timer channel for clockevent. Although these processors can 
>>>>>>> also use
>>>>>>>  "INGENIC_TIMER" driver, but the better choice is completely 
>>>>>>> \x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7findependent
>>>>>>>  use of "INGENIC_SYSOST" driver to provide higher precision 
>>>>>>> \x7f\x7f\x7f\x7f\x7fclocksource
>>>>>>>  and clockevent.
>>>>>>
>>>>>> Ok, the rating should do the job then.
>>>>>>
>>>>>> Thanks for the explanation.
>>>>>>
>>>>>>>  You may have already noticed that this independent SYSOST 
>>>>>>> \x7f\x7f\x7f\x7f\x7fcomponent is
>>>>>>>  like an upgraded and streamlined TCU, which only retains one 
>>>>>>> generic
>>>>>>>  timer channel that can generate interrupts, upgrade it from 
>>>>>>> 16bit to
>>>>>>>  32bit, and then retain the 64bit ost channel. so the driver 
>>>>>>> code and
>>>>>>>  Kconfig code of this patch is largely referenced
>>>>>>>  "INGENIC_TIMER" driver and "INGENIC_OST" driver.
>>>>>>>
>>>>>>>  Thanks and best regards!
>>>>>>>
>>>>>>>>>  +    default MACH_INGENIC
>>>>>>>>>  +    depends on MIPS || COMPILE_TEST
>>>>>>>>>  +    depends on COMMON_CLK
>>>>>>>>>  +    select MFD_SYSCON
>>>>>>>>>  +    select TIMER_OF
>>>>>>>>>  +    select IRQ_DOMAIN
>>>>>>>>>  +    help
>>>>>>>>>  +      Support for the SYSOST of the Ingenic X Series SoCs.
>>>>>>>>>  +
>>>>>>>>  [ ... ]
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>>
>>>>>> -- 
>>>>>> <http://www.linaro.org/> Linaro.org │ Open source software for 
>>>>>> ARM \x7f\x7f\x7f\x7fSoCs
>>>>>>
>>>>>> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
>>>>>> <http://twitter.com/#!/linaroorg> Twitter |
>>>>>> <http://www.linaro.org/linaro-blog/> Blog
>>>>>
>>>
>