Re: [PATCH 1/4] staging: most: hdm-usb: remove proprietary urb anchoring

[Greg KH]

On Mon, Sep 19, 2016 at 05:40:22PM +0200, Christian Gromm wrote:
> This patch removes the propietary tracking of URBs. Instead the structure
> usb_anchor of the USB subsystem is used.
> 
> Signed-off-by: Christian Gromm 

Ah, nice job, glad to see these changes made.

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/4] staging: most: hdm-usb: remove proprietary urb anchoring

[Christian Gromm]

This patch removes the propietary tracking of URBs. Instead the structure
usb_anchor of the USB subsystem is used.

Signed-off-by: Christian Gromm 
---
 drivers/staging/most/hdm-usb/hdm_usb.c | 75 ++
 1 file changed, 21 insertions(+), 54 deletions(-)

diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c 
b/drivers/staging/most/hdm-usb/hdm_usb.c
index 5b27e96..58f65a4 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -65,17 +65,6 @@
 #define DRCI_WRITE_REQ 0xA1
 
 /**
- * struct buf_anchor - used to create a list of pending URBs
- * @urb: pointer to USB request block
- * @list: linked list
- * @urb_completion:
- */
-struct buf_anchor {
-   struct urb *urb;
-   struct list_head list;
-};
-
-/**
  * struct most_dci_obj - Direct Communication Interface
  * @kobj:position in sysfs
  * @usb_device: pointer to the usb device
@@ -116,7 +105,7 @@ struct clear_hold_work {
  * @anchor_list_lock: locks list access
  * @padding_active: indicates channel uses padding
  * @is_channel_healthy: health status table of each channel
- * @anchor_list: list of anchored items
+ * @busy_urbs: list of anchored items
  * @io_mutex: synchronize I/O with disconnect
  * @link_stat_timer: timer for link status reports
  * @poll_work_obj: work for polling link status
@@ -137,7 +126,7 @@ struct most_dev {
bool padding_active[MAX_NUM_ENDPOINTS];
bool is_channel_healthy[MAX_NUM_ENDPOINTS];
struct clear_hold_work clear_work[MAX_NUM_ENDPOINTS];
-   struct list_head *anchor_list;
+   struct usb_anchor *busy_urbs;
struct mutex io_mutex;
struct timer_list link_stat_timer;
struct work_struct poll_work_obj;
@@ -207,29 +196,22 @@ static void free_anchored_buffers(struct most_dev *mdev, 
unsigned int channel,
  enum mbo_status_flags status)
 {
struct mbo *mbo;
-   struct buf_anchor *anchor, *tmp;
+   struct urb *urb;
spinlock_t *lock = mdev->anchor_list_lock + channel; /* temp. lock */
unsigned long flags;
 
spin_lock_irqsave(lock, flags);
-   list_for_each_entry_safe(anchor, tmp, >anchor_list[channel],
-list) {
-   struct urb *urb = anchor->urb;
-
+   while ((urb = usb_get_from_anchor(>busy_urbs[channel]))) {
spin_unlock_irqrestore(lock, flags);
-   if (likely(urb)) {
-   mbo = urb->context;
-   usb_kill_urb(urb);
-   if (mbo && mbo->complete) {
-   mbo->status = status;
-   mbo->processed_length = 0;
-   mbo->complete(mbo);
-   }
-   usb_free_urb(urb);
+   mbo = urb->context;
+   usb_kill_urb(urb);
+   if (mbo && mbo->complete) {
+   mbo->status = status;
+   mbo->processed_length = 0;
+   mbo->complete(mbo);
}
+   usb_free_urb(urb);
spin_lock_irqsave(lock, flags);
-   list_del(>list);
-   kfree(anchor);
}
spin_unlock_irqrestore(lock, flags);
 }
@@ -394,7 +376,6 @@ static int hdm_remove_padding(struct most_dev *mdev, int 
channel,
 static void hdm_write_completion(struct urb *urb)
 {
struct mbo *mbo = urb->context;
-   struct buf_anchor *anchor = mbo->priv;
struct most_dev *mdev = to_mdev(mbo->ifp);
unsigned int channel = mbo->hdm_channel_id;
struct device *dev = >usb_device->dev;
@@ -426,14 +407,13 @@ static void hdm_write_completion(struct urb *urb)
mbo->status = MBO_E_INVAL;
break;
}
+   usb_unanchor_urb(urb);
} else {
mbo->status = MBO_SUCCESS;
mbo->processed_length = urb->actual_length;
}
 
-   list_del(>list);
spin_unlock_irqrestore(lock, flags);
-   kfree(anchor);
 
if (likely(mbo->complete))
mbo->complete(mbo);
@@ -551,7 +531,6 @@ static void hdm_write_completion(struct urb *urb)
 static void hdm_read_completion(struct urb *urb)
 {
struct mbo *mbo = urb->context;
-   struct buf_anchor *anchor = mbo->priv;
struct most_dev *mdev = to_mdev(mbo->ifp);
unsigned int channel = mbo->hdm_channel_id;
struct device *dev = >usb_device->dev;
@@ -585,6 +564,7 @@ static void hdm_read_completion(struct urb *urb)
mbo->status = MBO_E_INVAL;
break;
}
+   usb_unanchor_urb(urb);
} else {
mbo->processed_length = urb->actual_length;
mbo->status = MBO_SUCCESS;
@@ -595,9 +575,7 @@ static void hdm_read_completion(struct urb *urb)