Re: [PATCH v12 07/13] clocksource: Add a new timer-ingenic driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

I'll make a V13 of this patchset on top of -rc6, any feedback
for this clocksource driver? It's been already one month.

Thanks,
-Paul



Le mar. 21 mai 2019 à 16:51, Paul Cercueil <paul@xxxxxxxxxxxxxxx> a écrit :
This driver handles the TCU (Timer Counter Unit) present on the Ingenic JZ47xx SoCs, and provides the kernel with a system timer, a clocksource
and a sched_clock.

Signed-off-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
---

Notes:
    v2: Use SPDX identifier for the license

    v3: - Move documentation to its own patch
    	- Search the devicetree for PWM clients, and use all the TCU
    	  channels that won't be used for PWM

    v4: - Add documentation about why we search for PWM clients
    	- Verify that the PWM clients are for the TCU PWM driver

    v5: Major overhaul. Too many changes to list. Consider it's a new
    	patch.

    v6: - Add two API functions ingenic_tcu_request_channel and
    	  ingenic_tcu_release_channel. To be used by the PWM driver to
request the use of a TCU channel. The driver will now dynamically
    	  move away the system timer or clocksource to a new TCU channel.
    	- The system timer now defaults to channel 0, the clocksource now
    	  defaults to channel 1 and is no more optional. The
ingenic,timer-channel and ingenic,clocksource-channel devicetree
    	  properties are now gone.
    	- Fix round_rate / set_rate not calculating the prescale divider
the same way. This caused problems when (parent_rate / div) would
    	  give a non-integer result. The behaviour is correct now.
    	- The clocksource clock is turned off on suspend now.

    v7: Fix section mismatch by using builtin_platform_driver_probe()

v8: - Removed ingenic_tcu_[request,release]_channel, and the mechanism
    	  to dynamically change the TCU channel of the system timer or
    	  the clocksource.
    	- The driver's devicetree node can now have two more children
    	  nodes, that correspond to the system timer and clocksource.
    	  For these two, the driver will use the TCU timer that
    	  correspond to the memory resource supplied in their
    	  respective node.

    v9: - Removed support for clocksource / timer children devicetree
    	  nodes. Now, we use a property "ingenic,pwm-channels-mask" to
    	  know which PWM channels are reserved for PWM use and should
    	  not be used as OS timers.

v10: - Use CLK_SET_RATE_UNGATE instead of CLK_SET_RATE_GATE + manually un-gating the clock before changing rate. Same for re-parenting.
    	 - Unconditionally create the clocksource and sched_clock even if
the SoC possesses a OS Timer. That gives the choice back to the
    	   user which clocksource should be selected.
- Use subsys_initcall() instead of builtin_platform_driver_probe().
    	   The OS Timer driver calls builtin_platform_driver_probe, which
    	   requires the device to be created before that.
    	 - Cosmetic cleanups

    v11: - Change prototype of exported function
    	   ingenic_tcu_pwm_can_use_chn(), use a struct device * as first
    	   argument.
    	 - Read clocksource using the regmap instead of bypassing it.
Bypassing the regmap makes sense only for the sched_clock where
    	   the read operation must be as fast as possible.
    	 - Fix incorrect format in pr_crit() macro

v12: - Clock handling and IRQ handling are gone, and are now handled
    	   in their own driver.
    	 - Obtain regmap from the ingenic-tcu MFD driver. As a result, we
    	   cannot bypass the regmap anymore for the sched_clock.

 drivers/clocksource/Kconfig         |  11 +
 drivers/clocksource/Makefile        |   1 +
drivers/clocksource/ingenic-timer.c | 357 ++++++++++++++++++++++++++++
 3 files changed, 369 insertions(+)
 create mode 100644 drivers/clocksource/ingenic-timer.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 6bcaa4e2e72c..bb5d5c044341 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -672,4 +672,15 @@ config MILBEAUT_TIMER
 	help
 	  Enables the support for Milbeaut timer driver.

+config INGENIC_TIMER
+	bool "Clocksource/timer using the TCU in Ingenic JZ SoCs"
+	default MACH_INGENIC
+	depends on MIPS || COMPILE_TEST
+	depends on COMMON_CLK
+	select INGENIC_TCU
+	select TIMER_OF
+	select IRQ_DOMAIN
+	help
+	  Support for the timer/counter unit of the Ingenic JZ SoCs.
+
 endmenu
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 236858fa7fbf..553f3c61717a 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -78,6 +78,7 @@ obj-$(CONFIG_ASM9260_TIMER)		+= asm9260_timer.o
 obj-$(CONFIG_H8300_TMR8)		+= h8300_timer8.o
 obj-$(CONFIG_H8300_TMR16)		+= h8300_timer16.o
 obj-$(CONFIG_H8300_TPU)			+= h8300_tpu.o
+obj-$(CONFIG_INGENIC_TIMER)		+= ingenic-timer.o
 obj-$(CONFIG_CLKSRC_ST_LPC)		+= clksrc_st_lpc.o
 obj-$(CONFIG_X86_NUMACHIP)		+= numachip.o
 obj-$(CONFIG_ATCPIT100_TIMER)		+= timer-atcpit100.o
diff --git a/drivers/clocksource/ingenic-timer.c b/drivers/clocksource/ingenic-timer.c
new file mode 100644
index 000000000000..9f4df64390f4
--- /dev/null
+++ b/drivers/clocksource/ingenic-timer.c
@@ -0,0 +1,357 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * JZ47xx SoCs TCU IRQ driver
+ * Copyright (C) 2019 Paul Cercueil <paul@xxxxxxxxxxxxxxx>
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/clocksource.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/ingenic-tcu.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/sched_clock.h>
+
+#include <dt-bindings/clock/ingenic,tcu.h>
+
+struct ingenic_soc_info {
+	unsigned int num_channels;
+};
+
+struct ingenic_tcu {
+	struct regmap *map;
+	struct clk *timer_clk, *cs_clk;
+
+	unsigned int timer_channel, cs_channel;
+	struct clock_event_device cevt;
+	struct clocksource cs;
+	char name[4];
+
+	unsigned long pwm_channels_mask;
+};
+
+static struct ingenic_tcu *ingenic_tcu;
+
+static u64 notrace ingenic_tcu_timer_read(void)
+{
+	struct ingenic_tcu *tcu = ingenic_tcu;
+	unsigned int count;
+
+	regmap_read(tcu->map, TCU_REG_TCNTc(tcu->cs_channel), &count);
+
+	return count;
+}
+
+static u64 notrace ingenic_tcu_timer_cs_read(struct clocksource *cs)
+{
+	return ingenic_tcu_timer_read();
+}
+
+static inline struct ingenic_tcu *to_ingenic_tcu(struct clock_event_device *evt)
+{
+	return container_of(evt, struct ingenic_tcu, cevt);
+}
+
+static int ingenic_tcu_cevt_set_state_shutdown(struct clock_event_device *evt)
+{
+	struct ingenic_tcu *tcu = to_ingenic_tcu(evt);
+
+	regmap_write(tcu->map, TCU_REG_TECR, BIT(tcu->timer_channel));
+
+	return 0;
+}
+
+static int ingenic_tcu_cevt_set_next(unsigned long next,
+				     struct clock_event_device *evt)
+{
+	struct ingenic_tcu *tcu = to_ingenic_tcu(evt);
+
+	if (next > 0xffff)
+		return -EINVAL;
+
+	regmap_write(tcu->map, TCU_REG_TDFRc(tcu->timer_channel), next);
+	regmap_write(tcu->map, TCU_REG_TCNTc(tcu->timer_channel), 0);
+	regmap_write(tcu->map, TCU_REG_TESR, BIT(tcu->timer_channel));
+
+	return 0;
+}
+
+static irqreturn_t ingenic_tcu_cevt_cb(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = dev_id;
+	struct ingenic_tcu *tcu = to_ingenic_tcu(evt);
+
+	regmap_write(tcu->map, TCU_REG_TECR, BIT(tcu->timer_channel));
+
+	if (evt->event_handler)
+		evt->event_handler(evt);
+
+	return IRQ_HANDLED;
+}
+
+static struct clk * __init ingenic_tcu_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_tcu_timer_init(struct device_node *np,
+					 struct ingenic_tcu *tcu)
+{
+	unsigned int timer_virq, channel = tcu->timer_channel;
+	struct irq_domain *domain;
+	unsigned long rate;
+	int err;
+
+	tcu->timer_clk = ingenic_tcu_get_clock(np, channel);
+	if (IS_ERR(tcu->timer_clk))
+		return PTR_ERR(tcu->timer_clk);
+
+	err = clk_prepare_enable(tcu->timer_clk);
+	if (err)
+		goto err_clk_put;
+
+	rate = clk_get_rate(tcu->timer_clk);
+	if (!rate) {
+		err = -EINVAL;
+		goto err_clk_disable;
+	}
+
+	domain = irq_find_host(np);
+	if (!domain) {
+		err = -ENODEV;
+		goto err_clk_disable;
+	}
+
+	timer_virq = irq_create_mapping(domain, channel);
+	if (!timer_virq) {
+		err = -EINVAL;
+		goto err_clk_disable;
+	}
+
+	snprintf(tcu->name, sizeof(tcu->name), "TCU");
+
+	err = request_irq(timer_virq, ingenic_tcu_cevt_cb, IRQF_TIMER,
+			  tcu->name, &tcu->cevt);
+	if (err)
+		goto err_irq_dispose_mapping;
+
+	tcu->cevt.cpumask = cpumask_of(smp_processor_id());
+	tcu->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
+	tcu->cevt.name = tcu->name;
+	tcu->cevt.rating = 200;
+	tcu->cevt.set_state_shutdown = ingenic_tcu_cevt_set_state_shutdown;
+	tcu->cevt.set_next_event = ingenic_tcu_cevt_set_next;
+
+	clockevents_config_and_register(&tcu->cevt, rate, 10, 0xffff);
+
+	return 0;
+
+err_irq_dispose_mapping:
+	irq_dispose_mapping(timer_virq);
+err_clk_disable:
+	clk_disable_unprepare(tcu->timer_clk);
+err_clk_put:
+	clk_put(tcu->timer_clk);
+	return err;
+}
+
+static int __init ingenic_tcu_clocksource_init(struct device_node *np,
+					       struct ingenic_tcu *tcu)
+{
+	unsigned int channel = tcu->cs_channel;
+	struct clocksource *cs = &tcu->cs;
+	unsigned long rate;
+	int err;
+
+	tcu->cs_clk = ingenic_tcu_get_clock(np, channel);
+	if (IS_ERR(tcu->cs_clk))
+		return PTR_ERR(tcu->cs_clk);
+
+	err = clk_prepare_enable(tcu->cs_clk);
+	if (err)
+		goto err_clk_put;
+
+	rate = clk_get_rate(tcu->cs_clk);
+	if (!rate) {
+		err = -EINVAL;
+		goto err_clk_disable;
+	}
+
+	/* Reset channel */
+	regmap_update_bits(tcu->map, TCU_REG_TCSRc(channel),
+			   0xffff & ~TCU_TCSR_RESERVED_BITS, 0);
+
+	/* Reset counter */
+	regmap_write(tcu->map, TCU_REG_TDFRc(channel), 0xffff);
+	regmap_write(tcu->map, TCU_REG_TCNTc(channel), 0);
+
+	/* Enable channel */
+	regmap_write(tcu->map, TCU_REG_TESR, BIT(channel));
+
+	cs->name = "ingenic-timer";
+	cs->rating = 200;
+	cs->flags = CLOCK_SOURCE_IS_CONTINUOUS;
+	cs->mask = CLOCKSOURCE_MASK(16);
+	cs->read = ingenic_tcu_timer_cs_read;
+
+	err = clocksource_register_hz(cs, rate);
+	if (err)
+		goto err_clk_disable;
+
+	return 0;
+
+err_clk_disable:
+	clk_disable_unprepare(tcu->cs_clk);
+err_clk_put:
+	clk_put(tcu->cs_clk);
+	return err;
+}
+
+static const struct ingenic_soc_info jz4740_soc_info = {
+	.num_channels = 8,
+};
+
+static const struct ingenic_soc_info jz4725b_soc_info = {
+	.num_channels = 6,
+};
+
+static const struct of_device_id ingenic_tcu_of_match[] = {
+	{ .compatible = "ingenic,jz4740-tcu", .data = &jz4740_soc_info, },
+	{ .compatible = "ingenic,jz4725b-tcu", .data = &jz4725b_soc_info, },
+	{ .compatible = "ingenic,jz4770-tcu", .data = &jz4740_soc_info, },
+	{ }
+};
+
+static int __init ingenic_tcu_init(struct device_node *np)
+{
+ const struct of_device_id *id = of_match_node(ingenic_tcu_of_match, np);
+	const struct ingenic_soc_info *soc_info = id->data;
+	struct ingenic_tcu *tcu;
+	struct regmap *map;
+	long rate;
+	int ret;
+
+	of_node_clear_flag(np, OF_POPULATED);
+
+	map = ingenic_tcu_get_regmap(np);
+	if (IS_ERR(map))
+		return PTR_ERR(map);
+
+	tcu = kzalloc(sizeof(*tcu), GFP_KERNEL);
+	if (!tcu)
+		return -ENOMEM;
+
+ /* Enable all TCU channels for PWM use by default except channels 0/1 */
+	tcu->pwm_channels_mask = GENMASK(soc_info->num_channels - 1, 2);
+	of_property_read_u32(np, "ingenic,pwm-channels-mask",
+			     (u32 *)&tcu->pwm_channels_mask);
+
+	/* Verify that we have at least two free channels */
+	if (hweight8(tcu->pwm_channels_mask) > soc_info->num_channels - 2) {
+		pr_crit("%s: Invalid PWM channel mask: 0x%02lx\n", __func__,
+			tcu->pwm_channels_mask);
+		ret = -EINVAL;
+		goto err_free_ingenic_tcu;
+	}
+
+	tcu->map = map;
+	ingenic_tcu = tcu;
+
+	tcu->timer_channel = find_first_zero_bit(&tcu->pwm_channels_mask,
+						 soc_info->num_channels);
+	tcu->cs_channel = find_next_zero_bit(&tcu->pwm_channels_mask,
+					     soc_info->num_channels,
+					     tcu->timer_channel + 1);
+
+	ret = ingenic_tcu_clocksource_init(np, tcu);
+	if (ret) {
+		pr_crit("%s: Unable to init clocksource: %i\n", __func__, ret);
+		goto err_free_ingenic_tcu;
+	}
+
+	ret = ingenic_tcu_timer_init(np, tcu);
+	if (ret)
+		goto err_tcu_clocksource_cleanup;
+
+ /* Register the sched_clock at the end as there's no way to undo it */
+	rate = clk_get_rate(tcu->cs_clk);
+	sched_clock_register(ingenic_tcu_timer_read, 16, rate);
+
+	return 0;
+
+err_tcu_clocksource_cleanup:
+	clocksource_unregister(&tcu->cs);
+	clk_disable_unprepare(tcu->cs_clk);
+	clk_put(tcu->cs_clk);
+err_free_ingenic_tcu:
+	kfree(tcu);
+	return ret;
+}
+
+TIMER_OF_DECLARE(jz4740_tcu_intc, "ingenic,jz4740-tcu", ingenic_tcu_init); +TIMER_OF_DECLARE(jz4725b_tcu_intc, "ingenic,jz4725b-tcu", ingenic_tcu_init); +TIMER_OF_DECLARE(jz4770_tcu_intc, "ingenic,jz4770-tcu", ingenic_tcu_init);
+
+
+static int __init ingenic_tcu_probe(struct platform_device *pdev)
+{
+	platform_set_drvdata(pdev, ingenic_tcu);
+
+	return 0;
+}
+
+static int __maybe_unused ingenic_tcu_suspend(struct device *dev)
+{
+	struct ingenic_tcu *tcu = dev_get_drvdata(dev);
+
+	clk_disable(tcu->cs_clk);
+	clk_disable(tcu->timer_clk);
+	return 0;
+}
+
+static int __maybe_unused ingenic_tcu_resume(struct device *dev)
+{
+	struct ingenic_tcu *tcu = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_enable(tcu->timer_clk);
+	if (ret)
+		return ret;
+
+	ret = clk_enable(tcu->cs_clk);
+	if (ret) {
+		clk_disable(tcu->timer_clk);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops __maybe_unused ingenic_tcu_pm_ops = {
+ /* _noirq: We want the TCU clocks to be gated last / ungated first */
+	.suspend_noirq = ingenic_tcu_suspend,
+	.resume_noirq  = ingenic_tcu_resume,
+};
+
+static struct platform_driver ingenic_tcu_driver = {
+	.driver = {
+		.name	= "ingenic-tcu-timer",
+#ifdef CONFIG_PM_SLEEP
+		.pm	= &ingenic_tcu_pm_ops,
+#endif
+		.of_match_table = ingenic_tcu_of_match,
+	},
+};
+builtin_platform_driver_probe(ingenic_tcu_driver, ingenic_tcu_probe);
--
2.21.0.593.g511ec345e18







[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux