On Fri, 14 Oct 2005, Franck wrote:
> > > > Also remember that URBs can be given back out of order if they are
> > > > unlinked. Then you have a race: URB is being given back because it was
> > > > unlinked, and at the same time your HCD is making it active.
> > > >
> > >
> > > I don't think this
2005/10/14, Alan Stern <[EMAIL PROTECTED]>:
> We're getting confused over definitions. I thought you used "active" to
> mean that this was the URB being sent by the hardware. Your HCD sends an
I was using your previous definition ;)
> URB to the hardware when it's at the head of the endpoint qu
On Fri, 14 Oct 2005, David Brownell wrote:
> > > > We will still have the problem that the HCDs don't have access to the
> > > > spinlock that protects the queue pointers. I suppose the spinlock could
> > > > be EXPORTed.
> > >
> > > That problem is ... what? Â Exporting that lock would imply ther
On Fri, 14 Oct 2005, Franck wrote:
> 2005/10/13, Alan Stern <[EMAIL PROTECTED]>:
> > On Thu, 13 Oct 2005, Franck wrote:
> > > obviously endpoint queues are usbcore's data but used by hcd. hcd
> > > seems to know when an urb is active. Therefore we could export a new
> > > usbcore's function which
> > > We will still have the problem that the HCDs don't have access to the
> > > spinlock that protects the queue pointers. I suppose the spinlock could
> > > be EXPORTed.
> >
> > That problem is ... what? Exporting that lock would imply there's some
> > good reason for code other than usbcore to
2005/10/13, Alan Stern <[EMAIL PROTECTED]>:
> On Thu, 13 Oct 2005, Franck wrote:
> > obviously endpoint queues are usbcore's data but used by hcd. hcd
> > seems to know when an urb is active. Therefore we could export a new
> > usbcore's function which would be called by hcd to make an urb active:
2005/10/14, David Brownell <[EMAIL PROTECTED]>:
> > We will still have the problem that the HCDs don't have access to the
> > spinlock that protects the queue pointers. I suppose the spinlock could
> > be EXPORTed.
>
> That problem is ... what? Exporting that lock would imply there's some
> good r
> We will still have the problem that the HCDs don't have access to the
> spinlock that protects the queue pointers. I suppose the spinlock could
> be EXPORTed.
That problem is ... what? Exporting that lock would imply there's some
good reason for code other than usbcore to use it. I haven't
On Thu, 13 Oct 2005, Franck wrote:
> > If every HCD is careful not to use URBs that aren't marked (i.e., for
> > which urb->hcpriv is NULL), then a single endpoint queue would be okay.
> > And it wouldn't be necessary to move the usb_get_urb and atomic_inc
> > statements either (or the DMA stuff).
On Thu, 13 Oct 2005, Franck wrote:
> 2005/10/12, Alan Stern <[EMAIL PROTECTED]>:
> > On Wed, 12 Oct 2005, Franck wrote:
> > > Does a patch that do this have any chance to be accepted ?
> >
> > I think we should decide on the _correct_ solution first. Then acceptance
> > will be obvious.
> >
>
>
2005/10/13, Alan Stern <[EMAIL PROTECTED]>:
> On Wed, 12 Oct 2005, David Brownell wrote:
> > The sl811 code should probably refuse to start() urbs it hasn't marked
> > yet (by setting urb->hcpriv) too. There's no point to maintaining a
> > parallel queue for each endpoint, but it should still igno
2005/10/12, Alan Stern <[EMAIL PROTECTED]>:
> On Wed, 12 Oct 2005, Franck wrote:
> > Does a patch that do this have any chance to be accepted ?
>
> I think we should decide on the _correct_ solution first. Then acceptance
> will be obvious.
>
ok, but my question was more: "does the usbcore need t
On Wed, 12 Oct 2005, David Brownell wrote:
> Potentially; the usb_get_urb() and atomic_inc() should be paired with the
> list_add_tail() -- under spinlock protection -- rather than being done as
> late as it's now done by hcd_submit_urb(). That's what Alan's patch does.
>
> Similarly, the urb->*
> > > If an interrupt occurs right after inserting the urb into ep's list,
> > > this urb can be transfered and gave back to the driver before
> > > usb_get_urb is called !
> >
> > Yep, that could do it.
>
> It was. I have no more badness in kref counter. By the way David, can
> this problem happe
On Wed, 12 Oct 2005, Franck wrote:
> > That's because the link belongs to usbcore, not to the HCD. Yes, you
> > could move the code that links the URB to just before calling urb_enqueue.
> > But then (a) you would have to handle the case of root-hub URBs
> > separately, and (b) you would also hav
2005/10/12, Alan Stern <[EMAIL PROTECTED]>:
> On Wed, 12 Oct 2005, Franck wrote:
>
> > > > A simple fix is to link urb right after
> > > > calling hcd->urb_enqueue. What do you think about this ?
> > >
> > > Not good. The URB must be linked before the HCD is called. Your idea
> > > runs the ris
On Wed, 12 Oct 2005, Franck wrote:
> > > A simple fix is to link urb right after
> > > calling hcd->urb_enqueue. What do you think about this ?
> >
> > Not good. The URB must be linked before the HCD is called. Your idea
> > runs the risk of the opposite problem: giving back the URB before it
2005/10/12, Alan Stern <[EMAIL PROTECTED]>:
> On Wed, 12 Oct 2005, Franck wrote:
>
> > > Remember that the only data protected by urb->lock is urb->status,
> > > urb->urb_list, and urb->reject. Your HCD should change nothing but the
> > > status.
> > >
> >
> > it also changes hcpriv, actual_length
On Wed, 12 Oct 2005, Franck wrote:
> > Remember that the only data protected by urb->lock is urb->status,
> > urb->urb_list, and urb->reject. Your HCD should change nothing but the
> > status.
> >
>
> it also changes hcpriv, actual_length, interval, start_frame...hcpriv
> and actual_length seem
2005/10/11, Alan Stern <[EMAIL PROTECTED]>:
> > hmm, this example may be correct not because you're _reading_ the
> > value but because printk statement doesn't _use_ urb's data
> > structure.
>
> Or because the printk statement doesn't require the URB's internal data
> not to change. Yes, but re
On Tue, 11 Oct 2005, Franck wrote:
> > Consider a simple example of reading:
> >
> > spin_lock(&urb->lock);
> > if (urb->status == -EINPROGRESS)
> > printk("URB is in progress\n");
> > else
> > printk("URB is not in progress\n");
> >
2005/10/11, Alan Stern <[EMAIL PROTECTED]>:
> On Tue, 11 Oct 2005, Franck wrote:
>
> > 2005/10/11, Alan Stern <[EMAIL PROTECTED]>:
> > > Be more careful here. It doesn't hold urb->lock while _reading_
> > > urb->status; there's no point. But it does hold urb->lock while
> > > _writing_ urb->statu
On Tue, 11 Oct 2005, Franck wrote:
> 2005/10/11, Alan Stern <[EMAIL PROTECTED]>:
> > Be more careful here. It doesn't hold urb->lock while _reading_
> > urb->status; there's no point. But it does hold urb->lock while
> > _writing_ urb->status. That is necessary.
> >
>
> hmm, sorry but I don't
2005/10/11, Alan Stern <[EMAIL PROTECTED]>:
> Be more careful here. It doesn't hold urb->lock while _reading_
> urb->status; there's no point. But it does hold urb->lock while
> _writing_ urb->status. That is necessary.
>
hmm, sorry but I don't see why it needs a lock while writing but not
whil
On Tue, 11 Oct 2005, Franck wrote:
> still tracking my bug...I just have 2 questions, I looked at uhci hcd
> to have an idea on how interrupts and locks must be handled in
> hcd->urb_enqueue method. uhci hcd simply disables IT in this method,
> so every allocations are atomic and mem_flags paramet
Alan
2005/10/10, Alan Stern <[EMAIL PROTECTED]>:
> The only way it can happen is if your HCD set urb->hcpriv to NULL earlier,
> or never set it to a non-NULL value. It must be a bug in the HCD.
>
still tracking my bug...I just have 2 questions, I looked at uhci hcd
to have an idea on how interru
Hi Alan.
2005/10/10, Alan Stern <[EMAIL PROTECTED]>:
> On Mon, 10 Oct 2005, Franck wrote:
> > But sometime this field is NULL when starting an URB transfer ! That's
> > normally impossible since _all_ URBs submitted to an HCD use
> > "hcd->urb_enqueue", isn't it ?
> > Could someone help me on this
On Mon, 10 Oct 2005, Franck wrote:
> Hi,
>
> I'm still debugging my hcd. I still have some issues, and one of them
> is related to urb->hcpriv usage. Following explain how I use it:
> urb->hcpriv keeps a pointer on a structure which gives an internal
> description of the urb for my hcd. This fiel
28 matches
Mail list logo
