[U-Boot,09/12] microblaze: timer: Add device-tree driver configuration

Submitted by Michal Simek on July 9, 2012, 9:20 a.m.

Details

Message ID 1341825639-23475-9-git-send-email-monstr@monstr.eu
State Accepted
Delegated to: Michal Simek
Headers show

Commit Message

Michal Simek July 9, 2012, 9:20 a.m.
Read configuration from DTB.

Signed-off-by: Michal Simek <monstr@monstr.eu>
---
 arch/microblaze/cpu/timer.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

Comments

Stephan Linz July 9, 2012, 6:25 p.m.
Hi Michal,

Am Montag, den 09.07.2012, 11:20 +0200 schrieb Michal Simek: 
> Read configuration from DTB.
> 
> Signed-off-by: Michal Simek <monstr@monstr.eu>
> ---
>  arch/microblaze/cpu/timer.c |   25 +++++++++++++++++++++++++
>  1 files changed, 25 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c
> index dfaaaf5..91ca42b 100644
> --- a/arch/microblaze/cpu/timer.c
> +++ b/arch/microblaze/cpu/timer.c
> @@ -25,6 +25,9 @@
>  #include <common.h>
>  #include <asm/microblaze_timer.h>
>  #include <asm/microblaze_intc.h>
> +#include <fdtdec.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
>  
>  volatile int timestamp = 0;
>  microblaze_timer_t *tmr;
> @@ -62,11 +65,33 @@ int timer_init (void)
>  	u32 preload = 0;
>  	u32 ret = 0;
>  
> +#ifndef CONFIG_OF_CONTROL
>  #if defined(CONFIG_SYS_TIMER_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM)
>  	preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ;
>  	irq = CONFIG_SYS_TIMER_0_IRQ;
>  	tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR);
>  #endif
> +#else
> +	int temp;
> +	int offset = 0;
> +
> +	offset = fdt_node_offset_by_compatible(gd->fdt_blob, offset,
> +				"xlnx,xps-timer-1.00.a");
> +	if (offset > 0) {
> +		temp = fdtdec_get_addr(gd->fdt_blob, offset, "reg");
> +		if (temp != FDT_ADDR_T_NONE) {
> +			tmr = (microblaze_timer_t *)temp;
> +			irq = fdtdec_get_int(gd->fdt_blob, offset,
> +							"interrupts", -1);
> +			if (irq == -1)

irq is u32 (unsigned) -- my be a problem. But yes, I'm missing a
compilation warning here ...


br,
Stephan

> +				panic("Connect IRQ to system timer\n");
> +			/* Set default clock frequency */
> +			temp = fdtdec_get_int(gd->fdt_blob, offset,
> +					"clock-frequency", 0);
> +			preload = temp / CONFIG_SYS_HZ;
> +		}
> +	}
> +#endif
>  
>  	if (tmr && irq && preload) {
>  		tmr->loadreg = preload;
Simon Glass July 9, 2012, 9:32 p.m.
Hi Michal,

On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek <monstr@monstr.eu> wrote:

> Read configuration from DTB.
>
> Signed-off-by: Michal Simek <monstr@monstr.eu>
> ---
>  arch/microblaze/cpu/timer.c |   25 +++++++++++++++++++++++++
>  1 files changed, 25 insertions(+), 0 deletions(-)
>
> diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c
> index dfaaaf5..91ca42b 100644
> --- a/arch/microblaze/cpu/timer.c
> +++ b/arch/microblaze/cpu/timer.c
> @@ -25,6 +25,9 @@
>  #include <common.h>
>  #include <asm/microblaze_timer.h>
>  #include <asm/microblaze_intc.h>
> +#include <fdtdec.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
>
>  volatile int timestamp = 0;
>  microblaze_timer_t *tmr;
> @@ -62,11 +65,33 @@ int timer_init (void)
>         u32 preload = 0;
>         u32 ret = 0;
>
> +#ifndef CONFIG_OF_CONTROL
>

Maybe make this #ifdef and put the fdt code first?


>  #if defined(CONFIG_SYS_TIMER_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM)
>         preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ;
>         irq = CONFIG_SYS_TIMER_0_IRQ;
>         tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR);
>  #endif
> +#else
> +       int temp;
> +       int offset = 0;
>

Can remove =0 I think.


> +
> +       offset = fdt_node_offset_by_compatible(gd->fdt_blob, offset,
> +                               "xlnx,xps-timer-1.00.a");
> +       if (offset > 0) {
> +               temp = fdtdec_get_addr(gd->fdt_blob, offset, "reg");
> +               if (temp != FDT_ADDR_T_NONE) {
> +                       tmr = (microblaze_timer_t *)temp;
> +                       irq = fdtdec_get_int(gd->fdt_blob, offset,
> +                                                       "interrupts", -1);
> +                       if (irq == -1)
> +                               panic("Connect IRQ to system timer\n");
>

That's a little cryptic - maybe should mention fdt?


> +                       /* Set default clock frequency */
> +                       temp = fdtdec_get_int(gd->fdt_blob, offset,
> +                                       "clock-frequency", 0);
> +                       preload = temp / CONFIG_SYS_HZ;
> +               }
> +       }
> +#endif
>
>         if (tmr && irq && preload) {
>                 tmr->loadreg = preload;
> --
> 1.7.0.4
>
>
Regards,
Simon
Michal Simek July 10, 2012, 9:23 a.m.
On 07/09/2012 11:32 PM, Simon Glass wrote:
> Hi Michal,
>
> On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu>> wrote:
>
>     Read configuration from DTB.
>
>     Signed-off-by: Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu>>
>     ---
>       arch/microblaze/cpu/timer.c |   25 +++++++++++++++++++++++++
>       1 files changed, 25 insertions(+), 0 deletions(-)
>
>     diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c
>     index dfaaaf5..91ca42b 100644
>     --- a/arch/microblaze/cpu/timer.c
>     +++ b/arch/microblaze/cpu/timer.c
>     @@ -25,6 +25,9 @@
>       #include <common.h>
>       #include <asm/microblaze_timer.h>
>       #include <asm/microblaze_intc.h>
>     +#include <fdtdec.h>
>     +
>     +DECLARE_GLOBAL_DATA_PTR;
>
>       volatile int timestamp = 0;
>       microblaze_timer_t *tmr;
>     @@ -62,11 +65,33 @@ int timer_init (void)
>              u32 preload = 0;
>              u32 ret = 0;
>
>     +#ifndef CONFIG_OF_CONTROL
>
>
> Maybe make this #ifdef and put the fdt code first?

I see only one reason to do it (and that's why I have done it in intc code too)
which is debug purpose. Because you can simple comment ifdefs and hardcoded values
below fdt code which you can debug.



>
>       #if defined(CONFIG_SYS_TIMER_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM)
>              preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ;
>              irq = CONFIG_SYS_TIMER_0_IRQ;
>              tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR);
>       #endif
>     +#else
>     +       int temp;
>     +       int offset = 0;
>
>
> Can remove =0 I think.

done.

>
>     +
>     +       offset = fdt_node_offset_by_compatible(gd->fdt_blob, offset,
>     + "xlnx,xps-timer-1.00.a");
>     +       if (offset > 0) {
>     +               temp = fdtdec_get_addr(gd->fdt_blob, offset, "reg");
>     +               if (temp != FDT_ADDR_T_NONE) {
>     +                       tmr = (microblaze_timer_t *)temp;
>     +                       irq = fdtdec_get_int(gd->fdt_blob, offset,
>     + "interrupts", -1);
>     +                       if (irq == -1)
>     +                               panic("Connect IRQ to system timer\n");
>
>
> That's a little cryptic - maybe should mention fdt?

What about this?
panic("Timer IRQ is not found in DTS/DTB\n");

I don't want to write there novels but this is also checked in different tool
which generates DTS that's why simple panic should be eliminated for most cases.

Thanks,
Michal
Michal Simek July 10, 2012, 9:34 a.m.
On 07/09/2012 11:32 PM, Simon Glass wrote:
> Hi Michal,
>
> On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu>> wrote:
>
>     Read configuration from DTB.
>
>     Signed-off-by: Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu>>
>     ---
>       arch/microblaze/cpu/timer.c |   25 +++++++++++++++++++++++++
>       1 files changed, 25 insertions(+), 0 deletions(-)
>
>     diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c
>     index dfaaaf5..91ca42b 100644
>     --- a/arch/microblaze/cpu/timer.c
>     +++ b/arch/microblaze/cpu/timer.c
>     @@ -25,6 +25,9 @@
>       #include <common.h>
>       #include <asm/microblaze_timer.h>
>       #include <asm/microblaze_intc.h>
>     +#include <fdtdec.h>
>     +
>     +DECLARE_GLOBAL_DATA_PTR;
>
>       volatile int timestamp = 0;
>       microblaze_timer_t *tmr;
>     @@ -62,11 +65,33 @@ int timer_init (void)
>              u32 preload = 0;
>              u32 ret = 0;
>
>     +#ifndef CONFIG_OF_CONTROL
>
>
> Maybe make this #ifdef and put the fdt code first?
>
>       #if defined(CONFIG_SYS_TIMER_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM)
>              preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ;
>              irq = CONFIG_SYS_TIMER_0_IRQ;
>              tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR);
>       #endif
>     +#else
>     +       int temp;
>     +       int offset = 0;
>
>
> Can remove =0 I think.
>
>     +
>     +       offset = fdt_node_offset_by_compatible(gd->fdt_blob, offset,
>     + "xlnx,xps-timer-1.00.a");


ok. It must be initialized because offset is passed as argument to the next function.
For interrupt code is fine to add there zero but for timer I would keep there offset
because system can contain more timers and we can change it to while loop soon.

(NOTE: We have had discussion to use aliases for system timer but we haven't finished it.

Thanks,
Michal
Simon Glass July 10, 2012, 9:04 p.m.
Hi Mich

On Tue, Jul 10, 2012 at 11:34 AM, Michal Simek <monstr@monstr.eu> wrote:

> On 07/09/2012 11:32 PM, Simon Glass wrote:
>
>> Hi Michal,
>>
>>
>> On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek <monstr@monstr.eu <mailto:
>> monstr@monstr.eu>> wrote:
>>
>>     Read configuration from DTB.
>>
>>     Signed-off-by: Michal Simek <monstr@monstr.eu <mailto:
>> monstr@monstr.eu>>
>>
>>     ---
>>       arch/microblaze/cpu/timer.c |   25 +++++++++++++++++++++++++
>>       1 files changed, 25 insertions(+), 0 deletions(-)
>>
>>     diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c
>>     index dfaaaf5..91ca42b 100644
>>     --- a/arch/microblaze/cpu/timer.c
>>     +++ b/arch/microblaze/cpu/timer.c
>>     @@ -25,6 +25,9 @@
>>       #include <common.h>
>>       #include <asm/microblaze_timer.h>
>>       #include <asm/microblaze_intc.h>
>>     +#include <fdtdec.h>
>>     +
>>     +DECLARE_GLOBAL_DATA_PTR;
>>
>>       volatile int timestamp = 0;
>>       microblaze_timer_t *tmr;
>>     @@ -62,11 +65,33 @@ int timer_init (void)
>>              u32 preload = 0;
>>              u32 ret = 0;
>>
>>     +#ifndef CONFIG_OF_CONTROL
>>
>>
>> Maybe make this #ifdef and put the fdt code first?
>>
>>       #if defined(CONFIG_SYS_TIMER_0_**ADDR) &&
>> defined(CONFIG_SYS_INTC_0_NUM)
>>              preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ;
>>              irq = CONFIG_SYS_TIMER_0_IRQ;
>>              tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR);
>>       #endif
>>     +#else
>>     +       int temp;
>>     +       int offset = 0;
>>
>>
>> Can remove =0 I think.
>>
>>     +
>>     +       offset = fdt_node_offset_by_compatible(**gd->fdt_blob,
>> offset,
>>     + "xlnx,xps-timer-1.00.a");
>>
>
>
> ok. It must be initialized because offset is passed as argument to the
> next function.
> For interrupt code is fine to add there zero but for timer I would keep
> there offset
> because system can contain more timers and we can change it to while loop
> soon.
>
> Yes, well maybe then:

    +       offset = fdt_node_offset_by_compatible(**gd->fdt_blob, 0,



> (NOTE: We have had discussion to use aliases for system timer but we
> haven't finished it.
>
>
> Thanks,
> Michal
>
>
>
> --
> Michal Simek, Ing. (M.Eng)
> w: www.monstr.eu p: +42-0-721842854
> Maintainer of Linux kernel 2.6 Microblaze Linux -
> http://www.monstr.eu/fdt/
> Microblaze U-BOOT custodian
>

Regards,
Simon
Michal Simek July 11, 2012, 6:18 a.m.
On 07/10/2012 11:04 PM, Simon Glass wrote:
> Hi Mich
>
> On Tue, Jul 10, 2012 at 11:34 AM, Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu>> wrote:
>
>     On 07/09/2012 11:32 PM, Simon Glass wrote:
>
>         Hi Michal,
>
>
>         On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr@monstr.eu <mailto:monstr@monstr.eu>>> wrote:
>
>              Read configuration from DTB.
>
>              Signed-off-by: Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr@monstr.eu <mailto:monstr@monstr.eu>>>
>
>              ---
>                arch/microblaze/cpu/timer.c |   25 +++++++++++++++++++++++++
>                1 files changed, 25 insertions(+), 0 deletions(-)
>
>              diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c
>              index dfaaaf5..91ca42b 100644
>              --- a/arch/microblaze/cpu/timer.c
>              +++ b/arch/microblaze/cpu/timer.c
>              @@ -25,6 +25,9 @@
>                #include <common.h>
>                #include <asm/microblaze_timer.h>
>                #include <asm/microblaze_intc.h>
>              +#include <fdtdec.h>
>              +
>              +DECLARE_GLOBAL_DATA_PTR;
>
>                volatile int timestamp = 0;
>                microblaze_timer_t *tmr;
>              @@ -62,11 +65,33 @@ int timer_init (void)
>                       u32 preload = 0;
>                       u32 ret = 0;
>
>              +#ifndef CONFIG_OF_CONTROL
>
>
>         Maybe make this #ifdef and put the fdt code first?
>
>                #if defined(CONFIG_SYS_TIMER_0___ADDR) && defined(CONFIG_SYS_INTC_0_NUM)
>                       preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ;
>                       irq = CONFIG_SYS_TIMER_0_IRQ;
>                       tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR);
>                #endif
>              +#else
>              +       int temp;
>              +       int offset = 0;
>
>
>         Can remove =0 I think.
>
>              +
>              +       offset = fdt_node_offset_by_compatible(__gd->fdt_blob, offset,
>              + "xlnx,xps-timer-1.00.a");
>
>
>
>     ok. It must be initialized because offset is passed as argument to the next function.
>     For interrupt code is fine to add there zero but for timer I would keep there offset
>     because system can contain more timers and we can change it to while loop soon.
>
> Yes, well maybe then:
>
>      +       offset = fdt_node_offset_by_compatible(__gd->fdt_blob, 0,

Both options just works. I can look at asm dump to see which solution is faster
and better to use.

Thanks,
Michal
Simon Glass July 11, 2012, 9:53 a.m.
Hi Michal,

On Wed, Jul 11, 2012 at 8:18 AM, Michal Simek <monstr@monstr.eu> wrote:

> On 07/10/2012 11:04 PM, Simon Glass wrote:
>
>> Hi Mich
>>
>>
>> On Tue, Jul 10, 2012 at 11:34 AM, Michal Simek <monstr@monstr.eu <mailto:
>> monstr@monstr.eu>> wrote:
>>
>>     On 07/09/2012 11:32 PM, Simon Glass wrote:
>>
>>         Hi Michal,
>>
>>
>>         On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek <monstr@monstr.eu<mailto:
>> monstr@monstr.eu> <mailto:monstr@monstr.eu <mailto:monstr@monstr.eu>>>
>> wrote:
>>
>>              Read configuration from DTB.
>>
>>              Signed-off-by: Michal Simek <monstr@monstr.eu <mailto:
>> monstr@monstr.eu> <mailto:monstr@monstr.eu <mailto:monstr@monstr.eu>>>
>>
>>
>>              ---
>>                arch/microblaze/cpu/timer.c |   25
>> +++++++++++++++++++++++++
>>                1 files changed, 25 insertions(+), 0 deletions(-)
>>
>>              diff --git a/arch/microblaze/cpu/timer.c
>> b/arch/microblaze/cpu/timer.c
>>              index dfaaaf5..91ca42b 100644
>>              --- a/arch/microblaze/cpu/timer.c
>>              +++ b/arch/microblaze/cpu/timer.c
>>              @@ -25,6 +25,9 @@
>>                #include <common.h>
>>                #include <asm/microblaze_timer.h>
>>                #include <asm/microblaze_intc.h>
>>              +#include <fdtdec.h>
>>              +
>>              +DECLARE_GLOBAL_DATA_PTR;
>>
>>                volatile int timestamp = 0;
>>                microblaze_timer_t *tmr;
>>              @@ -62,11 +65,33 @@ int timer_init (void)
>>                       u32 preload = 0;
>>                       u32 ret = 0;
>>
>>              +#ifndef CONFIG_OF_CONTROL
>>
>>
>>         Maybe make this #ifdef and put the fdt code first?
>>
>>                #if defined(CONFIG_SYS_TIMER_0___**ADDR) &&
>> defined(CONFIG_SYS_INTC_0_NUM)
>>
>>                       preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ;
>>                       irq = CONFIG_SYS_TIMER_0_IRQ;
>>                       tmr = (microblaze_timer_t *)
>> (CONFIG_SYS_TIMER_0_ADDR);
>>                #endif
>>              +#else
>>              +       int temp;
>>              +       int offset = 0;
>>
>>
>>         Can remove =0 I think.
>>
>>              +
>>              +       offset = fdt_node_offset_by_compatible(**__gd->fdt_blob,
>> offset,
>>
>>              + "xlnx,xps-timer-1.00.a");
>>
>>
>>
>>     ok. It must be initialized because offset is passed as argument to
>> the next function.
>>     For interrupt code is fine to add there zero but for timer I would
>> keep there offset
>>     because system can contain more timers and we can change it to while
>> loop soon.
>>
>> Yes, well maybe then:
>>
>>      +       offset = fdt_node_offset_by_compatible(**__gd->fdt_blob, 0,
>>
>
> Both options just works. I can look at asm dump to see which solution is
> faster
> and better to use.


I doubt there will be any difference there, but OK.


>
> Thanks,
> Michal
>
>
>
>
> --
> Michal Simek, Ing. (M.Eng)
> w: www.monstr.eu p: +42-0-721842854
> Maintainer of Linux kernel 2.6 Microblaze Linux -
> http://www.monstr.eu/fdt/
> Microblaze U-BOOT custodian
>

Patch hide | download patch | download mbox

diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c
index dfaaaf5..91ca42b 100644
--- a/arch/microblaze/cpu/timer.c
+++ b/arch/microblaze/cpu/timer.c
@@ -25,6 +25,9 @@ 
 #include <common.h>
 #include <asm/microblaze_timer.h>
 #include <asm/microblaze_intc.h>
+#include <fdtdec.h>
+
+DECLARE_GLOBAL_DATA_PTR;
 
 volatile int timestamp = 0;
 microblaze_timer_t *tmr;
@@ -62,11 +65,33 @@  int timer_init (void)
 	u32 preload = 0;
 	u32 ret = 0;
 
+#ifndef CONFIG_OF_CONTROL
 #if defined(CONFIG_SYS_TIMER_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM)
 	preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ;
 	irq = CONFIG_SYS_TIMER_0_IRQ;
 	tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR);
 #endif
+#else
+	int temp;
+	int offset = 0;
+
+	offset = fdt_node_offset_by_compatible(gd->fdt_blob, offset,
+				"xlnx,xps-timer-1.00.a");
+	if (offset > 0) {
+		temp = fdtdec_get_addr(gd->fdt_blob, offset, "reg");
+		if (temp != FDT_ADDR_T_NONE) {
+			tmr = (microblaze_timer_t *)temp;
+			irq = fdtdec_get_int(gd->fdt_blob, offset,
+							"interrupts", -1);
+			if (irq == -1)
+				panic("Connect IRQ to system timer\n");
+			/* Set default clock frequency */
+			temp = fdtdec_get_int(gd->fdt_blob, offset,
+					"clock-frequency", 0);
+			preload = temp / CONFIG_SYS_HZ;
+		}
+	}
+#endif
 
 	if (tmr && irq && preload) {
 		tmr->loadreg = preload;