Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1235365 > unrolled thread

Trivial clocksource driver

Back to article view | Back to linux.kernel

Contents

  Trivial clocksource driver Mason <slash.tmp@free.fr> - 2015-09-29 18:30 +0200
    Re: Trivial clocksource driver Thomas Gleixner <tglx@linutronix.de> - 2015-09-29 20:40 +0200
      Re: Trivial clocksource driver Mason <slash.tmp@free.fr> - 2015-09-29 21:50 +0200
        Re: Trivial clocksource driver Måns Rullgård <mans@mansr.com> - 2015-09-29 22:20 +0200
          Re: Trivial clocksource driver Thomas Gleixner <tglx@linutronix.de> - 2015-09-29 23:00 +0200
          Re: Trivial clocksource driver Mason <slash.tmp@free.fr> - 2015-09-29 23:20 +0200
            Re: Trivial clocksource driver Måns Rullgård <mans@mansr.com> - 2015-09-30 00:00 +0200

#1235365 — Trivial clocksource driver

Hello everyone,

I am trying to submit a new ARM port, and Arnd pointed out that the
clocksource code could not live in arch/arm/$PLATFORM, but had to
move to drivers/clocksource (and it had to support DT).

Did I understand correctly? Is this the right place to submit code
as provided below?

Regards.


#include <linux/delay.h>	/* register_current_timer_delay	*/
#include <linux/clocksource.h>	/* clocksource_register_hz	*/
#include <linux/sched_clock.h>	/* sched_clock_register		*/
#include <linux/of_address.h>	/* of_iomap			*/
#include <linux/clk.h>		/* of_clk_get, clk_get_rate	*/

static void __iomem *xtal_in_cnt;
static struct delay_timer delay_timer;

static unsigned long read_xtal_counter(void)
{
	return readl_relaxed(xtal_in_cnt);
}

static u64 read_sched_clock(void)
{
	return read_xtal_counter();
}

static cycle_t read_clocksource(struct clocksource *cs)
{
	return read_xtal_counter();
}

static struct clocksource tango_xtal = {
	.name	= "tango-xtal",
	.rating	= 350,
	.read	= read_clocksource,
	.mask	= CLOCKSOURCE_MASK(32),
	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
};

static void __init tango_clksrc_init(struct device_node *np)
{
	struct clk *clk = of_clk_get(np, 0);
	unsigned int xtal_freq = clk_get_rate(clk);
	xtal_in_cnt = of_iomap(np, 0);

	delay_timer.freq = xtal_freq;
	delay_timer.read_current_timer = read_xtal_counter;
	register_current_timer_delay(&delay_timer);
	sched_clock_register(read_sched_clock, 32, xtal_freq);
	clocksource_register_hz(&tango_xtal, xtal_freq);
}

CLOCKSOURCE_OF_DECLARE(tango, "sigma,xtal_in_cnt", tango_clksrc_init);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1235472

On Tue, 29 Sep 2015, Mason wrote:
> Hello everyone,
> 
> I am trying to submit a new ARM port, and Arnd pointed out that the
> clocksource code could not live in arch/arm/$PLATFORM, but had to
> move to drivers/clocksource (and it had to support DT).
> 
> Did I understand correctly? Is this the right place to submit code
> as provided below?


Yes, drivers/clocksource is the right place. You just need to submit a
formal patch, which includes a proper subject line, changelog, plus
the necessary Makefile and Kconfig modifications.
 
> #include <linux/delay.h>	/* register_current_timer_delay	*/

Please get rid of these silly tail comments. They provide absolutely
no value.

Other than that this looks reasonable.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1235509

On 29/09/2015 20:32, Thomas Gleixner wrote:

> On Tue, 29 Sep 2015, Mason wrote:
> 
>> I am trying to submit a new ARM port, and Arnd pointed out that the
>> clocksource code could not live in arch/arm/$PLATFORM, but had to
>> move to drivers/clocksource (and it had to support DT).
>>
>> Did I understand correctly? Is this the right place to submit code
>> as provided below?
> 
> Yes, drivers/clocksource is the right place. You just need to submit a
> formal patch, which includes a proper subject line, changelog, plus
> the necessary Makefile and Kconfig modifications.

OK, I'll send a formal patch tomorrow.
There are no Kconfig modifications, is that OK?

Also, that patch is part of a larger patch-set (most of the
patches intended for arch/arm). I should send you only the
clocksource patch, or the whole patch-set?

>> #include <linux/delay.h>	/* register_current_timer_delay	*/
> 
> Please get rid of these silly tail comments. They provide absolutely
> no value.

I will remove them, since you asked.

In my opinion, they serve one purpose: if code is refactored,
and the function call is removed, the comment is a reminder
to also remove the relevant include directive.

Do you disagree?

> Other than that this looks reasonable.

Just wanted to ask:
Can register_current_timer_delay, sched_clock_register, and
clocksource_register_hz be called in any order?

Regards.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1235519

Mason <slash.tmp@free.fr> writes:

> On 29/09/2015 20:32, Thomas Gleixner wrote:
>
>> On Tue, 29 Sep 2015, Mason wrote:
>> 
>>> I am trying to submit a new ARM port, and Arnd pointed out that the
>>> clocksource code could not live in arch/arm/$PLATFORM, but had to
>>> move to drivers/clocksource (and it had to support DT).
>>>
>>> Did I understand correctly? Is this the right place to submit code
>>> as provided below?
>> 
>> Yes, drivers/clocksource is the right place. You just need to submit a
>> formal patch, which includes a proper subject line, changelog, plus
>> the necessary Makefile and Kconfig modifications.
>
> OK, I'll send a formal patch tomorrow.
> There are no Kconfig modifications, is that OK?

Why don't you use my driver?  It's even simpler, and it works just fine
on the 87xx chip.

https://github.com/mansr/linux-tangox/blob/master/drivers/clocksource/clksrc-tangox.c

-- 
Måns Rullgård
mans@mansr.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1235551

[Multipart message — attachments visible in raw view] — view raw

On Tue, 29 Sep 2015, Måns Rullgård wrote:
> Mason <slash.tmp@free.fr> writes:
> 
> > On 29/09/2015 20:32, Thomas Gleixner wrote:
> >
> >> On Tue, 29 Sep 2015, Mason wrote:
> >> 
> >>> I am trying to submit a new ARM port, and Arnd pointed out that the
> >>> clocksource code could not live in arch/arm/$PLATFORM, but had to
> >>> move to drivers/clocksource (and it had to support DT).
> >>>
> >>> Did I understand correctly? Is this the right place to submit code
> >>> as provided below?
> >> 
> >> Yes, drivers/clocksource is the right place. You just need to submit a
> >> formal patch, which includes a proper subject line, changelog, plus
> >> the necessary Makefile and Kconfig modifications.
> >
> > OK, I'll send a formal patch tomorrow.
> > There are no Kconfig modifications, is that OK?
> 
> Why don't you use my driver?  It's even simpler, and it works just fine
> on the 87xx chip.
> 
> https://github.com/mansr/linux-tangox/blob/master/drivers/clocksource/clksrc-tangox.c

Perhaps because that driver is not upstream either?

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1235562

Hello Mans,

On 29/09/2015 22:18, Måns Rullgård wrote:

> Mason writes:
> 
>> On 29/09/2015 20:32, Thomas Gleixner wrote:
>>
>>> On Tue, 29 Sep 2015, Mason wrote:
>>>
>>>> I am trying to submit a new ARM port, and Arnd pointed out that the
>>>> clocksource code could not live in arch/arm/$PLATFORM, but had to
>>>> move to drivers/clocksource (and it had to support DT).
>>>>
>>>> Did I understand correctly? Is this the right place to submit code
>>>> as provided below?
>>>
>>> Yes, drivers/clocksource is the right place. You just need to submit a
>>> formal patch, which includes a proper subject line, changelog, plus
>>> the necessary Makefile and Kconfig modifications.
>>
>> OK, I'll send a formal patch tomorrow.
>> There are no Kconfig modifications, is that OK?
> 
> Why don't you use my driver?  It's even simpler, and it works just fine
> on the 87xx chip.
> 
> https://github.com/mansr/linux-tangox/blob/master/drivers/clocksource/clksrc-tangox.c

As you know, I am using two of your complex drivers, namely
ethernet and interrupt controller (which would have taken me
several weeks to write). I will be submitting them upstream
shortly, is that OK with you?

I'm not using this particular driver of yours because I had
already written the code, and porting it to DT was a good
learning process. This is probably my only opportunity to
actually write any kind of code for this port.

Regards.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1235575

Mason <slash.tmp@free.fr> writes:

> Hello Mans,
>
> On 29/09/2015 22:18, Måns Rullgård wrote:
>
>> Mason writes:
>> 
>>> On 29/09/2015 20:32, Thomas Gleixner wrote:
>>>
>>>> On Tue, 29 Sep 2015, Mason wrote:
>>>>
>>>>> I am trying to submit a new ARM port, and Arnd pointed out that the
>>>>> clocksource code could not live in arch/arm/$PLATFORM, but had to
>>>>> move to drivers/clocksource (and it had to support DT).
>>>>>
>>>>> Did I understand correctly? Is this the right place to submit code
>>>>> as provided below?
>>>>
>>>> Yes, drivers/clocksource is the right place. You just need to submit a
>>>> formal patch, which includes a proper subject line, changelog, plus
>>>> the necessary Makefile and Kconfig modifications.
>>>
>>> OK, I'll send a formal patch tomorrow.
>>> There are no Kconfig modifications, is that OK?
>> 
>> Why don't you use my driver?  It's even simpler, and it works just fine
>> on the 87xx chip.
>> 
>> https://github.com/mansr/linux-tangox/blob/master/drivers/clocksource/clksrc-tangox.c
>
> As you know, I am using two of your complex drivers, namely
> ethernet and interrupt controller (which would have taken me
> several weeks to write). I will be submitting them upstream
> shortly, is that OK with you?

I want to test them a bit more on the 87xx first.  It's booting, but
there are a couple of niggles that should be looked into.  Getting some
documentation would expedite that process.

> I'm not using this particular driver of yours because I had
> already written the code, and porting it to DT was a good
> learning process. This is probably my only opportunity to
> actually write any kind of code for this port.

You should still be using the existing clocksource_mmio helper.  In
fact, that interface could be exposed using a generic DT binding.

-- 
Måns Rullgård
mans@mansr.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web