Hi, On Thu, Feb 21, 2013 at 03:22:54AM +0000, Paul Zimmerman wrote: > > From: Felipe Balbi [mailto:balbi@xxxxxx] > > Sent: Wednesday, February 20, 2013 12:29 AM > > On Tue, Feb 19, 2013 at 06:50:03PM -0800, Paul Zimmerman wrote: > > > Here is v4 of the DWC2 patch set. I made most of the changes you asked > > > for, except for the following: > > > - I did not convert to a threaded IRQ handler. I would like to postpone > > > that for now. > > > > without any reasoning ? > > The HSOTG core enables SOF interrupts in some cases, so it has to respond > in less than 125us to interrupts. I'm afraid with a threaded IRQ handler the > latency for handling SOFs could be too large. IRQ threads are the next thing scheduler will fetch after hardirq handlers. It also gives you the oportunity to set IRQ priorities just right for your use case when you have linux-rt patch applied. > I'm not saying I won't do it eventually, I'd just rather not open that possible > can of worms right now. fair enough ;-) > > > - I did not convert to devm_request_and_ioremap() in the probe function, > > > because that requires a struct resource, which you don't have in a PCI > > > driver. Perhaps I'm missing something. > > > > look at line 287 on include/linux/pci.h > > Ah, that's what I was missing. Yes, that makes the probe() code much > nicer, thanks. np > > > - I removed a few unneeded module parameters, but left the rest for now. > > > I discovered that some of the parameters need to be set to non- > > > hardware-default values, or else the driver doesn't work correctly. > > > > isn't that something which is wrong with your FPGA model ? IIRC you can > > change register default values via coreConsultant. > > Building and validating a new FPGA image is a lengthy task. If I can make > things work by tweaking some module parameters, so much the better. > Plus, as part of our validation testing we try with different values for things > like FIFO sizes and DMA modes. Building a new FPGA image to tweak > stuff like that is just not practical. Is there at least part of that stuff which you could just do by default without relying on a module parameter ? I mean, if you know default value is bogus, can you not compute a good value in runtime and just use it ? I mean, the amount of module parameters in this driver is much more than anyone will be able to understand and use correctly. You really want to decrease the amount of module parameters. dwc3, for instance, has a single module parameter which is used for testing purposes only (forcing the IP to work on lower speeds). MUSB has 3 module parameters only, net2272 has 4... anyway, usually anything bigger than 4 to 5 module parameters will already cause problems, specially on cases where loading the driver without any module parameter is likely to not work. my 2 cents. -- balbi
Attachment:
signature.asc
Description: Digital signature
