On Thu, 2 Apr 2009, Darius Augulis wrote:

> 
> > > + * struct mx1_camera_pdata - i.MX1/i.MXL camera platform data
> > > + * @init:        Init board resources
> > > + * @exit:        Release board resources
> > > + * @mclk_10khz:  master clock frequency in 10kHz units
> > > + * @flags:       MX1 camera platform flags
> > > + */
> > > +struct mx1_camera_pdata {
> > > + int (*init)(struct device *);
> > > + int (*exit)(struct device *);
> > >     
> > 
> > I thought the agreement was to avoid these .init() and .exit() hooks in new
> > code...
> >   
> 
> Should I config board statically during system start-up?

Unless you have some special requirements why you want to do this 
dynamically - yes.

> 
> 
> > > +static void mx1_videobuf_queue(struct videobuf_queue *vq,
> > > +                                         struct videobuf_buffer *vb)
> > > +{
> > > + struct soc_camera_device *icd = vq->priv_data;
> > > + struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
> > > + struct mx1_camera_dev *pcdev = ici->priv;
> > > + struct mx1_buffer *buf = container_of(vb, struct mx1_buffer, vb);
> > > +
> > > + dev_dbg(&icd->dev, "%s (vb=0x%p) 0x%08lx %d\n", __func__,
> > > +         vb, vb->baddr, vb->bsize);
> > > +
> > > + list_add_tail(&vb->queue, &pcdev->capture);
> > >     
> > 
> > No, you had a spinlock here and in DMA ISR in the previous version, and it
> > was correct. Without that lock the above list_add races with list_del_init()
> > in mx1_camera_wakeup().
> >   
> 
> what can save and help for the spinlock on single-core system? mx3 there does
> not have spinlock.

The IRQ can interrupt you while you're manipulating the list in 
mx1_videobuf_queue(), and corrupt the list. This is not just a spinlock, 
it also disables interrupts. Also, think about preemptible kernels.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to