Re: [PATCH] usb: host: Add MAX3421E HCD support.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 24, 2014 at 03:22:10PM -0400, Alan Stern wrote:
> On Mon, 24 Mar 2014, Felipe Balbi wrote:
> 
> > Hi,
> > 
> > On Mon, Mar 24, 2014 at 09:21:40AM -0600, David Mosberger wrote:
> > > Felipe,
> > > 
> > > On Mon, Mar 24, 2014 at 7:31 AM, Felipe Balbi <balbi@xxxxxx> wrote:
> > > 
> > > > Why do you need to run your IRQ handler when an URQ gets enqueued ? That
> > > > doesn't make much sense :-s
> > > 
> > > Please see the comments at the start of the file: spi_sync() is
> > > blocking and must not be called while holding spinlocks or with
> > > interrupts disabled.  The purpose of max3421_spi_thread() is to handle
> > 
> > sure, but this is why we have threaded IRQ support.
> > 
> > > all SPI transactions (implicitly serializing them).  An interrupt is
> > > just one of the reasons to wake up the SPI thread.  Adding URBs,
> > > removing them, or performing a bus reset are other reasons.
> > 
> > you didn't tell me why. Why do you need to run that inside the same
> > thread ? Why can't you do that from inside ->urb_enqueue() itself ?
> > 
> > > You can think of max3421_spi_thread() as implementing the state
> > > machine that would normally be implemented in silicon with a "smart"
> > > controller such as OHCI or similar.
> > 
> > yada, yada, yada... still didn't answer my original question.
> 
> It sounds like David is saying that he needs a process context to 
> communicate with the host controller over SPI, and this communication 
> has to take place whenever an URB is enqueued or dequeued as well as 
> whenever an interrupt occurs.
> 
> If that's correct, it seems a kernel thread would be a better solution 
> than a threaded IRQ handler.

looking at the driver, there's still quite a bit of duplication between
his kthread implementation and what threaded IRQs would give for free...

I see what you're saying, from urb_enqueue the EP schedule needs to be
advanced, so he needs to run his kthread and that might be alright, code
still looks a bit fishy, though.

-- 
balbi

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux