IMHO the xhci driver is already far too complicated, and this probably just makes
it even worse.
This is an attempt to make it cleaner and less complicated.
In the end this series removes far more more code than it adds:
5 files changed, 373 insertions(+), 530 deletions(-)
The timers are also simplified, so now we only have one timeout timer
instead of each command running its own timer.
The fact that you are having to allocate memory ion an ISR ought also to be
ringing alarm bells.
It did.
Other options are as you said to use a 'software command ring'. Either
just pre-allocate a full command ring (64 trbs * sizeof(struct
xhci_command)), roughly 2300 bytes when driver loads,
or then a smaller pool of commands just used for ISR submitted commands.
(We then need to either either expand pool, or start allocating in isr
if pool is empty)
These solutions require some ring management and possibly expansion
code, and increases the complexity again. I selected this simpler
approach as I understood that the frequency of commands allocated in ISR
is quite low.
This also feels like trying to optimize before we get the main
changes working.
Have you considered adding a 'software command ring' (indexed with the
same value as the hardware one) and using it to hold additional parameters?
It might even be worth only putting a single command into the hardware ring!
That might simplify the timer code.
This is something I haven't thought about. Could be one possibility, but
feels like artificially restricting something the HW is designed to care
of. The timer code isn't that complex anyway. In addition to init/exit
parts we basically got:
queue_command(...)
{
if (command list is empty)
/* start timer for new command added to empty list*/
mod_timer(cmd_timer, timeout);
}
handle_cmd_completion(...) /* in ISR */
{
/* A commandcompleted, remove timeout timer */
del_timer(cmd_timer)
do_other_stuff
if(more commands in list)
mod_timer(cmd_timer, timeout); /* start timer for next command */
}
This still has a fixed constraint on the number of queued commands, but
I suspect that is bounded anyway (a few per device?).
64 commands fit on the command ring segment, I'm not aware of any
per-device limits?
Thanks for the comments and ideas
-Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
