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


Groups > linux.kernel > #1240576 > unrolled thread

[PATCH v1] clocksource: Sigma Designs Tango 27 MHz xtal

Started byMarc Gonzalez <marc_gonzalez@sigmadesigns.com>
First post2015-10-06 17:40 +0200
Last post2015-10-09 18:40 +0200
Articles 20 on this page of 24 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v1] clocksource: Sigma Designs Tango 27 MHz xtal Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2015-10-06 17:40 +0200
    Re: [PATCH v1] clocksource: Sigma Designs Tango 27 MHz xtal Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-10-07 01:10 +0200
      Re: [PATCH v1] clocksource: Sigma Designs Tango 27 MHz xtal Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2015-10-07 10:30 +0200
        Re: [PATCH v1] clocksource: Sigma Designs Tango 27 MHz xtal Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-10-07 11:50 +0200
          Re: [PATCH v1] clocksource: Sigma Designs Tango 27 MHz xtal Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2015-10-07 13:20 +0200
            Re: [PATCH v1] clocksource: Sigma Designs Tango 27 MHz xtal Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-10-07 13:40 +0200
        [PATCH v2] clocksource/drivers/tango_xtal: Add new timer for Tango  SoCs Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2015-10-07 13:40 +0200
          Re: [PATCH v2] clocksource/drivers/tango_xtal: Add new timer for  Tango SoCs Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-10-07 14:40 +0200
            Re: [PATCH v2] clocksource/drivers/tango_xtal: Add new timer for  Tango SoCs Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2015-10-07 15:20 +0200
              Re: [PATCH v2] clocksource/drivers/tango_xtal: Add new timer for  Tango SoCs Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-10-07 18:10 +0200
                Re: [PATCH v2] clocksource/drivers/tango_xtal: Add new timer for  Tango SoCs Mason <slash.tmp@free.fr> - 2015-10-08 15:20 +0200
                  [PATCH v3] clocksource/drivers/tango_xtal: Add new timer for Tango  SoCs Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2015-10-09 14:30 +0200
                    Re: [PATCH v3] clocksource/drivers/tango_xtal: Add new timer for  Tango SoCs Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-10-09 15:30 +0200
                      Re: [PATCH v3] clocksource/drivers/tango_xtal: Add new timer for  Tango SoCs Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2015-10-09 15:50 +0200
                        Re: [PATCH v3] clocksource/drivers/tango_xtal: Add new timer for  Tango SoCs Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-10-09 16:30 +0200
                          Re: [PATCH v3] clocksource/drivers/tango_xtal: Add new timer for  Tango SoCs Nicolas Pitre <nicolas.pitre@linaro.org> - 2015-10-09 18:40 +0200
                      [PATCH v4] clocksource/drivers/tango_xtal: Add new timer for Tango  SoCs Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2015-10-09 16:40 +0200
                        Re: [PATCH v4] clocksource/drivers/tango_xtal: Add new timer for  Tango SoCs Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-10-09 16:50 +0200
                          [PATCH v5] clocksource/drivers/tango_xtal: Add new timer for Tango  SoCs Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2015-10-09 17:00 +0200
                            Re: [PATCH v5] clocksource/drivers/tango_xtal: Add new timer for  Tango SoCs Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-10-16 14:10 +0200
                        Re: [PATCH v4] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs Måns Rullgård <mans@mansr.com> - 2015-10-09 17:00 +0200
                          Re: [PATCH v4] clocksource/drivers/tango_xtal: Add new timer for  Tango SoCs Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2015-10-09 17:50 +0200
                            Re: [PATCH v4] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs Måns Rullgård <mans@mansr.com> - 2015-10-09 18:10 +0200
                              Re: [PATCH v4] clocksource/drivers/tango_xtal: Add new timer for  Tango SoCs Marc Gonzalez <marc_gonzalez@sigmadesigns.com> - 2015-10-09 18:40 +0200

Page 1 of 2  [1] 2  Next page →


#1240576 — [PATCH v1] clocksource: Sigma Designs Tango 27 MHz xtal

FromMarc Gonzalez <marc_gonzalez@sigmadesigns.com>
Date2015-10-06 17:40 +0200
Subject[PATCH v1] clocksource: Sigma Designs Tango 27 MHz xtal
Message-ID<qgCvo-1OO-17@gated-at.bofh.it>
Date: Tue, 6 Oct 2015 16:49:28 +0200
Subject: [PATCH] clocksource: Sigma Designs Tango 27 MHz xtal

Sigma Designs Tango platforms provide a 27 MHz crystal oscillator.
Use it for clocksource, sched_clock, and delay_timer.

Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
 drivers/clocksource/Makefile     |  1 +
 drivers/clocksource/tango_xtal.c | 46 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)
 create mode 100644 drivers/clocksource/tango_xtal.c

diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index f228354961ca..46e405673d75 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_ARCH_CLPS711X)	+= clps711x-timer.o
 obj-$(CONFIG_ARCH_ATLAS7)	+= timer-atlas7.o
 obj-$(CONFIG_ARCH_MOXART)	+= moxart_timer.o
 obj-$(CONFIG_ARCH_MXS)		+= mxs_timer.o
+obj-$(CONFIG_ARCH_TANGOX)	+= tango_xtal.o
 obj-$(CONFIG_CLKSRC_PXA)	+= pxa_timer.o
 obj-$(CONFIG_ARCH_PRIMA2)	+= timer-prima2.o
 obj-$(CONFIG_ARCH_U300)		+= timer-u300.o
diff --git a/drivers/clocksource/tango_xtal.c b/drivers/clocksource/tango_xtal.c
new file mode 100644
index 000000000000..0f2fb293ab75
--- /dev/null
+++ b/drivers/clocksource/tango_xtal.c
@@ -0,0 +1,46 @@
+#include <linux/clocksource.h>
+#include <linux/sched_clock.h>
+#include <linux/of_address.h>
+#include <linux/delay.h>
+#include <linux/clk.h>
+
+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_clocksource_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,tick-counter", tango_clocksource_init);
-- 
2.4.5

--
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]


#1241071

FromDaniel Lezcano <daniel.lezcano@linaro.org>
Date2015-10-07 01:10 +0200
Message-ID<qgJwR-3HK-17@gated-at.bofh.it>
In reply to#1240576
Hi Marc,

On 10/06/2015 05:10 PM, Marc Gonzalez wrote:
> Date: Tue, 6 Oct 2015 16:49:28 +0200
> Subject: [PATCH] clocksource: Sigma Designs Tango 27 MHz xtal

Fix the patch format.

Subject is clocksource/drivers/tango_xtal: Add new timer ...

> Sigma Designs Tango platforms provide a 27 MHz crystal oscillator.
> Use it for clocksource, sched_clock, and delay_timer.
>
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---
>   drivers/clocksource/Makefile     |  1 +
>   drivers/clocksource/tango_xtal.c | 46 ++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 47 insertions(+)
>   create mode 100644 drivers/clocksource/tango_xtal.c
>
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index f228354961ca..46e405673d75 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_ARCH_CLPS711X)	+= clps711x-timer.o
>   obj-$(CONFIG_ARCH_ATLAS7)	+= timer-atlas7.o
>   obj-$(CONFIG_ARCH_MOXART)	+= moxart_timer.o
>   obj-$(CONFIG_ARCH_MXS)		+= mxs_timer.o
> +obj-$(CONFIG_ARCH_TANGOX)	+= tango_xtal.o

Please use the following scheme:

Add in the clocksource's Kconfig an entry:

config CLKSRC_TANGO_XTAL
	bool "bla bla" if COMPILE_TEST
	select CLKSRC_OF

and then in the arch's Kconfig:

select CLKSRC_TANGO_XTAL


>   obj-$(CONFIG_CLKSRC_PXA)	+= pxa_timer.o
>   obj-$(CONFIG_ARCH_PRIMA2)	+= timer-prima2.o
>   obj-$(CONFIG_ARCH_U300)		+= timer-u300.o
> diff --git a/drivers/clocksource/tango_xtal.c b/drivers/clocksource/tango_xtal.c
> new file mode 100644
> index 000000000000..0f2fb293ab75
> --- /dev/null
> +++ b/drivers/clocksource/tango_xtal.c
> @@ -0,0 +1,46 @@
> +#include <linux/clocksource.h>
> +#include <linux/sched_clock.h>
> +#include <linux/of_address.h>
> +#include <linux/delay.h>
> +#include <linux/clk.h>

#include <linux/init.h> is missing.

> +
> +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 u64 *notrace* read_sched_clock(void)

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

check return code.

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

check return code.

> +}
> +
> +CLOCKSOURCE_OF_DECLARE(tango, "sigma,tick-counter", tango_clocksource_init);
>


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

--
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]


#1241232

FromMarc Gonzalez <marc_gonzalez@sigmadesigns.com>
Date2015-10-07 10:30 +0200
Message-ID<qgSgO-7JK-17@gated-at.bofh.it>
In reply to#1241071
Hello everyone,

On 07/10/2015 01:09, Daniel Lezcano wrote:

> On 10/06/2015 05:10 PM, Marc Gonzalez wrote:
>> Date: Tue, 6 Oct 2015 16:49:28 +0200
>> Subject: [PATCH] clocksource: Sigma Designs Tango 27 MHz xtal
> 
> Fix the patch format.

OK.

> Subject is clocksource/drivers/tango_xtal: Add new timer ...

OK.

(84583983c319 didn't seem to follow that convention)

>> Sigma Designs Tango platforms provide a 27 MHz crystal oscillator.
>> Use it for clocksource, sched_clock, and delay_timer.
>>
>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>> ---
>>   drivers/clocksource/Makefile     |  1 +
>>   drivers/clocksource/tango_xtal.c | 46 ++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 47 insertions(+)
>>   create mode 100644 drivers/clocksource/tango_xtal.c
>>
>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>> index f228354961ca..46e405673d75 100644
>> --- a/drivers/clocksource/Makefile
>> +++ b/drivers/clocksource/Makefile
>> @@ -24,6 +24,7 @@ obj-$(CONFIG_ARCH_CLPS711X)	+= clps711x-timer.o
>>   obj-$(CONFIG_ARCH_ATLAS7)	+= timer-atlas7.o
>>   obj-$(CONFIG_ARCH_MOXART)	+= moxart_timer.o
>>   obj-$(CONFIG_ARCH_MXS)		+= mxs_timer.o
>> +obj-$(CONFIG_ARCH_TANGOX)	+= tango_xtal.o
> 
> Please use the following scheme:
> 
> Add in the clocksource's Kconfig an entry:
> 
> config CLKSRC_TANGO_XTAL
> 	bool "bla bla" if COMPILE_TEST
> 	select CLKSRC_OF
> 
> and then in the arch's Kconfig:
> 
> select CLKSRC_TANGO_XTAL

OK.

>>   obj-$(CONFIG_CLKSRC_PXA)	+= pxa_timer.o
>>   obj-$(CONFIG_ARCH_PRIMA2)	+= timer-prima2.o
>>   obj-$(CONFIG_ARCH_U300)		+= timer-u300.o
>> diff --git a/drivers/clocksource/tango_xtal.c b/drivers/clocksource/tango_xtal.c
>> new file mode 100644
>> index 000000000000..0f2fb293ab75
>> --- /dev/null
>> +++ b/drivers/clocksource/tango_xtal.c
>> @@ -0,0 +1,46 @@
>> +#include <linux/clocksource.h>
>> +#include <linux/sched_clock.h>
>> +#include <linux/of_address.h>
>> +#include <linux/delay.h>
>> +#include <linux/clk.h>
> 
> #include <linux/init.h> is missing.

For my education: this header is already indirectly included;
The directive should be explicit to prevent breakage in case
other (implicit) includes are reorganized?

>> +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 u64 *notrace* read_sched_clock(void)

What about read_clocksource? and 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_clocksource_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);
> 
> check return code.

OK.

>> +	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);
> 
> check return code.

OK.

New patch coming up.

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]


#1241299

FromDaniel Lezcano <daniel.lezcano@linaro.org>
Date2015-10-07 11:50 +0200
Message-ID<qgTwe-10x-23@gated-at.bofh.it>
In reply to#1241232
On 10/07/2015 10:23 AM, Marc Gonzalez wrote:

[ ... ]

>>> +++ b/drivers/clocksource/tango_xtal.c
>>> @@ -0,0 +1,46 @@
>>> +#include <linux/clocksource.h>
>>> +#include <linux/sched_clock.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/clk.h>
>>
>> #include <linux/init.h> is missing.
>
> For my education: this header is already indirectly included;
> The directive should be explicit to prevent breakage in case
> other (implicit) includes are reorganized?

Yes, exactly.

The 'clocksource.h' header includes 'init.h' only for the 
'clocksource_default_clock' function. If this function disappears, or 
the __init section is removed, then the 'init.h' inclusion won't make 
sense anymore in the header file. By removing it, that will break all 
code relying on the implicit inclusion.

So the rule of thumb I apply to myself is always to explicitly include 
the headers in the C file even if it is already implicitly included. 
This rule is valid only if the header file is not using intensively code 
coming from the implicit inclusion like static inlines etc ...

>>> +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 u64 *notrace* read_sched_clock(void)
>
> What about read_clocksource? and read_xtal_counter?

See commit 89e6a13b88.

   -- Daniel


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

--
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]


#1241385

FromMarc Gonzalez <marc_gonzalez@sigmadesigns.com>
Date2015-10-07 13:20 +0200
Message-ID<qgUVk-39p-25@gated-at.bofh.it>
In reply to#1241299
On 07/10/2015 11:47, Daniel Lezcano wrote:
> On 10/07/2015 10:23 AM, Marc Gonzalez wrote:
>> Daniel Lezcano wrote:
>>
>>> static u64 *notrace* read_sched_clock(void)
>>
>> What about read_clocksource? and read_xtal_counter?
> 
> See commit 89e6a13b88.

If I understand correctly, since read_sched_clock calls read_xtal_counter,
both functions should be marked notrace.

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]


#1241401

FromDaniel Lezcano <daniel.lezcano@linaro.org>
Date2015-10-07 13:40 +0200
Message-ID<qgVeG-3vT-21@gated-at.bofh.it>
In reply to#1241385
On 10/07/2015 01:14 PM, Marc Gonzalez wrote:
> On 07/10/2015 11:47, Daniel Lezcano wrote:
>> On 10/07/2015 10:23 AM, Marc Gonzalez wrote:
>>> Daniel Lezcano wrote:
>>>
>>>> static u64 *notrace* read_sched_clock(void)
>>>
>>> What about read_clocksource? and read_xtal_counter?
>>
>> See commit 89e6a13b88.
>
> If I understand correctly, since read_sched_clock calls read_xtal_counter,
> both functions should be marked notrace.

Yes. We have to prevent the recursion with the ftrace timestamp.


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

--
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]


#1241402 — [PATCH v2] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs

FromMarc Gonzalez <marc_gonzalez@sigmadesigns.com>
Date2015-10-07 13:40 +0200
Subject[PATCH v2] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs
Message-ID<qgVeF-3vT-19@gated-at.bofh.it>
In reply to#1241232
Sigma Designs Tango platforms provide a 27 MHz crystal oscillator.
Use it for clocksource, sched_clock, and delay_timer.

Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
AFAICS, clocksource_register_hz does not report failures via its
return value (always 0) but writes warnings to stdout?

Open question: can I call register_current_timer_delay,
sched_clock_register, clocksource_register_hz in any order?
---
 drivers/clocksource/Kconfig      |  4 ++++
 drivers/clocksource/Makefile     |  1 +
 drivers/clocksource/tango_xtal.c | 49 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+)
 create mode 100644 drivers/clocksource/tango_xtal.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 4e57730e0be4..63c878d4d9e3 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -275,6 +275,10 @@ config CLKSRC_MIPS_GIC
 	depends on MIPS_GIC
 	select CLKSRC_OF
 
+config CLKSRC_TANGO_XTAL
+	bool
+	select CLKSRC_OF
+
 config CLKSRC_PXA
 	def_bool y if ARCH_PXA || ARCH_SA1100
 	select CLKSRC_OF if USE_OF
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index f228354961ca..160970a3963b 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_ARCH_KEYSTONE)		+= timer-keystone.o
 obj-$(CONFIG_ARCH_INTEGRATOR_AP)	+= timer-integrator-ap.o
 obj-$(CONFIG_CLKSRC_VERSATILE)		+= versatile.o
 obj-$(CONFIG_CLKSRC_MIPS_GIC)		+= mips-gic-timer.o
+obj-$(CONFIG_CLKSRC_TANGO_XTAL)		+= tango_xtal.o
 obj-$(CONFIG_CLKSRC_IMX_GPT)		+= timer-imx-gpt.o
 obj-$(CONFIG_ASM9260_TIMER)		+= asm9260_timer.o
 obj-$(CONFIG_H8300)			+= h8300_timer8.o
diff --git a/drivers/clocksource/tango_xtal.c b/drivers/clocksource/tango_xtal.c
new file mode 100644
index 000000000000..c67599457782
--- /dev/null
+++ b/drivers/clocksource/tango_xtal.c
@@ -0,0 +1,49 @@
+#include <linux/clocksource.h>
+#include <linux/sched_clock.h>
+#include <linux/of_address.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/clk.h>
+
+static void __iomem *xtal_in_cnt;
+static struct delay_timer delay_timer;
+
+static unsigned long notrace read_xtal_counter(void)
+{
+	return readl_relaxed(xtal_in_cnt);
+}
+
+static u64 notrace 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_clocksource_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);
+	if (xtal_in_cnt == NULL)
+		panic("%s: of_iomap failed\n", np->full_name);
+
+	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,tick-counter", tango_clocksource_init);
-- 
2.4.5

--
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]


#1241453 — Re: [PATCH v2] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs

FromDaniel Lezcano <daniel.lezcano@linaro.org>
Date2015-10-07 14:40 +0200
SubjectRe: [PATCH v2] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs
Message-ID<qgWaM-4Te-53@gated-at.bofh.it>
In reply to#1241402
On 10/07/2015 01:35 PM, Marc Gonzalez wrote:
> Sigma Designs Tango platforms provide a 27 MHz crystal oscillator.
> Use it for clocksource, sched_clock, and delay_timer.
>
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---
> AFAICS, clocksource_register_hz does not report failures via its
> return value (always 0) but writes warnings to stdout?

Yeah, it returns always 0. I suggest you assume it is returning an error 
code, that will be safer for future changes in the framework (if any).

> Open question: can I call register_current_timer_delay,
> sched_clock_register, clocksource_register_hz in any order?
> ---

Yes, I think so. Thomas ?

[ ... ]

> +static void __init tango_clocksource_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);
> +	if (xtal_in_cnt == NULL)
> +		panic("%s: of_iomap failed\n", np->full_name);

   ^^^^^^^^^^^

That does not comply with the Linux kernel coding style.

	xtal_in_cnt = of_iomap(np, 0);
	if (!xtal_in_cnt) {
		pr_err("Argh!");
		return;
	}

	clk = of_clk_get(np, 0);
	if (!clk) {
		pr_err("grumf!");
		return;
	}

	freq = clk_get_rate(clk);


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

ret = clocksource_register_hz(&tango_xtal, xtal_freq);
if (ret) {
	pr_err("oups!");
	return;
}



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

--
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]


#1241477 — Re: [PATCH v2] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs

FromMarc Gonzalez <marc_gonzalez@sigmadesigns.com>
Date2015-10-07 15:20 +0200
SubjectRe: [PATCH v2] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs
Message-ID<qgWNt-5TU-19@gated-at.bofh.it>
In reply to#1241453
On 07/10/2015 14:31, Daniel Lezcano wrote:
> On 10/07/2015 01:35 PM, Marc Gonzalez wrote:
>> Sigma Designs Tango platforms provide a 27 MHz crystal oscillator.
>> Use it for clocksource, sched_clock, and delay_timer.
>>
>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>> ---
>> AFAICS, clocksource_register_hz does not report failures via its
>> return value (always 0) but writes warnings to stdout?
> 
> Yeah, it returns always 0. I suggest you assume it is returning an error
> code, that will be safer for future changes in the framework (if any).

I suppose I'd also need to check the return value of of_clk_get?
(Looks like you mention it implicitly below.)

>> Open question: can I call register_current_timer_delay,
>> sched_clock_register, clocksource_register_hz in any order?
>> ---
> 
> Yes, I think so. Thomas ?
> 
> [ ... ]
> 
>> +static void __init tango_clocksource_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);
>> +	if (xtal_in_cnt == NULL)
>> +		panic("%s: of_iomap failed\n", np->full_name);
> 
>    ^^^^^^^^^^^
> 
> That does not comply with the Linux kernel coding style.

<confused>

scripts/checkpatch.pl only complains about a missing blank line after
the declaration block. (Sorry, I'll fix that.)

> 	xtal_in_cnt = of_iomap(np, 0);
> 	if (!xtal_in_cnt) {
> 		pr_err("Argh!");
> 		return;
> 	}

I know "!xtal_in_cnt" is equivalent to "xtal_in_cnt == NULL" but I'd
rather emphasize the fact that xtal_in_cnt is a pointer, not a bool.
(Documentation/CodingStyle does not mandate this particular idiom.)

I'm also confused that you've replaced panic() with pr_err/return.
AFAIU, if I don't have a clocksource/sched_clock, the system is dead
in the water. Might as well stop there, and wait for the operator to
fix whatever needs fixing. (Several clksrc drivers do this.)

> 	clk = of_clk_get(np, 0);
> 	if (!clk) {

AFAICT, checking for NULL is not good enough here.
of_clk_get returns ERR_PTR(rc) style errors.
Looks like I'd need "if (IS_ERR(clk))"

Thanks for the review.

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]


#1241640 — Re: [PATCH v2] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs

FromDaniel Lezcano <daniel.lezcano@linaro.org>
Date2015-10-07 18:10 +0200
SubjectRe: [PATCH v2] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs
Message-ID<qgZrZ-1jR-21@gated-at.bofh.it>
In reply to#1241477
On 10/07/2015 03:17 PM, Marc Gonzalez wrote:
> On 07/10/2015 14:31, Daniel Lezcano wrote:
>> On 10/07/2015 01:35 PM, Marc Gonzalez wrote:
>>> Sigma Designs Tango platforms provide a 27 MHz crystal oscillator.
>>> Use it for clocksource, sched_clock, and delay_timer.
>>>
>>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>>> ---
>>> AFAICS, clocksource_register_hz does not report failures via its
>>> return value (always 0) but writes warnings to stdout?
>>
>> Yeah, it returns always 0. I suggest you assume it is returning an error
>> code, that will be safer for future changes in the framework (if any).
>
> I suppose I'd also need to check the return value of of_clk_get?
> (Looks like you mention it implicitly below.)
>
>>> Open question: can I call register_current_timer_delay,
>>> sched_clock_register, clocksource_register_hz in any order?
>>> ---
>>
>> Yes, I think so. Thomas ?
>>
>> [ ... ]
>>
>>> +static void __init tango_clocksource_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);
>>> +	if (xtal_in_cnt == NULL)
>>> +		panic("%s: of_iomap failed\n", np->full_name);
>>
>>     ^^^^^^^^^^^
>>
>> That does not comply with the Linux kernel coding style.
>
> <confused>
>
> scripts/checkpatch.pl only complains about a missing blank line after
> the declaration block. (Sorry, I'll fix that.)
>
>> 	xtal_in_cnt = of_iomap(np, 0);
>> 	if (!xtal_in_cnt) {
>> 		pr_err("Argh!");
>> 		return;
>> 	}
>
> I know "!xtal_in_cnt" is equivalent to "xtal_in_cnt == NULL" but I'd
> rather emphasize the fact that xtal_in_cnt is a pointer, not a bool.
> (Documentation/CodingStyle does not mandate this particular idiom.)

Yes, if you prefer you can use != NULL.

> I'm also confused that you've replaced panic() with pr_err/return.
> AFAIU, if I don't have a clocksource/sched_clock, the system is dead
> in the water. Might as well stop there, and wait for the operator to
> fix whatever needs fixing. (Several clksrc drivers do this.)

Hmm, yeah that's true but also we have platforms with different 
clocksources, so we don't want to panic if the next clocksource will 
succeed. That's the logic behind not doing panic. There is some legacy 
code still using panic but that should be fixed.

I don't know if your platform can fall under this category, but it would 
be a good practice to pr_err or pr_warn instead of panic in order to be 
consistent with the current direction in the recent drivers.

>> 	clk = of_clk_get(np, 0);
>> 	if (!clk) {
>
> AFAICT, checking for NULL is not good enough here.
> of_clk_get returns ERR_PTR(rc) style errors.
> Looks like I'd need "if (IS_ERR(clk))"

Yep :)


   -- Daniel



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

--
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]


#1242330 — Re: [PATCH v2] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs

FromMason <slash.tmp@free.fr>
Date2015-10-08 15:20 +0200
SubjectRe: [PATCH v2] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs
Message-ID<qhjgZ-4AF-1@gated-at.bofh.it>
In reply to#1241640
On 07/10/2015 18:03, Daniel Lezcano wrote:

> On 10/07/2015 03:17 PM, Marc Gonzalez wrote:
>
>> I'm also confused that you've replaced panic() with pr_err/return.
>> AFAIU, if I don't have a clocksource/sched_clock, the system is dead
>> in the water. Might as well stop there, and wait for the operator to
>> fix whatever needs fixing. (Several clksrc drivers do this.)
> 
> Hmm, yeah that's true but also we have platforms with different 
> clocksources, so we don't want to panic if the next clocksource will 
> succeed. That's the logic behind not doing panic. There is some legacy 
> code still using panic but that should be fixed.

There's so much legacy code lying around that it's really hard
to tell what the current best practices are :-(

> I don't know if your platform can fall under this category, but it would 
> be a good practice to pr_err or pr_warn instead of panic in order to be 
> consistent with the current direction in the recent drivers.

I think the system falls back to using the "jiffies clock" when
it doesn't find anything better? However, on my system, the
clockevent device is running at cpuclk/2, so once I add cpufreq
DFS into the mix, I don't think the "jiffies" clock is a very
good clocksource.

Anyway, updated patch is on the way.

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]


#1243306 — [PATCH v3] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs

FromMarc Gonzalez <marc_gonzalez@sigmadesigns.com>
Date2015-10-09 14:30 +0200
Subject[PATCH v3] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs
Message-ID<qhEYa-21U-7@gated-at.bofh.it>
In reply to#1242330
Sigma Designs Tango platforms provide a 27 MHz crystal oscillator.
Use it for clocksource, sched_clock, and delay_timer.

Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
I have a nagging feeling that the QUIT_IF macro will get this patch NAKed ;-)
My rationale: error-handling tends to take the focus away from the normal
path, and put it on the error path. Hiding the details away in a macro
helps to keep the error-handling noise to a minimum.
---
 drivers/clocksource/Kconfig      |  4 +++
 drivers/clocksource/Makefile     |  1 +
 drivers/clocksource/tango_xtal.c | 61 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 66 insertions(+)
 create mode 100644 drivers/clocksource/tango_xtal.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 4e57730e0be4..63c878d4d9e3 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -275,6 +275,10 @@ config CLKSRC_MIPS_GIC
 	depends on MIPS_GIC
 	select CLKSRC_OF
 
+config CLKSRC_TANGO_XTAL
+	bool
+	select CLKSRC_OF
+
 config CLKSRC_PXA
 	def_bool y if ARCH_PXA || ARCH_SA1100
 	select CLKSRC_OF if USE_OF
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index f228354961ca..160970a3963b 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_ARCH_KEYSTONE)		+= timer-keystone.o
 obj-$(CONFIG_ARCH_INTEGRATOR_AP)	+= timer-integrator-ap.o
 obj-$(CONFIG_CLKSRC_VERSATILE)		+= versatile.o
 obj-$(CONFIG_CLKSRC_MIPS_GIC)		+= mips-gic-timer.o
+obj-$(CONFIG_CLKSRC_TANGO_XTAL)		+= tango_xtal.o
 obj-$(CONFIG_CLKSRC_IMX_GPT)		+= timer-imx-gpt.o
 obj-$(CONFIG_ASM9260_TIMER)		+= asm9260_timer.o
 obj-$(CONFIG_H8300)			+= h8300_timer8.o
diff --git a/drivers/clocksource/tango_xtal.c b/drivers/clocksource/tango_xtal.c
new file mode 100644
index 000000000000..a3c265eb7236
--- /dev/null
+++ b/drivers/clocksource/tango_xtal.c
@@ -0,0 +1,61 @@
+#include <linux/clocksource.h>
+#include <linux/sched_clock.h>
+#include <linux/of_address.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/clk.h>
+
+#define QUIT_IF(cond, fmt, ...) do if (cond) {		\
+	static const char format[] __initconst = fmt;	\
+	pr_err(format, ## __VA_ARGS__);			\
+	return;						\
+} while (0)
+
+static void __iomem *xtal_in_cnt;
+static struct delay_timer delay_timer;
+
+static unsigned long notrace read_xtal_counter(void)
+{
+	return readl_relaxed(xtal_in_cnt);
+}
+
+static u64 notrace 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_clocksource_init(struct device_node *np)
+{
+	struct clk *clk;
+	int xtal_freq, err;
+
+	xtal_in_cnt = of_iomap(np, 0);
+	QUIT_IF(xtal_in_cnt == NULL, "%s: invalid address\n", np->full_name);
+
+	clk = of_clk_get(np, 0);
+	QUIT_IF(IS_ERR(clk), "%s: invalid clock\n", np->full_name);
+
+	xtal_freq = clk_get_rate(clk);
+	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);
+	err = clocksource_register_hz(&tango_xtal, xtal_freq);
+	QUIT_IF(err, "%s: registration failed\n", np->full_name);
+}
+
+CLOCKSOURCE_OF_DECLARE(tango, "sigma,tick-counter", tango_clocksource_init);
-- 
2.4.5

--
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]


#1243368 — Re: [PATCH v3] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs

FromDaniel Lezcano <daniel.lezcano@linaro.org>
Date2015-10-09 15:30 +0200
SubjectRe: [PATCH v3] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs
Message-ID<qhFUe-3mZ-15@gated-at.bofh.it>
In reply to#1243306
On 10/09/2015 02:13 PM, Marc Gonzalez wrote:
> Sigma Designs Tango platforms provide a 27 MHz crystal oscillator.
> Use it for clocksource, sched_clock, and delay_timer.
>
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---
> I have a nagging feeling that the QUIT_IF macro will get this patch NAKed ;-)
> My rationale: error-handling tends to take the focus away from the normal
> path, and put it on the error path. Hiding the details away in a macro
> helps to keep the error-handling noise to a minimum.

It is a right feeling :)

The Linux kernel code follows the same path all across the different 
sub-systems. So it is not a problem to write: if (err) ..., people is 
used to read such code and by introducing this macro, that makes the 
code less readable for them.

Moreover, the way you wrote the macro is strongly discouraged in the 
CodingStyle document because there is a 'return' inside.

   -- DAniel


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

--
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]


#1243375 — Re: [PATCH v3] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs

FromMarc Gonzalez <marc_gonzalez@sigmadesigns.com>
Date2015-10-09 15:50 +0200
SubjectRe: [PATCH v3] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs
Message-ID<qhGdz-3Jr-3@gated-at.bofh.it>
In reply to#1243368
On 09/10/2015 15:24, Daniel Lezcano wrote:
> On 10/09/2015 02:13 PM, Marc Gonzalez wrote:
>> Sigma Designs Tango platforms provide a 27 MHz crystal oscillator.
>> Use it for clocksource, sched_clock, and delay_timer.
>>
>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>> ---
>> I have a nagging feeling that the QUIT_IF macro will get this patch NAKed ;-)
>> My rationale: error-handling tends to take the focus away from the normal
>> path, and put it on the error path. Hiding the details away in a macro
>> helps to keep the error-handling noise to a minimum.
> 
> It is a right feeling :)
> 
> The Linux kernel code follows the same path all across the different 
> sub-systems. So it is not a problem to write: if (err) ..., people is 
> used to read such code and by introducing this macro, that makes the 
> code less readable for them.
> 
> Moreover, the way you wrote the macro is strongly discouraged in the 
> CodingStyle document because there is a 'return' inside.

New patch coming right up.

On a tangential subject, it would seem that platforms with verbose logs
at init might benefit from marking as __initconst strings used in __init
functions.

I discussed this some time ago:
https://lkml.org/lkml/2015/3/25/688

which pointed to an earlier discussion:
https://lkml.org/lkml/2014/8/21/255
https://lkml.org/lkml/2014/6/22/149

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]


#1243414 — Re: [PATCH v3] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs

FromDaniel Lezcano <daniel.lezcano@linaro.org>
Date2015-10-09 16:30 +0200
SubjectRe: [PATCH v3] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs
Message-ID<qhGQi-4IM-11@gated-at.bofh.it>
In reply to#1243375
On 10/09/2015 03:46 PM, Marc Gonzalez wrote:
> On 09/10/2015 15:24, Daniel Lezcano wrote:

[ ... ]

> On a tangential subject, it would seem that platforms with verbose logs
> at init might benefit from marking as __initconst strings used in __init
> functions.
>
> I discussed this some time ago:
> https://lkml.org/lkml/2015/3/25/688
>
> which pointed to an earlier discussion:
> https://lkml.org/lkml/2014/8/21/255
> https://lkml.org/lkml/2014/6/22/149

Thanks for the pointer.

Nicolas Pitre (Cc'ed) is working on optimizing the memory consumption by 
stripping unreferenced code at compile time. Perhaps he thought a 
solution for this particular use case.


   -- Daniel

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

--
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]


#1243525 — Re: [PATCH v3] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs

FromNicolas Pitre <nicolas.pitre@linaro.org>
Date2015-10-09 18:40 +0200
SubjectRe: [PATCH v3] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs
Message-ID<qhIS8-7BS-49@gated-at.bofh.it>
In reply to#1243414
On Fri, 9 Oct 2015, Daniel Lezcano wrote:

> On 10/09/2015 03:46 PM, Marc Gonzalez wrote:
> > On 09/10/2015 15:24, Daniel Lezcano wrote:
> 
> [ ... ]
> 
> > On a tangential subject, it would seem that platforms with verbose logs
> > at init might benefit from marking as __initconst strings used in __init
> > functions.
> > 
> > I discussed this some time ago:
> > https://lkml.org/lkml/2015/3/25/688
> > 
> > which pointed to an earlier discussion:
> > https://lkml.org/lkml/2014/8/21/255
> > https://lkml.org/lkml/2014/6/22/149
> 
> Thanks for the pointer.
> 
> Nicolas Pitre (Cc'ed) is working on optimizing the memory consumption by
> stripping unreferenced code at compile time. Perhaps he thought a solution for
> this particular use case.

This is different.  Here those strings are actually referenced and can't 
be discarded.  The solution is to mark them so they go in the 
appropriate section as suggested.

It might be worth investigating the addition of a new gcc function 
attribute that provides a replacement section for .data, .rodata, etc. 
in addition to .text, that would apply to data objects created within 
that function.


Nicolas
--
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]


#1243429 — [PATCH v4] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs

FromMarc Gonzalez <marc_gonzalez@sigmadesigns.com>
Date2015-10-09 16:40 +0200
Subject[PATCH v4] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs
Message-ID<qhGZY-4Tm-3@gated-at.bofh.it>
In reply to#1243368
Sigma Designs Tango platforms provide a 27 MHz crystal oscillator.
Use it for clocksource, sched_clock, and delay_timer.

Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
 drivers/clocksource/Kconfig      |  4 +++
 drivers/clocksource/Makefile     |  1 +
 drivers/clocksource/tango_xtal.c | 71 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+)
 create mode 100644 drivers/clocksource/tango_xtal.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 4e57730e0be4..63c878d4d9e3 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -275,6 +275,10 @@ config CLKSRC_MIPS_GIC
 	depends on MIPS_GIC
 	select CLKSRC_OF
 
+config CLKSRC_TANGO_XTAL
+	bool
+	select CLKSRC_OF
+
 config CLKSRC_PXA
 	def_bool y if ARCH_PXA || ARCH_SA1100
 	select CLKSRC_OF if USE_OF
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index f228354961ca..160970a3963b 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_ARCH_KEYSTONE)		+= timer-keystone.o
 obj-$(CONFIG_ARCH_INTEGRATOR_AP)	+= timer-integrator-ap.o
 obj-$(CONFIG_CLKSRC_VERSATILE)		+= versatile.o
 obj-$(CONFIG_CLKSRC_MIPS_GIC)		+= mips-gic-timer.o
+obj-$(CONFIG_CLKSRC_TANGO_XTAL)		+= tango_xtal.o
 obj-$(CONFIG_CLKSRC_IMX_GPT)		+= timer-imx-gpt.o
 obj-$(CONFIG_ASM9260_TIMER)		+= asm9260_timer.o
 obj-$(CONFIG_H8300)			+= h8300_timer8.o
diff --git a/drivers/clocksource/tango_xtal.c b/drivers/clocksource/tango_xtal.c
new file mode 100644
index 000000000000..da67bc2aec63
--- /dev/null
+++ b/drivers/clocksource/tango_xtal.c
@@ -0,0 +1,71 @@
+#include <linux/clocksource.h>
+#include <linux/sched_clock.h>
+#include <linux/of_address.h>
+#include <linux/printk.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/clk.h>
+
+#define QUIT_IF(cond, fmt, ...) do if (cond) {		\
+	static const char format[] __initconst = fmt;	\
+	pr_err(format, ## __VA_ARGS__);			\
+	return;						\
+} while (0)
+
+static void __iomem *xtal_in_cnt;
+static struct delay_timer delay_timer;
+
+static unsigned long notrace read_xtal_counter(void)
+{
+	return readl_relaxed(xtal_in_cnt);
+}
+
+static u64 notrace 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_clocksource_init(struct device_node *np)
+{
+	struct clk *clk;
+	int xtal_freq, ret;
+
+	xtal_in_cnt = of_iomap(np, 0);
+	if (xtal_in_cnt == NULL) {
+		pr_err("%s: invalid address\n", np->full_name);
+		return;
+	}
+
+	clk = of_clk_get(np, 0);
+	if (IS_ERR(clk)) {
+		pr_err("%s: invalid clock\n", np->full_name);
+		return;
+	}
+
+	xtal_freq = clk_get_rate(clk);
+	delay_timer.freq = xtal_freq;
+	delay_timer.read_current_timer = read_xtal_counter;
+
+	ret = clocksource_register_hz(&tango_xtal, xtal_freq);
+	if (ret != 0) {
+		pr_err("%s: registration failed\n", np->full_name);
+		return;
+	}
+	sched_clock_register(read_sched_clock, 32, xtal_freq);
+	register_current_timer_delay(&delay_timer);
+}
+
+CLOCKSOURCE_OF_DECLARE(tango, "sigma,tick-counter", tango_clocksource_init);
-- 
2.4.5

--
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]


#1243445 — Re: [PATCH v4] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs

FromDaniel Lezcano <daniel.lezcano@linaro.org>
Date2015-10-09 16:50 +0200
SubjectRe: [PATCH v4] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs
Message-ID<qhH9E-54I-19@gated-at.bofh.it>
In reply to#1243429
On 10/09/2015 04:37 PM, Marc Gonzalez wrote:
> Sigma Designs Tango platforms provide a 27 MHz crystal oscillator.
> Use it for clocksource, sched_clock, and delay_timer.
>
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---

[ ... ]

> +
> +#define QUIT_IF(cond, fmt, ...) do if (cond) {		\
> +	static const char format[] __initconst = fmt;	\
> +	pr_err(format, ## __VA_ARGS__);			\
> +	return;						\
> +} while (0)

hmm ... forget something ?

> +
> +	ret = clocksource_register_hz(&tango_xtal, xtal_freq);
> +	if (ret != 0) {
> +		pr_err("%s: registration failed\n", np->full_name);
> +		return;
> +	}

missing line.

> +	sched_clock_register(read_sched_clock, 32, xtal_freq);
> +	register_current_timer_delay(&delay_timer);
> +}
> +
> +CLOCKSOURCE_OF_DECLARE(tango, "sigma,tick-counter", tango_clocksource_init);
>


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

--
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]


#1243447 — [PATCH v5] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs

FromMarc Gonzalez <marc_gonzalez@sigmadesigns.com>
Date2015-10-09 17:00 +0200
Subject[PATCH v5] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs
Message-ID<qhHjj-5g5-1@gated-at.bofh.it>
In reply to#1243445
Sigma Designs Tango platforms provide a 27 MHz crystal oscillator.
Use it for clocksource, sched_clock, and delay_timer.

Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
 drivers/clocksource/Kconfig      |  4 +++
 drivers/clocksource/Makefile     |  1 +
 drivers/clocksource/tango_xtal.c | 66 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+)
 create mode 100644 drivers/clocksource/tango_xtal.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 4e57730e0be4..63c878d4d9e3 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -275,6 +275,10 @@ config CLKSRC_MIPS_GIC
 	depends on MIPS_GIC
 	select CLKSRC_OF
 
+config CLKSRC_TANGO_XTAL
+	bool
+	select CLKSRC_OF
+
 config CLKSRC_PXA
 	def_bool y if ARCH_PXA || ARCH_SA1100
 	select CLKSRC_OF if USE_OF
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index f228354961ca..160970a3963b 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_ARCH_KEYSTONE)		+= timer-keystone.o
 obj-$(CONFIG_ARCH_INTEGRATOR_AP)	+= timer-integrator-ap.o
 obj-$(CONFIG_CLKSRC_VERSATILE)		+= versatile.o
 obj-$(CONFIG_CLKSRC_MIPS_GIC)		+= mips-gic-timer.o
+obj-$(CONFIG_CLKSRC_TANGO_XTAL)		+= tango_xtal.o
 obj-$(CONFIG_CLKSRC_IMX_GPT)		+= timer-imx-gpt.o
 obj-$(CONFIG_ASM9260_TIMER)		+= asm9260_timer.o
 obj-$(CONFIG_H8300)			+= h8300_timer8.o
diff --git a/drivers/clocksource/tango_xtal.c b/drivers/clocksource/tango_xtal.c
new file mode 100644
index 000000000000..d297b30d2bc0
--- /dev/null
+++ b/drivers/clocksource/tango_xtal.c
@@ -0,0 +1,66 @@
+#include <linux/clocksource.h>
+#include <linux/sched_clock.h>
+#include <linux/of_address.h>
+#include <linux/printk.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/clk.h>
+
+static void __iomem *xtal_in_cnt;
+static struct delay_timer delay_timer;
+
+static unsigned long notrace read_xtal_counter(void)
+{
+	return readl_relaxed(xtal_in_cnt);
+}
+
+static u64 notrace 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_clocksource_init(struct device_node *np)
+{
+	struct clk *clk;
+	int xtal_freq, ret;
+
+	xtal_in_cnt = of_iomap(np, 0);
+	if (xtal_in_cnt == NULL) {
+		pr_err("%s: invalid address\n", np->full_name);
+		return;
+	}
+
+	clk = of_clk_get(np, 0);
+	if (IS_ERR(clk)) {
+		pr_err("%s: invalid clock\n", np->full_name);
+		return;
+	}
+
+	xtal_freq = clk_get_rate(clk);
+	delay_timer.freq = xtal_freq;
+	delay_timer.read_current_timer = read_xtal_counter;
+
+	ret = clocksource_register_hz(&tango_xtal, xtal_freq);
+	if (ret != 0) {
+		pr_err("%s: registration failed\n", np->full_name);
+		return;
+	}
+
+	sched_clock_register(read_sched_clock, 32, xtal_freq);
+	register_current_timer_delay(&delay_timer);
+}
+
+CLOCKSOURCE_OF_DECLARE(tango, "sigma,tick-counter", tango_clocksource_init);
-- 
2.4.5

--
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]


#1248678 — Re: [PATCH v5] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs

FromDaniel Lezcano <daniel.lezcano@linaro.org>
Date2015-10-16 14:10 +0200
SubjectRe: [PATCH v5] clocksource/drivers/tango_xtal: Add new timer for Tango SoCs
Message-ID<qkbZF-5OX-27@gated-at.bofh.it>
In reply to#1243447
On 10/09/2015 04:59 PM, Marc Gonzalez wrote:
> Sigma Designs Tango platforms provide a 27 MHz crystal oscillator.
> Use it for clocksource, sched_clock, and delay_timer.
>
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---

Applied to my tree for 4.4

Thanks !

   -- Daniel

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

--
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]


Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel


csiph-web