Thanks for the review Kevin!
+struct mutex_s usb_hid_lock;
What is this lock for? The "thread" implementation in SeaBIOS is cooperative, so I don't see how the code could be preempted between lock/unlock anyway. Maybe I'm missing something.
Good question. I'm not the most versed in system programming but I noticed that if a hub is found in usb.c:configure_usb_device -> usb-hub.c:usb_hub_setup -> usb.c:enumerate is called which calls run_thread(usb_hub_port_setup) -> usb.c:usb_hub_port_setup -> usb.c:configure_usb_device. I assumed this meant that multiple threads may find keyboards or mice which is why I put this lock in.
With this information, is it still the case that I don't need a lock here?
I'll keep it in the version I'm attaching to this email pending your reply
- for (struct pipe_node *node = GET_GLOBAL(keyboard_pipe_node);
node; node = GET_GLOBALFLAT(node->next)) {
Please wrap the lines to 80 characters - in keeping with the existing code style for this file.
Will do! Updated in the latest version attached to this email
struct usb_pipe *pipe = GET_GLOBALFLAT(node->pipe);if (!pipe)continue;How could pipe ever be null?
Based on your question I'm guessing this won't be the case. I noticed that usb.c can make usb_free_pipe calls so I thought I should check just to be sure. I've just removed these checks now
What's the best way to post the latest changes, is just an attachment preferred or in the email body as well?
Cheers, - Daniel K.