[U-Boot] [PATCH] arm: rename timer init callback timer_init

[Wolfgang Denk]

Dear Jean-Christophe PLAGNIOL-VILLARD,

In message <1240047101-6787-1-git-send-email-plagnioj at jcrosoft.com> you wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj at jcrosoft.com>

The Subject: indicates a mere formal change (a rename of a function
name), but the patch actually does other things as well:

...
> diff --git a/cpu/ixp/interrupts.c b/cpu/ixp/interrupts.c
> index ee0129e..a05e439 100644
> --- a/cpu/ixp/interrupts.c
> +++ b/cpu/ixp/interrupts.c
> @@ -33,14 +33,6 @@
>  #include <asm/arch/ixp425.h>
>  #include <asm/proc-armv/ptrace.h>
>  
> -/*
> - * When interrupts are enabled, use timer 2 for time/delay generation...
> - */
> -
> -#define FREQ		66666666
> -#define CLOCK_TICK_RATE	(((FREQ / CONFIG_SYS_HZ & ~IXP425_OST_RELOAD_MASK) + 1) * CONFIG_SYS_HZ)
> -#define LATCH		((CLOCK_TICK_RATE + CONFIG_SYS_HZ/2) / CONFIG_SYS_HZ)	/* For divider */
> -
>  struct _irq_handler {
>  	void                *m_data;
>  	void (*m_func)( void *data);
> @@ -48,8 +40,6 @@ struct _irq_handler {
>  
>  static struct _irq_handler IRQ_HANDLER[N_IRQS];
>  
> -static volatile ulong timestamp;
> -
>  static void default_isr(void *data)
>  {
>  	printf("default_isr():  called for IRQ %d, Interrupt Status=%x PR=%x\n",
> @@ -61,33 +51,20 @@ static int next_irq(void)
>  	return (((*IXP425_ICIH & 0x000000fc) >> 2) - 1);
>  }
>  
> -static void timer_isr(void *data)
> -{
> -	unsigned int *pTime = (unsigned int *)data;
> -
> -	(*pTime)++;
> -
> -	/*
> -	 * Reset IRQ source
> -	 */
> -	*IXP425_OSST = IXP425_OSST_TIMER_2_PEND;
> -}
> -
> -ulong get_timer (ulong base)
> +void do_irq (struct pt_regs *pt_regs)
>  {
> -	return timestamp - base;
> -}
> +	int irq = next_irq();
>  
> -void reset_timer (void)
> -{
> -	timestamp = 0;
> +	IRQ_HANDLER[irq].m_func(IRQ_HANDLER[irq].m_data);
>  }
>  
> -void do_irq (struct pt_regs *pt_regs)
> +void irq_install_handler (int irq, interrupt_handler_t handle_irq, void *data)
>  {
> -	int irq = next_irq();
> +	if (irq >= N_IRQS || !handle_irq)
> +		return;
>  
> -	IRQ_HANDLER[irq].m_func(IRQ_HANDLER[irq].m_data);
> +	IRQ_HANDLER[irq].m_data = data;
> +	IRQ_HANDLER[irq].m_func = handle_irq;
>  }
>  
>  int interrupt_init (void)
> @@ -95,23 +72,11 @@ int interrupt_init (void)
>  	int i;
>  
>  	/* install default interrupt handlers */
> -	for (i = 0; i < N_IRQS; i++) {
> -		IRQ_HANDLER[i].m_data = (void *)i;
> -		IRQ_HANDLER[i].m_func = default_isr;
> -	}
> -
> -	/* install interrupt handler for timer */
> -	IRQ_HANDLER[IXP425_TIMER_2_IRQ].m_data = (void *)&timestamp;
> -	IRQ_HANDLER[IXP425_TIMER_2_IRQ].m_func = timer_isr;
> -
> -	/* setup the Timer counter value */
> -	*IXP425_OSRT2 = (LATCH & ~IXP425_OST_RELOAD_MASK) | IXP425_OST_ENABLE;
> +	for (i = 0; i < N_IRQS; i++)
> +		irq_install_handler(i, default_isr, (void *)i);
>  
>  	/* configure interrupts for IRQ mode */
>  	*IXP425_ICLR = 0x00000000;
>  
> -	/* enable timer irq */
> -	*IXP425_ICMR = (1 << IXP425_TIMER_2_IRQ);
> -
>  	return (0);
>  }

This is far beyond a mere rename. from the patch it is not even easy
to judge if the resulting code is equivalent or not.

I think the commit message should explain in more detail what exactly
is being changed here, and why.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The one who says it cannot be done should never interrupt the one who
is doing it.