All of lore.kernel.org
 help / color / mirror / Atom feed
From: Romain Naour <romain.naour@openwide.fr>
To: Gilles Chanteperdrix <gilles.chanteperdrix@xenomai.org>
Cc: xenomai@xenomai.org
Subject: Re: [Xenomai] s3c24xx with clocksource/clockevent (kernel 3.2.21)
Date: Mon, 22 Oct 2012 01:33:18 +0200	[thread overview]
Message-ID: <5084863E.7030900@openwide.fr> (raw)
In-Reply-To: <50844242.7010107@xenomai.org>

Le 21/10/2012 20:43, Gilles Chanteperdrix a écrit :
> On 10/21/2012 03:35 AM, Romain Naour wrote:
>
>>> You mean the patch for the UART ? I can not take that patch. You have to
>>> understand what goes wrong in the I-pipe core, or irq chip callbacks
>>> implementation for this processor, and fix it there, not in the irq
>>> handler for the serial interrupt.
>> I think there is a problem when Linux unmask an IRQ line with
>> irq_enable() and when there is an IRQ pending behind.
>> I made some comments on the ipipe-tracer log
>> (UART-samsung-bug-ipipe-tracer.txt)
>
> Ok: first comment: thanks to this patch:
> http://git.xenomai.org/?p=ipipe-gch.git;a=commitdiff;h=352ee07368a89133df74948f3dd71ae657c519a0;hp=5e7413789eb3d1ca1613e986cbeb6c979082a217
>
> We know that irq_state_clr_masked is called when unmask_irq is called.
>
> Second, to know why cond_unmask_irq does not unmask the irq, you can use
> ipipe_trace_special to print the value of the various conditions.
>
> It would have been better to include the relevant parts of the trace in
> the mail body, this would have made answering simpler.
>
Ok, sorry for that.

here are some new trace with id :
(1): IRQ enabled == 1
(2): IRQ masked == 1
(3): Treads oneshot == 1

I-pipe worst-case tracing service on 3.2.21/ipipe release #1
-------------------------------------------------------------
CPU: 0, Begin: 328856864 cycles, Trace Points: 28 (-3500/+92), Length: 
1559 us
Calibrated minimum trace-point overhead: 0.777 us

  +----- Hard IRQs ('|': locked)
  |+-- Xenomai
  ||+- Linux ('*': domain stalled, '+': current, '#': current+stalled)
  |||                          +---------- Delay flag ('+': > 1 us, '!': 
 > 10 us)
  |||                          |        +- NMI noise ('N')
  |||                          |        |
       Type    User Val.   Time    Delay  Function (Parent)
      +func                -190    2.111  uart_start+0x10 (uart_write+0x134)
      #func                -188    2.666  __uart_start+0x10 
(uart_start+0x50)
// driver start a new transmission, so it enable the IRQ.
      #func                -185    4.222 s3c24xx_serial_start_tx+0x10 
(__uart_start+0x54)
      #func                -181    2.000  enable_irq+0x14 
(s3c24xx_serial_start_tx+0xc4)
      #func                -179    4.888  __irq_get_desc_lock+0x10 
(enable_irq+0x2c)
      #func                -174    2.555  __enable_irq+0x10 
(enable_irq+0x64)
      #func                -171    3.444  irq_enable+0x10 
(__enable_irq+0xac)
      #(0x01)  0x00000001  -168    1.444  irq_enable+0x30 
(__enable_irq+0xac) // enabled
      #(0x02)  0x00000000  -167    3.111  irq_enable+0x48 
(__enable_irq+0xac) // not masked
      #func                -163    4.777  s3c_irq_uart0_unmask+0x10 
(irq_enable+0x70)
  |   #func                -159    2.666  __ipipe_grab_irq+0x10 
(__irq_svc+0x24)
  |   #func                -156    3.888  __ipipe_dispatch_irq+0x10 
(__ipipe_grab_irq+0x54)
  |   #func                -152    1.666  __ipipe_ack_irq+0x10 
(__ipipe_dispatch_irq+0xe0)
  |   #func                -150    2.333  s3c_irq_demux_uart0+0x10 
(__ipipe_ack_irq+0x1c)
  |   #func                -148    2.111  s3c_irq_demux_uart+0x10 
(s3c_irq_demux_uart0+0x1c)
  |   #func                -146    1.444  __ipipe_dispatch_irq+0x10 
(s3c_irq_demux_uart+0x54)
  |   #func                -145    1.666  __ipipe_ack_irq+0x10 
(__ipipe_dispatch_irq+0xe0)
  |   #func                -143    1.666 __ipipe_ack_level_irq+0x10 
(__ipipe_ack_irq+0x1c)
  |   #func                -141    2.666  s3c_irq_uart0_ack+0x10 
(__ipipe_ack_level_irq+0x34)
  |   #(0x01)  0x00000001  -139    1.111 __ipipe_ack_level_irq+0x78 
(__ipipe_ack_irq+0x1c) // enabled
  |   #(0x02)  0x00000001  -137    2.555 __ipipe_ack_level_irq+0x90 
(__ipipe_ack_irq+0x1c) // Adeos mask the IRQ
  |   #func                -135    5.222 __ipipe_set_irq_pending+0x10 
(__ipipe_dispatch_irq+0x16c)
  |   #func                -130    3.444 __ipipe_do_sync_pipeline+0x10 
(__ipipe_dispatch_irq+0x1b8)
  |   #func                -126    1.444  __ipipe_exit_irq+0x10 
(__ipipe_grab_irq+0x5c)
  |   #func                -125    2.777 
__ipipe_check_root_interruptible+0x10 (__irq_svc+0x94)
      #func                -122    1.555  check_irq_resend+0x10 
(__enable_irq+0xb8)
      #(0x01)  0x00000001  -120    1.111  __enable_irq+0xd0 
(enable_irq+0x64) // enabled
      #(0x02)  0x00000000  -119    1.111  __enable_irq+0xe8 
(enable_irq+0x64) // Linux reset the IRQ status flag IRQD_IRQ_MASKED, 
but irq still masked at hardware level !!
      #func                -118    2.444 __irq_put_desc_unlock+0x10 
(enable_irq+0x74)
      #func                -116    1.666  ipipe_unstall_root+0x10 
(uart_start+0x5c)
  |   +func                -114    4.000 __ipipe_do_sync_stage+0x10 
(ipipe_unstall_root+0x3c)
      #func                -110    2.000  __ipipe_do_IRQ+0x10 
(__ipipe_do_sync_stage+0x1bc)
      #func                -108    2.333  handle_IRQ+0x10 
(__ipipe_do_IRQ+0x1c)
      #func                -106    1.555  irq_enter+0x10 (handle_IRQ+0x28)
      #func                -104    3.666  idle_cpu+0x10 (irq_enter+0x1c)
      #func                -101    1.777  generic_handle_irq+0x10 
(handle_IRQ+0x6c)
      #func                 -99    3.777  handle_level_irq+0x10 
(generic_handle_irq+0x34)
      #func                 -95    2.222  handle_irq_event+0x10 
(handle_level_irq+0x98)
      #func                 -93    3.222 handle_irq_event_percpu+0x14 
(handle_irq_event+0x64)
      #func                 -90   23.888 s3c24xx_serial_tx_chars+0x10 
(handle_irq_event_percpu+0xc0)
      #func                 -66    2.111  uart_write_wakeup+0x10 
(s3c24xx_serial_tx_chars+0xe0)
      #func                 -64    2.222  tty_wakeup+0x10 
(uart_write_wakeup+0x2c)
      #func                 -61    2.555  __wake_up+0x14 (tty_wakeup+0x64)
      #func                 -59    2.222  __wake_up_common+0x10 
(__wake_up+0x5c)
      #func                 -57    1.888 default_wake_function+0x10 
(__wake_up_common+0x58)
      #func                 -55    3.222  try_to_wake_up+0x10 
(default_wake_function+0x1c)
      #func                 -51    3.111 
ttwu_do_wakeup.constprop.138+0x10 (try_to_wake_up+0xa8)
      #func                 -48    2.555  check_preempt_curr+0x10 
(ttwu_do_wakeup.constprop.138+0xb8)
      #func                 -46    3.000  check_preempt_wakeup+0x10 
(check_preempt_curr+0x38)
      #func                 -43    5.777 __task_rq_unlock.isra.122+0x10 
(try_to_wake_up+0xb0)
      #func                 -37    3.666  note_interrupt+0x10 
(handle_irq_event_percpu+0x290)
      #(0x01)  0x00000001   -33    1.111  handle_level_irq+0xe0 
(generic_handle_irq+0x34) // enabled
      #(0x02)  0x00000000   -32    1.333  handle_level_irq+0x100 
(generic_handle_irq+0x34) // not masked
      #(0x03)  0x00000001   -31    1.888  handle_level_irq+0x118 
(generic_handle_irq+0x34) // not oneshot
// so we won't unmask the IRQ.
      #func                 -29    3.000  irq_exit+0x10 (handle_IRQ+0x70)
      +func                 -26    1.666  mutex_unlock+0x10 
(n_tty_write+0x258)
      +func                 -24    0.777  process_output+0x10 
(n_tty_write+0x288)
      +func                 -24    1.222  mutex_lock+0x10 
(process_output+0x28)
      +func                 -22    1.000  tty_write_room+0x10 
(process_output+0x30)
      +func                 -21    0.888  uart_write_room+0x10 
(tty_write_room+0x2c)
      #func                 -20    1.666  ipipe_unstall_root+0x10 
(uart_write_room+0x74)
      +func                 -19    2.444  do_output_char+0x10 
(process_output+0x40)
      +func                 -16    1.444  uart_write+0x10 
(do_output_char+0x9c)
      #func                 -15    1.111  ipipe_unstall_root+0x10 
(uart_write+0x108)
      +func                 -14    0.888  uart_start+0x10 (uart_write+0x134)
      #func                 -13    1.222  __uart_start+0x10 
(uart_start+0x50)
      #func                 -12    2.444 s3c24xx_serial_start_tx+0x10 
(__uart_start+0x54) // (!tx_enabled(port)) == 0
      #begin   0x52120007    -9    1.888 s3c24xx_serial_start_tx+0xd8 
(__uart_start+0x54)
      #end     0x52120007    -7    1.666 s3c24xx_serial_start_tx+0xe0 
(__uart_start+0x54)
      #func                  -6    1.777  ipipe_unstall_root+0x10 
(uart_start+0x5c)
      +func                  -4    1.444  mutex_unlock+0x10 
(process_output+0x4c)
      +func                  -2    0.888  uart_flush_chars+0x10 
(n_tty_write+0x2bc)
      +func                  -2    0.888  uart_start+0x10 
(uart_flush_chars+0x18)
      #func                  -1    1.111  __uart_start+0x10 
(uart_start+0x50)
 >    #func                   0! 1493.333 s3c24xx_serial_start_tx+0x10 
(__uart_start+0x54)
:|   #func                1493+   4.222  __ipipe_grab_irq+0x10 
(__irq_svc+0x24)
:|   #func                1497+   2.555  __ipipe_dispatch_irq+0x10 
(__ipipe_grab_irq+0x54)

> Without looking at the trace, if the irqs need to be off during the
> execution of an irq chip callback, we usually do this inside the chip
> callback. When there is a spinlock, simply turn it into an ipipe
> spinlock and use spin_lock_irqsave_cond, as explained here:
> http://www.xenomai.org/index.php/I-pipe-core:ArmPorting#call-backs_implementation

Thank you for the explanation, I found a __SPIN_LOCK_UNLOCKED into the 
samsung driver code.
I modified it into a IPIPE_SPIN_LOCK_UNLOCKED.
It works, now :)

      +func                -220    1.333  uart_start+0x10 (uart_write+0xec)
      +func                -219    1.666 __ipipe_spin_lock_irqsave+0x10 
(uart_start+0x28)    << now, we use an IPIPE_SPINLOCK, IRQs are desabled.
  |   #func                -217    2.666  __uart_start+0x10 
(uart_start+0x34)
  |   #func                -215    4.222 s3c24xx_serial_start_tx+0x10 
(__uart_start+0x54)
  |   #func                -210    2.333  enable_irq+0x14 
(s3c24xx_serial_start_tx+0x78)
  |   #func                -208    4.111  __irq_get_desc_lock+0x10 
(enable_irq+0x2c)
  |   #func                -204    3.111  __enable_irq+0x10 
(enable_irq+0x64)
  |   #func                -201    3.333  irq_enable+0x10 
(__enable_irq+0xac)
  |   #(0x01)  0x00000001  -197    1.000  irq_enable+0x30 
(__enable_irq+0xac)
  |   #(0x02)  0x00000001  -196    3.000  irq_enable+0x48 
(__enable_irq+0xac)
  |   #func                -193    3.000  s3c_irq_uart0_unmask+0x10 
(irq_enable+0x70)
  |   #func                -190    1.666  check_irq_resend+0x10 
(__enable_irq+0xb8)
  |   #(0x01)  0x00000001  -189    1.000  __enable_irq+0xd0 
(enable_irq+0x64)
  |   #(0x02)  0x00000000  -188    1.111  __enable_irq+0xe8 
(enable_irq+0x64)            << enable_irq end correctly
  |   #func                -187    2.333 __irq_put_desc_unlock+0x10 
(enable_irq+0x74)
  |   #func                -184    3.555 
__ipipe_spin_unlock_irqrestore+0x10 (uart_start+0x40)  << ipipe_spinlock 
is unlocked here, IRQ are enabled
  |   +func                -181    2.444  __ipipe_grab_irq+0x10 
(__irq_svc+0x24)
  |   +func                -178    3.555  __ipipe_dispatch_irq+0x10 
(__ipipe_grab_irq+0x54)
  |   +func                -175    1.333  __ipipe_ack_irq+0x10 
(__ipipe_dispatch_irq+0xe0)
  |   +func                -173    1.555  s3c_irq_demux_uart0+0x10 
(__ipipe_ack_irq+0x1c)
  |   +func                -172    1.666  s3c_irq_demux_uart+0x10 
(s3c_irq_demux_uart0+0x1c)
  |   +func                -170    1.555  __ipipe_dispatch_irq+0x10 
(s3c_irq_demux_uart+0x54)
  |   +func                -169    1.111  __ipipe_ack_irq+0x10 
(__ipipe_dispatch_irq+0xe0)
  |   +func                -168    1.444 __ipipe_ack_level_irq+0x10 
(__ipipe_ack_irq+0x1c)
  |   +func                -166    2.111  s3c_irq_uart0_ack+0x10 
(__ipipe_ack_level_irq+0x34)
  |   +(0x01)  0x00000001  -164    1.111 __ipipe_ack_level_irq+0x78 
(__ipipe_ack_irq+0x1c)
  |   +(0x02)  0x00000001  -163    2.555 __ipipe_ack_level_irq+0x90 
(__ipipe_ack_irq+0x1c)
  |   +func                -160    4.666 __ipipe_set_irq_pending+0x10 
(__ipipe_dispatch_irq+0x16c)
  |   +func                -156    2.888 __ipipe_do_sync_pipeline+0x10 
(__ipipe_dispatch_irq+0x1b8)
  |   +func                -153    3.777 __ipipe_do_sync_stage+0x10 
(__ipipe_do_sync_pipeline+0x90)
      #func                -149    1.777  __ipipe_do_IRQ+0x10 
(__ipipe_do_sync_stage+0x1bc)
      #func                -147    1.555  handle_IRQ+0x10 
(__ipipe_do_IRQ+0x1c)
      #func                -146    1.555  irq_enter+0x10 (handle_IRQ+0x28)
      #func                -144    2.555  idle_cpu+0x10 (irq_enter+0x1c)
      #func                -142    1.888  generic_handle_irq+0x10 
(handle_IRQ+0x6c)
      #func                -140    2.888  handle_level_irq+0x10 
(generic_handle_irq+0x34)
      #func                -137    2.222  handle_irq_event+0x10 
(handle_level_irq+0x98)
      #func                -135    2.555 handle_irq_event_percpu+0x14 
(handle_irq_event+0x64)
      #func                -132   23.222 s3c24xx_serial_tx_chars+0x10 
(handle_irq_event_percpu+0xc0)
      #func                -109    2.111  uart_write_wakeup+0x10 
(s3c24xx_serial_tx_chars+0xe0)
      #func                -107    2.333  tty_wakeup+0x10 
(uart_write_wakeup+0x2c)
      #func                -104    2.444  __wake_up+0x14 (tty_wakeup+0x64)
      #func                -102    2.555  __wake_up_common+0x10 
(__wake_up+0x5c)
      #func                 -99    1.555 default_wake_function+0x10 
(__wake_up_common+0x58)
      #func                 -98    2.666  try_to_wake_up+0x10 
(default_wake_function+0x1c)
      #func                 -95    2.333 
ttwu_do_wakeup.constprop.138+0x10 (try_to_wake_up+0xa8)
      #func                 -93    1.777  check_preempt_curr+0x10 
(ttwu_do_wakeup.constprop.138+0xb8)
      #func                 -91    2.555  check_preempt_wakeup+0x10 
(check_preempt_curr+0x38)
      #func                 -88    5.333 __task_rq_unlock.isra.122+0x10 
(try_to_wake_up+0xb0)
      #func                 -83    3.444  note_interrupt+0x10 
(handle_irq_event_percpu+0x290)
      #func                 -80    1.333  unmask_irq+0x10 
(handle_level_irq+0xc0)
  |   #func                 -78    1.888  s3c_irq_uart0_unmask+0x10 
(unmask_irq+0x40)
  |   #func                 -76    1.111  __ipipe_grab_irq+0x10 
(__irq_svc+0x24)
  |   #func                 -75    0.888  __ipipe_dispatch_irq+0x10 
(__ipipe_grab_irq+0x54)
  |   #func                 -74    1.111  __ipipe_ack_irq+0x10 
(__ipipe_dispatch_irq+0xe0)
  |   #func                 -73    1.111  s3c_irq_demux_uart0+0x10 
(__ipipe_ack_irq+0x1c)
  |   #func                 -72    1.111  s3c_irq_demux_uart+0x10 
(s3c_irq_demux_uart0+0x1c)
  |   #func                 -71    1.000  __ipipe_dispatch_irq+0x10 
(s3c_irq_demux_uart+0x54)
  |   #func                 -70    1.111  __ipipe_ack_irq+0x10 
(__ipipe_dispatch_irq+0xe0)
  |   #func                 -69    1.111 __ipipe_ack_level_irq+0x10 
(__ipipe_ack_irq+0x1c)
  |   #func                 -68    1.444  s3c_irq_uart0_ack+0x10 
(__ipipe_ack_level_irq+0x34)
  |   #(0x01)  0x00000001   -66    0.888 __ipipe_ack_level_irq+0x78 
(__ipipe_ack_irq+0x1c)
  |   #(0x02)  0x00000001   -66    1.222 __ipipe_ack_level_irq+0x90 
(__ipipe_ack_irq+0x1c)
  |   #func                 -64    1.333 __ipipe_set_irq_pending+0x10 
(__ipipe_dispatch_irq+0x16c)
  |   #func                 -63    1.444 __ipipe_do_sync_pipeline+0x10 
(__ipipe_dispatch_irq+0x1b8)
  |   #func                 -62    1.444  __ipipe_exit_irq+0x10 
(__ipipe_grab_irq+0x5c)
  |   #func                 -60    3.666 
__ipipe_check_root_interruptible+0x10 (__irq_svc+0x94)
      #func                 -56    2.444  irq_exit+0x10 (handle_IRQ+0x70)
      #func                 -54    1.444  __ipipe_do_IRQ+0x10 
(__ipipe_do_sync_stage+0x1bc)
      #func                 -53    0.777  handle_IRQ+0x10 
(__ipipe_do_IRQ+0x1c)
      #func                 -52    1.111  irq_enter+0x10 (handle_IRQ+0x28)
      #func                 -51    1.111  idle_cpu+0x10 (irq_enter+0x1c)
      #func                 -50    1.111  generic_handle_irq+0x10 
(handle_IRQ+0x6c)
      #func                 -48    0.888  handle_level_irq+0x10 
(generic_handle_irq+0x34)
      #func                 -48    1.111  handle_irq_event+0x10 
(handle_level_irq+0x98)
      #func                 -46    1.111 handle_irq_event_percpu+0x14 
(handle_irq_event+0x64)
      #func                 -45    1.555 s3c24xx_serial_tx_chars+0x10 
(handle_irq_event_percpu+0xc0)
      #func                 -44    0.888  uart_write_wakeup+0x10 
(s3c24xx_serial_tx_chars+0xe0)
      #func                 -43    1.111  tty_wakeup+0x10 
(uart_write_wakeup+0x2c)
      #func                 -42    1.555  __wake_up+0x14 (tty_wakeup+0x64)
      #func                 -40    1.000  __wake_up_common+0x10 
(__wake_up+0x5c)
      #func                 -39    0.888 default_wake_function+0x10 
(__wake_up_common+0x58)
      #func                 -38    1.555  try_to_wake_up+0x10 
(default_wake_function+0x1c)
      #func                 -37    1.222  note_interrupt+0x10 
(handle_irq_event_percpu+0x290)
      #func                 -36    1.111  unmask_irq+0x10 
(handle_level_irq+0xc0)
  |   #func                 -34    1.222  s3c_irq_uart0_unmask+0x10 
(unmask_irq+0x40)
      #func                 -33    1.333  irq_exit+0x10 (handle_IRQ+0x70)
  |   +func                 -32    1.333  __ipipe_exit_irq+0x10 
(__ipipe_grab_irq+0x5c)
  |   +func                 -31    2.000 
__ipipe_check_root_interruptible+0x10 (__irq_svc+0x94)

This patch is needed for kernel 3.0, 3.2 and probably for newer version.

Regards
Romain
-------------- next part --------------
A non-text attachment was scrubbed...
Name: linux-0003-Samsung-UART-convert-spinlock-to-ipipe_spinlock.patch
Type: text/x-patch
Size: 2152 bytes
Desc: not available
URL: <http://www.xenomai.org/pipermail/xenomai/attachments/20121022/9d838d18/attachment.bin>

  reply	other threads:[~2012-10-21 23:33 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-19 10:19 [Xenomai] s3c24xx with clocksource/clockevent (kernel 3.2.21) Romain Naour
2012-10-19 12:18 ` Gilles Chanteperdrix
2012-10-20 16:12   ` Romain Naour
2012-10-20 17:26     ` Gilles Chanteperdrix
2012-10-21  1:35       ` Romain Naour
2012-10-21 18:29         ` Gilles Chanteperdrix
2012-10-21 18:43         ` Gilles Chanteperdrix
2012-10-21 23:33           ` Romain Naour [this message]
2012-10-22 13:18             ` Gilles Chanteperdrix
2012-10-22 14:05               ` Gilles Chanteperdrix
2012-10-22 16:26                 ` Romain Naour

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5084863E.7030900@openwide.fr \
    --to=romain.naour@openwide.fr \
    --cc=gilles.chanteperdrix@xenomai.org \
    --cc=xenomai@xenomai.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.