Re: [Xenomai] s3c24xx with clocksource/clockevent (kernel 3.2.21)

[Romain Naour <romain.naour@openwide.fr>]

Le 22/10/2012 16:05, Gilles Chanteperdrix a écrit :
> On 10/22/2012 03:18 PM, Gilles Chanteperdrix wrote:
>
>> Another candidate would be irq_enable in kernel/irq/chip.c, shut off the
>> irqs around the whole section, or move irq_state_clk_masked before the
>> call to irq_unmask with a compiler barrier.
>
> Actually, the following patch fixes another bug I had, so, could you
> check if it fixes yours? (and thanks for the explanations, they were
> very helpful).
>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index f5d8d5e..cef1e78 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -167,8 +167,10 @@ int irq_startup(struct irq_desc *desc, bool resend)
>   	desc->depth = 0;
>   
>   	if (desc->irq_data.chip->irq_startup) {
> +		unsigned long flags = hard_cond_local_irq_save();
>   		ret = desc->irq_data.chip->irq_startup(&desc->irq_data);
>   		irq_state_clr_masked(desc);
> +		hard_cond_local_irq_restore(flags);
>   	} else {
>   		irq_enable(desc);
>   	}
> @@ -192,12 +194,14 @@ void irq_shutdown(struct irq_desc *desc)
>   
>   void irq_enable(struct irq_desc *desc)
>   {
> +	unsigned long flags = hard_cond_local_irq_save();
>   	irq_state_clr_disabled(desc);
>   	if (desc->irq_data.chip->irq_enable)
>   		desc->irq_data.chip->irq_enable(&desc->irq_data);
>   	else
>   		desc->irq_data.chip->irq_unmask(&desc->irq_data);
>   	irq_state_clr_masked(desc);
> +	hard_cond_local_irq_restore(flags);
>   }
>   
>   void irq_disable(struct irq_desc *desc)
> @@ -211,11 +215,13 @@ void irq_disable(struct irq_desc *desc)
>   
>   void irq_percpu_enable(struct irq_desc *desc, unsigned int cpu)
>   {
> +	unsigned long flags = hard_cond_local_irq_save();
>   	if (desc->irq_data.chip->irq_enable)
>   		desc->irq_data.chip->irq_enable(&desc->irq_data);
>   	else
>   		desc->irq_data.chip->irq_unmask(&desc->irq_data);
>   	cpumask_set_cpu(cpu, desc->percpu_enabled);
> +	hard_cond_local_irq_restore(flags);
>   }
>   
>   void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu)
>
>
Indeed, it fixes my bug without modifying the samsung driver :)
Thank you.

> As explained on the page I directed you too, converting a spinlock to an
> ipipe_spinlock is not something that should be done lightly. Otherwise,
> you might well end-up with large masking sections, so, I would try and
> find what section needs to run with irqs off, and only disable irqs for
> that section.
I can see that, IRQs are only disabled during the call of irq_enable().
It's better :)

Here is the trace:
I-pipe worst-case tracing service on 3.2.21/ipipe release #1
-------------------------------------------------------------
CPU: 0, Begin: 156240209 cycles, Trace Points: 22 (-3500/+85), Length: 
1380 us
Calibrated minimum trace-point overhead: 0.600 us

  +----- Hard IRQs ('|': locked)
  |+-- Xenomai
  ||+- Linux ('*': domain stalled, '+': current, '#': current+stalled)
  |||                          +---------- Delay flag ('+': > 1 us, '!': 
 > 10 us)
  |||                          |        +- NMI noise ('N')
  |||                          |        |
      +func               -1750    2.400  uart_start+0x10 (uart_write+0x134)
      #func               -1747    2.800  __uart_start+0x10 
(uart_start+0x50)
      #func               -1745    3.200 s3c24xx_serial_start_tx+0x10 
(__uart_start+0x54)
      #func               -1741    1.600  enable_irq+0x14 
(s3c24xx_serial_start_tx+0xc4)
      #func               -1740    4.600 __irq_get_desc_lock+0x10 
(enable_irq+0x2c)
      #func               -1735    2.200  __enable_irq+0x10 
(enable_irq+0x64)
      #func               -1733    3.200  irq_enable+0x10 
(__enable_irq+0xac)
  |   #(0x01)  0x00000001 -1730    1.000  irq_enable+0x3c 
(__enable_irq+0xac)
  |   #(0x02)  0x00000001 -1729    2.000  irq_enable+0x54 
(__enable_irq+0xac)
  |   #func               -1727    2.200 s3c_irq_uart0_unmask+0x10 
(irq_enable+0x7c)
  |   #(0x01)  0x00000001 -1725    1.000  irq_enable+0x9c 
(__enable_irq+0xac)
  |   #(0x02)  0x00000000 -1724    3.800  irq_enable+0xb4 
(__enable_irq+0xac)
  |   #func               -1720    2.800 __ipipe_grab_irq+0x10 
(__irq_svc+0x24)
  |   #func               -1717    2.800 __ipipe_dispatch_irq+0x10 
(__ipipe_grab_irq+0x54)
  |   #func               -1714    1.800 __ipipe_ack_irq+0x10 
(__ipipe_dispatch_irq+0x94)
  |   #func               -1712    1.600 s3c_irq_demux_uart0+0x10 
(__ipipe_ack_irq+0x1c)
  |   #func               -1711    1.600 s3c_irq_demux_uart+0x10 
(s3c_irq_demux_uart0+0x1c)
  |   #func               -1709    1.400 __ipipe_dispatch_irq+0x10 
(s3c_irq_demux_uart+0x54)
  |   #func               -1708    1.800 __ipipe_ack_irq+0x10 
(__ipipe_dispatch_irq+0x94)
  |   #func               -1706    1.200 __ipipe_ack_level_irq+0x10 
(__ipipe_ack_irq+0x1c)
  |   #func               -1705    2.600 s3c_irq_uart0_ack+0x10 
(__ipipe_ack_level_irq+0x34)
  |   #(0x01)  0x00000001 -1702    1.000 __ipipe_ack_level_irq+0x78 
(__ipipe_ack_irq+0x1c)
  |   #(0x02)  0x00000001 -1701    3.000 __ipipe_ack_level_irq+0x90 
(__ipipe_ack_irq+0x1c)
  |   #func               -1698    4.600 __ipipe_set_irq_pending+0x10 
(__ipipe_dispatch_irq+0x120)
  |   #func               -1694    2.200 __ipipe_do_sync_pipeline+0x10 
(__ipipe_dispatch_irq+0x16c)
  |   #func               -1691    1.600 __ipipe_exit_irq+0x10 
(__ipipe_grab_irq+0x5c)
  |   #func               -1690    2.400 
__ipipe_check_root_interruptible+0x10 (__irq_svc+0x94)
      #func               -1687    1.600 check_irq_resend+0x10 
(__enable_irq+0xb8)
      #func               -1686    1.600 __irq_put_desc_unlock+0x10 
(enable_irq+0x74)
      #func               -1684    1.200 ipipe_unstall_root+0x10 
(uart_start+0x5c)
  |   +func               -1683    3.600 __ipipe_do_sync_stage+0x10 
(ipipe_unstall_root+0x3c)

Regards,
Romain