[Letux-kernel] jz4730-i2c - clocksource

[H. Nikolaus Schaller]

Hi Paul,
I may have found something new... I did simply loop
and collect the timer values and the sched_clock()
in an array with later printk(). So that data collection
is not slowed down by printk().

The interesting thing is that the timer register overflows
and restarts - and at the same moment the sched_clock()
jumps back. Which means that there was no cevt triggered
snapshot in between.

My idea now was: what happens if the cevt interrupts
are not processed correctly?

Now, ingenic-timer does:

static int ingenic_tcu_setup_cevt(unsigned int cpu)
{
	struct ingenic_tcu *tcu = ingenic_tcu;
	struct ingenic_tcu_timer *timer = &tcu->timers[cpu];
	unsigned int timer_virq;
	struct irq_domain *domain;
	unsigned long rate;
	int err;

...
	domain = irq_find_host(tcu->np);
	if (!domain) {
		err = -ENODEV;
		goto err_clk_disable;
	}

	timer_virq = irq_create_mapping(domain, timer->channel);
	if (!timer_virq) {
		err = -EINVAL;
		goto err_clk_disable;
	}

...

	err = request_irq(timer_virq, ingenic_tcu_cevt_cb, IRQF_TIMER,
			  timer->name, timer);


while ingenic-sysost.c does:

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;

...

	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);

So the cevt interrupt is setup differently!

Interesting is that irq_create_mapping expects some irq_hw_number_t hwirq
but I wonder why passing the timer->channel number should be the
right value. Well, the magic "domain" could do some translation.

The sysost uses a direct device tree based approach. Maybe nobody did care
about OF conversion since this feature wasn't used in the jz4740 setup...

If my finding is correct this would mean that the ingenic_tcu_cevt_cb()
is not called by the timer expiry interrupt but some other interrupt
which happens to be fairly regular.

This would mean again that all cevt based timings are wrong and may
come too early or too late.

I'll give it a try...

BTW: I have replaced all regmap code by direct readl/writel and IMHO
there is a very strong argument to bypass regmap: it takes an unknown
amount of processor cycles to read through a regmap and that it is
used for a timer with sub-microsecond precision... Here we should
save every unnecessary instruction to get it as fast as possible.

iMHO the same would be wise to be done for the jz4740...

BR,
Nikolaus