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

[Geert Uytterhoeven]

Hi Daniel,

On Wed, Jun 1, 2016 at 10:34 AM, Daniel Lezcano
 wrote:
> The macro CLOCKSOURCE_OF_DECLARE is widely used in the timer drivers.
>
> Basically, this macro is defined to insert in a table a tuple name,function.
> This function is an init function called when the name matches the DT node and
> its signature is:
>
> typedef void (*of_init_fn_1)(struct device_node *);
>
> It does not return an error code.
>
> That results in the clocksource-probe not being able to figure out if the 
> driver
> was correctly initialized or not, the different drivers to act as they were 
> the
> only ones on the system (panic, instead of failing gracefully), and duplicated
> code for error reporting.
>
> This series initiates the logic change and centralize the error handling in 
> the
> clocksource probe code.
>
> In order to do the changes little by little, a new macro was introduced:
>
>  CLOCKSOURCE_OF_DECLARE_RET()

As I have no other thread to reply to, I'm using this related one.

commit bcbe219f9306da478b77e705a7273843c2660d7b
Author: Daniel Lezcano 
Date:   Tue Jun 7 00:27:44 2016 +0200

clocksources: Switch back to the clksrc table

Signed-off-by: Daniel Lezcano 

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)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

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

[Daniel Lezcano]

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

Hi Daniel,

On Wed, Jun 1, 2016 at 10:34 AM, Daniel Lezcano
 wrote:

The macro CLOCKSOURCE_OF_DECLARE is widely used in the timer drivers.

Basically, this macro is defined to insert in a table a tuple name,function.
This function is an init function called when the name matches the DT node and
its signature is:

 typedef void (*of_init_fn_1)(struct device_node *);

It does not return an error code.

That results in the clocksource-probe not being able to figure out if the driver
was correctly initialized or not, the different drivers to act as they were the
only ones on the system (panic, instead of failing gracefully), and duplicated
code for error reporting.

This series initiates the logic change and centralize the error handling in the
clocksource probe code.

In order to do the changes little by little, a new macro was introduced:

  CLOCKSOURCE_OF_DECLARE_RET()


As I have no other thread to reply to, I'm using this related one.

commit bcbe219f9306da478b77e705a7273843c2660d7b
Author: Daniel Lezcano 
Date:   Tue Jun 7 00:27:44 2016 +0200

 clocksources: Switch back to the clksrc table

 Signed-off-by: Daniel Lezcano 

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:


Ah, thanks for reporting.

I will fix it.

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

[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

[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

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

[Geert Uytterhoeven]

Hi Daniel,

On Wed, Jun 8, 2016 at 4:10 PM, Daniel Lezcano
 wrote:
> 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 ?

I can confirm the issue is fixed in today's clockevents/next
(52be039599e1339e).

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

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

[Daniel Lezcano]

On 06/09/2016 09:46 AM, Geert Uytterhoeven wrote:

Hi Daniel,

On Wed, Jun 8, 2016 at 4:10 PM, Daniel Lezcano
 wrote:

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 ?


I can confirm the issue is fixed in today's clockevents/next
(52be039599e1339e).


Great ! Thanks for testing.

  -- Daniel


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

Follow Linaro:   Facebook |
 Twitter |
 Blog