* [PATCH 1/8] staging: most: hdm-usb: fix race between enqueue and most_stop_enqueue
2016-08-16 14:10 [PATCH 0/8] staging: most: fix hdm-usb issues Christian Gromm
@ 2016-08-16 14:10 ` Christian Gromm
2016-08-16 14:10 ` [PATCH 2/8] staging: most: hdm-usb: assign spinlock to local variable Christian Gromm
` (7 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Christian Gromm @ 2016-08-16 14:10 UTC (permalink / raw)
To: gregkh; +Cc: Christian Gromm, driverdev-devel, Andrey Shvetsov
The "broken in pipe" handler of the USB-HDM calls most_stop_enqueue() to
stop the MBO traffic before returning all MBOs back to the Mostcore. As
the enqueue() call from the Mostcore may run in parallel with the
most_stop_enqueue(), the HDM may run into the inconsistent state and
crash the kernel.
This patch synchronizes enqueue(), most_stop_enqueue() and
most_resume_enqueue() with a mutex, hence avoiding the race condition.
Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
drivers/staging/most/hdm-usb/hdm_usb.c | 3 +-
drivers/staging/most/mostcore/core.c | 53 ++++++++++++++++++++++++----------
2 files changed, 38 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c b/drivers/staging/most/hdm-usb/hdm_usb.c
index 7f00aaf..8d8c72c 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -415,7 +415,6 @@ static void hdm_write_completion(struct urb *urb)
switch (urb->status) {
case -EPIPE:
dev_warn(dev, "Broken OUT pipe detected\n");
- most_stop_enqueue(&mdev->iface, channel);
mdev->is_channel_healthy[channel] = false;
mbo->status = MBO_E_INVAL;
mdev->clear_work[channel].pipe = urb->pipe;
@@ -578,7 +577,6 @@ static void hdm_read_completion(struct urb *urb)
switch (urb->status) {
case -EPIPE:
dev_warn(dev, "Broken IN pipe detected\n");
- most_stop_enqueue(&mdev->iface, channel);
mdev->is_channel_healthy[channel] = false;
mbo->status = MBO_E_INVAL;
mdev->clear_work[channel].pipe = urb->pipe;
@@ -928,6 +926,7 @@ static void wq_clear_halt(struct work_struct *wq_obj)
int pipe = clear_work->pipe;
mutex_lock(&mdev->io_mutex);
+ most_stop_enqueue(&mdev->iface, channel);
free_anchored_buffers(mdev, channel, MBO_E_INVAL);
if (usb_clear_halt(mdev->usb_device, pipe))
dev_warn(&mdev->usb_device->dev, "Failed to reset endpoint.\n");
diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index b03cdc9..db0606ca 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -51,6 +51,7 @@ struct most_c_obj {
u16 channel_id;
bool is_poisoned;
struct mutex start_mutex;
+ struct mutex nq_mutex; /* nq thread synchronization */
int is_starving;
struct most_interface *iface;
struct most_inst_obj *inst;
@@ -1131,18 +1132,18 @@ static inline void trash_mbo(struct mbo *mbo)
spin_unlock_irqrestore(&c->fifo_lock, flags);
}
-static struct mbo *get_hdm_mbo(struct most_c_obj *c)
+static bool hdm_mbo_ready(struct most_c_obj *c)
{
- unsigned long flags;
- struct mbo *mbo;
+ bool empty;
- spin_lock_irqsave(&c->fifo_lock, flags);
- if (c->enqueue_halt || list_empty(&c->halt_fifo))
- mbo = NULL;
- else
- mbo = list_pop_mbo(&c->halt_fifo);
- spin_unlock_irqrestore(&c->fifo_lock, flags);
- return mbo;
+ if (c->enqueue_halt)
+ return false;
+
+ spin_lock_irq(&c->fifo_lock);
+ empty = list_empty(&c->halt_fifo);
+ spin_unlock_irq(&c->fifo_lock);
+
+ return !empty;
}
static void nq_hdm_mbo(struct mbo *mbo)
@@ -1160,20 +1161,32 @@ static int hdm_enqueue_thread(void *data)
{
struct most_c_obj *c = data;
struct mbo *mbo;
+ int ret;
typeof(c->iface->enqueue) enqueue = c->iface->enqueue;
while (likely(!kthread_should_stop())) {
wait_event_interruptible(c->hdm_fifo_wq,
- (mbo = get_hdm_mbo(c)) ||
+ hdm_mbo_ready(c) ||
kthread_should_stop());
- if (unlikely(!mbo))
+ mutex_lock(&c->nq_mutex);
+ spin_lock_irq(&c->fifo_lock);
+ if (unlikely(c->enqueue_halt || list_empty(&c->halt_fifo))) {
+ spin_unlock_irq(&c->fifo_lock);
+ mutex_unlock(&c->nq_mutex);
continue;
+ }
+
+ mbo = list_pop_mbo(&c->halt_fifo);
+ spin_unlock_irq(&c->fifo_lock);
if (c->cfg.direction == MOST_CH_RX)
mbo->buffer_length = c->cfg.buffer_size;
- if (unlikely(enqueue(mbo->ifp, mbo->hdm_channel_id, mbo))) {
+ ret = enqueue(mbo->ifp, mbo->hdm_channel_id, mbo);
+ mutex_unlock(&c->nq_mutex);
+
+ if (unlikely(ret)) {
pr_err("hdm enqueue failed\n");
nq_hdm_mbo(mbo);
c->hdm_enqueue_task = NULL;
@@ -1759,6 +1772,7 @@ struct kobject *most_register_interface(struct most_interface *iface)
init_completion(&c->cleanup);
atomic_set(&c->mbo_ref, 0);
mutex_init(&c->start_mutex);
+ mutex_init(&c->nq_mutex);
list_add_tail(&c->list, &inst->channel_list);
}
pr_info("registered new MOST device mdev%d (%s)\n",
@@ -1824,8 +1838,12 @@ void most_stop_enqueue(struct most_interface *iface, int id)
{
struct most_c_obj *c = get_channel_by_iface(iface, id);
- if (likely(c))
- c->enqueue_halt = true;
+ if (!c)
+ return;
+
+ mutex_lock(&c->nq_mutex);
+ c->enqueue_halt = true;
+ mutex_unlock(&c->nq_mutex);
}
EXPORT_SYMBOL_GPL(most_stop_enqueue);
@@ -1841,9 +1859,12 @@ void most_resume_enqueue(struct most_interface *iface, int id)
{
struct most_c_obj *c = get_channel_by_iface(iface, id);
- if (unlikely(!c))
+ if (!c)
return;
+
+ mutex_lock(&c->nq_mutex);
c->enqueue_halt = false;
+ mutex_unlock(&c->nq_mutex);
wake_up_interruptible(&c->hdm_fifo_wq);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/8] staging: most: hdm-usb: assign spinlock to local variable
2016-08-16 14:10 [PATCH 0/8] staging: most: fix hdm-usb issues Christian Gromm
2016-08-16 14:10 ` [PATCH 1/8] staging: most: hdm-usb: fix race between enqueue and most_stop_enqueue Christian Gromm
@ 2016-08-16 14:10 ` Christian Gromm
2016-08-16 14:10 ` [PATCH 3/8] staging: most: hdm-usb: synchronize release of struct buf_anchor Christian Gromm
` (6 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Christian Gromm @ 2016-08-16 14:10 UTC (permalink / raw)
To: gregkh; +Cc: Christian Gromm, driverdev-devel, Andrey Shvetsov
This patch assigns the spinlock of struct mdev to local spinlock_t
variable to get rid of all the ugly dereferencing.
Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
drivers/staging/most/hdm-usb/hdm_usb.c | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c b/drivers/staging/most/hdm-usb/hdm_usb.c
index 8d8c72c..4156a30 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -206,14 +206,15 @@ static void free_anchored_buffers(struct most_dev *mdev, unsigned int channel,
{
struct mbo *mbo;
struct buf_anchor *anchor, *tmp;
+ spinlock_t *lock = mdev->anchor_list_lock + channel; /* temp. lock */
unsigned long flags;
- spin_lock_irqsave(&mdev->anchor_list_lock[channel], flags);
+ spin_lock_irqsave(lock, flags);
list_for_each_entry_safe(anchor, tmp, &mdev->anchor_list[channel],
list) {
struct urb *urb = anchor->urb;
- spin_unlock_irqrestore(&mdev->anchor_list_lock[channel], flags);
+ spin_unlock_irqrestore(lock, flags);
if (likely(urb)) {
mbo = urb->context;
if (!irqs_disabled()) {
@@ -229,11 +230,11 @@ static void free_anchored_buffers(struct most_dev *mdev, unsigned int channel,
}
usb_free_urb(urb);
}
- spin_lock_irqsave(&mdev->anchor_list_lock[channel], flags);
+ spin_lock_irqsave(lock, flags);
list_del(&anchor->list);
kfree(anchor);
}
- spin_unlock_irqrestore(&mdev->anchor_list_lock[channel], flags);
+ spin_unlock_irqrestore(lock, flags);
}
/**
@@ -397,12 +398,14 @@ static void hdm_write_completion(struct urb *urb)
struct device *dev;
unsigned int channel;
unsigned long flags;
+ spinlock_t *lock; /* temp. lock */
mbo = urb->context;
anchor = mbo->priv;
mdev = to_mdev(mbo->ifp);
channel = mbo->hdm_channel_id;
dev = &mdev->usb_device->dev;
+ lock = mdev->anchor_list_lock + channel;
if ((urb->status == -ENOENT) || (urb->status == -ECONNRESET) ||
(!mdev->is_channel_healthy[channel])) {
@@ -433,9 +436,9 @@ static void hdm_write_completion(struct urb *urb)
mbo->processed_length = urb->actual_length;
}
- spin_lock_irqsave(&mdev->anchor_list_lock[channel], flags);
+ spin_lock_irqsave(lock, flags);
list_del(&anchor->list);
- spin_unlock_irqrestore(&mdev->anchor_list_lock[channel], flags);
+ spin_unlock_irqrestore(lock, flags);
kfree(anchor);
if (likely(mbo->complete))
@@ -559,12 +562,14 @@ static void hdm_read_completion(struct urb *urb)
struct device *dev;
unsigned long flags;
unsigned int channel;
+ spinlock_t *lock; /* temp. lock */
mbo = urb->context;
anchor = mbo->priv;
mdev = to_mdev(mbo->ifp);
channel = mbo->hdm_channel_id;
dev = &mdev->usb_device->dev;
+ lock = mdev->anchor_list_lock + channel;
if ((urb->status == -ENOENT) || (urb->status == -ECONNRESET) ||
(!mdev->is_channel_healthy[channel])) {
@@ -601,9 +606,9 @@ static void hdm_read_completion(struct urb *urb)
mbo->status = MBO_E_INVAL;
}
}
- spin_lock_irqsave(&mdev->anchor_list_lock[channel], flags);
+ spin_lock_irqsave(lock, flags);
list_del(&anchor->list);
- spin_unlock_irqrestore(&mdev->anchor_list_lock[channel], flags);
+ spin_unlock_irqrestore(lock, flags);
kfree(anchor);
if (likely(mbo->complete))
@@ -638,6 +643,7 @@ static int hdm_enqueue(struct most_interface *iface, int channel,
unsigned long flags;
unsigned long length;
void *virt_address;
+ spinlock_t *lock; /* temp. lock */
if (unlikely(!iface || !mbo))
return -EIO;
@@ -697,9 +703,10 @@ static int hdm_enqueue(struct most_interface *iface, int channel,
}
urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
- spin_lock_irqsave(&mdev->anchor_list_lock[channel], flags);
+ lock = mdev->anchor_list_lock + channel;
+ spin_lock_irqsave(lock, flags);
list_add_tail(&anchor->list, &mdev->anchor_list[channel]);
- spin_unlock_irqrestore(&mdev->anchor_list_lock[channel], flags);
+ spin_unlock_irqrestore(lock, flags);
retval = usb_submit_urb(urb, GFP_KERNEL);
if (retval) {
@@ -709,9 +716,9 @@ static int hdm_enqueue(struct most_interface *iface, int channel,
return 0;
_error_1:
- spin_lock_irqsave(&mdev->anchor_list_lock[channel], flags);
+ spin_lock_irqsave(lock, flags);
list_del(&anchor->list);
- spin_unlock_irqrestore(&mdev->anchor_list_lock[channel], flags);
+ spin_unlock_irqrestore(lock, flags);
kfree(anchor);
_error:
usb_free_urb(urb);
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/8] staging: most: hdm-usb: synchronize release of struct buf_anchor
2016-08-16 14:10 [PATCH 0/8] staging: most: fix hdm-usb issues Christian Gromm
2016-08-16 14:10 ` [PATCH 1/8] staging: most: hdm-usb: fix race between enqueue and most_stop_enqueue Christian Gromm
2016-08-16 14:10 ` [PATCH 2/8] staging: most: hdm-usb: assign spinlock to local variable Christian Gromm
@ 2016-08-16 14:10 ` Christian Gromm
2016-08-16 14:10 ` [PATCH 4/8] staging: most: hdm-usb: remove completion object Christian Gromm
` (5 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Christian Gromm @ 2016-08-16 14:10 UTC (permalink / raw)
To: gregkh; +Cc: Christian Gromm, driverdev-devel, Andrey Shvetsov
In case a channel that is going to be destroyed has been tagged as not
"healthy" by the function hdm_poison_channel() while the functions
hdm_write_completion() or hdm_read_completion() are being executed, they
race for destruction of buf_anchor.
This patch fixes the problem.
Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
drivers/staging/most/hdm-usb/hdm_usb.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c b/drivers/staging/most/hdm-usb/hdm_usb.c
index 4156a30..26b5c1b 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -285,6 +285,8 @@ static unsigned int get_stream_frame_size(struct most_channel_config *cfg)
static int hdm_poison_channel(struct most_interface *iface, int channel)
{
struct most_dev *mdev;
+ unsigned long flags;
+ spinlock_t *lock; /* temp. lock */
mdev = to_mdev(iface);
if (unlikely(!iface)) {
@@ -296,7 +298,10 @@ static int hdm_poison_channel(struct most_interface *iface, int channel)
return -ECHRNG;
}
+ lock = mdev->anchor_list_lock + channel;
+ spin_lock_irqsave(lock, flags);
mdev->is_channel_healthy[channel] = false;
+ spin_unlock_irqrestore(lock, flags);
cancel_work_sync(&mdev->clear_work[channel].ws);
@@ -407,8 +412,10 @@ static void hdm_write_completion(struct urb *urb)
dev = &mdev->usb_device->dev;
lock = mdev->anchor_list_lock + channel;
+ spin_lock_irqsave(lock, flags);
if ((urb->status == -ENOENT) || (urb->status == -ECONNRESET) ||
(!mdev->is_channel_healthy[channel])) {
+ spin_unlock_irqrestore(lock, flags);
complete(&anchor->urb_compl);
return;
}
@@ -419,6 +426,7 @@ static void hdm_write_completion(struct urb *urb)
case -EPIPE:
dev_warn(dev, "Broken OUT pipe detected\n");
mdev->is_channel_healthy[channel] = false;
+ spin_unlock_irqrestore(lock, flags);
mbo->status = MBO_E_INVAL;
mdev->clear_work[channel].pipe = urb->pipe;
schedule_work(&mdev->clear_work[channel].ws);
@@ -436,7 +444,6 @@ static void hdm_write_completion(struct urb *urb)
mbo->processed_length = urb->actual_length;
}
- spin_lock_irqsave(lock, flags);
list_del(&anchor->list);
spin_unlock_irqrestore(lock, flags);
kfree(anchor);
@@ -571,8 +578,10 @@ static void hdm_read_completion(struct urb *urb)
dev = &mdev->usb_device->dev;
lock = mdev->anchor_list_lock + channel;
+ spin_lock_irqsave(lock, flags);
if ((urb->status == -ENOENT) || (urb->status == -ECONNRESET) ||
(!mdev->is_channel_healthy[channel])) {
+ spin_unlock_irqrestore(lock, flags);
complete(&anchor->urb_compl);
return;
}
@@ -583,6 +592,7 @@ static void hdm_read_completion(struct urb *urb)
case -EPIPE:
dev_warn(dev, "Broken IN pipe detected\n");
mdev->is_channel_healthy[channel] = false;
+ spin_unlock_irqrestore(lock, flags);
mbo->status = MBO_E_INVAL;
mdev->clear_work[channel].pipe = urb->pipe;
schedule_work(&mdev->clear_work[channel].ws);
@@ -606,7 +616,7 @@ static void hdm_read_completion(struct urb *urb)
mbo->status = MBO_E_INVAL;
}
}
- spin_lock_irqsave(lock, flags);
+
list_del(&anchor->list);
spin_unlock_irqrestore(lock, flags);
kfree(anchor);
--
1.9.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/8] staging: most: hdm-usb: remove completion object
2016-08-16 14:10 [PATCH 0/8] staging: most: fix hdm-usb issues Christian Gromm
` (2 preceding siblings ...)
2016-08-16 14:10 ` [PATCH 3/8] staging: most: hdm-usb: synchronize release of struct buf_anchor Christian Gromm
@ 2016-08-16 14:10 ` Christian Gromm
2016-08-16 14:10 ` [PATCH 5/8] staging: most: hdm-usb: remove redundant parenthesis Christian Gromm
` (4 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Christian Gromm @ 2016-08-16 14:10 UTC (permalink / raw)
To: gregkh; +Cc: Christian Gromm, driverdev-devel, Andrey Shvetsov
Waiting for the urb_compl object to complete evaluates always as false.
This patch removes this unnecessary completion object.
Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
drivers/staging/most/hdm-usb/hdm_usb.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c b/drivers/staging/most/hdm-usb/hdm_usb.c
index 26b5c1b..29e98dc 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -72,7 +72,6 @@
struct buf_anchor {
struct urb *urb;
struct list_head list;
- struct completion urb_compl;
};
/**
@@ -217,12 +216,7 @@ static void free_anchored_buffers(struct most_dev *mdev, unsigned int channel,
spin_unlock_irqrestore(lock, flags);
if (likely(urb)) {
mbo = urb->context;
- if (!irqs_disabled()) {
- usb_kill_urb(urb);
- } else {
- usb_unlink_urb(urb);
- wait_for_completion(&anchor->urb_compl);
- }
+ usb_kill_urb(urb);
if ((mbo) && (mbo->complete)) {
mbo->status = status;
mbo->processed_length = 0;
@@ -416,7 +410,6 @@ static void hdm_write_completion(struct urb *urb)
if ((urb->status == -ENOENT) || (urb->status == -ECONNRESET) ||
(!mdev->is_channel_healthy[channel])) {
spin_unlock_irqrestore(lock, flags);
- complete(&anchor->urb_compl);
return;
}
@@ -582,7 +575,6 @@ static void hdm_read_completion(struct urb *urb)
if ((urb->status == -ENOENT) || (urb->status == -ECONNRESET) ||
(!mdev->is_channel_healthy[channel])) {
spin_unlock_irqrestore(lock, flags);
- complete(&anchor->urb_compl);
return;
}
@@ -678,7 +670,6 @@ static int hdm_enqueue(struct most_interface *iface, int channel,
}
anchor->urb = urb;
- init_completion(&anchor->urb_compl);
mbo->priv = anchor;
if ((mdev->padding_active[channel]) &&
--
1.9.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/8] staging: most: hdm-usb: remove redundant parenthesis
2016-08-16 14:10 [PATCH 0/8] staging: most: fix hdm-usb issues Christian Gromm
` (3 preceding siblings ...)
2016-08-16 14:10 ` [PATCH 4/8] staging: most: hdm-usb: remove completion object Christian Gromm
@ 2016-08-16 14:10 ` Christian Gromm
2016-08-16 14:10 ` [PATCH 6/8] staging: most: hdm-usb: init variables at declaration time Christian Gromm
` (3 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Christian Gromm @ 2016-08-16 14:10 UTC (permalink / raw)
To: gregkh; +Cc: Christian Gromm, driverdev-devel, Andrey Shvetsov
This patch removes unnecessary parenthesis in boolean expressions.
Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
drivers/staging/most/hdm-usb/hdm_usb.c | 51 +++++++++++++++++-----------------
1 file changed, 25 insertions(+), 26 deletions(-)
diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c b/drivers/staging/most/hdm-usb/hdm_usb.c
index 29e98dc..ffe1519 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -217,7 +217,7 @@ static void free_anchored_buffers(struct most_dev *mdev, unsigned int channel,
if (likely(urb)) {
mbo = urb->context;
usb_kill_urb(urb);
- if ((mbo) && (mbo->complete)) {
+ if (mbo && mbo->complete) {
mbo->status = status;
mbo->processed_length = 0;
mbo->complete(mbo);
@@ -287,7 +287,7 @@ static int hdm_poison_channel(struct most_interface *iface, int channel)
dev_warn(&mdev->usb_device->dev, "Poison: Bad interface.\n");
return -EIO;
}
- if (unlikely((channel < 0) || (channel >= iface->num_channels))) {
+ if (unlikely(channel < 0 || channel >= iface->num_channels)) {
dev_warn(&mdev->usb_device->dev, "Channel ID out of range.\n");
return -ECHRNG;
}
@@ -407,8 +407,8 @@ static void hdm_write_completion(struct urb *urb)
lock = mdev->anchor_list_lock + channel;
spin_lock_irqsave(lock, flags);
- if ((urb->status == -ENOENT) || (urb->status == -ECONNRESET) ||
- (!mdev->is_channel_healthy[channel])) {
+ if (urb->status == -ENOENT || urb->status == -ECONNRESET ||
+ !mdev->is_channel_healthy[channel]) {
spin_unlock_irqrestore(lock, flags);
return;
}
@@ -572,8 +572,8 @@ static void hdm_read_completion(struct urb *urb)
lock = mdev->anchor_list_lock + channel;
spin_lock_irqsave(lock, flags);
- if ((urb->status == -ENOENT) || (urb->status == -ECONNRESET) ||
- (!mdev->is_channel_healthy[channel])) {
+ if (urb->status == -ENOENT || urb->status == -ECONNRESET ||
+ !mdev->is_channel_healthy[channel]) {
spin_unlock_irqrestore(lock, flags);
return;
}
@@ -649,7 +649,7 @@ static int hdm_enqueue(struct most_interface *iface, int channel,
if (unlikely(!iface || !mbo))
return -EIO;
- if (unlikely(iface->num_channels <= channel) || (channel < 0))
+ if (unlikely(iface->num_channels <= channel || channel < 0))
return -ECHRNG;
mdev = to_mdev(iface);
@@ -672,12 +672,11 @@ static int hdm_enqueue(struct most_interface *iface, int channel,
anchor->urb = urb;
mbo->priv = anchor;
- if ((mdev->padding_active[channel]) &&
- (conf->direction & MOST_CH_TX))
- if (hdm_add_padding(mdev, channel, mbo)) {
- retval = -EIO;
- goto _error;
- }
+ if ((conf->direction & MOST_CH_TX) && mdev->padding_active[channel] &&
+ hdm_add_padding(mdev, channel, mbo)) {
+ retval = -EIO;
+ goto _error;
+ }
urb->transfer_dma = mbo->bus_address;
virt_address = mbo->virt_address;
@@ -753,18 +752,18 @@ static int hdm_configure_channel(struct most_interface *iface, int channel,
dev_err(dev, "Bad interface or config pointer.\n");
return -EINVAL;
}
- if (unlikely((channel < 0) || (channel >= iface->num_channels))) {
+ if (unlikely(channel < 0 || channel >= iface->num_channels)) {
dev_err(dev, "Channel ID out of range.\n");
return -EINVAL;
}
- if ((!conf->num_buffers) || (!conf->buffer_size)) {
+ if (!conf->num_buffers || !conf->buffer_size) {
dev_err(dev, "Misconfig: buffer size or #buffers zero.\n");
return -EINVAL;
}
- if (!(conf->data_type == MOST_CH_SYNC) &&
- !((conf->data_type == MOST_CH_ISOC_AVP) &&
- (conf->packets_per_xact != 0xFF))) {
+ if (conf->data_type != MOST_CH_SYNC &&
+ !(conf->data_type == MOST_CH_ISOC_AVP &&
+ conf->packets_per_xact != 0xFF)) {
mdev->padding_active[channel] = false;
goto exit;
}
@@ -773,7 +772,7 @@ static int hdm_configure_channel(struct most_interface *iface, int channel,
temp_size = conf->buffer_size;
frame_size = get_stream_frame_size(conf);
- if ((frame_size == 0) || (frame_size > USB_MTU)) {
+ if (frame_size == 0 || frame_size > USB_MTU) {
dev_warn(dev, "Misconfig: frame size wrong\n");
return -EINVAL;
}
@@ -909,13 +908,13 @@ static void wq_netinfo(struct work_struct *wq_obj)
if (hdm_update_netinfo(mdev) < 0)
return;
- if ((prev_link_stat != mdev->link_stat) ||
- (prev_hw_addr[0] != mdev->hw_addr[0]) ||
- (prev_hw_addr[1] != mdev->hw_addr[1]) ||
- (prev_hw_addr[2] != mdev->hw_addr[2]) ||
- (prev_hw_addr[3] != mdev->hw_addr[3]) ||
- (prev_hw_addr[4] != mdev->hw_addr[4]) ||
- (prev_hw_addr[5] != mdev->hw_addr[5]))
+ if (prev_link_stat != mdev->link_stat ||
+ prev_hw_addr[0] != mdev->hw_addr[0] ||
+ prev_hw_addr[1] != mdev->hw_addr[1] ||
+ prev_hw_addr[2] != mdev->hw_addr[2] ||
+ prev_hw_addr[3] != mdev->hw_addr[3] ||
+ prev_hw_addr[4] != mdev->hw_addr[4] ||
+ prev_hw_addr[5] != mdev->hw_addr[5])
most_deliver_netinfo(&mdev->iface, mdev->link_stat,
&mdev->hw_addr[0]);
}
--
1.9.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 6/8] staging: most: hdm-usb: init variables at declaration time
2016-08-16 14:10 [PATCH 0/8] staging: most: fix hdm-usb issues Christian Gromm
` (4 preceding siblings ...)
2016-08-16 14:10 ` [PATCH 5/8] staging: most: hdm-usb: remove redundant parenthesis Christian Gromm
@ 2016-08-16 14:10 ` Christian Gromm
2016-08-16 14:10 ` [PATCH 7/8] staging: most: hdm-usb: remove unnecessary status assignment Christian Gromm
` (2 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Christian Gromm @ 2016-08-16 14:10 UTC (permalink / raw)
To: gregkh; +Cc: Christian Gromm, driverdev-devel, Andrey Shvetsov
This patch initializes variables by the time they are declared.
Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
drivers/staging/most/hdm-usb/hdm_usb.c | 72 +++++++++++-----------------------
1 file changed, 23 insertions(+), 49 deletions(-)
diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c b/drivers/staging/most/hdm-usb/hdm_usb.c
index ffe1519..453b641 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -278,11 +278,10 @@ static unsigned int get_stream_frame_size(struct most_channel_config *cfg)
*/
static int hdm_poison_channel(struct most_interface *iface, int channel)
{
- struct most_dev *mdev;
+ struct most_dev *mdev = to_mdev(iface);
unsigned long flags;
spinlock_t *lock; /* temp. lock */
- mdev = to_mdev(iface);
if (unlikely(!iface)) {
dev_warn(&mdev->usb_device->dev, "Poison: Bad interface.\n");
return -EIO;
@@ -391,20 +390,13 @@ static int hdm_remove_padding(struct most_dev *mdev, int channel,
*/
static void hdm_write_completion(struct urb *urb)
{
- struct mbo *mbo;
- struct buf_anchor *anchor;
- struct most_dev *mdev;
- struct device *dev;
- unsigned int channel;
+ 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 = &mdev->usb_device->dev;
+ spinlock_t *lock = mdev->anchor_list_lock + channel; /* temp. lock */
unsigned long flags;
- spinlock_t *lock; /* temp. lock */
-
- mbo = urb->context;
- anchor = mbo->priv;
- mdev = to_mdev(mbo->ifp);
- channel = mbo->hdm_channel_id;
- dev = &mdev->usb_device->dev;
- lock = mdev->anchor_list_lock + channel;
spin_lock_irqsave(lock, flags);
if (urb->status == -ENOENT || urb->status == -ECONNRESET ||
@@ -556,20 +548,13 @@ static void hdm_write_completion(struct urb *urb)
*/
static void hdm_read_completion(struct urb *urb)
{
- struct mbo *mbo;
- struct buf_anchor *anchor;
- struct most_dev *mdev;
- struct device *dev;
+ 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 = &mdev->usb_device->dev;
+ spinlock_t *lock = mdev->anchor_list_lock + channel; /* temp. lock */
unsigned long flags;
- unsigned int channel;
- spinlock_t *lock; /* temp. lock */
-
- mbo = urb->context;
- anchor = mbo->priv;
- mdev = to_mdev(mbo->ifp);
- channel = mbo->hdm_channel_id;
- dev = &mdev->usb_device->dev;
- lock = mdev->anchor_list_lock + channel;
spin_lock_irqsave(lock, flags);
if (urb->status == -ENOENT || urb->status == -ECONNRESET ||
@@ -738,15 +723,13 @@ static int hdm_configure_channel(struct most_interface *iface, int channel,
unsigned int frame_size;
unsigned int temp_size;
unsigned int tail_space;
- struct most_dev *mdev;
- struct device *dev;
+ struct most_dev *mdev = to_mdev(iface);
+ struct device *dev = &mdev->usb_device->dev;
- mdev = to_mdev(iface);
mdev->is_channel_healthy[channel] = true;
mdev->clear_work[channel].channel = channel;
mdev->clear_work[channel].mdev = mdev;
INIT_WORK(&mdev->clear_work[channel].ws, wq_clear_halt);
- dev = &mdev->usb_device->dev;
if (unlikely(!iface || !conf)) {
dev_err(dev, "Bad interface or config pointer.\n");
@@ -896,13 +879,10 @@ static void link_stat_timer_handler(unsigned long data)
*/
static void wq_netinfo(struct work_struct *wq_obj)
{
- struct most_dev *mdev;
- int i, prev_link_stat;
+ struct most_dev *mdev = to_mdev_from_work(wq_obj);
+ int i, prev_link_stat = mdev->link_stat;
u8 prev_hw_addr[6];
- mdev = to_mdev_from_work(wq_obj);
- prev_link_stat = mdev->link_stat;
-
for (i = 0; i < 6; i++)
prev_hw_addr[i] = mdev->hw_addr[i];
@@ -1174,10 +1154,9 @@ static struct kobj_type most_dci_ktype = {
static struct
most_dci_obj *create_most_dci_obj(struct kobject *parent)
{
- struct most_dci_obj *most_dci;
+ struct most_dci_obj *most_dci = kzalloc(sizeof(*most_dci), GFP_KERNEL);
int retval;
- most_dci = kzalloc(sizeof(*most_dci), GFP_KERNEL);
if (!most_dci)
return NULL;
@@ -1214,21 +1193,17 @@ static void destroy_most_dci_obj(struct most_dci_obj *p)
static int
hdm_probe(struct usb_interface *interface, const struct usb_device_id *id)
{
+ struct usb_host_interface *usb_iface_desc = interface->cur_altsetting;
+ struct usb_device *usb_dev = interface_to_usbdev(interface);
+ struct device *dev = &usb_dev->dev;
+ struct most_dev *mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
unsigned int i;
unsigned int num_endpoints;
struct most_channel_capability *tmp_cap;
- struct most_dev *mdev;
- struct usb_device *usb_dev;
- struct device *dev;
- struct usb_host_interface *usb_iface_desc;
struct usb_endpoint_descriptor *ep_desc;
int ret = 0;
int err;
- usb_iface_desc = interface->cur_altsetting;
- usb_dev = interface_to_usbdev(interface);
- dev = &usb_dev->dev;
- mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
if (!mdev)
goto exit_ENOMEM;
@@ -1378,9 +1353,8 @@ exit_ENOMEM:
*/
static void hdm_disconnect(struct usb_interface *interface)
{
- struct most_dev *mdev;
+ struct most_dev *mdev = usb_get_intfdata(interface);
- mdev = usb_get_intfdata(interface);
mutex_lock(&mdev->io_mutex);
usb_set_intfdata(interface, NULL);
mdev->usb_device = NULL;
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 7/8] staging: most: hdm-usb: remove unnecessary status assignment
2016-08-16 14:10 [PATCH 0/8] staging: most: fix hdm-usb issues Christian Gromm
` (5 preceding siblings ...)
2016-08-16 14:10 ` [PATCH 6/8] staging: most: hdm-usb: init variables at declaration time Christian Gromm
@ 2016-08-16 14:10 ` Christian Gromm
2016-08-16 14:10 ` [PATCH 8/8] staging: most: hdm-usb: add support for new USB gadget Christian Gromm
2016-08-18 15:18 ` [PATCH 0/8] staging: most: fix hdm-usb issues Greg KH
8 siblings, 0 replies; 12+ messages in thread
From: Christian Gromm @ 2016-08-16 14:10 UTC (permalink / raw)
To: gregkh; +Cc: Christian Gromm, driverdev-devel, Andrey Shvetsov
The USB completion callbacks set the status field of an MBO object before
scheduling the clear_work. This patch removes this redundant assignment as
the work_struct does the same for all MBOs.
Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
drivers/staging/most/hdm-usb/hdm_usb.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c b/drivers/staging/most/hdm-usb/hdm_usb.c
index 453b641..f44f27e 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -412,7 +412,6 @@ static void hdm_write_completion(struct urb *urb)
dev_warn(dev, "Broken OUT pipe detected\n");
mdev->is_channel_healthy[channel] = false;
spin_unlock_irqrestore(lock, flags);
- mbo->status = MBO_E_INVAL;
mdev->clear_work[channel].pipe = urb->pipe;
schedule_work(&mdev->clear_work[channel].ws);
return;
@@ -570,7 +569,6 @@ static void hdm_read_completion(struct urb *urb)
dev_warn(dev, "Broken IN pipe detected\n");
mdev->is_channel_healthy[channel] = false;
spin_unlock_irqrestore(lock, flags);
- mbo->status = MBO_E_INVAL;
mdev->clear_work[channel].pipe = urb->pipe;
schedule_work(&mdev->clear_work[channel].ws);
return;
--
1.9.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 8/8] staging: most: hdm-usb: add support for new USB gadget
2016-08-16 14:10 [PATCH 0/8] staging: most: fix hdm-usb issues Christian Gromm
` (6 preceding siblings ...)
2016-08-16 14:10 ` [PATCH 7/8] staging: most: hdm-usb: remove unnecessary status assignment Christian Gromm
@ 2016-08-16 14:10 ` Christian Gromm
2016-08-18 15:18 ` [PATCH 0/8] staging: most: fix hdm-usb issues Greg KH
8 siblings, 0 replies; 12+ messages in thread
From: Christian Gromm @ 2016-08-16 14:10 UTC (permalink / raw)
To: gregkh; +Cc: Christian Gromm, driverdev-devel
This patch is needed to make the driver support Microchip's OS81210 USB
MOST network interface controller. It simply adds the gadget's product
ID to the driver's ID table.
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
drivers/staging/most/hdm-usb/hdm_usb.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c b/drivers/staging/most/hdm-usb/hdm_usb.c
index f44f27e..08c4a3b 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -45,6 +45,7 @@
#define USB_DEV_ID_BRDG 0xC001 /* PID: USB Bridge */
#define USB_DEV_ID_OS81118 0xCF18 /* PID: USB OS81118 */
#define USB_DEV_ID_OS81119 0xCF19 /* PID: USB OS81119 */
+#define USB_DEV_ID_OS81210 0xCF30 /* PID: USB OS81210 */
/* DRCI Addresses */
#define DRCI_REG_NI_STATE 0x0100
#define DRCI_REG_PACKET_BW 0x0101
@@ -935,6 +936,7 @@ static struct usb_device_id usbid[] = {
{ USB_DEVICE(USB_VENDOR_ID_SMSC, USB_DEV_ID_BRDG), },
{ USB_DEVICE(USB_VENDOR_ID_SMSC, USB_DEV_ID_OS81118), },
{ USB_DEVICE(USB_VENDOR_ID_SMSC, USB_DEV_ID_OS81119), },
+ { USB_DEVICE(USB_VENDOR_ID_SMSC, USB_DEV_ID_OS81210), },
{ } /* Terminating entry */
};
@@ -1304,7 +1306,8 @@ hdm_probe(struct usb_interface *interface, const struct usb_device_id *id)
mutex_lock(&mdev->io_mutex);
if (le16_to_cpu(usb_dev->descriptor.idProduct) == USB_DEV_ID_OS81118 ||
- le16_to_cpu(usb_dev->descriptor.idProduct) == USB_DEV_ID_OS81119) {
+ le16_to_cpu(usb_dev->descriptor.idProduct) == USB_DEV_ID_OS81119 ||
+ le16_to_cpu(usb_dev->descriptor.idProduct) == USB_DEV_ID_OS81210) {
/* this increments the reference count of the instance
* object of the core
*/
--
1.9.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 0/8] staging: most: fix hdm-usb issues
2016-08-16 14:10 [PATCH 0/8] staging: most: fix hdm-usb issues Christian Gromm
` (7 preceding siblings ...)
2016-08-16 14:10 ` [PATCH 8/8] staging: most: hdm-usb: add support for new USB gadget Christian Gromm
@ 2016-08-18 15:18 ` Greg KH
2016-08-19 7:55 ` Christian Gromm
8 siblings, 1 reply; 12+ messages in thread
From: Greg KH @ 2016-08-18 15:18 UTC (permalink / raw)
To: Christian Gromm; +Cc: driverdev-devel
On Tue, Aug 16, 2016 at 04:10:26PM +0200, Christian Gromm wrote:
> This patch set is needed to fix issues of the hdm-usb module.
You sent me 2 patch series for this same driver right now. One 9
patches and one 8 patches.
Which go first? Why 2 different sets? What am I supposed to do, guess?
I'm dropping both of them, please resend _one_ set of patches, numbered,
so I know what to do. Please make it very obvious, otherwise I will get
it wrong...
Think about what you would want if someone were sending you patches...
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 0/8] staging: most: fix hdm-usb issues
2016-08-18 15:18 ` [PATCH 0/8] staging: most: fix hdm-usb issues Greg KH
@ 2016-08-19 7:55 ` Christian Gromm
2016-08-19 8:14 ` Greg KH
0 siblings, 1 reply; 12+ messages in thread
From: Christian Gromm @ 2016-08-19 7:55 UTC (permalink / raw)
To: Greg KH; +Cc: driverdev-devel
On Thu, 18 Aug 2016 17:18:56 +0200
Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Aug 16, 2016 at 04:10:26PM +0200, Christian Gromm wrote:
> > This patch set is needed to fix issues of the hdm-usb module.
>
> You sent me 2 patch series for this same driver right now. One 9
> patches and one 8 patches.
>
> Which go first? Why 2 different sets? What am I supposed to do, guess?
>
I thouhgt the different patch series are going to be applied in the
chronological order like they hit your inbox. Doesn't the date *when* a
certain patch set s'been received mean anything?
The two sets result from our internal development progress. Not every update
on the driver gets immediately pushed upstream. That's why I ended up with two
sets for a single module.
> I'm dropping both of them, please resend _one_ set of patches, numbered,
> so I know what to do. Please make it very obvious, otherwise I will get
> it wrong...
I'll go wrap them up again in one set.
thanks,
Chris
>
> Think about what you would want if someone were sending you patches...
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/8] staging: most: fix hdm-usb issues
2016-08-19 7:55 ` Christian Gromm
@ 2016-08-19 8:14 ` Greg KH
0 siblings, 0 replies; 12+ messages in thread
From: Greg KH @ 2016-08-19 8:14 UTC (permalink / raw)
To: Christian Gromm; +Cc: driverdev-devel
On Fri, Aug 19, 2016 at 09:55:33AM +0200, Christian Gromm wrote:
> On Thu, 18 Aug 2016 17:18:56 +0200
> Greg KH <gregkh@linuxfoundation.org> wrote:
>
> > On Tue, Aug 16, 2016 at 04:10:26PM +0200, Christian Gromm wrote:
> > > This patch set is needed to fix issues of the hdm-usb module.
> >
> > You sent me 2 patch series for this same driver right now. One 9
> > patches and one 8 patches.
> >
> > Which go first? Why 2 different sets? What am I supposed to do, guess?
> >
> I thouhgt the different patch series are going to be applied in the
> chronological order like they hit your inbox. Doesn't the date *when* a
> certain patch set s'been received mean anything?
Nope, never trust email timestamps, they are always wrong, that's why we
number patches. email can get stored on servers and delayed for random
amounts of time, it's part of the protocol :)
> The two sets result from our internal development progress. Not every update
> on the driver gets immediately pushed upstream. That's why I ended up with two
> sets for a single module.
That's fine, but please put them in a single set.
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 12+ messages in thread