[Linux1394-cvslog] rev 551 - trunk
Brought to you by:
aeb,
bencollins
From: SVN U. <dm...@li...> - 2002-08-14 01:14:56
|
Author: dmaas Date: 2002-08-14 05:58:04 -0400 (Wed, 14 Aug 2002) New Revision: 551 Modified: trunk/dv1394-private.h trunk/dv1394.c trunk/ohci1394.c trunk/ohci1394.h Log: - dv1394: expand the spinlock to include video->ohci_i*_ctx (Drew Perttula reported race conditions between irq_handler and do_dv1394_shutdown) - ohci1394: add a spinlock to cover hooking/unhooking IRQ handlers Modified: trunk/ohci1394.c ============================================================================== --- trunk/ohci1394.c (original) +++ trunk/ohci1394.c 2002-08-14 05:58:05.000000000 -0400 @@ -2211,6 +2211,7 @@ ohci->irq_hooks[i].irq_handler = NULL; ohci->irq_hooks[i].data = NULL; } + spin_lock_init(&ohci->irq_hooks_lock); if (request_irq(dev->irq, ohci_irq_handler, SA_SHIRQ, OHCI1394_DRIVER_NAME, ohci)) @@ -2359,12 +2360,15 @@ quadlet_t isoXmitEvent) { int i; + + spin_lock(&ohci->irq_hooks_lock); for(i = 0; i < OHCI1394_MAX_IRQ_HOOKS; i++) { if(ohci->irq_hooks[i].irq_handler != NULL) { ohci->irq_hooks[i].irq_handler(ohci->id, isoRecvEvent, isoXmitEvent, ohci->irq_hooks[i].data); } } + spin_unlock(&ohci->irq_hooks_lock); } int ohci1394_hook_irq(struct ti_ohci *ohci, @@ -2372,6 +2376,10 @@ void *data) { int i; + int ret = -EBUSY; + unsigned long flags; + + spin_lock_irqsave(&ohci->irq_hooks_lock, flags); /* find a free slot */ for(i = 0; i < OHCI1394_MAX_IRQ_HOOKS; i++) { @@ -2380,15 +2388,19 @@ } if(i >= OHCI1394_MAX_IRQ_HOOKS) - return -EBUSY; + goto out; ohci->irq_hooks[i].irq_handler = irq_handler; ohci->irq_hooks[i].data = data; + ret = 0; + +out: + spin_unlock_irqrestore(&ohci->irq_hooks_lock, flags); + return ret; + /* ohci1394 will never be unloaded while an IRQ hook is in use, because the user must reference this symbol */ - - return 0; } void ohci1394_unhook_irq(struct ti_ohci *ohci, @@ -2396,6 +2408,9 @@ void *data) { int i; + unsigned long flags; + + spin_lock_irqsave(&ohci->irq_hooks_lock, flags); for(i = 0; i < OHCI1394_MAX_IRQ_HOOKS; i++) { if( (ohci->irq_hooks[i].irq_handler == irq_handler) && @@ -2407,6 +2422,8 @@ ohci->irq_hooks[i].irq_handler = NULL; ohci->irq_hooks[i].data = NULL; } + + spin_unlock_irqrestore(&ohci->irq_hooks_lock, flags); } EXPORT_SYMBOL(ohci1394_stop_context); Modified: trunk/dv1394-private.h ============================================================================== --- trunk/dv1394-private.h (original) +++ trunk/dv1394-private.h 2002-08-14 05:58:05.000000000 -0400 @@ -477,6 +477,9 @@ immediately. This is safe because the interrupt handler will never advance active_frame onto a frame that is not READY (and the spinlock must be held while marking a frame READY). + + spinlock is also used to protect ohci_it_ctx and ohci_ir_ctx, + which can be accessed from both process and interrupt context */ spinlock_t spinlock; Modified: trunk/ohci1394.h ============================================================================== --- trunk/ohci1394.h (original) +++ trunk/ohci1394.h 2002-08-14 05:58:05.000000000 -0400 @@ -212,6 +212,8 @@ void *data; } irq_hooks[OHCI1394_MAX_IRQ_HOOKS]; + spinlock_t irq_hooks_lock; + /* Swap the selfid buffer? */ unsigned int selfid_swap:1; /* Some Apple chipset seem to swap incoming headers for us */ Modified: trunk/dv1394.c ============================================================================== --- trunk/dv1394.c (original) +++ trunk/dv1394.c 2002-08-14 05:58:05.000000000 -0400 @@ -982,6 +982,7 @@ do_dv1394_shutdown(video, 0); + /* try to claim the ISO channel */ spin_lock_irqsave(&video->ohci->IR_channel_lock, flags); if(video->ohci->ISO_channel_usage & chan_mask) { @@ -994,9 +995,46 @@ video->channel = init->channel; + /* initialize misc. fields of video */ + video->n_frames = init->n_frames; + video->pal_or_ntsc = init->format; + + + video->cip_accum = 0; + video->continuity_counter = 0; + + video->active_frame = -1; + video->first_clear_frame = 0; + video->n_clear_frames = video->n_frames; + video->dropped_frames = 0; + + video->write_off = 0; + + video->first_run = 1; + video->current_packet = -1; + video->first_frame = 0; + + + if(video->pal_or_ntsc == DV1394_NTSC) { + video->cip_n = init->cip_n != 0 ? init->cip_n : CIP_N_NTSC; + video->cip_d = init->cip_d != 0 ? init->cip_d : CIP_D_NTSC; + video->frame_size = DV1394_NTSC_FRAME_SIZE; + } else { + video->cip_n = init->cip_n != 0 ? init->cip_n : CIP_N_PAL; + video->cip_d = init->cip_d != 0 ? init->cip_d : CIP_D_PAL; + video->frame_size = DV1394_PAL_FRAME_SIZE; + } + + video->syt_offset = init->syt_offset; - /* find and claim DMA contexts on the OHCI card */ + /* find and claim DMA contexts on the OHCI card */ + + /* XXX this should be the last step of initialization, since the interrupt + handler uses ohci_i*_ctx to indicate whether or not it is safe to touch + frames. I'm not making this change quite yet, since it would be better + to clean up the init/shutdown process first.*/ + if(video->ohci_it_ctx == -1) { for(i = 0; i < video->ohci->nb_iso_xmit_ctx; i++) { @@ -1045,38 +1083,8 @@ } } - /* initialize misc. fields of video */ - video->n_frames = init->n_frames; - video->pal_or_ntsc = init->format; - video->cip_accum = 0; - video->continuity_counter = 0; - - video->active_frame = -1; - video->first_clear_frame = 0; - video->n_clear_frames = video->n_frames; - video->dropped_frames = 0; - - video->write_off = 0; - - video->first_run = 1; - video->current_packet = -1; - video->first_frame = 0; - - - if(video->pal_or_ntsc == DV1394_NTSC) { - video->cip_n = init->cip_n != 0 ? init->cip_n : CIP_N_NTSC; - video->cip_d = init->cip_d != 0 ? init->cip_d : CIP_D_NTSC; - video->frame_size = DV1394_NTSC_FRAME_SIZE; - } else { - video->cip_n = init->cip_n != 0 ? init->cip_n : CIP_N_PAL; - video->cip_d = init->cip_d != 0 ? init->cip_d : CIP_D_PAL; - video->frame_size = DV1394_PAL_FRAME_SIZE; - } - - video->syt_offset = init->syt_offset; - if(video->user_buf == NULL) { unsigned int i; @@ -1303,25 +1311,14 @@ static int do_dv1394_shutdown(struct video_card *video, int free_user_buf) { int i; - + unsigned long flags; + debug_printk("dv1394: shutdown...\n"); /* stop DMA if in progress */ stop_dma(video); - /* release the ISO channel */ - if(video->channel != -1) { - u64 chan_mask; - unsigned long flags; - - chan_mask = (u64)1 << video->channel; - - spin_lock_irqsave(&video->ohci->IR_channel_lock, flags); - video->ohci->ISO_channel_usage &= ~(chan_mask); - spin_unlock_irqrestore(&video->ohci->IR_channel_lock, flags); - - video->channel = -1; - } + spin_lock_irqsave(&video->spinlock, flags); /* release the DMA contexts */ if(video->ohci_it_ctx != -1) { @@ -1351,6 +1348,22 @@ video->ohci_ir_ctx = -1; } + spin_unlock_irqrestore(&video->spinlock, flags); + + /* release the ISO channel */ + if(video->channel != -1) { + u64 chan_mask; + unsigned long flags; + + chan_mask = (u64)1 << video->channel; + + spin_lock_irqsave(&video->ohci->IR_channel_lock, flags); + video->ohci->ISO_channel_usage &= ~(chan_mask); + spin_unlock_irqrestore(&video->ohci->IR_channel_lock, flags); + + video->channel = -1; + } + /* free the frame structs */ for(i = 0; i < DV1394_MAX_FRAMES; i++) { if(video->frames[i]) @@ -2311,6 +2324,8 @@ int wake = 0; struct video_card *video = (struct video_card*) data; + spin_lock(&video->spinlock); + irq_printk("INTERRUPT! Video = %08lx Iso event Recv: %08x Xmit: %08x\n", (unsigned long) video, isoRecvIntEvent, isoXmitIntEvent); irq_printk("ContextControl = %08x, CommandPtr = %08x\n", @@ -2326,7 +2341,7 @@ struct frame *f; unsigned int frame, i; - spin_lock(&video->spinlock); + if(video->active_frame == -1) frame = 0; else @@ -2439,8 +2454,6 @@ } /* for(each frame) */ - spin_unlock(&video->spinlock); - } /* end XMIT portion */ /***** RECEIVE INTERRUPT and DMA ACTIVE *****/ @@ -2452,8 +2465,6 @@ int sof=0; /* start-of-frame flag */ struct frame *f; u16 packet_length, packet_time; - - spin_lock(&video->spinlock); packet_length = le16_to_cpu(video->packet_buffer[video->current_packet].data_length); packet_time = le16_to_cpu(video->packet_buffer[video->current_packet].timestamp); @@ -2520,12 +2531,12 @@ /* advance packet_buffer cursor */ video->current_packet = (video->current_packet + 1) % MAX_PACKET_BUFFER; - spin_unlock(&video->spinlock); - wake = 1; /* why the hell not? */ } /* receive interrupt */ + spin_unlock(&video->spinlock); + if(wake) { /* send SIGIO */ @@ -2922,12 +2933,13 @@ if(!video) return; + + spin_lock_irqsave(&video->spinlock, flags); + /* check IT context */ if(video->ohci_it_ctx != -1) { u32 ctx; - spin_lock_irqsave(&video->spinlock, flags); - ctx = reg_read(video->ohci, video->ohci_IsoXmitContextControlSet); /* if(RUN but not ACTIVE) */ @@ -2958,16 +2970,12 @@ reg_read(video->ohci, video->ohci_IsoXmitContextControlSet), reg_read(video->ohci, video->ohci_IsoXmitCommandPtr)); } - - spin_unlock_irqrestore(&video->spinlock, flags); } /* check IR context */ if(video->ohci_ir_ctx != -1) { u32 ctx; - spin_lock_irqsave(&video->spinlock, flags); - ctx = reg_read(video->ohci, video->ohci_IsoRcvContextControlSet); /* if(RUN but not ACTIVE) */ @@ -2999,10 +3007,10 @@ reg_read(video->ohci, video->ohci_IsoRcvContextControlSet), reg_read(video->ohci, video->ohci_IsoRcvCommandPtr)); } - - spin_unlock_irqrestore(&video->spinlock, flags); } - + + spin_unlock_irqrestore(&video->spinlock, flags); + /* wake readers/writers/ioctl'ers */ wake_up_interruptible(&video->waitq); } |