| Message ID | 1341825639-23475-9-git-send-email-monstr@monstr.eu |
|---|---|
| State | Accepted |
| Delegated to: | Michal Simek |
| Headers | show |
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;
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
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
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
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
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
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 >
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;
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(-)