Re: [PATCH 0/9] clocksource/drivers/clksrc-of: Improve error handling

2016-06-07 Thread Daniel Lezcano 

On 06/07/2016 11:54 AM, Geert Uytterhoeven wrote:

Hi Daniel,


Hi Geert,

[ ... ]


Using "earlycon keep_bootcon" on koelsch (this doesn't help on arm64)
reveals it's stuck at:

 clocksource_probe: no matching clocksources found
 sched_clock: 32 bits at 100 Hz, resolution 1000ns, wraps every
2147483647500ns
 Calibrating delay loop...



Ok, so the "renesas,cmt-48-gen2" is not impacted by the changes because 
it uses the platform approach. Very likely, the issue is coming from the 
arch_arm_timer failing somewhere.


Or I added a regression and the timer is wrongly returning an error, or 
the error is always there and now it is caught.


Can you give the traces before 'clocksource_probe: no matching 
clocksources found' ?


  -- Daniel

--
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH 0/9] clocksource/drivers/clksrc-of: Improve error handling

2016-06-08 Thread Daniel Lezcano 

On 06/07/2016 11:54 AM, Geert Uytterhoeven wrote:

[ ... ]


in clockevents/clockevents/next breaks the boot on e.g. r8a7791/koelsch
(arm32) and r8a7795/salvator-x (arm64).

Using "earlycon keep_bootcon" on koelsch (this doesn't help on arm64)
reveals it's stuck at:

 clocksource_probe: no matching clocksources found
 sched_clock: 32 bits at 100 Hz, resolution 1000ns, wraps every
2147483647500ns
 Calibrating delay loop...

With the above commit reverted, it works again:

 Architected cp15 timer(s) running at 10.00MHz (virt).
 clocksource: arch_sys_counter: mask: 0xff max_cycles:
0x24e6a1710, max_idle_ns: 440795202120 ns
 sched_clock: 56 bits at 10MHz, resolution 100ns, wraps every 
4398046511100ns
 Switching to timer-based delay loop, resolution 100ns
 Calibrating delay loop (skipped), value calculated using timer
frequency.. 20.00 BogoMIPS (lpj=10)


I think it is fixed now and pushed on my tree. Is it possible to confirm 
your boards are working correctly again after the linux-next is updated 
with my latest changes ?


Thanks for the report.

  -- Daniel


--
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

[PATCH 06/10] clocksource/drivers/ostm: Add renesas-ostm timer driver

2017-02-08 Thread Daniel Lezcano 
From: Chris Brandt <chris.bra...@renesas.com>

This patch adds a OSTM driver for the Renesas architecture.
The OS Timer (OSTM) has independent channels that can be
used as a freerun or interval times.
This driver uses the first probed device as a clocksource
and then any additional devices as clock events.

Signed-off-by: Chris Brandt <chris.bra...@renesas.com>
Signed-off-by: Daniel Lezcano <daniel.lezc...@linaro.org>
---
 arch/arm/mach-shmobile/Kconfig |   1 +
 drivers/clocksource/Kconfig|   7 +
 drivers/clocksource/Makefile   |   1 +
 drivers/clocksource/renesas-ostm.c | 265 +
 4 files changed, 274 insertions(+)
 create mode 100644 drivers/clocksource/renesas-ostm.c

diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
index 2bb4b09..ad7d604 100644
--- a/arch/arm/mach-shmobile/Kconfig
+++ b/arch/arm/mach-shmobile/Kconfig
@@ -57,6 +57,7 @@ config ARCH_R7S72100
select PM
select PM_GENERIC_DOMAINS
select SYS_SUPPORTS_SH_MTU2
+   select RENESAS_OSTM
 
 config ARCH_R8A73A4
bool "R-Mobile APE6 (R8A73A40)"
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index afef0e8..4002d6d 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -484,6 +484,13 @@ config SH_TIMER_MTU2
  Timer Pulse Unit 2 (MTU2) hardware available on SoCs from Renesas.
  This hardware comes with 16 bit-timer registers.
 
+config RENESAS_OSTM
+   bool "Renesas OSTM timer driver" if COMPILE_TEST
+   depends on GENERIC_CLOCKEVENTS
+   select CLKSRC_MMIO
+   help
+ Enables the support for the Renesas OSTM.
+
 config SH_TIMER_TMU
bool "Renesas TMU timer driver" if COMPILE_TEST
depends on GENERIC_CLOCKEVENTS
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index dbbee80..d227d13 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_CS5535_CLOCK_EVENT_SRC)+= cs5535-clockevt.o
 obj-$(CONFIG_CLKSRC_JCORE_PIT) += jcore-pit.o
 obj-$(CONFIG_SH_TIMER_CMT) += sh_cmt.o
 obj-$(CONFIG_SH_TIMER_MTU2)+= sh_mtu2.o
+obj-$(CONFIG_RENESAS_OSTM) += renesas-ostm.o
 obj-$(CONFIG_SH_TIMER_TMU) += sh_tmu.o
 obj-$(CONFIG_EM_TIMER_STI) += em_sti.o
 obj-$(CONFIG_CLKBLD_I8253) += i8253.o
diff --git a/drivers/clocksource/renesas-ostm.c 
b/drivers/clocksource/renesas-ostm.c
new file mode 100644
index 000..c76f576
--- /dev/null
+++ b/drivers/clocksource/renesas-ostm.c
@@ -0,0 +1,265 @@
+/*
+ * Renesas Timer Support - OSTM
+ *
+ * Copyright (C) 2017 Renesas Electronics America, Inc.
+ * Copyright (C) 2017 Chris Brandt
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * The OSTM contains independent channels.
+ * The first OSTM channel probed will be set up as a free running
+ * clocksource. Additionally we will use this clocksource for the system
+ * schedule timer sched_clock().
+ *
+ * The second (or more) channel probed will be set up as an interrupt
+ * driven clock event.
+ */
+
+struct ostm_device {
+   void __iomem *base;
+   unsigned long ticks_per_jiffy;
+   struct clock_event_device ced;
+};
+
+static void __iomem *system_clock; /* For sched_clock() */
+
+/* OSTM REGISTERS */
+#defineOSTM_CMP0x000   /* RW,32 */
+#defineOSTM_CNT0x004   /* R,32 */
+#defineOSTM_TE 0x010   /* R,8 */
+#defineOSTM_TS 0x014   /* W,8 */
+#defineOSTM_TT 0x018   /* W,8 */
+#defineOSTM_CTL0x020   /* RW,8 */
+
+#defineTE  0x01
+#defineTS  0x01
+#defineTT  0x01
+#defineCTL_PERIODIC0x00
+#defineCTL_ONESHOT 0x02
+#defineCTL_FREERUN 0x02
+
+static struct ostm_device *ced_to_ostm(struct clock_event_device *ced)
+{
+   return container_of(ced, struct ostm_device, ced);
+}
+
+static void ostm_timer_stop(struct ostm_device *ostm)
+{
+   if (readb(ostm->base + OSTM_TE) & TE) {
+   writeb(TT, ostm->base + OSTM_TT);
+
+   /*
+* Read back the register simply to confirm the write operation
+* has completed since I/O writes can somet

Re: [PATCH v3 2/2] clocksource: Add renesas-ostm timer driver

2017-01-25 Thread Daniel Lezcano 
On Tue, Jan 24, 2017 at 08:19:50PM +, Chris Brandt wrote:
> Hi Daniel,
> 
> On Tuesday, January 24, 2017, Daniel Lezcano wrote:
> > > > > +early_platform_init("earlytimer", _timer);
> > > > > +subsys_initcall(ostm_init); module_exit(ostm_exit);
> > > > > +
> > > > > +MODULE_AUTHOR("Chris Brandt");
> > > > > +MODULE_DESCRIPTION("Renesas OSTM Timer Driver");
> > > > > +MODULE_LICENSE("GPL v2");
> > > >
> > > > Maybe you can try with builtin_platform ?
> > >
> > > Good idea. But, now I get a "Section mismatch" during link time so
> > > I'll have to figure out why that is.
> > 
> > Mmh, I think it would be more consistent to convert this to:
> > 
> > CLOCKSOURCE_OF_DECLARE(ostm, "renesas,ostm", ostm_init);
> > 
> > The only problem is to get the struct device associated to the of_node
> > passed as parameter to ostm_init in order to use the devm_* API.
> > 
> > I think of_find_device_by_node should return the platform_device, then
> > pdev->dev. If that works the other drivers will benefit from that to
> > pdev->remove all
> > the rollback code everywhere.
> 
> 
> It was a good ideabut it will not work.
> 
> While CLOCKSOURCE_OF_DECLARE is good at putting the driver at the
> front of the line for loading, it's too early in boot to detect
> a platform_device.
> 
> of_find_device_by_node calls bus_find_device. But the first thing that
> bus_find_device does is:
> 
>   if (!bus || !bus->p)
>   return NULL;
> 
> But bus->p=0 so it returns immediately.
> 
> 
> Of course changing the code over to:
> devm_kzalloc -> devm_kzalloc
> devm_ioremap_nocache -> of_io_request_and_map
> platform_get_irq -> irq_of_parse_and_map
>  dev_err -> pr_err
> 
> 
> Then things work, but I'm back to managing the rollback code manually.
> 
> 
> Any other ideas on how to get the corresponding platform_device for
> a DT node?

No :/

So up to you.
- CLOCKSOURCE_OF_DECLARE consistent but need rollback
or
- platform_device but with another timer available at early time

-- 

 <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 v7 0/2] clocksource: Add renesas-ostm timer driver

2017-01-30 Thread Daniel Lezcano 
On Fri, Jan 27, 2017 at 03:02:13PM -0500, Chris Brandt wrote:
> This patch set adds a new clocksource driver that uses the OS Timer
> (OSTM) that exists in the R7S72100 (RZ/A1) SoC.
> 
> The operation of the driver was tested with a simple user application
> that does multiple calls to nanosleep() and gettimeofday().
> 
> The purpose of adding this driver is to get better time keeping
> accuracy over the default MTU2 clocksource timer.
> 
> v7:
> * initialize *ostm_clk=NULL to remove 'may be used uninitialized'
>   warning. Reported by "kbuild test robot"

Hi Chris,

I queued up the patches for 4.11.

Thanks !

  -- Daniel

Re: [PATCH v3 2/2] clocksource: Add renesas-ostm timer driver

2017-01-23 Thread Daniel Lezcano 
On Mon, Jan 23, 2017 at 08:54:23AM -0500, Chris Brandt wrote:
> This patch adds a OSTM driver for the Renesas architecture.

As it is a new driver, please give technical details for the log.
 
Replace ioread/write by readl/writel in the code.

> Signed-off-by: Chris Brandt 
> 
> ---
> v2:
> * changed implementation to be independent channel nodes
> ---
>  arch/arm/mach-shmobile/Kconfig |   1 +
>  drivers/clocksource/Kconfig|  12 ++
>  drivers/clocksource/Makefile   |   1 +
>  drivers/clocksource/renesas-ostm.c | 349 
> +
>  4 files changed, 363 insertions(+)
>  create mode 100644 drivers/clocksource/renesas-ostm.c
> 
> diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> index 2bb4b09..b928634 100644
> --- a/arch/arm/mach-shmobile/Kconfig
> +++ b/arch/arm/mach-shmobile/Kconfig
> @@ -57,6 +57,7 @@ config ARCH_R7S72100
>   select PM
>   select PM_GENERIC_DOMAINS
>   select SYS_SUPPORTS_SH_MTU2
> + select SYS_SUPPORTS_RENESAS_OSTM

- select SYS_SUPPORTS_RENESAS_OSTM
+ select RENESAS_OSTM

>  
>  config ARCH_R8A73A4
>   bool "R-Mobile APE6 (R8A73A40)"
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 4866f7a..95c8d56 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -431,6 +431,9 @@ config MTK_TIMER
>  config SYS_SUPPORTS_SH_MTU2
>  bool
>  
> +config SYS_SUPPORTS_RENESAS_OSTM
> +bool
> +

-config SYS_SUPPORTS_RENESAS_OSTM
-bool
-

>  config SYS_SUPPORTS_SH_TMU
>  bool
>  
> @@ -467,6 +470,15 @@ config SH_TIMER_MTU2
> Timer Pulse Unit 2 (MTU2) hardware available on SoCs from Renesas.
> This hardware comes with 16 bit-timer registers.
>  
> +config RENESAS_OSTM
> + bool "Renesas OSTM timer driver" if COMPILE_TEST
> + depends on GENERIC_CLOCKEVENTS
> + select CLKSRC_MMIO
> + default SYS_SUPPORTS_RENESAS_OSTM

- default SYS_SUPPORTS_RENESAS_OSTM

Except I missing something, I don' the point of having this intermediate
config option.

> + help
> +   This enables the build of the OSTM timer driver.
> +   It creates a clock source and clock event device.
> +
>  config SH_TIMER_TMU
>   bool "Renesas TMU timer driver" if COMPILE_TEST
>   depends on GENERIC_CLOCKEVENTS
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index a14111e..bbd163b 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_CS5535_CLOCK_EVENT_SRC)  += cs5535-clockevt.o
>  obj-$(CONFIG_CLKSRC_JCORE_PIT)   += jcore-pit.o
>  obj-$(CONFIG_SH_TIMER_CMT)   += sh_cmt.o
>  obj-$(CONFIG_SH_TIMER_MTU2)  += sh_mtu2.o
> +obj-$(CONFIG_RENESAS_OSTM)   += renesas-ostm.o
>  obj-$(CONFIG_SH_TIMER_TMU)   += sh_tmu.o
>  obj-$(CONFIG_EM_TIMER_STI)   += em_sti.o
>  obj-$(CONFIG_CLKBLD_I8253)   += i8253.o
> diff --git a/drivers/clocksource/renesas-ostm.c 
> b/drivers/clocksource/renesas-ostm.c
> new file mode 100644
> index 000..37f2461
> --- /dev/null
> +++ b/drivers/clocksource/renesas-ostm.c
> @@ -0,0 +1,349 @@
> +/*
> + * Renesas Timer Support - OSTM
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Please remove everything module related here and below. This driver is not
supposed to be removed and it is compiled in with the Kconfig option.

> +#include 
> +#include 
> +
> +/*
> + * The OSTM contains independent channels.
> + * The first OSTM channel probed will be set up as a free running
> + * clocksource. Additionally we will use this clocksource for the system
> + * schedule timer sched_clock().
> + *
> + * The second (or more) channel probed will be set up as an interrupt
> + * driven clock event.
> + */
> +
> +struct ostm_device {
> + struct platform_device *pdev;
> + int irq;
> + struct clk *clk;
> + unsigned long rate;
> + void __iomem *base;
> + unsigned long ticks_per_jiffy;
> + struct clock_event_device ced;
> +};

You can probably reduce the size of this structure by removing some fields
which are used only at init time.

> +
> +static void __iomem *system_clock;   /* For sched_clock() */
> +
> +/* OSTM REGISTERS */
> +#define  OSTM_CMP0x000   /* RW,32 */
> +#define  OSTM_CNT0x004   /* R,32 */
> +#define  OSTM_TE 0x010   /* R,8 */
> +#define  OSTM_TS 0x014

Re: [PATCH v3 2/2] clocksource: Add renesas-ostm timer driver

2017-01-24 Thread Daniel Lezcano 
On Tue, Jan 24, 2017 at 04:45:47AM +, Chris Brandt wrote:

Hi Chris,

[ ... ]

> > > + bool "Renesas OSTM timer driver" if COMPILE_TEST
> > > + depends on GENERIC_CLOCKEVENTS
> > > + select CLKSRC_MMIO
> > > + default SYS_SUPPORTS_RENESAS_OSTM
> > 
> > - default SYS_SUPPORTS_RENESAS_OSTM
> > 
> > Except I missing something, I don' the point of having this intermediate
> > config option.
> 
> I was basically following what all the other clocksource drivers do.
> Selecting an ARCH will then set SYS_SUPPORTS_xxTIMERxx which will then
> force xxTIMERxx=y.

Yeah, little by little these options are clean up to be more consistent across
the different drivers. The sh/mtu drivers are different from the other drivers.

The idea with the config option is to let the platform to select the drivers,
and optionnaly allows the compilation on other architecture to increase the
compilation test coverage. That is the reason of COMPILE_TEST.

> But, if "COMPILE_TEST" is set, you can choose what clocksource timers
> you want to build in.
> 
> Is your suggestion to do away with the COMPILE_TEST ability and
> just force RENESAS_OSTM=y if ARCH_R7S72100 is selected?

Just like that:

config RENESAS_OSTM
bool "Renesas OSTM timer driver" if COMPILE_TEST
depends on GENERIC_CLOCKEVENTS
select CLKSRC_MMIO
help
  Enables the support for the Renesas OSTM

And then from arch/arm/mach-shmobile/Kconfig:

select RENESAS_OSTM


> > > +#include 
> > 
> > Please remove everything module related here and below. This driver is not
> > supposed to be removed and it is compiled in with the Kconfig option.
> 
> Good point. I will remove.
> 
> I guess if you can't compile it as a module, you don't need things like
> 'MODULE_LICENSE'.

Right.

[ ... ]

> > > +static int __init ostm_init_clksrc(struct ostm_device *ostm) {
> > > + int ret;
> > > +
> > > + /* irq not used (clock sources don't use interrupts) */
> > > +
> > > + /* stop counter */
> > 
> > One line comment is usually in the network code. Please use multiline.
> > 
> > /*
> >  * Bla bla
> >  */
> 
> I'm confused. Do you mean:
> 
> A) use multiline formatting for every single-line comment throughout
>the file?
> 
> B) use multiline for any place where you have 2 or more single-line
>comments back to back?

I think it is simpler if you just ignore my comment, remove all your comments
in this function, implement a couple of wrapper (eg. clksrc_stop, clksrc_start)
which will speak by themselves.

By the way, is it normal stopping the clockevent is the same code than stopping
the clocksource ?

[ ... ]

> > > + if (!is_early_platform_device(pdev)) {
> > > + pm_runtime_set_active(>dev);
> > > + pm_runtime_enable(>dev);
> > > + }
> > 
> > Can you clarify why the 'early' is needed here ?
> 
> Actually, it means nothing.
> 
> I was using sh_mtu2.c and sh_cmt.c as reference and they were registering
> an "earlytimer" which made me think the kernel would probe this driver
> early in bootbutnow I see that is just a SH4 kernel specific thing.
> 
> So, I will remove all the early platform reference stuff.
> 
> Of course I still have the question of how are you supposed to get a
> clocksource up and running early in boot. (but I'll figure that out later).

Until the hardware clocksource is registered, the jiffies are used as source of
time. That happens at the very early boot time. The clockevent must be
registered early to update the jiffies but you won't have to care about that if
you use the macro below.

[ ... ]

> > > +early_platform_init("earlytimer", _timer);
> > > +subsys_initcall(ostm_init); module_exit(ostm_exit);
> > > +
> > > +MODULE_AUTHOR("Chris Brandt");
> > > +MODULE_DESCRIPTION("Renesas OSTM Timer Driver"); MODULE_LICENSE("GPL
> > > +v2");
> > 
> > Maybe you can try with builtin_platform ?
> 
> Good idea. But, now I get a "Section mismatch" during link time so I'll
> have to figure out why that is.

Mmh, I think it would be more consistent to convert this to:

CLOCKSOURCE_OF_DECLARE(ostm, "renesas,ostm", ostm_init);

The only problem is to get the struct device associated to the of_node passed
as parameter to ostm_init in order to use the devm_* API.

I think of_find_device_by_node should return the platform_device, then
pdev->dev. If that works the other drivers will benefit from that to remove all
the rollback code everywhere.

  -- Daniel

-- 

  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH v4 00/06] clocksource: sh_cmt: DT binding rework V4

2017-07-31 Thread Daniel Lezcano 
On 11/07/2017 13:56, Simon Horman wrote:
> On Thu, Nov 24, 2016 at 11:58:43AM +0100, Simon Horman wrote:
>> Hi Magnus,
>>
>> On Mon, Mar 14, 2016 at 11:23:42PM +0900, Magnus Damm wrote:
>>> clocksource: sh_cmt: DT binding rework V4
>>>
>>> [PATCH v4 01/06] devicetree: bindings: Remove sh7372 CMT binding
>>> [PATCH v4 02/06] devicetree: bindings: R-Car Gen2 CMT0 and CMT1 bindings
>>> [PATCH v4 03/06] devicetree: bindings: r8a73a4 and R-Car Gen2 CMT bindings
>>> [PATCH v4 04/06] devicetree: bindings: Deprecate property, update example
>>> [PATCH v4 05/06] devicetree: bindings: Remove unused 32-bit CMT bindings
>>> [PATCH v4 06/06] devicetree: bindings: Remove deprecated properties
>>>
>>> Here is the latest and hopefully final take on updating the CMT DT
>>> bindings for R-Car Gen2. In total there are 6 patches that have acks
>>> and are ready to be picked up and merged. Other earlier posted changes
>>> such as driver modification and SoC DTS bits depend on this series.
>>
>> I am wondering what the state of this work is.
>> I see only one minor review comment for this series.
>> It would be great to see it merged.
> 
> Ping

Up.



-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH v4 00/06] clocksource: sh_cmt: DT binding rework V4

2017-08-10 Thread Daniel Lezcano 
On 10/08/2017 11:01, Geert Uytterhoeven wrote:
> On Tue, Jul 11, 2017 at 1:56 PM, Simon Horman  wrote:
>> On Thu, Nov 24, 2016 at 11:58:43AM +0100, Simon Horman wrote:
>>> On Mon, Mar 14, 2016 at 11:23:42PM +0900, Magnus Damm wrote:
 clocksource: sh_cmt: DT binding rework V4

 [PATCH v4 01/06] devicetree: bindings: Remove sh7372 CMT binding
 [PATCH v4 02/06] devicetree: bindings: R-Car Gen2 CMT0 and CMT1 bindings
 [PATCH v4 03/06] devicetree: bindings: r8a73a4 and R-Car Gen2 CMT bindings
 [PATCH v4 04/06] devicetree: bindings: Deprecate property, update example
 [PATCH v4 05/06] devicetree: bindings: Remove unused 32-bit CMT bindings
 [PATCH v4 06/06] devicetree: bindings: Remove deprecated properties

 Here is the latest and hopefully final take on updating the CMT DT
 bindings for R-Car Gen2. In total there are 6 patches that have acks
 and are ready to be picked up and merged. Other earlier posted changes
 such as driver modification and SoC DTS bits depend on this series.
>>>
>>> I am wondering what the state of this work is.
>>> I see only one minor review comment for this series.
>>> It would be great to see it merged.
>>
>> Ping
> 
> Recently, at +1800m, I realized that if we want to continue this work, we
> better do it soon, so it can be included in the big R-Car Gen2 flag day
> requiring APMU, CPG/MSSR, ICRAM, RST, and SYSC being described in DT.

Applied.

Thanks.

  -- Daniel



-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH v3 0/7] clocksource: sh_cmt: Update driver for DT binding rework

2017-09-25 Thread Daniel Lezcano 

Hi all,

last call for comments before I merge this series.

Thanks.

  -- Daniel


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH v3 0/7] clocksource: sh_cmt: Update driver for DT binding rework

2017-09-26 Thread Daniel Lezcano 
On 26/09/2017 10:26, Laurent Pinchart wrote:
> Reviewed-by: Laurent Pinchart 

Thanks. I applied the whole series.

 -- Daniel


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH v2 1/3] dt-bindings: timer: renesas, cmt: Document r8a774[35] CMT support

2018-01-25 Thread Daniel Lezcano 
On 24/01/2018 17:49, Fabrizio Castro wrote:
> Hello Daniel,
> 
> I am sorry to bother you, just wondering if you think there are any chances 
> for this patch to applied on top of v4.16?

You can take it through the renesas tree.

Acked-by: Daniel Lezcano <daniel.lezc...@linaro.org>


>> Subject: [PATCH v2 1/3] dt-bindings: timer: renesas, cmt: Document 
>> r8a774[35] CMT support
>>
>> Document SoC specific compatible strings for r8a7743 and r8a7745.
>> No driver change is needed as the fallback strings will activate
>> the right code.
>>
>> Signed-off-by: Fabrizio Castro <fabrizio.cas...@bp.renesas.com>
>> Reviewed-by: Biju Das <biju@bp.renesas.com>
>> Reviewed-by: Geert Uytterhoeven <geert+rene...@glider.be>
>> Reviewed-by: Rob Herring <r...@kernel.org>
>> Reviewed-by: Simon Horman <ho...@verge.net.au>
>> ---
>> v1->v2:
>> * "R-Car Gen2 or RZ/G1." -> "R-Car Gen2 and RZ/G1."
>> * "all the R-Car" -> "R-Car"
>>
>>  Documentation/devicetree/bindings/timer/renesas,cmt.txt | 14 ++
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/timer/renesas,cmt.txt 
>> b/Documentation/devicetree/bindings/timer/renesas,cmt.txt
>> index d740989..b40add2 100644
>> --- a/Documentation/devicetree/bindings/timer/renesas,cmt.txt
>> +++ b/Documentation/devicetree/bindings/timer/renesas,cmt.txt
>> @@ -22,6 +22,10 @@ Required Properties:
>>
>>  - "renesas,r8a73a4-cmt0" for the 32-bit CMT0 device included in r8a73a4.
>>  - "renesas,r8a73a4-cmt1" for the 48-bit CMT1 device included in r8a73a4.
>> +- "renesas,r8a7743-cmt0" for the 32-bit CMT0 device included in r8a7743.
>> +- "renesas,r8a7743-cmt1" for the 48-bit CMT1 device included in r8a7743.
>> +- "renesas,r8a7745-cmt0" for the 32-bit CMT0 device included in r8a7745.
>> +- "renesas,r8a7745-cmt1" for the 48-bit CMT1 device included in r8a7745.
>>  - "renesas,r8a7790-cmt0" for the 32-bit CMT0 device included in r8a7790.
>>  - "renesas,r8a7790-cmt1" for the 48-bit CMT1 device included in r8a7790.
>>  - "renesas,r8a7791-cmt0" for the 32-bit CMT0 device included in r8a7791.
>> @@ -31,10 +35,12 @@ Required Properties:
>>  - "renesas,r8a7794-cmt0" for the 32-bit CMT0 device included in r8a7794.
>>  - "renesas,r8a7794-cmt1" for the 48-bit CMT1 device included in r8a7794.
>>
>> -- "renesas,rcar-gen2-cmt0" for 32-bit CMT0 devices included in R-Car 
>> Gen2.
>> -- "renesas,rcar-gen2-cmt1" for 48-bit CMT1 devices included in R-Car 
>> Gen2.
>> -These are fallbacks for r8a73a4 and all the R-Car Gen2
>> -entrieslisted above.
>> +- "renesas,rcar-gen2-cmt0" for 32-bit CMT0 devices included in R-Car 
>> Gen2
>> +and RZ/G1.
>> +- "renesas,rcar-gen2-cmt1" for 48-bit CMT1 devices included in R-Car 
>> Gen2
>> +and RZ/G1.
>> +These are fallbacks for r8a73a4, R-Car Gen2 and RZ/G1 entries
>> +listed above.
>>
>>- reg: base address and length of the registers block for the timer 
>> module.
>>- interrupts: interrupt-specifier for the timer, one per channel.
>> --
>> 2.7.4
> 
> 
> 
> 
> Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, 
> Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered 
> No. 04586709.
> 


-- 
 <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 v2 1/3] dt-bindings: timer: renesas, cmt: Document r8a774[35] CMT support

2018-01-25 Thread Daniel Lezcano 
On 25/01/2018 09:21, Geert Uytterhoeven wrote:
> Hi Daniel,
> 
> On Thu, Jan 25, 2018 at 9:09 AM, Daniel Lezcano
> <daniel.lezc...@linaro.org> wrote:
>> On 24/01/2018 17:49, Fabrizio Castro wrote:
>>> I am sorry to bother you, just wondering if you think there are any chances 
>>> for this patch to applied on top of v4.16?
>>
>> You can take it through the renesas tree.
>>
>> Acked-by: Daniel Lezcano <daniel.lezc...@linaro.org>
> 
> Thank you!
> 
> Unfortunately it's a bit late to get non-bugfixes in v4.16 through the renesas
> tree and arm-soc.

I thought they were going through the renesas tree, so I discarded them :/

May be Thomas can take them in the tip/timers tree ?

Thomas ?


-- 
 <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] thermal: rcar_gen3_thermal: convert to SPDX identifiers

2018-07-30 Thread Daniel Lezcano 
On 30/07/2018 09:57, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto 
> 
> Signed-off-by: Kuninori Morimoto 

Reviewed-by: Daniel Lezcano 

Adding Cc: Philippe Ombredanne 

> ---
>  drivers/thermal/rcar_gen3_thermal.c | 11 +--
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/thermal/rcar_gen3_thermal.c 
> b/drivers/thermal/rcar_gen3_thermal.c
> index 766521e..7aed533 100644
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -1,19 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
>  /*
>   *  R-Car Gen3 THS thermal sensor driver
>   *  Based on rcar_thermal.c and work from Hien Dang and Khiem Nguyen.
>   *
>   * Copyright (C) 2016 Renesas Electronics Corporation.
>   * Copyright (C) 2016 Sang Engineering
> - *
> - *  This program is free software; you can redistribute it and/or modify
> - *  it under the terms of the GNU General Public License as published by
> - *  the Free Software Foundation; version 2 of the License.
> - *
> - *  This program is distributed in the hope that it will be useful, but
> - *  WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> - *  General Public License for more details.
> - *
>   */
>  #include 
>  #include 
> 


-- 
 <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] thermal: rcar_thermal: convert to SPDX identifiers

2018-07-30 Thread Daniel Lezcano 
On 30/07/2018 09:56, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto 
> 
> As original license mentioned, it is GPL-2.0 in SPDX.
> Then, MODULE_LICENSE() should be "GPL v2" instead of "GPL".
> See ${LINUX}/include/linux/module.h
> 
>   "GPL"   [GNU Public License v2 or later]
>   "GPL v2"[GNU Public License v2]
> 
> Signed-off-by: Kuninori Morimoto 

Reviewed-by: Daniel Lezcano 

Adding Cc: Philippe Ombredanne 



> ---
>  drivers/thermal/rcar_thermal.c | 16 ++--
>  1 file changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
> index 45fb284..14ab4a9 100644
> --- a/drivers/thermal/rcar_thermal.c
> +++ b/drivers/thermal/rcar_thermal.c
> @@ -1,21 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0
>  /*
>   *  R-Car THS/TSC thermal sensor driver
>   *
>   * Copyright (C) 2012 Renesas Solutions Corp.
>   * Kuninori Morimoto 
> - *
> - *  This program is free software; you can redistribute it and/or modify
> - *  it under the terms of the GNU General Public License as published by
> - *  the Free Software Foundation; version 2 of the License.
> - *
> - *  This program is distributed in the hope that it will be useful, but
> - *  WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> - *  General Public License for more details.
> - *
> - *  You should have received a copy of the GNU General Public License along
> - *  with this program; if not, write to the Free Software Foundation, Inc.,
> - *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
>   */
>  #include 
>  #include 
> @@ -660,6 +648,6 @@ static struct platform_driver rcar_thermal_driver = {
>  };
>  module_platform_driver(rcar_thermal_driver);
>  
> -MODULE_LICENSE("GPL");
> +MODULE_LICENSE("GPL v2");
>  MODULE_DESCRIPTION("R-Car THS/TSC thermal sensor driver");
>  MODULE_AUTHOR("Kuninori Morimoto ");
> 


-- 
 <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 1/2] clocksource/drivers/ostm: Delay driver registration

2018-08-29 Thread Daniel Lezcano 
On 29/08/2018 17:44, Chris Brandt wrote:
> On Wednesday, August 29, 2018 1, Daniel Lezcano wrote:
>> Can the boot constraints [1] solve this issue instead of the changes you
>> are proposing ?
>>
>> [1] https://lwn.net/Articles/747250/
> 
> Thanks for the suggestion, but...
> 
> I grepped for "boot_constraint" and it shows up nowhere in the current 
> kernel.
> 
> Also, this article was written Feb 16, 2018, and I can see that the 
> patch series was still being submitted (V7) as of Feb 23, 2018.

Ah ok, fair enough, I thought it was merged. In any case, after thinking
about it, it wouldn't have helped.

My concern is if we can avoid changing the TIMER_OF_DECLARE because of
the boot order, it would be better.

Can returning EPROBE_DEFER fix this issue? Or use the 'complex
dependencies' [1]?

I'm pretty busy ATM, so I can not investigate now if the suggestions
above are fine or just stupid. I will have a look as soon as I can.

  -- Daniel

[1] https://www.kernel.org/doc/html/v4.15/driver-api/device_link.html




-- 
 <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 1/2] clocksource/drivers/ostm: Delay driver registration

2018-08-29 Thread Daniel Lezcano 
On 29/08/2018 15:29, Chris Brandt wrote:
> The newer RZ/A clock control driver no longer registers all its clocks
> using DT, therefore the clocks required by this driver are no longer
> present at the beginning of boot.
> 
> Because of this, TIMER_OF_DECLARE can no longer be used because this
> causes the driver to get probed too early before the parent clock exists,
> and the probe will fail with "ostm: Failed to get clock".
> 
> So, we'll change this driver to register/probe during subsys_initcall which
> is after the appropriate clocks have been registered.

Can the boot constraints [1] solve this issue instead of the changes you
are proposing ?

[1] https://lwn.net/Articles/747250/




-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH 1/2] clocksource/drivers/ostm: Delay driver registration

2018-08-30 Thread Daniel Lezcano 
On 30/08/2018 09:54, Geert Uytterhoeven wrote:
> Hi Daniel,
> 
> On Wed, Aug 29, 2018 at 6:26 PM Daniel Lezcano
>  wrote:
>> On 29/08/2018 17:44, Chris Brandt wrote:
>>> On Wednesday, August 29, 2018 1, Daniel Lezcano wrote:
>>>> Can the boot constraints [1] solve this issue instead of the changes you
>>>> are proposing ?
>>>>
>>>> [1] https://lwn.net/Articles/747250/
>>>
>>> Thanks for the suggestion, but...
>>>
>>> I grepped for "boot_constraint" and it shows up nowhere in the current
>>> kernel.
>>>
>>> Also, this article was written Feb 16, 2018, and I can see that the
>>> patch series was still being submitted (V7) as of Feb 23, 2018.
>>
>> Ah ok, fair enough, I thought it was merged. In any case, after thinking
>> about it, it wouldn't have helped.
>>
>> My concern is if we can avoid changing the TIMER_OF_DECLARE because of
>> the boot order, it would be better.
>>
>> Can returning EPROBE_DEFER fix this issue? Or use the 'complex
>> dependencies' [1]?
> 
> *_OF_DECLARE() is not compatible with EPROBE_DEFER, which causes
> issues with complex dependencies.

What causes issues ? Add support for EPROBE_DEFER with OF_DECLARE or not
having support of it ?

> That's exactly why many subsystems are moving away from it.
> E.g. IOMMU_OF_DECLARE was removed in v4.19-rc1.

Ok, thanks for information.


-- 
 <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 1/2] clocksource/drivers/ostm: Delay driver registration

2018-08-30 Thread Daniel Lezcano 
On 30/08/2018 10:27, Geert Uytterhoeven wrote:
> Hi Daniel,
> 
> On Thu, Aug 30, 2018 at 10:09 AM Daniel Lezcano
>  wrote:
>> On 30/08/2018 09:54, Geert Uytterhoeven wrote:
>>> On Wed, Aug 29, 2018 at 6:26 PM Daniel Lezcano
>>>  wrote:
>>>> On 29/08/2018 17:44, Chris Brandt wrote:
>>>>> On Wednesday, August 29, 2018 1, Daniel Lezcano wrote:
>>>>>> Can the boot constraints [1] solve this issue instead of the changes you
>>>>>> are proposing ?
>>>>>>
>>>>>> [1] https://lwn.net/Articles/747250/
>>>>>
>>>>> Thanks for the suggestion, but...
>>>>>
>>>>> I grepped for "boot_constraint" and it shows up nowhere in the current
>>>>> kernel.
>>>>>
>>>>> Also, this article was written Feb 16, 2018, and I can see that the
>>>>> patch series was still being submitted (V7) as of Feb 23, 2018.
>>>>
>>>> Ah ok, fair enough, I thought it was merged. In any case, after thinking
>>>> about it, it wouldn't have helped.
>>>>
>>>> My concern is if we can avoid changing the TIMER_OF_DECLARE because of
>>>> the boot order, it would be better.
>>>>
>>>> Can returning EPROBE_DEFER fix this issue? Or use the 'complex
>>>> dependencies' [1]?
>>>
>>> *_OF_DECLARE() is not compatible with EPROBE_DEFER, which causes
>>> issues with complex dependencies.
>>
>> What causes issues ? Add support for EPROBE_DEFER with OF_DECLARE or not
>> having support of it ?
> 
> Driver init functions declared using *_OF_DECLARE() are called exactly once.
> Hence if they fail, they are never retried. Calling order among subsystems is
> hardcoded, and the actual order was determined by historical reasons (legacy
> PC systems with always-on clocks and power domains ;-).
> This breaks as soon as e.g. timers or interrupt controllers start depending
> on clocks and/or power domains.
> 
> Drivers using the device driver framework are retried later when their init
> function returns -EPROBE_DEFER. So (eventually) they all succeed
> initialization.

Yeah, I got this point. But it is the meaning of your sentence: "...
which causes issues with complex dependencies.".

It is ambiguous *what* causes the issues.

Did you meant an attempt was done to support EPROBE_DEFER with
*_OF_DECLARE but caused too much issues with the complex dependencies?

Or the current situation is causing too much issues with the complex
dependencies?

(I know the latter is true, it is about the meaning of the sentence).


-- 
 <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 1/2] clocksource/drivers/ostm: Delay driver registration

2018-08-30 Thread Daniel Lezcano 


[Added Arnd Bergmann, Bartosz Golaszewski and Mark Brown]

On 30/08/2018 10:48, Geert Uytterhoeven wrote:
> Hi Daniel,

[ ... ]

>> Yeah, I got this point. But it is the meaning of your sentence: "...
>> which causes issues with complex dependencies.".
>>
>> It is ambiguous *what* causes the issues.
>>
>> Did you meant an attempt was done to support EPROBE_DEFER with
>> *_OF_DECLARE but caused too much issues with the complex dependencies?
>>
>> Or the current situation is causing too much issues with the complex
>> dependencies?
>>
>> (I know the latter is true, it is about the meaning of the sentence).
> 
> I meant the latter.
> 
> AFAIK no attempt was done to support EPROBE_DEFER with *_OF_DECLARE.
> IMHO it would be pointless, as it would be much easier to just switch to real
> platform drivers.

May be, may be not.

>From your point of view, the change is simple because it touches only a
single driver.

>From my point of view, the change implies a split in the approach while
I'm trying to unify the drivers little by little and there are hundred
of them.

It is not the first time we face this situation and Bartosz Golaszewski
has a similar problem [1].

We have all the frameworks we need to solve this properly but I would
like something we can propagate to all drivers (OF and !OF) so we end up
with unified code.

It is time we clearly state the dependency issues and we find a proper
way to solve it.




[1] https://lkml.org/lkml/2018/4/26/657

-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH] thermal: rcar_thermal: fix duplicate IRQ request

2018-10-04 Thread Daniel Lezcano 
On 03/10/2018 22:47, Sergei Shtylyov wrote:
> The driver on R8A77995 requests the same IRQ twice since
> platform_get_resource() is always called for the 1st IRQ resource. 
> 
> Fixes: 1969d9dc2079 ("thermal: rcar_thermal: add r8a77995 support")
> Signed-off-by: Sergei Shtylyov 
> 
> ---

Reviewed-by: Daniel Lezcano 

-- 
 <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] dt-bindings: thermal: rcar: Add device tree support for r8a7744

2018-09-25 Thread Daniel Lezcano 
On 25/09/2018 19:01, Biju Das wrote:
> Add thermal sensor support for r8a7744 SoC. The Renesas RZ/G1N
> (r8a7744) thermal sensor module is identical to the R-Car Gen2 family.
> 
> No driver change is needed due to the fallback compatible value
> "renesas,rcar-gen2-thermal".
> 
> Signed-off-by: Biju Das 
> Reviewed-by: Chris Paterson 

Reviewed-by: Daniel Lezcano 

> ---
>  Documentation/devicetree/bindings/thermal/rcar-thermal.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/rcar-thermal.txt 
> b/Documentation/devicetree/bindings/thermal/rcar-thermal.txt
> index 67c563f..01d30a97 100644
> --- a/Documentation/devicetree/bindings/thermal/rcar-thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/rcar-thermal.txt
> @@ -8,6 +8,7 @@ Required properties:
> Examples with soctypes are:
>   - "renesas,thermal-r8a73a4" (R-Mobile APE6)
>   - "renesas,thermal-r8a7743" (RZ/G1M)
> + - "renesas,thermal-r8a7744" (RZ/G1N)
>   - "renesas,thermal-r8a7779" (R-Car H1)
>   - "renesas,thermal-r8a7790" (R-Car H2)
>   - "renesas,thermal-r8a7791" (R-Car M2-W)
> 


-- 
 <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] arm64: dts: renesas: r8a7796: Add CMT device nodes

2018-11-22 Thread Daniel Lezcano 
On 22/11/2018 10:46, Biju Das wrote:
> Hello Daniel,
> 
> Thanks for the feedback.
> 
>> -Original Message-----
>> From: Daniel Lezcano 
>> Sent: 19 November 2018 17:15
>> To: Biju Das ; Rob Herring
>> ; Mark Rutland 
>> Cc: Simon Horman ; Magnus Damm
>> ; linux-renesas-soc@vger.kernel.org;
>> devicet...@vger.kernel.org; Geert Uytterhoeven
>> ; Chris Paterson
>> ; Thomas Gleixner ;
>> John Stultz ; Fabrizio Castro
>> 
>> Subject: Re: [PATCH] arm64: dts: renesas: r8a7796: Add CMT device nodes
>>
>> On 19/11/2018 16:50, Biju Das wrote:
>>> Hi Daniel,
>>>
>>> Thanks for the feedback.
>>>
>>>>>> Subject: Re: [PATCH] arm64: dts: renesas: r8a7796: Add CMT device
>>>>>> nodes
>>>>>>
>>>>>> On 26/10/2018 10:25, Biju Das wrote:
>>>>>>> This patch adds CMT{0|1|2|3} device nodes for r8a7796 SoC.
>>>>>>>
>>>>>>> Signed-off-by: Biju Das 
>>>>>>> ---
>>>>>>> This patch is tested against renesas-dev
>>>>>>>
>>>>>>> I have executed on inconsistency-check, nanosleep and
>>>>>>> clocksource_switch selftests on this arm64 SoC. The
>>>>>>> inconsistency-check and nanosleep tests are working fine.The
>>>>>>> clocksource_switch asynchronous test is failing due to
>>>>>>> inconsistency-check
>>>>>> failure on "arch_sys_counter".
>>>>>>>
>>>>>>> But if i skip the clocksource_switching of "arch_sys_counter", the
>>>>>>> asynchronous test is passing for CMT0/1/2/3 timer.
>>>>>>>
>>>>>>> Has any one noticed this issue?
>>>>>>
>>>>>> So now that you mention that, I've been through the
>>>>>> clocksource_switch on another ARM64 platform (hikey960) and
>>>>>> disabled the
>>>>>> ARM64_ERRATUM_858921 config option. I can see the same issue.
>>>>>>
>>>>>> Is this option set on your config ?
>>>>>
>>>>> No.  As per  " config ARM64_ERRATUM_858921", it is "Workaround for
>>>> Cortex-A73 erratum 858921"
>>>>>
>>>>> Our SoC is 2xCA-57 + 4 x CA-53.  Does  it impact CA-57 + CA_53?
>>>>
>>>> Dunno :/
>>>>
>>>>> Any way I will enable this config option and will provide you the results.
>>>>
>>>> Ok, thanks!
>>>
>>> The following config is enabled by default on upstream
>>> kernel(4.20-rc3) CONFIG_ARM_ARCH_TIMER_EVTSTREAM=y
>>> CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND=y
>>> CONFIG_FSL_ERRATUM_A008585=y
>>> CONFIG_HISILICON_ERRATUM_161010101=y
>>> CONFIG_ARM64_ERRATUM_858921=y
>>>
>>> For a quick testing,  I have activated the erratum using the property
>> "fsl,erratum-a008585" on device tree.
>>> With this I confirm the issue is fixed.
>>>
>>> I have  some questions on this.
>>> 1) Based  on the test result ,do you think renesas soc also  impacted by the
>> ARM64_ERRATUM_858921?
>>> 2) Is there any way to find, is this Erratum actually causing the
>> asynchronous test to fail?
>>
>> I guess, you can hack the __fsl_a008585_read_reg macro and check if the
>> invalid condition is reached.
>>
>> This thread https://lkml.org/lkml/2018/5/10/773 will give you all the answers
>> you are looking for (well very likely).
>>
>> Let me know if it helped.
> 
> In our case , Delta: 174760 ns
> 
> 1530553351:205762284
> 1530553351:205762404
> 
> 1530553351:205951226
> 1530553351:205776466
> 
> 
> I have tried the workaround for ARM64_ERRATUM_858921, that also fixes the 
> issue.
> 
> But all the workaround disables ARM64 VDSO. How do we conclude that is it 
> VDSO issue or ARM64_ERRATUM issue?

May be disable all errata and set vdso_default to false?

[ ... ]

-static bool vdso_default = true;
+static bool vdso_default = false;

[ ... ]

>>> timer {
>>> compatible = "arm,armv8-timer";
>>> interrupts-extended = < GIC_PPI 13
>> (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
>>>   < GIC_PPI 14 
>>> (GIC_CPU_MASK_SIMPLE(6) |
>> IRQ_TYPE_LEVEL_LOW)>,
>>>   < GIC_PPI 11 
>>> (GIC_CPU_MASK_SIMPLE(6) |
>> IRQ_TYPE_LEVEL_LOW)>,
>>>   < GIC_PPI 10
>>> (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>;
>>> +fsl,erratum-a008585;
>>> }
>>
>>
>>
>>
>> --
>>  <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
> 
> 
> 
> 
> Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, 
> Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered 
> No. 04586709.
> 


-- 
 <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] dt-bindings: timer: renesas, cmt: Document r8a774a1 CMT support

2018-11-22 Thread Daniel Lezcano 
On 19/11/2018 17:14, Biju Das wrote:
> Document SoC specific bindings for RZ/G2M (r8a774a1) SoC.
> 
> Signed-off-by: Biju Das 

Please send to lkml@ also.

> ---
>  Documentation/devicetree/bindings/timer/renesas,cmt.txt | 11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/timer/renesas,cmt.txt 
> b/Documentation/devicetree/bindings/timer/renesas,cmt.txt
> index eb602c5..862a80f 100644
> --- a/Documentation/devicetree/bindings/timer/renesas,cmt.txt
> +++ b/Documentation/devicetree/bindings/timer/renesas,cmt.txt
> @@ -30,6 +30,8 @@ Required Properties:
>  - "renesas,r8a7745-cmt1" for the 48-bit CMT1 device included in r8a7745.
>  - "renesas,r8a77470-cmt0" for the 32-bit CMT0 device included in 
> r8a77470.
>  - "renesas,r8a77470-cmt1" for the 48-bit CMT1 device included in 
> r8a77470.
> +- "renesas,r8a774a1-cmt0" for the 32-bit CMT0 device included in 
> r8a774a1.
> +- "renesas,r8a774a1-cmt1" for the 48-bit CMT1 device included in 
> r8a774a1.
>  - "renesas,r8a7790-cmt0" for the 32-bit CMT0 device included in r8a7790.
>  - "renesas,r8a7790-cmt1" for the 48-bit CMT1 device included in r8a7790.
>  - "renesas,r8a7791-cmt0" for the 32-bit CMT0 device included in r8a7791.
> @@ -51,9 +53,12 @@ Required Properties:
>   and RZ/G1.
>   These are fallbacks for r8a73a4, R-Car Gen2 and RZ/G1 entries
>   listed above.
> -- "renesas,rcar-gen3-cmt0" for 32-bit CMT0 devices included in R-Car 
> Gen3.
> -- "renesas,rcar-gen3-cmt1" for 48-bit CMT1 devices included in R-Car 
> Gen3.
> - These are fallbacks for R-Car Gen3 entries listed above.
> +- "renesas,rcar-gen3-cmt0" for 32-bit CMT0 devices included in R-Car Gen3
> + and RZ/G2.
> +- "renesas,rcar-gen3-cmt1" for 48-bit CMT1 devices included in R-Car Gen3
> + and RZ/G2.
> + These are fallbacks for R-Car Gen3 and RZ/G2 entries listed
> + above.
>  
>- reg: base address and length of the registers block for the timer module.
>- interrupts: interrupt-specifier for the timer, one per channel.
> 


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH] dt-bindings: timer: renesas, cmt: Document r8a774a1 CMT support

2018-11-22 Thread Daniel Lezcano 
On 19/11/2018 17:14, Biju Das wrote:
> Document SoC specific bindings for RZ/G2M (r8a774a1) SoC.
> 

Applied, thanks!

  -- Daniel


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH] arm64: dts: renesas: r8a7796: Add CMT device nodes

2018-11-19 Thread Daniel Lezcano 
On 26/10/2018 10:25, Biju Das wrote:
> This patch adds CMT{0|1|2|3} device nodes for r8a7796 SoC.
> 
> Signed-off-by: Biju Das 
> ---
> This patch is tested against renesas-dev
> 
> I have executed on inconsistency-check, nanosleep and clocksource_switch
> selftests on this arm64 SoC. The inconsistency-check and nanosleep tests
> are working fine.The clocksource_switch asynchronous test is failing due
> to inconsistency-check failure on "arch_sys_counter".
> 
> But if i skip the clocksource_switching of "arch_sys_counter", the
> asynchronous test is passing for CMT0/1/2/3 timer.
> 
> Has any one noticed this issue?

Were you able to narrow down the issue?

> ---
>  arch/arm64/boot/dts/renesas/r8a7796.dtsi | 70 
> 
>  1 file changed, 70 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi 
> b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> index 1ec6aaa..d62febd0 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> @@ -401,6 +401,76 @@
>   reg = <0 0xe606 0 0x50c>;
>   };
>  
> + cmt0: timer@e60f {
> + compatible = "renesas,r8a7796-cmt0",
> +  "renesas,rcar-gen3-cmt0";
> + reg = <0 0xe60f 0 0x1004>;
> + interrupts = ,
> +  ;
> + clocks = < CPG_MOD 303>;
> + clock-names = "fck";
> + power-domains = < R8A7796_PD_ALWAYS_ON>;
> + resets = < 303>;
> + status = "disabled";
> + };
> +
> + cmt1: timer@e613 {
> + compatible = "renesas,r8a7796-cmt1",
> +  "renesas,rcar-gen3-cmt1";
> + reg = <0 0xe613 0 0x1004>;
> + interrupts = ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ;
> + clocks = < CPG_MOD 302>;
> + clock-names = "fck";
> + power-domains = < R8A7796_PD_ALWAYS_ON>;
> + resets = < 302>;
> + status = "disabled";
> + };
> +
> + cmt2: timer@e614 {
> + compatible = "renesas,r8a7796-cmt1",
> +  "renesas,rcar-gen3-cmt1";
> + reg = <0 0xe614 0 0x1004>;
> + interrupts = ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ;
> + clocks = < CPG_MOD 301>;
> + clock-names = "fck";
> + power-domains = < R8A7796_PD_ALWAYS_ON>;
> + resets = < 301>;
> + status = "disabled";
> + };
> +
> + cmt3: timer@e6148000 {
> + compatible = "renesas,r8a7796-cmt1",
> +  "renesas,rcar-gen3-cmt1";
> + reg = <0 0xe6148000 0 0x1004>;
> + interrupts = ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ;
> + clocks = < CPG_MOD 300>;
> + clock-names = "fck";
> + power-domains = < R8A7796_PD_ALWAYS_ON>;
> + resets = < 300>;
> + status = "disabled";
> + };
> +
>   cpg: clock-controller@e615 {
>   compatible = "renesas,r8a7796-cpg-mssr";
>   reg = <0 0xe615 0 0x1000>;
> 


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH] arm64: dts: renesas: r8a7796: Add CMT device nodes

2018-11-19 Thread Daniel Lezcano 
On 26/10/2018 10:25, Biju Das wrote:
> This patch adds CMT{0|1|2|3} device nodes for r8a7796 SoC.
> 
> Signed-off-by: Biju Das 
> ---
> This patch is tested against renesas-dev
> 
> I have executed on inconsistency-check, nanosleep and clocksource_switch
> selftests on this arm64 SoC. The inconsistency-check and nanosleep tests
> are working fine.The clocksource_switch asynchronous test is failing due
> to inconsistency-check failure on "arch_sys_counter".
> 
> But if i skip the clocksource_switching of "arch_sys_counter", the
> asynchronous test is passing for CMT0/1/2/3 timer.
> 
> Has any one noticed this issue?

So now that you mention that, I've been through the clocksource_switch
on another ARM64 platform (hikey960) and disabled the
ARM64_ERRATUM_858921 config option. I can see the same issue.

Is this option set on your config ?


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH] arm64: dts: renesas: r8a7796: Add CMT device nodes

2018-11-19 Thread Daniel Lezcano 
On 19/11/2018 11:35, Biju Das wrote:
> Hi Daniel,
> 
> Thanks for the feedback.
> 
>> -Original Message-----
>> From: Daniel Lezcano 
>> Sent: 19 November 2018 10:26
>> To: Biju Das ; Rob Herring
>> ; Mark Rutland 
>> Cc: Simon Horman ; Magnus Damm
>> ; linux-renesas-soc@vger.kernel.org;
>> devicet...@vger.kernel.org; Geert Uytterhoeven
>> ; Chris Paterson
>> ; Thomas Gleixner ;
>> John Stultz ; Fabrizio Castro
>> 
>> Subject: Re: [PATCH] arm64: dts: renesas: r8a7796: Add CMT device nodes
>>
>> On 26/10/2018 10:25, Biju Das wrote:
>>> This patch adds CMT{0|1|2|3} device nodes for r8a7796 SoC.
>>>
>>> Signed-off-by: Biju Das 
>>> ---
>>> This patch is tested against renesas-dev
>>>
>>> I have executed on inconsistency-check, nanosleep and
>>> clocksource_switch selftests on this arm64 SoC. The
>>> inconsistency-check and nanosleep tests are working fine.The
>>> clocksource_switch asynchronous test is failing due to inconsistency-check
>> failure on "arch_sys_counter".
>>>
>>> But if i skip the clocksource_switching of "arch_sys_counter", the
>>> asynchronous test is passing for CMT0/1/2/3 timer.
>>>
>>> Has any one noticed this issue?
>>
>> So now that you mention that, I've been through the clocksource_switch on
>> another ARM64 platform (hikey960) and disabled the
>> ARM64_ERRATUM_858921 config option. I can see the same issue.
>>
>> Is this option set on your config ?
> 
> No.  As per  " config ARM64_ERRATUM_858921", it is "Workaround for Cortex-A73 
> erratum 858921"
> 
> Our SoC is 2xCA-57 + 4 x CA-53.  Does  it impact CA-57 + CA_53?

Dunno :/

> Any way I will enable this config option and will provide you the results.

Ok, thanks!

> The following errata is set in our kernel config.
> 
> CONFIG_ARM64_ERRATUM_826319=y
> CONFIG_ARM64_ERRATUM_827319=y
> CONFIG_ARM64_ERRATUM_824069=y
> CONFIG_ARM64_ERRATUM_819472=y
> CONFIG_ARM64_ERRATUM_832075=y
> CONFIG_ARM64_ERRATUM_834220=y
> CONFIG_ARM64_ERRATUM_845719=y
> CONFIG_ARM64_ERRATUM_843419=y
> CONFIG_ARM64_ERRATUM_1024718=y
> CONFIG_ARM64_ERRATUM_1188873=y
> 
> Regards.
> Biju
> 
> 
> 
> Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, 
> Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered 
> No. 04586709.
> 


-- 
 <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] dt-bindings: timer: renesas, cmt: Document r8a7796 CMT support

2018-11-17 Thread Daniel Lezcano 
On 26/10/2018 10:01, Biju Das wrote:
> Document SoC specific bindings for R-Car M3-W (r8a7796) SoC.
> 
> Signed-off-by: Biju Das 
> ---
> This patch is tested against linu-next
> ---

Applied, thanks!


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH] dt-bindings: timer: renesas, cmt: Document r8a77470 CMT support

2018-11-17 Thread Daniel Lezcano 
On 26/10/2018 10:36, Biju Das wrote:
> Document SoC specific compatible strings for r8a77470. No driver change
> is needed as the fallback strings will activate the right code.
> 
> Signed-off-by: Biju Das 
> ---

Applied also, thanks!


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH] clocksource: sh_cmt: fix clocksource width for 32-bit machines

2018-09-13 Thread Daniel Lezcano 
On 10/09/2018 22:22, Sergei Shtylyov wrote:
> The driver seems to abuse *unsigned long* not only for the (32-bit)
> register values but also for the 'sh_cmt_channel::total_cycles' which
> needs to always be 64-bit -- as a result, the clocksource's mask is
> needlessly clamped down to 32-bits on the 32-bit machines...
> 
> Reported-by: Geert Uytterhoeven 
> Signed-off-by: Sergei Shtylyov 
> 
> ---
> This patch is against the 'tip.git' repo's 'timers/core' branch plus the fixup
> for the 64-bit machines reposted last Saturday...

Why not against timers/urgent ?



-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH 1/2] clocksource/drivers/ostm: Delay driver registration

2018-09-13 Thread Daniel Lezcano 
On 11/09/2018 20:42, Chris Brandt wrote:
> On Tuesday, September 11, 2018 1, Rob Herring wrote:
>> Well before we get to initcalls, the kernel calls the arch specific
>> time_init() which (on ARM) calls of_clk_init (for all the reasons
>> above) and then timer_probe(). When timer_probe returns, it is
>> expected that you have setup a clocksource and clockevent. If you
>> haven't, then at some point before we get to initcalls we should hang
>> because we're not getting any timer interrupts and time is not
>> advancing.
> 
> What I get now is:
> 
> [0.00] timer_probe: no matching timers found
> ...
> ...
>  [0.00] clocksource: jiffies: mask: 0x max_cycles: 
> 0x, max_idle_ns: 1911260446275 ns
> ...
> ...
> 
> 
> But then later on in boot, I'll get (with my subsys_initcall timer fix)
> 
> ...
> ...
> [0.00] SCSI subsystem initialized
> [0.00] usbcore: registered new interface driver usbfs
> [0.00] usbcore: registered new interface driver hub
> [0.00] usbcore: registered new device driver usb
> [0.00] clocksource: ostm: mask: 0x max_cycles: 0x, 
> max_idle_ns: 28958491609 ns
> [0.000619] sched_clock: 32 bits at 66MHz, resolution 15ns, wraps every 
> 32537631224ns
> [0.008599] ostm: used for clocksource
> [0.018926] ostm: used for clock events
> [0.19] clocksource: Switched to clocksource ostm
> [0.821474] NET: Registered protocol family 2
> [0.840624] tcp_listen_portaddr_hash hash table entries: 512 (order: 0, 
> 4096 bytes)
> [0.850549] TCP established hash table entries: 1024 (order: 0, 4096 bytes)
> ...
> ...
> 
> 
> 
>> Maybe you
>> just get lucky and it works as long as no thread blocks (e.g. on a
>> msleep).
> 
> You're right. If I put in a msleep() before my timer is up and running, it 
> hangs.
> 
> As far as I can tell, nothing before device_initcall seems to call anything 
> like msleep.
> 
>> If things changed and you can setup a timer in an initcall,
>> then why are folks still trying to do things like early platform
>> drivers. Regular drivers would work and we should be able to
>> completely remove CLK_OF_DECLARE and TIMER_OF_DECLARE.
> 
> I wonder if the reason is because you can't assign a priority to your 
> driver when you declare it with xxx_initcall( ). So, your driver ends up 
> in the same table as all the other drivers and you are not guaranteed the
> order in which they probe. So, the answer was to make a NEW table and 
> register it using TIMER_OF_DECLARE and CLOCK_OF_DECLARE.
> 
> I wonder they just didn't make a clock_initcall() and timer_initcall() 
> instead.

What happens if you place the clk_init() before board_time_init() ? in
arch/sh/kernel/time.c

void __init time_init(void)
{
if (board_time_init)
board_time_init();

clk_init();

late_time_init = sh_late_time_init;
}



-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH v2 0/3] Add R-Car gen3 SoC support to the CMT driver

2018-09-14 Thread Daniel Lezcano 
On 12/09/2018 22:10, Sergei Shtylyov wrote:
> Hello!
> 
> Here's the set of 3 patches against the 'tip.git' repo's 'timers/core' branch
> plus the CMT driver fixups for the 32/64-bit machines posted recently. We're
> adding support for the CMT types0/1 found in the R-Car gen3 SoCs.
> 
> [1/3] clocksource: sh_cmt: properly line-wrap sh_cmt_of_table[] initializer
> [2/3] dt-bindings: timer: renesas: cmt: document R-Car gen3 support
> [3/3] clocksource: sh_cmt: add R-Car gen3 support

Applied for 4.20, thanks.

-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH] clocksource: sh_cmt: fix clocksource width for 32-bit machines

2018-09-14 Thread Daniel Lezcano 
On 10/09/2018 22:22, Sergei Shtylyov wrote:
> The driver seems to abuse *unsigned long* not only for the (32-bit)
> register values but also for the 'sh_cmt_channel::total_cycles' which
> needs to always be 64-bit -- as a result, the clocksource's mask is
> needlessly clamped down to 32-bits on the 32-bit machines...
> 
> Reported-by: Geert Uytterhoeven 
> Signed-off-by: Sergei Shtylyov 
> 
> ---

Applied for 4.20, thanks.

Re: [PATCH v3] clocksource: sh_cmt: fixup for 64-bit machines

2018-09-14 Thread Daniel Lezcano 
On 08/09/2018 22:54, Sergei Shtylyov wrote:
> When trying to use CMT for clockevents on R-Car gen3 SoCs, I noticed
> that 'max_delta_ns' for the broadcast timer (CMT) was shown as 1000 in
> /proc/timer_list. It turned out that when calculating it, the driver did
> 1 << 32 (causing what I think was undefined behavior) resulting in a zero
> delta, later clamped to 1000 by cev_delta2ns(). The root cause turned out
> to be that the driver abused *unsigned long* for the CMT register values
> (which are 16/32-bit), so that the calculation of 'ch->max_match_value'
> in sh_cmt_setup_channel() used the wrong branch. Using more proper 'u32'
> instead fixed 'max_delta_ns' and even fixed the switching an active
> clocksource to CMT (which caused the system to turn non-interactive
> before).
> 
> Signed-off-by: Sergei Shtylyov 
> 
> ---
> This patch is against the 'tip.git' repo's 'timers/core' branch.
> The CMT driver wasn't ever used on SH64; on R-Car gen3 (ARM64) I'm only
> enabling building of this driver now, so not sure how urgent is this...
> 
> Changes in version 3:
> - made the 'overflow_bit' and 'clear_bits' members of 'struct sh_cmt_info'
>   'u32';
> - made the 2nd parameter of sh_cmt_write_cmstr() 'u32';
> - made the result, the 2nd parameter, and 'o{1|2}' local variables of
>   sh_cmt_get_counter() 'u32';
> - made the 'has_wrapped' local variables 'u32' in sh_cmt_clocksource_read()
>   and sh_cmt_clock_event_program_verify();
> - fixed a typo in the patch description.
> 
> Changes in version 2:
> - completely redid the patch, fixing abuse of *unsigned long* for the CMT
>   register values.
> 
>  drivers/clocksource/sh_cmt.c |   72 
> +++
>  1 file changed, 33 insertions(+), 39 deletions(-)
> 

Applied for 4.20, thanks.

-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH v3] clocksource: sh_cmt: fixup for 64-bit machines

2018-09-13 Thread Daniel Lezcano 
On 08/09/2018 22:54, Sergei Shtylyov wrote:
> When trying to use CMT for clockevents on R-Car gen3 SoCs, I noticed
> that 'max_delta_ns' for the broadcast timer (CMT) was shown as 1000 in
> /proc/timer_list. It turned out that when calculating it, the driver did
> 1 << 32 (causing what I think was undefined behavior) resulting in a zero
> delta, later clamped to 1000 by cev_delta2ns(). The root cause turned out
> to be that the driver abused *unsigned long* for the CMT register values
> (which are 16/32-bit), so that the calculation of 'ch->max_match_value'
> in sh_cmt_setup_channel() used the wrong branch. Using more proper 'u32'
> instead fixed 'max_delta_ns' and even fixed the switching an active
> clocksource to CMT (which caused the system to turn non-interactive
> before).
> 
> Signed-off-by: Sergei Shtylyov 

Geert any comments ?

Sergei, in the future please Cc people who did comments on your patch.

Thanks

  -- Daniel

> ---
> This patch is against the 'tip.git' repo's 'timers/core' branch.
> The CMT driver wasn't ever used on SH64; on R-Car gen3 (ARM64) I'm only
> enabling building of this driver now, so not sure how urgent is this...
> 
> Changes in version 3:
> - made the 'overflow_bit' and 'clear_bits' members of 'struct sh_cmt_info'
>   'u32';
> - made the 2nd parameter of sh_cmt_write_cmstr() 'u32';
> - made the result, the 2nd parameter, and 'o{1|2}' local variables of
>   sh_cmt_get_counter() 'u32';
> - made the 'has_wrapped' local variables 'u32' in sh_cmt_clocksource_read()
>   and sh_cmt_clock_event_program_verify();
> - fixed a typo in the patch description.
> 
> Changes in version 2:
> - completely redid the patch, fixing abuse of *unsigned long* for the CMT
>   register values.
> 
>  drivers/clocksource/sh_cmt.c |   72 
> +++
>  1 file changed, 33 insertions(+), 39 deletions(-)
> 
> Index: tip/drivers/clocksource/sh_cmt.c
> ===
> --- tip.orig/drivers/clocksource/sh_cmt.c
> +++ tip/drivers/clocksource/sh_cmt.c
> @@ -78,18 +78,17 @@ struct sh_cmt_info {
>   unsigned int channels_mask;
>  
>   unsigned long width; /* 16 or 32 bit version of hardware block */
> - unsigned long overflow_bit;
> - unsigned long clear_bits;
> + u32 overflow_bit;
> + u32 clear_bits;
>  
>   /* callbacks for CMSTR and CMCSR access */
> - unsigned long (*read_control)(void __iomem *base, unsigned long offs);
> + u32 (*read_control)(void __iomem *base, unsigned long offs);
>   void (*write_control)(void __iomem *base, unsigned long offs,
> -   unsigned long value);
> +   u32 value);
>  
>   /* callbacks for CMCNT and CMCOR access */
> - unsigned long (*read_count)(void __iomem *base, unsigned long offs);
> - void (*write_count)(void __iomem *base, unsigned long offs,
> - unsigned long value);
> + u32 (*read_count)(void __iomem *base, unsigned long offs);
> + void (*write_count)(void __iomem *base, unsigned long offs, u32 value);
>  };
>  
>  struct sh_cmt_channel {
> @@ -103,9 +102,9 @@ struct sh_cmt_channel {
>  
>   unsigned int timer_bit;
>   unsigned long flags;
> - unsigned long match_value;
> - unsigned long next_match_value;
> - unsigned long max_match_value;
> + u32 match_value;
> + u32 next_match_value;
> + u32 max_match_value;
>   raw_spinlock_t lock;
>   struct clock_event_device ced;
>   struct clocksource cs;
> @@ -160,24 +159,22 @@ struct sh_cmt_device {
>  #define SH_CMT32_CMCSR_CKS_RCLK1 (7 << 0)
>  #define SH_CMT32_CMCSR_CKS_MASK  (7 << 0)
>  
> -static unsigned long sh_cmt_read16(void __iomem *base, unsigned long offs)
> +static u32 sh_cmt_read16(void __iomem *base, unsigned long offs)
>  {
>   return ioread16(base + (offs << 1));
>  }
>  
> -static unsigned long sh_cmt_read32(void __iomem *base, unsigned long offs)
> +static u32 sh_cmt_read32(void __iomem *base, unsigned long offs)
>  {
>   return ioread32(base + (offs << 2));
>  }
>  
> -static void sh_cmt_write16(void __iomem *base, unsigned long offs,
> -unsigned long value)
> +static void sh_cmt_write16(void __iomem *base, unsigned long offs, u32 value)
>  {
>   iowrite16(value, base + (offs << 1));
>  }
>  
> -static void sh_cmt_write32(void __iomem *base, unsigned long offs,
> -unsigned long value)
> +static void sh_cmt_write32(void __iomem *base, unsigned long offs, u32 value)
>  {
>   iowrite32(value, base + (offs << 2));
>  }
> @@ -242,7 +239,7 @@ static const struct sh_cmt_info sh_cmt_i
>  #define CMCNT 1 /* channel register */
>  #define CMCOR 2 /* channel register */
>  
> -static inline unsigned long sh_cmt_read_cmstr(struct sh_cmt_channel *ch)
> +static inline u32 sh_cmt_read_cmstr(struct sh_cmt_channel *ch)
>  {
>   if (ch->iostart)
>   return