On Thu, Aug 22, 2013 at 06:20:14PM +0300, Xenia Ragiadakou wrote:
This patch refactors the switch statement in handle_cmd_completion() by
creating a separate function for each branch, named 'xhci_handle_cmd_type'.
Other changes introduced by this patch are:
- Renaming the already existed functions by adding the prefix
'xhci_handle_cmd_' for consistency. Also, for handle_stopped_endpoint()
the order of arguments was changed too, so that the prototype of
'xhci_handle_cmd_' be similar and easy to follow.
- Additional local variables were added, such as cmd_trb, cmd_comp_code,
cmd_type, add_flags and drop_flags, and the label 'update_ring',
mainly to reduce code dublication and line length.
- The variable ep_ring, that was assigned in the case TRB_CONFIG_EP
but never used, was removed.
Can you break these changes into several patches, in order to make it
easier to review? Something like:
1. Rename existing functions.
2. Change ordering of arguments for consistency.
3. Remove the unused variable from the TRB_CONFIG_EP case.
4. Several patches to refactor the code.
For the refactoring, break each refactoring into a separate patch. I.e.
refactoring and creating xhci_handle_cmd_enable_slot goes in one patch,
xhci_handle_cmd_config_ep goes in another patch.
This patch is nearing the 400-line fatigue point of most reviewers, see
the article on Greg's google plus post for an interesting analysis:
https://plus.google.com/111049168280159033135/posts/2KcFNwzzEgC
Sarah Sharp
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
Differences from version 1:
Create separate functions for each switch case branch, instead of a single
function holding the entire switch
drivers/usb/host/xhci-ring.c | 310
++-
1 file changed, 185 insertions(+), 125 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ddbda35..bc9ce03 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -755,8 +755,8 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd
*xhci,
* 2. Otherwise, we turn all the TRBs in the TD into No-op TRBs (with the
chain
* bit cleared) so that the HW will skip over them.
*/
-static void handle_stopped_endpoint(struct xhci_hcd *xhci,
- union xhci_trb *trb, struct xhci_event_cmd *event)
+static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event, union xhci_trb *trb)
{
unsigned int slot_id;
unsigned int ep_index;
@@ -1063,9 +1063,8 @@ static void update_ring_for_set_deq_completion(struct
xhci_hcd *xhci,
* endpoint doorbell to restart the ring, but only if there aren't more
* cancellations pending.
*/
-static void handle_set_deq_completion(struct xhci_hcd *xhci,
- struct xhci_event_cmd *event,
- union xhci_trb *trb)
+static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event, union xhci_trb *trb)
{
unsigned int slot_id;
unsigned int ep_index;
@@ -1157,9 +1156,8 @@ static void handle_set_deq_completion(struct xhci_hcd
*xhci,
ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
}
-static void handle_reset_ep_completion(struct xhci_hcd *xhci,
- struct xhci_event_cmd *event,
- union xhci_trb *trb)
+static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event, union xhci_trb *trb)
{
int slot_id;
unsigned int ep_index;
@@ -1372,21 +1370,160 @@ static int handle_stopped_cmd_ring(struct xhci_hcd
*xhci,
return cur_trb_is_good;
}
-static void handle_cmd_completion(struct xhci_hcd *xhci,
+static void xhci_handle_cmd_enable_slot(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event, u32 cmd_comp_code)
+{
+ int slot_id;
+
+ slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event-flags));
+ if (cmd_comp_code == COMP_SUCCESS)
+ xhci-slot_id = slot_id;
+ else
+ xhci-slot_id = 0;
+ complete(xhci-addr_dev);
+}
+
+static void xhci_handle_cmd_disable_slot(struct xhci_hcd *xhci,
struct xhci_event_cmd *event)
{
- int slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event-flags));
- u64 cmd_dma;
- dma_addr_t cmd_dequeue_dma;
- struct xhci_input_control_ctx *ctrl_ctx;
+ int slot_id;
struct xhci_virt_device *virt_dev;
+
+ slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event-flags));
+ virt_dev = xhci-devs[slot_id];
+ if (!virt_dev)
+ return;
+ if (xhci-quirks XHCI_EP_LIMIT_QUIRK)
+ /* Delete default control endpoint resources */
+ xhci_free_device_endpoint_resources(xhci, virt_dev, true);
+ xhci_free_virt_device(xhci, slot_id);
+}
+
+static void xhci_handle_cmd_config_ep(struct xhci_hcd *xhci,
+ struct
