linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] clocksource: dw_apb_timer_of: return EPROBE_DEFER if no clock available
@ 2020-11-19 12:12 Dinh Nguyen
  2020-11-20 10:02 ` Jisheng Zhang
  2020-12-03 10:09 ` Daniel Lezcano
  0 siblings, 2 replies; 4+ messages in thread
From: Dinh Nguyen @ 2020-11-19 12:12 UTC (permalink / raw)
  To: linux-kernel; +Cc: dinguyen, daniel.lezcano, tglx, p.zabel, Jisheng.Zhang, arnd

commit ("b0fc70ce1f02 arm64: berlin: Select DW_APB_TIMER_OF") added the
support for the dw_apb_timer into the arm64 defconfig. However, for some
platforms like the Intel Stratix10 and Agilex, the clock manager doesn't
get probed until after the timer driver is probed. Thus, the driver hits
the panic "No clock nor clock-frequency property for %" because it cannot
properly get the clock.

This patch adds support for EPROBE_DEFER so the kernel can come back to
finish probing this timer driver after the clock driver is probed.

Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
 drivers/clocksource/dw_apb_timer_of.c | 86 ++++++++++++++++-----------
 1 file changed, 51 insertions(+), 35 deletions(-)

diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
index ab3ddebe8344..a8ce980c5146 100644
--- a/drivers/clocksource/dw_apb_timer_of.c
+++ b/drivers/clocksource/dw_apb_timer_of.c
@@ -14,7 +14,7 @@
 #include <linux/reset.h>
 #include <linux/sched_clock.h>
 
-static void __init timer_get_base_and_rate(struct device_node *np,
+static int __init timer_get_base_and_rate(struct device_node *np,
 				    void __iomem **base, u32 *rate)
 {
 	struct clk *timer_clk;
@@ -47,65 +47,77 @@ static void __init timer_get_base_and_rate(struct device_node *np,
 				np);
 
 	timer_clk = of_clk_get_by_name(np, "timer");
-	if (IS_ERR(timer_clk))
-		goto try_clock_freq;
+	if (IS_ERR(timer_clk)) {
+		if (PTR_ERR(timer_clk) != -EPROBE_DEFER) {
+			pr_err("Failed to get clock for %pOF\n", np);
+			goto try_clock_freq;
+		}
+		return PTR_ERR(timer_clk);
+	}
 
 	if (!clk_prepare_enable(timer_clk)) {
 		*rate = clk_get_rate(timer_clk);
-		return;
+		return 0;
 	}
 
 try_clock_freq:
 	if (of_property_read_u32(np, "clock-freq", rate) &&
 	    of_property_read_u32(np, "clock-frequency", rate))
 		panic("No clock nor clock-frequency property for %pOFn", np);
+	return 0;
 }
 
-static void __init add_clockevent(struct device_node *event_timer)
+static int __init add_clockevent(struct device_node *event_timer)
 {
 	void __iomem *iobase;
 	struct dw_apb_clock_event_device *ced;
 	u32 irq, rate;
+	int ret = 0;
 
 	irq = irq_of_parse_and_map(event_timer, 0);
 	if (irq == 0)
 		panic("No IRQ for clock event timer");
 
-	timer_get_base_and_rate(event_timer, &iobase, &rate);
-
-	ced = dw_apb_clockevent_init(-1, event_timer->name, 300, iobase, irq,
+	ret = timer_get_base_and_rate(event_timer, &iobase, &rate);
+	if (ret == 0) {
+		ced = dw_apb_clockevent_init(-1, event_timer->name, 300, iobase, irq,
 				     rate);
-	if (!ced)
-		panic("Unable to initialise clockevent device");
+		if (!ced)
+			panic("Unable to initialise clockevent device");
 
-	dw_apb_clockevent_register(ced);
+		dw_apb_clockevent_register(ced);
+	}
+	return ret;
 }
 
 static void __iomem *sched_io_base;
 static u32 sched_rate;
 
-static void __init add_clocksource(struct device_node *source_timer)
+static int __init add_clocksource(struct device_node *source_timer)
 {
 	void __iomem *iobase;
 	struct dw_apb_clocksource *cs;
 	u32 rate;
-
-	timer_get_base_and_rate(source_timer, &iobase, &rate);
-
-	cs = dw_apb_clocksource_init(300, source_timer->name, iobase, rate);
-	if (!cs)
-		panic("Unable to initialise clocksource device");
-
-	dw_apb_clocksource_start(cs);
-	dw_apb_clocksource_register(cs);
-
-	/*
-	 * Fallback to use the clocksource as sched_clock if no separate
-	 * timer is found. sched_io_base then points to the current_value
-	 * register of the clocksource timer.
-	 */
-	sched_io_base = iobase + 0x04;
-	sched_rate = rate;
+	int ret;
+
+	ret = timer_get_base_and_rate(source_timer, &iobase, &rate);
+	if (ret == 0) {
+		cs = dw_apb_clocksource_init(300, source_timer->name, iobase, rate);
+		if (!cs)
+			panic("Unable to initialise clocksource device");
+
+		dw_apb_clocksource_start(cs);
+		dw_apb_clocksource_register(cs);
+
+		/*
+		 * Fallback to use the clocksource as sched_clock if no separate
+		 * timer is found. sched_io_base then points to the current_value
+		 * register of the clocksource timer.
+		 */
+		sched_io_base = iobase + 0x04;
+		sched_rate = rate;
+	}
+	return ret;
 }
 
 static u64 notrace read_sched_clock(void)
@@ -146,25 +158,29 @@ static struct delay_timer dw_apb_delay_timer = {
 static int num_called;
 static int __init dw_apb_timer_init(struct device_node *timer)
 {
+	int ret = 0;
+
 	switch (num_called) {
 	case 1:
 		pr_debug("%s: found clocksource timer\n", __func__);
-		add_clocksource(timer);
-		init_sched_clock();
+		ret = add_clocksource(timer);
+		if (ret == 0) {
+			init_sched_clock();
 #ifdef CONFIG_ARM
-		dw_apb_delay_timer.freq = sched_rate;
-		register_current_timer_delay(&dw_apb_delay_timer);
+			dw_apb_delay_timer.freq = sched_rate;
+			register_current_timer_delay(&dw_apb_delay_timer);
 #endif
+		}
 		break;
 	default:
 		pr_debug("%s: found clockevent timer\n", __func__);
-		add_clockevent(timer);
+		ret = add_clockevent(timer);
 		break;
 	}
 
 	num_called++;
 
-	return 0;
+	return ret;
 }
 TIMER_OF_DECLARE(pc3x2_timer, "picochip,pc3x2-timer", dw_apb_timer_init);
 TIMER_OF_DECLARE(apb_timer_osc, "snps,dw-apb-timer-osc", dw_apb_timer_init);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] clocksource: dw_apb_timer_of: return EPROBE_DEFER if no clock available
  2020-11-19 12:12 [PATCH] clocksource: dw_apb_timer_of: return EPROBE_DEFER if no clock available Dinh Nguyen
@ 2020-11-20 10:02 ` Jisheng Zhang
  2020-12-02 15:33   ` Dinh Nguyen
  2020-12-03 10:09 ` Daniel Lezcano
  1 sibling, 1 reply; 4+ messages in thread
From: Jisheng Zhang @ 2020-11-20 10:02 UTC (permalink / raw)
  To: Dinh Nguyen; +Cc: linux-kernel, daniel.lezcano, tglx, p.zabel, arnd

On Thu, 19 Nov 2020 06:12:25 -0600
Dinh Nguyen <dinguyen@kernel.org> wrote:


> 
> 
> commit ("b0fc70ce1f02 arm64: berlin: Select DW_APB_TIMER_OF") added the
> support for the dw_apb_timer into the arm64 defconfig. However, for some
> platforms like the Intel Stratix10 and Agilex, the clock manager doesn't
> get probed until after the timer driver is probed. Thus, the driver hits
> the panic "No clock nor clock-frequency property for %" because it cannot
> properly get the clock.
> 
> This patch adds support for EPROBE_DEFER so the kernel can come back to
> finish probing this timer driver after the clock driver is probed.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>

Reviewed-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>

> ---
>  drivers/clocksource/dw_apb_timer_of.c | 86 ++++++++++++++++-----------
>  1 file changed, 51 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
> index ab3ddebe8344..a8ce980c5146 100644
> --- a/drivers/clocksource/dw_apb_timer_of.c
> +++ b/drivers/clocksource/dw_apb_timer_of.c
> @@ -14,7 +14,7 @@
>  #include <linux/reset.h>
>  #include <linux/sched_clock.h>
> 
> -static void __init timer_get_base_and_rate(struct device_node *np,
> +static int __init timer_get_base_and_rate(struct device_node *np,
>                                     void __iomem **base, u32 *rate)
>  {
>         struct clk *timer_clk;
> @@ -47,65 +47,77 @@ static void __init timer_get_base_and_rate(struct device_node *np,
>                                 np);
> 
>         timer_clk = of_clk_get_by_name(np, "timer");
> -       if (IS_ERR(timer_clk))
> -               goto try_clock_freq;
> +       if (IS_ERR(timer_clk)) {
> +               if (PTR_ERR(timer_clk) != -EPROBE_DEFER) {
> +                       pr_err("Failed to get clock for %pOF\n", np);
> +                       goto try_clock_freq;
> +               }
> +               return PTR_ERR(timer_clk);
> +       }
> 
>         if (!clk_prepare_enable(timer_clk)) {
>                 *rate = clk_get_rate(timer_clk);
> -               return;
> +               return 0;
>         }
> 
>  try_clock_freq:
>         if (of_property_read_u32(np, "clock-freq", rate) &&
>             of_property_read_u32(np, "clock-frequency", rate))
>                 panic("No clock nor clock-frequency property for %pOFn", np);
> +       return 0;
>  }
> 
> -static void __init add_clockevent(struct device_node *event_timer)
> +static int __init add_clockevent(struct device_node *event_timer)
>  {
>         void __iomem *iobase;
>         struct dw_apb_clock_event_device *ced;
>         u32 irq, rate;
> +       int ret = 0;
> 
>         irq = irq_of_parse_and_map(event_timer, 0);
>         if (irq == 0)
>                 panic("No IRQ for clock event timer");
> 
> -       timer_get_base_and_rate(event_timer, &iobase, &rate);
> -
> -       ced = dw_apb_clockevent_init(-1, event_timer->name, 300, iobase, irq,
> +       ret = timer_get_base_and_rate(event_timer, &iobase, &rate);
> +       if (ret == 0) {
> +               ced = dw_apb_clockevent_init(-1, event_timer->name, 300, iobase, irq,
>                                      rate);
> -       if (!ced)
> -               panic("Unable to initialise clockevent device");
> +               if (!ced)
> +                       panic("Unable to initialise clockevent device");
> 
> -       dw_apb_clockevent_register(ced);
> +               dw_apb_clockevent_register(ced);
> +       }
> +       return ret;
>  }
> 
>  static void __iomem *sched_io_base;
>  static u32 sched_rate;
> 
> -static void __init add_clocksource(struct device_node *source_timer)
> +static int __init add_clocksource(struct device_node *source_timer)
>  {
>         void __iomem *iobase;
>         struct dw_apb_clocksource *cs;
>         u32 rate;
> -
> -       timer_get_base_and_rate(source_timer, &iobase, &rate);
> -
> -       cs = dw_apb_clocksource_init(300, source_timer->name, iobase, rate);
> -       if (!cs)
> -               panic("Unable to initialise clocksource device");
> -
> -       dw_apb_clocksource_start(cs);
> -       dw_apb_clocksource_register(cs);
> -
> -       /*
> -        * Fallback to use the clocksource as sched_clock if no separate
> -        * timer is found. sched_io_base then points to the current_value
> -        * register of the clocksource timer.
> -        */
> -       sched_io_base = iobase + 0x04;
> -       sched_rate = rate;
> +       int ret;
> +
> +       ret = timer_get_base_and_rate(source_timer, &iobase, &rate);
> +       if (ret == 0) {
> +               cs = dw_apb_clocksource_init(300, source_timer->name, iobase, rate);
> +               if (!cs)
> +                       panic("Unable to initialise clocksource device");
> +
> +               dw_apb_clocksource_start(cs);
> +               dw_apb_clocksource_register(cs);
> +
> +               /*
> +                * Fallback to use the clocksource as sched_clock if no separate
> +                * timer is found. sched_io_base then points to the current_value
> +                * register of the clocksource timer.
> +                */
> +               sched_io_base = iobase + 0x04;
> +               sched_rate = rate;
> +       }
> +       return ret;
>  }
> 
>  static u64 notrace read_sched_clock(void)
> @@ -146,25 +158,29 @@ static struct delay_timer dw_apb_delay_timer = {
>  static int num_called;
>  static int __init dw_apb_timer_init(struct device_node *timer)
>  {
> +       int ret = 0;
> +
>         switch (num_called) {
>         case 1:
>                 pr_debug("%s: found clocksource timer\n", __func__);
> -               add_clocksource(timer);
> -               init_sched_clock();
> +               ret = add_clocksource(timer);
> +               if (ret == 0) {
> +                       init_sched_clock();
>  #ifdef CONFIG_ARM
> -               dw_apb_delay_timer.freq = sched_rate;
> -               register_current_timer_delay(&dw_apb_delay_timer);
> +                       dw_apb_delay_timer.freq = sched_rate;
> +                       register_current_timer_delay(&dw_apb_delay_timer);
>  #endif
> +               }
>                 break;
>         default:
>                 pr_debug("%s: found clockevent timer\n", __func__);
> -               add_clockevent(timer);
> +               ret = add_clockevent(timer);
>                 break;
>         }
> 
>         num_called++;
> 
> -       return 0;
> +       return ret;
>  }
>  TIMER_OF_DECLARE(pc3x2_timer, "picochip,pc3x2-timer", dw_apb_timer_init);
>  TIMER_OF_DECLARE(apb_timer_osc, "snps,dw-apb-timer-osc", dw_apb_timer_init);
> --
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] clocksource: dw_apb_timer_of: return EPROBE_DEFER if no clock available
  2020-11-20 10:02 ` Jisheng Zhang
@ 2020-12-02 15:33   ` Dinh Nguyen
  0 siblings, 0 replies; 4+ messages in thread
From: Dinh Nguyen @ 2020-12-02 15:33 UTC (permalink / raw)
  To: Jisheng Zhang; +Cc: linux-kernel, daniel.lezcano, tglx, p.zabel, arnd

Gentle ping?

On 11/20/20 4:02 AM, Jisheng Zhang wrote:
> On Thu, 19 Nov 2020 06:12:25 -0600
> Dinh Nguyen <dinguyen@kernel.org> wrote:
> 
> 
>>
>>
>> commit ("b0fc70ce1f02 arm64: berlin: Select DW_APB_TIMER_OF") added the
>> support for the dw_apb_timer into the arm64 defconfig. However, for some
>> platforms like the Intel Stratix10 and Agilex, the clock manager doesn't
>> get probed until after the timer driver is probed. Thus, the driver hits
>> the panic "No clock nor clock-frequency property for %" because it cannot
>> properly get the clock.
>>
>> This patch adds support for EPROBE_DEFER so the kernel can come back to
>> finish probing this timer driver after the clock driver is probed.
>>
>> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> 
> Reviewed-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> 
>> ---
>>   drivers/clocksource/dw_apb_timer_of.c | 86 ++++++++++++++++-----------
>>   1 file changed, 51 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
>> index ab3ddebe8344..a8ce980c5146 100644
>> --- a/drivers/clocksource/dw_apb_timer_of.c
>> +++ b/drivers/clocksource/dw_apb_timer_of.c
>> @@ -14,7 +14,7 @@
>>   #include <linux/reset.h>
>>   #include <linux/sched_clock.h>
>>
>> -static void __init timer_get_base_and_rate(struct device_node *np,
>> +static int __init timer_get_base_and_rate(struct device_node *np,
>>                                      void __iomem **base, u32 *rate)
>>   {
>>          struct clk *timer_clk;
>> @@ -47,65 +47,77 @@ static void __init timer_get_base_and_rate(struct device_node *np,
>>                                  np);
>>
>>          timer_clk = of_clk_get_by_name(np, "timer");
>> -       if (IS_ERR(timer_clk))
>> -               goto try_clock_freq;
>> +       if (IS_ERR(timer_clk)) {
>> +               if (PTR_ERR(timer_clk) != -EPROBE_DEFER) {
>> +                       pr_err("Failed to get clock for %pOF\n", np);
>> +                       goto try_clock_freq;
>> +               }
>> +               return PTR_ERR(timer_clk);
>> +       }
>>
>>          if (!clk_prepare_enable(timer_clk)) {
>>                  *rate = clk_get_rate(timer_clk);
>> -               return;
>> +               return 0;
>>          }
>>
>>   try_clock_freq:
>>          if (of_property_read_u32(np, "clock-freq", rate) &&
>>              of_property_read_u32(np, "clock-frequency", rate))
>>                  panic("No clock nor clock-frequency property for %pOFn", np);
>> +       return 0;
>>   }
>>
>> -static void __init add_clockevent(struct device_node *event_timer)
>> +static int __init add_clockevent(struct device_node *event_timer)
>>   {
>>          void __iomem *iobase;
>>          struct dw_apb_clock_event_device *ced;
>>          u32 irq, rate;
>> +       int ret = 0;
>>
>>          irq = irq_of_parse_and_map(event_timer, 0);
>>          if (irq == 0)
>>                  panic("No IRQ for clock event timer");
>>
>> -       timer_get_base_and_rate(event_timer, &iobase, &rate);
>> -
>> -       ced = dw_apb_clockevent_init(-1, event_timer->name, 300, iobase, irq,
>> +       ret = timer_get_base_and_rate(event_timer, &iobase, &rate);
>> +       if (ret == 0) {
>> +               ced = dw_apb_clockevent_init(-1, event_timer->name, 300, iobase, irq,
>>                                       rate);
>> -       if (!ced)
>> -               panic("Unable to initialise clockevent device");
>> +               if (!ced)
>> +                       panic("Unable to initialise clockevent device");
>>
>> -       dw_apb_clockevent_register(ced);
>> +               dw_apb_clockevent_register(ced);
>> +       }
>> +       return ret;
>>   }
>>
>>   static void __iomem *sched_io_base;
>>   static u32 sched_rate;
>>
>> -static void __init add_clocksource(struct device_node *source_timer)
>> +static int __init add_clocksource(struct device_node *source_timer)
>>   {
>>          void __iomem *iobase;
>>          struct dw_apb_clocksource *cs;
>>          u32 rate;
>> -
>> -       timer_get_base_and_rate(source_timer, &iobase, &rate);
>> -
>> -       cs = dw_apb_clocksource_init(300, source_timer->name, iobase, rate);
>> -       if (!cs)
>> -               panic("Unable to initialise clocksource device");
>> -
>> -       dw_apb_clocksource_start(cs);
>> -       dw_apb_clocksource_register(cs);
>> -
>> -       /*
>> -        * Fallback to use the clocksource as sched_clock if no separate
>> -        * timer is found. sched_io_base then points to the current_value
>> -        * register of the clocksource timer.
>> -        */
>> -       sched_io_base = iobase + 0x04;
>> -       sched_rate = rate;
>> +       int ret;
>> +
>> +       ret = timer_get_base_and_rate(source_timer, &iobase, &rate);
>> +       if (ret == 0) {
>> +               cs = dw_apb_clocksource_init(300, source_timer->name, iobase, rate);
>> +               if (!cs)
>> +                       panic("Unable to initialise clocksource device");
>> +
>> +               dw_apb_clocksource_start(cs);
>> +               dw_apb_clocksource_register(cs);
>> +
>> +               /*
>> +                * Fallback to use the clocksource as sched_clock if no separate
>> +                * timer is found. sched_io_base then points to the current_value
>> +                * register of the clocksource timer.
>> +                */
>> +               sched_io_base = iobase + 0x04;
>> +               sched_rate = rate;
>> +       }
>> +       return ret;
>>   }
>>
>>   static u64 notrace read_sched_clock(void)
>> @@ -146,25 +158,29 @@ static struct delay_timer dw_apb_delay_timer = {
>>   static int num_called;
>>   static int __init dw_apb_timer_init(struct device_node *timer)
>>   {
>> +       int ret = 0;
>> +
>>          switch (num_called) {
>>          case 1:
>>                  pr_debug("%s: found clocksource timer\n", __func__);
>> -               add_clocksource(timer);
>> -               init_sched_clock();
>> +               ret = add_clocksource(timer);
>> +               if (ret == 0) {
>> +                       init_sched_clock();
>>   #ifdef CONFIG_ARM
>> -               dw_apb_delay_timer.freq = sched_rate;
>> -               register_current_timer_delay(&dw_apb_delay_timer);
>> +                       dw_apb_delay_timer.freq = sched_rate;
>> +                       register_current_timer_delay(&dw_apb_delay_timer);
>>   #endif
>> +               }
>>                  break;
>>          default:
>>                  pr_debug("%s: found clockevent timer\n", __func__);
>> -               add_clockevent(timer);
>> +               ret = add_clockevent(timer);
>>                  break;
>>          }
>>
>>          num_called++;
>>
>> -       return 0;
>> +       return ret;
>>   }
>>   TIMER_OF_DECLARE(pc3x2_timer, "picochip,pc3x2-timer", dw_apb_timer_init);
>>   TIMER_OF_DECLARE(apb_timer_osc, "snps,dw-apb-timer-osc", dw_apb_timer_init);
>> --
>> 2.17.1
>>
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] clocksource: dw_apb_timer_of: return EPROBE_DEFER if no clock available
  2020-11-19 12:12 [PATCH] clocksource: dw_apb_timer_of: return EPROBE_DEFER if no clock available Dinh Nguyen
  2020-11-20 10:02 ` Jisheng Zhang
@ 2020-12-03 10:09 ` Daniel Lezcano
  1 sibling, 0 replies; 4+ messages in thread
From: Daniel Lezcano @ 2020-12-03 10:09 UTC (permalink / raw)
  To: Dinh Nguyen, linux-kernel; +Cc: tglx, p.zabel, Jisheng.Zhang, arnd


Hi Dinh,

On 19/11/2020 13:12, Dinh Nguyen wrote:
> commit ("b0fc70ce1f02 arm64: berlin: Select DW_APB_TIMER_OF") added the
> support for the dw_apb_timer into the arm64 defconfig. However, for some
> platforms like the Intel Stratix10 and Agilex, the clock manager doesn't
> get probed until after the timer driver is probed. Thus, the driver hits
> the panic "No clock nor clock-frequency property for %" because it cannot
> properly get the clock.
> 
> This patch adds support for EPROBE_DEFER so the kernel can come back to
> finish probing this timer driver after the clock driver is probed.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>

A few comments below.

> ---
>  drivers/clocksource/dw_apb_timer_of.c | 86 ++++++++++++++++-----------
>  1 file changed, 51 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
> index ab3ddebe8344..a8ce980c5146 100644
> --- a/drivers/clocksource/dw_apb_timer_of.c
> +++ b/drivers/clocksource/dw_apb_timer_of.c
> @@ -14,7 +14,7 @@
>  #include <linux/reset.h>
>  #include <linux/sched_clock.h>
>  
> -static void __init timer_get_base_and_rate(struct device_node *np,
> +static int __init timer_get_base_and_rate(struct device_node *np,
>  				    void __iomem **base, u32 *rate)
>  {
>  	struct clk *timer_clk;
> @@ -47,65 +47,77 @@ static void __init timer_get_base_and_rate(struct device_node *np,
>  				np);
>  
>  	timer_clk = of_clk_get_by_name(np, "timer");
> -	if (IS_ERR(timer_clk))
> -		goto try_clock_freq;
> +	if (IS_ERR(timer_clk)) {
> +		if (PTR_ERR(timer_clk) != -EPROBE_DEFER) {
> +			pr_err("Failed to get clock for %pOF\n", np);
> +			goto try_clock_freq;
> +		}
> +		return PTR_ERR(timer_clk);
> +	}

May be massage the changes by moving the of-rate check first

 	if (!of_property_read_u32(np, "clock-freq", rate) &&
  	    !of_property_read_u32(np, "clock-frequency", rate))
		return 0;

	timer_clk = of_clk_get_by_name(np, "timer");

	/*
	 * Whatever the result, we return
	 */
	ret = PTR_ERR(timer_clk);
	if (ret)
		return ret;

	ret = clk_prepare_enable(timer_clk);
	if (ret)
		return ret;

	*rate = clk_get_rate(timer_clk);
	if (!(*rate))
		return -EINVAL;

	return 0;

>  
>  	if (!clk_prepare_enable(timer_clk)) {
>  		*rate = clk_get_rate(timer_clk);
> -		return;
> +		return 0;
>  	}
>  
>  try_clock_freq:
>  	if (of_property_read_u32(np, "clock-freq", rate) &&
>  	    of_property_read_u32(np, "clock-frequency", rate))
>  		panic("No clock nor clock-frequency property for %pOFn", np);
>
> +	return 0;
>  }
>  
> -static void __init add_clockevent(struct device_node *event_timer)
> +static int __init add_clockevent(struct device_node *event_timer)
>  {
>  	void __iomem *iobase;
>  	struct dw_apb_clock_event_device *ced;
>  	u32 irq, rate;
> +	int ret = 0;
>  
>  	irq = irq_of_parse_and_map(event_timer, 0);
>  	if (irq == 0)
>  		panic("No IRQ for clock event timer");
>  
> -	timer_get_base_and_rate(event_timer, &iobase, &rate);
> -
> -	ced = dw_apb_clockevent_init(-1, event_timer->name, 300, iobase, irq,
> +	ret = timer_get_base_and_rate(event_timer, &iobase, &rate);
> +	if (ret == 0) {
> +		ced = dw_apb_clockevent_init(-1, event_timer->name, 300, iobase, irq,
>  				     rate);
> -	if (!ced)
> -		panic("Unable to initialise clockevent device");
> +		if (!ced)
> +			panic("Unable to initialise clockevent device");

	ret = timer_get_base_and_rate(event_timer, &iobase, &rate);
	if (ret)
		return ret;

	ced = dw_apb_clockevent_init(-1, event_timer->name, 300,
				iobase, irq, rate);
	if (!ced)
		return -EINVAL;

	dw_apb_clockevent_register(ced);

	return 0;

> -	dw_apb_clockevent_register(ced);
> +		dw_apb_clockevent_register(ced);
> +	}
> +	return ret;
>  }
>  
>  static void __iomem *sched_io_base;
>  static u32 sched_rate;
>  
> -static void __init add_clocksource(struct device_node *source_timer)
> +static int __init add_clocksource(struct device_node *source_timer)
>  {
>  	void __iomem *iobase;
>  	struct dw_apb_clocksource *cs;
>  	u32 rate;
> -
> -	timer_get_base_and_rate(source_timer, &iobase, &rate);
> -
> -	cs = dw_apb_clocksource_init(300, source_timer->name, iobase, rate);
> -	if (!cs)
> -		panic("Unable to initialise clocksource device");
> -
> -	dw_apb_clocksource_start(cs);
> -	dw_apb_clocksource_register(cs);
> -
> -	/*
> -	 * Fallback to use the clocksource as sched_clock if no separate
> -	 * timer is found. sched_io_base then points to the current_value
> -	 * register of the clocksource timer.
> -	 */
> -	sched_io_base = iobase + 0x04;
> -	sched_rate = rate;
> +	int ret;
> +
> +	ret = timer_get_base_and_rate(source_timer, &iobase, &rate);
> +	if (ret == 0) {
> +		cs = dw_apb_clocksource_init(300, source_timer->name, iobase, rate);
> +		if (!cs)
> +			panic("Unable to initialise clocksource device");
> +
> +		dw_apb_clocksource_start(cs);
> +		dw_apb_clocksource_register(cs);
> +
> +		/*
> +		 * Fallback to use the clocksource as sched_clock if no separate
> +		 * timer is found. sched_io_base then points to the current_value
> +		 * register of the clocksource timer.
> +		 */
> +		sched_io_base = iobase + 0x04;
> +		sched_rate = rate;
> +	}
> +	return ret;

Same suggestion as above

>  }
>  
>  static u64 notrace read_sched_clock(void)
> @@ -146,25 +158,29 @@ static struct delay_timer dw_apb_delay_timer = {
>  static int num_called;
>  static int __init dw_apb_timer_init(struct device_node *timer)
>  {
> +	int ret = 0;
> +
>  	switch (num_called) {
>  	case 1:
>  		pr_debug("%s: found clocksource timer\n", __func__);
> -		add_clocksource(timer);
> -		init_sched_clock();
> +		ret = add_clocksource(timer);
> +		if (ret == 0) {
> +			init_sched_clock();

		ret = add_clocksource(timer);
		if (ret)
			return ret;

		init_sched_clock();
		dw_apb_delay_timer.freq = sched_rate;
		register_current_timer_delay(&dw_apb_delay_timer);

>  #ifdef CONFIG_ARM
> -		dw_apb_delay_timer.freq = sched_rate;
> -		register_current_timer_delay(&dw_apb_delay_timer);
> +			dw_apb_delay_timer.freq = sched_rate;
> +			register_current_timer_delay(&dw_apb_delay_timer);
>  #endif
> +		}
>  		break;
>  	default:
>  		pr_debug("%s: found clockevent timer\n", __func__);
> -		add_clockevent(timer);
> +		ret = add_clockevent(timer);

		ret = add_clockevent(timer);
		if (ret)
			return ret;

>  		break;
>  	}
>  
>  	num_called++;
>  
> -	return 0;
> +	return ret;

	return 0;

>  }
>  TIMER_OF_DECLARE(pc3x2_timer, "picochip,pc3x2-timer", dw_apb_timer_init);
>  TIMER_OF_DECLARE(apb_timer_osc, "snps,dw-apb-timer-osc", dw_apb_timer_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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-12-03 10:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-19 12:12 [PATCH] clocksource: dw_apb_timer_of: return EPROBE_DEFER if no clock available Dinh Nguyen
2020-11-20 10:02 ` Jisheng Zhang
2020-12-02 15:33   ` Dinh Nguyen
2020-12-03 10:09 ` Daniel Lezcano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).