All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] staging: most: fix driver modules
@ 2016-10-04 15:10 Christian Gromm
  2016-10-04 15:10 ` [PATCH 01/10] staging: most: core: remove member add_link Christian Gromm
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Christian Gromm @ 2016-10-04 15:10 UTC (permalink / raw)
  To: gregkh; +Cc: Christian Gromm, driverdev-devel

This patch set is needed to fix up issues of the driver.

Andrey Shvetsov (10):
  staging: most: core: remove member add_link
  staging: most: core: remove read option from remove_link
  staging: most: core: remove processing of deprecated names
  staging: most: core: update examples on how to link channels
  staging: most: aim-network: fix startup scenario.
  staging: most: aim-network: setup mac address before ifup has finished
  staging: most: aim-network: avoid calling netdev_info()
  staging: most: hdm-usb: remove filtering of
  staging: most: hdm-dim2: remove tracing of mac address
  staging: most: hdm-usb: fix mbo buffer leak

 drivers/staging/most/aim-network/networking.c |  47 ++++---
 drivers/staging/most/hdm-dim2/dim2_hdm.c      |   5 +-
 drivers/staging/most/hdm-usb/hdm_usb.c        | 189 ++++++++------------------
 drivers/staging/most/mostcore/core.c          |  39 ++----
 4 files changed, 99 insertions(+), 181 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 01/10] staging: most: core: remove member add_link
  2016-10-04 15:10 [PATCH 00/10] staging: most: fix driver modules Christian Gromm
@ 2016-10-04 15:10 ` Christian Gromm
  2016-10-04 15:10 ` [PATCH 02/10] staging: most: core: remove read option from remove_link Christian Gromm
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Gromm @ 2016-10-04 15:10 UTC (permalink / raw)
  To: gregkh; +Cc: Andrey Shvetsov, driverdev-devel, Christian Gromm

From: Andrey Shvetsov <andrey.shvetsov@k2l.de>

This patch removes the unused field add_link of struct most_aim_obj.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/mostcore/core.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index 329109c..9c15b63 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -763,7 +763,6 @@ struct most_aim_obj {
 	struct kobject kobj;
 	struct list_head list;
 	struct most_aim *driver;
-	char add_link[STRING_SIZE];
 	char remove_link[STRING_SIZE];
 };
 
@@ -984,7 +983,6 @@ static ssize_t store_add_link(struct most_aim_obj *aim_obj,
 	size_t max_len = min_t(size_t, len + 1, STRING_SIZE);
 
 	strlcpy(buffer, buf, max_len);
-	strlcpy(aim_obj->add_link, buf, max_len);
 
 	ret = split_string(buffer, &mdev, &mdev_ch, &mdev_devnod);
 	if (ret)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 02/10] staging: most: core: remove read option from remove_link
  2016-10-04 15:10 [PATCH 00/10] staging: most: fix driver modules Christian Gromm
  2016-10-04 15:10 ` [PATCH 01/10] staging: most: core: remove member add_link Christian Gromm
@ 2016-10-04 15:10 ` Christian Gromm
  2016-10-04 15:10 ` [PATCH 03/10] staging: most: core: remove processing of deprecated names Christian Gromm
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Gromm @ 2016-10-04 15:10 UTC (permalink / raw)
  To: gregkh; +Cc: Andrey Shvetsov, driverdev-devel, Christian Gromm

From: Andrey Shvetsov <andrey.shvetsov@k2l.de>

The attribute "remove_link" returns the latest link that has been removed
of a certain channel. Since this piece information isn't particulary
useful this patch is going to remove it.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/mostcore/core.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index 9c15b63..6507263 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -763,7 +763,6 @@ struct most_aim_obj {
 	struct kobject kobj;
 	struct list_head list;
 	struct most_aim *driver;
-	char remove_link[STRING_SIZE];
 };
 
 #define to_aim_obj(d) container_of(d, struct most_aim_obj, kobj)
@@ -1019,13 +1018,6 @@ static ssize_t store_add_link(struct most_aim_obj *aim_obj,
 static struct most_aim_attribute most_aim_attr_add_link =
 	__ATTR(add_link, S_IRUGO | S_IWUSR, show_add_link, store_add_link);
 
-static ssize_t show_remove_link(struct most_aim_obj *aim_obj,
-				struct most_aim_attribute *attr,
-				char *buf)
-{
-	return snprintf(buf, PAGE_SIZE, "%s\n", aim_obj->remove_link);
-}
-
 /**
  * store_remove_link - store function for remove_link attribute
  * @aim_obj: pointer to AIM object
@@ -1049,7 +1041,6 @@ static ssize_t store_remove_link(struct most_aim_obj *aim_obj,
 	size_t max_len = min_t(size_t, len + 1, STRING_SIZE);
 
 	strlcpy(buffer, buf, max_len);
-	strlcpy(aim_obj->remove_link, buf, max_len);
 	ret = split_string(buffer, &mdev, &mdev_ch, NULL);
 	if (ret)
 		return ret;
@@ -1068,8 +1059,7 @@ static ssize_t store_remove_link(struct most_aim_obj *aim_obj,
 }
 
 static struct most_aim_attribute most_aim_attr_remove_link =
-	__ATTR(remove_link, S_IRUGO | S_IWUSR, show_remove_link,
-	       store_remove_link);
+	__ATTR(remove_link, S_IWUSR, NULL, store_remove_link);
 
 static struct attribute *most_aim_def_attrs[] = {
 	&most_aim_attr_add_link.attr,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 03/10] staging: most: core: remove processing of deprecated names
  2016-10-04 15:10 [PATCH 00/10] staging: most: fix driver modules Christian Gromm
  2016-10-04 15:10 ` [PATCH 01/10] staging: most: core: remove member add_link Christian Gromm
  2016-10-04 15:10 ` [PATCH 02/10] staging: most: core: remove read option from remove_link Christian Gromm
@ 2016-10-04 15:10 ` Christian Gromm
  2016-10-04 15:10 ` [PATCH 04/10] staging: most: core: update examples on how to link channels Christian Gromm
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Gromm @ 2016-10-04 15:10 UTC (permalink / raw)
  To: gregkh; +Cc: Andrey Shvetsov, driverdev-devel, Christian Gromm

From: Andrey Shvetsov <andrey.shvetsov@k2l.de>

The USB HDM does not use the "@" character for channel names anymore. This
patch cleans up the code processing such names and adapts the corresponding
examples on how to use the properties "add_link" and "remove_link".

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/mostcore/core.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index 6507263..835a5871 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -883,16 +883,16 @@ static ssize_t show_add_link(struct most_aim_obj *aim_obj,
  *
  * Examples:
  *
- * Input: "mdev0:ch0@ep_81:my_channel\n" or
- *        "mdev0:ch0@ep_81:my_channel"
+ * Input: "mdev0:ch6:my_channel\n" or
+ *        "mdev0:ch6:my_channel"
  *
- * Output: *a -> "mdev0", *b -> "ch0@ep_81", *c -> "my_channel"
+ * Output: *a -> "mdev0", *b -> "ch6", *c -> "my_channel"
  *
- * Input: "mdev0:ch0@ep_81\n"
- * Output: *a -> "mdev0", *b -> "ch0@ep_81", *c -> ""
+ * Input: "mdev1:ep81\n"
+ * Output: *a -> "mdev1", *b -> "ep81", *c -> ""
  *
- * Input: "mdev0:ch0@ep_81"
- * Output: *a -> "mdev0", *b -> "ch0@ep_81", *c == NULL
+ * Input: "mdev1:ep81"
+ * Output: *a -> "mdev1", *b -> "ep81", *c == NULL
  */
 static int split_string(char *buf, char **a, char **b, char **c)
 {
@@ -960,11 +960,11 @@ most_c_obj *get_channel_by_name(char *mdev, char *mdev_ch)
  * Searches for a pair of device and channel and probes the AIM
  *
  * Example:
- * (1) echo -n -e "mdev0:ch0@ep_81:my_rxchannel\n" >add_link
- * (2) echo -n -e "mdev0:ch0@ep_81\n" >add_link
+ * (1) echo -n -e "mdev0:ch6:my_rxchannel\n" >add_link
+ * (2) echo -n -e "mdev1:ep81\n" >add_link
  *
  * (1) would create the device node /dev/my_rxchannel
- * (2) would create the device node /dev/mdev0-ch0@ep_81
+ * (2) would create the device node /dev/mdev1-ep81
  */
 static ssize_t store_add_link(struct most_aim_obj *aim_obj,
 			      struct most_aim_attribute *attr,
@@ -1026,7 +1026,7 @@ static struct most_aim_attribute most_aim_attr_add_link =
  * @len: buffer length
  *
  * Example:
- * echo -n -e "mdev0:ch0@ep_81\n" >remove_link
+ * echo -n -e "mdev0:ep81\n" >remove_link
  */
 static ssize_t store_remove_link(struct most_aim_obj *aim_obj,
 				 struct most_aim_attribute *attr,
@@ -1749,9 +1749,6 @@ struct kobject *most_register_interface(struct most_interface *iface)
 
 		if (!name_suffix)
 			snprintf(channel_name, STRING_SIZE, "ch%d", i);
-		else if (name_suffix[0] == '@')
-			snprintf(channel_name, STRING_SIZE, "ch%d%s", i,
-				 name_suffix);
 		else
 			snprintf(channel_name, STRING_SIZE, "%s", name_suffix);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 04/10] staging: most: core: update examples on how to link channels
  2016-10-04 15:10 [PATCH 00/10] staging: most: fix driver modules Christian Gromm
                   ` (2 preceding siblings ...)
  2016-10-04 15:10 ` [PATCH 03/10] staging: most: core: remove processing of deprecated names Christian Gromm
@ 2016-10-04 15:10 ` Christian Gromm
  2016-10-04 15:10 ` [PATCH 05/10] staging: most: aim-network: fix startup scenario Christian Gromm
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Gromm @ 2016-10-04 15:10 UTC (permalink / raw)
  To: gregkh; +Cc: Andrey Shvetsov, driverdev-devel, Christian Gromm

From: Andrey Shvetsov <andrey.shvetsov@k2l.de>

This patch updates the comments with examples on how to use the "add_link"
and "remove_link" properties.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/mostcore/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index 835a5871..4c580d1 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -960,8 +960,8 @@ most_c_obj *get_channel_by_name(char *mdev, char *mdev_ch)
  * Searches for a pair of device and channel and probes the AIM
  *
  * Example:
- * (1) echo -n -e "mdev0:ch6:my_rxchannel\n" >add_link
- * (2) echo -n -e "mdev1:ep81\n" >add_link
+ * (1) echo "mdev0:ch6:my_rxchannel" >add_link
+ * (2) echo "mdev1:ep81" >add_link
  *
  * (1) would create the device node /dev/my_rxchannel
  * (2) would create the device node /dev/mdev1-ep81
@@ -1026,7 +1026,7 @@ static struct most_aim_attribute most_aim_attr_add_link =
  * @len: buffer length
  *
  * Example:
- * echo -n -e "mdev0:ep81\n" >remove_link
+ * echo "mdev0:ep81" >remove_link
  */
 static ssize_t store_remove_link(struct most_aim_obj *aim_obj,
 				 struct most_aim_attribute *attr,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 05/10] staging: most: aim-network: fix startup scenario.
  2016-10-04 15:10 [PATCH 00/10] staging: most: fix driver modules Christian Gromm
                   ` (3 preceding siblings ...)
  2016-10-04 15:10 ` [PATCH 04/10] staging: most: core: update examples on how to link channels Christian Gromm
@ 2016-10-04 15:10 ` Christian Gromm
  2016-10-04 15:10 ` [PATCH 06/10] staging: most: aim-network: setup mac address before ifup has finished Christian Gromm
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Gromm @ 2016-10-04 15:10 UTC (permalink / raw)
  To: gregkh; +Cc: Andrey Shvetsov, driverdev-devel, Christian Gromm

From: Andrey Shvetsov <andrey.shvetsov@k2l.de>

In case the networking interface (running on MediaLB) is being started
before the MOST network is, it remains disabled even after the MOST network
has transitioned to active mode.

This patch removes the dependency on the MOST link status to keep the
networking queue active and the networking interface working for the
case described above.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-network/networking.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/most/aim-network/networking.c b/drivers/staging/most/aim-network/networking.c
index 4659a64..5728270 100644
--- a/drivers/staging/most/aim-network/networking.c
+++ b/drivers/staging/most/aim-network/networking.c
@@ -67,7 +67,6 @@ struct net_dev_context {
 	struct most_interface *iface;
 	bool channels_opened;
 	bool is_mamac;
-	unsigned char link_stat;
 	struct net_device *dev;
 	struct net_dev_channel rx;
 	struct net_dev_channel tx;
@@ -203,13 +202,10 @@ static int most_nd_open(struct net_device *dev)
 	}
 
 	nd->channels_opened = true;
+	netif_wake_queue(dev);
 
-	if (nd->is_mamac) {
-		nd->link_stat = 1;
-		netif_wake_queue(dev);
-	} else {
+	if (!nd->is_mamac)
 		nd->iface->request_netinfo(nd->iface, nd->tx.ch_id);
-	}
 
 	return 0;
 }
@@ -561,14 +557,6 @@ void most_deliver_netinfo(struct most_interface *iface,
 
 	if (mac_addr)
 		ether_addr_copy(dev->dev_addr, mac_addr);
-
-	if (nd->link_stat != link_stat) {
-		nd->link_stat = link_stat;
-		if (nd->link_stat)
-			netif_wake_queue(dev);
-		else
-			netif_stop_queue(dev);
-	}
 }
 EXPORT_SYMBOL(most_deliver_netinfo);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 06/10] staging: most: aim-network: setup mac address before ifup has finished
  2016-10-04 15:10 [PATCH 00/10] staging: most: fix driver modules Christian Gromm
                   ` (4 preceding siblings ...)
  2016-10-04 15:10 ` [PATCH 05/10] staging: most: aim-network: fix startup scenario Christian Gromm
@ 2016-10-04 15:10 ` Christian Gromm
  2016-10-04 15:10 ` [PATCH 07/10] staging: most: aim-network: avoid calling netdev_info() Christian Gromm
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Gromm @ 2016-10-04 15:10 UTC (permalink / raw)
  To: gregkh; +Cc: Andrey Shvetsov, driverdev-devel, Christian Gromm

From: Andrey Shvetsov <andrey.shvetsov@k2l.de>

The networking AIM implements a non-standard behavior as it starts the
networking interface with an invalid MAC address and changes it by the time
a valid one is available.

This patch introduces a completion object to set the MAC address of the
networking interface before the .ndo_open callback (ifup) of the net_device
returns.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-network/networking.c | 36 ++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/most/aim-network/networking.c b/drivers/staging/most/aim-network/networking.c
index 5728270..9e1b19b 100644
--- a/drivers/staging/most/aim-network/networking.c
+++ b/drivers/staging/most/aim-network/networking.c
@@ -70,6 +70,7 @@ struct net_dev_context {
 	struct net_device *dev;
 	struct net_dev_channel rx;
 	struct net_dev_channel tx;
+	struct completion mac_compl;
 	struct list_head list;
 };
 
@@ -180,6 +181,7 @@ static int most_nd_set_mac_address(struct net_device *dev, void *p)
 static int most_nd_open(struct net_device *dev)
 {
 	struct net_dev_context *nd = dev->ml_priv;
+	long wait_res;
 
 	netdev_info(dev, "open net device\n");
 
@@ -204,8 +206,21 @@ static int most_nd_open(struct net_device *dev)
 	nd->channels_opened = true;
 	netif_wake_queue(dev);
 
-	if (!nd->is_mamac)
-		nd->iface->request_netinfo(nd->iface, nd->tx.ch_id);
+	if (is_valid_ether_addr(dev->dev_addr))
+		return 0;
+
+	nd->iface->request_netinfo(nd->iface, nd->tx.ch_id);
+	wait_res = wait_for_completion_interruptible_timeout(
+				&nd->mac_compl, msecs_to_jiffies(5000));
+	if (!wait_res) {
+		netdev_err(dev, "mac timeout\n");
+		return -EBUSY;
+	}
+
+	if (wait_res < 0) {
+		netdev_warn(dev, "mac waiting interrupted\n");
+		return wait_res;
+	}
 
 	return 0;
 }
@@ -328,6 +343,7 @@ static int aim_probe_channel(struct most_interface *iface, int channel_idx,
 		if (!nd)
 			return -ENOMEM;
 
+		init_completion(&nd->mac_compl);
 		nd->iface = iface;
 
 		spin_lock_irqsave(&list_lock, flags);
@@ -544,8 +560,7 @@ void most_deliver_netinfo(struct most_interface *iface,
 {
 	struct net_dev_context *nd;
 	struct net_device *dev;
-
-	pr_info("Received netinfo from %s\n", iface->description);
+	const u8 *m = mac_addr;
 
 	nd = get_net_dev_context(iface);
 	if (!nd)
@@ -555,8 +570,17 @@ void most_deliver_netinfo(struct most_interface *iface,
 	if (!dev)
 		return;
 
-	if (mac_addr)
-		ether_addr_copy(dev->dev_addr, mac_addr);
+	if (m && is_valid_ether_addr(m)) {
+		if (!is_valid_ether_addr(dev->dev_addr)) {
+			netdev_info(dev, "set mac %02x-%02x-%02x-%02x-%02x-%02x\n",
+				    m[0], m[1], m[2], m[3], m[4], m[5]);
+			ether_addr_copy(dev->dev_addr, m);
+			complete(&nd->mac_compl);
+		} else if (!ether_addr_equal(dev->dev_addr, m)) {
+			netdev_warn(dev, "reject mac %02x-%02x-%02x-%02x-%02x-%02x\n",
+				    m[0], m[1], m[2], m[3], m[4], m[5]);
+		}
+	}
 }
 EXPORT_SYMBOL(most_deliver_netinfo);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 07/10] staging: most: aim-network: avoid calling netdev_info()
  2016-10-04 15:10 [PATCH 00/10] staging: most: fix driver modules Christian Gromm
                   ` (5 preceding siblings ...)
  2016-10-04 15:10 ` [PATCH 06/10] staging: most: aim-network: setup mac address before ifup has finished Christian Gromm
@ 2016-10-04 15:10 ` Christian Gromm
  2016-10-04 15:10 ` [PATCH 08/10] staging: most: hdm-usb: remove filtering of networking state Christian Gromm
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Gromm @ 2016-10-04 15:10 UTC (permalink / raw)
  To: gregkh; +Cc: Andrey Shvetsov, driverdev-devel, Christian Gromm

From: Andrey Shvetsov <andrey.shvetsov@k2l.de>

This patch removes the needless call of function netdev_info() from
function most_nd_setup().

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-network/networking.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/most/aim-network/networking.c b/drivers/staging/most/aim-network/networking.c
index 9e1b19b..fe0d516 100644
--- a/drivers/staging/most/aim-network/networking.c
+++ b/drivers/staging/most/aim-network/networking.c
@@ -211,7 +211,7 @@ static int most_nd_open(struct net_device *dev)
 
 	nd->iface->request_netinfo(nd->iface, nd->tx.ch_id);
 	wait_res = wait_for_completion_interruptible_timeout(
-				&nd->mac_compl, msecs_to_jiffies(5000));
+			   &nd->mac_compl, msecs_to_jiffies(5000));
 	if (!wait_res) {
 		netdev_err(dev, "mac timeout\n");
 		return -EBUSY;
@@ -288,7 +288,6 @@ static const struct net_device_ops most_nd_ops = {
 
 static void most_nd_setup(struct net_device *dev)
 {
-	netdev_info(dev, "setup net device\n");
 	ether_setup(dev);
 	dev->netdev_ops = &most_nd_ops;
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 08/10] staging: most: hdm-usb: remove filtering of networking state
  2016-10-04 15:10 [PATCH 00/10] staging: most: fix driver modules Christian Gromm
                   ` (6 preceding siblings ...)
  2016-10-04 15:10 ` [PATCH 07/10] staging: most: aim-network: avoid calling netdev_info() Christian Gromm
@ 2016-10-04 15:10 ` Christian Gromm
  2016-10-04 15:10 ` [PATCH 09/10] staging: most: hdm-dim2: remove tracing of mac address Christian Gromm
  2016-10-04 15:10 ` [PATCH 10/10] staging: most: hdm-usb: fix mbo buffer leak Christian Gromm
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Gromm @ 2016-10-04 15:10 UTC (permalink / raw)
  To: gregkh; +Cc: Andrey Shvetsov, driverdev-devel, Christian Gromm

From: Andrey Shvetsov <andrey.shvetsov@k2l.de>

In case the networking interface goes down and up again, the USB HDM does
not report the state the MOST networking interface controller is in. This
might lead to nonfunctional network.

Since the networking AIM already takes care of hardware address checking
and tracing it can be removed from the HDM USB, which is what this patch is
doing.

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 | 104 ++++++++++-----------------------
 1 file changed, 32 insertions(+), 72 deletions(-)

diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c b/drivers/staging/most/hdm-usb/hdm_usb.c
index 26c9adb..70e58c4 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -97,9 +97,7 @@ struct clear_hold_work {
  * @cap: channel capabilities
  * @conf: channel configuration
  * @dci: direct communication interface of hardware
- * @hw_addr: MAC address of hardware
  * @ep_address: endpoint address table
- * @link_stat: link status of hardware
  * @description: device description
  * @suffix: suffix for channel name
  * @channel_lock: synchronize channel access
@@ -117,9 +115,7 @@ struct most_dev {
 	struct most_channel_capability *cap;
 	struct most_channel_config *conf;
 	struct most_dci_obj *dci;
-	u8 hw_addr[6];
 	u8 *ep_address;
-	u16 link_stat;
 	char description[MAX_STRING_LEN];
 	char suffix[MAX_NUM_ENDPOINTS][MAX_SUFFIX_LEN];
 	spinlock_t channel_lock[MAX_NUM_ENDPOINTS]; /* sync channel access */
@@ -738,56 +734,6 @@ exit:
 }
 
 /**
- * hdm_update_netinfo - retrieve latest networking information
- * @mdev: device interface
- *
- * This triggers the USB vendor requests to read the hardware address and
- * the current link status of the attached device.
- */
-static int hdm_update_netinfo(struct most_dev *mdev)
-{
-	struct usb_device *usb_device = mdev->usb_device;
-	struct device *dev = &usb_device->dev;
-	u16 hi, mi, lo, link;
-
-	if (!is_valid_ether_addr(mdev->hw_addr)) {
-		if (drci_rd_reg(usb_device, DRCI_REG_HW_ADDR_HI, &hi) < 0) {
-			dev_err(dev, "Vendor request \"hw_addr_hi\" failed\n");
-			return -EFAULT;
-		}
-
-		if (drci_rd_reg(usb_device, DRCI_REG_HW_ADDR_MI, &mi) < 0) {
-			dev_err(dev, "Vendor request \"hw_addr_mid\" failed\n");
-			return -EFAULT;
-		}
-
-		if (drci_rd_reg(usb_device, DRCI_REG_HW_ADDR_LO, &lo) < 0) {
-			dev_err(dev, "Vendor request \"hw_addr_low\" failed\n");
-			return -EFAULT;
-		}
-
-		mutex_lock(&mdev->io_mutex);
-		mdev->hw_addr[0] = hi >> 8;
-		mdev->hw_addr[1] = hi;
-		mdev->hw_addr[2] = mi >> 8;
-		mdev->hw_addr[3] = mi;
-		mdev->hw_addr[4] = lo >> 8;
-		mdev->hw_addr[5] = lo;
-		mutex_unlock(&mdev->io_mutex);
-	}
-
-	if (drci_rd_reg(usb_device, DRCI_REG_NI_STATE, &link) < 0) {
-		dev_err(dev, "Vendor request \"link status\" failed\n");
-		return -EFAULT;
-	}
-
-	mutex_lock(&mdev->io_mutex);
-	mdev->link_stat = link;
-	mutex_unlock(&mdev->io_mutex);
-	return 0;
-}
-
-/**
  * hdm_request_netinfo - request network information
  * @iface: pointer to interface
  * @channel: channel ID
@@ -807,7 +753,7 @@ static void hdm_request_netinfo(struct most_interface *iface, int channel)
 }
 
 /**
- * link_stat_timer_handler - add work to link_stat work queue
+ * link_stat_timer_handler - schedule work obtaining mac address and link status
  * @data: pointer to USB device instance
  *
  * The handler runs in interrupt context. That's why we need to defer the
@@ -823,33 +769,47 @@ static void link_stat_timer_handler(unsigned long data)
 }
 
 /**
- * wq_netinfo - work queue function
+ * wq_netinfo - work queue function to deliver latest networking information
  * @wq_obj: object that holds data for our deferred work to do
  *
  * This retrieves the network interface status of the USB INIC
- * and compares it with the current status. If the status has
- * changed, it updates the status of the core.
  */
 static void wq_netinfo(struct work_struct *wq_obj)
 {
 	struct most_dev *mdev = to_mdev_from_work(wq_obj);
-	int i, prev_link_stat = mdev->link_stat;
-	u8 prev_hw_addr[6];
+	struct usb_device *usb_device = mdev->usb_device;
+	struct device *dev = &usb_device->dev;
+	u16 hi, mi, lo, link;
+	u8 hw_addr[6];
 
-	for (i = 0; i < 6; i++)
-		prev_hw_addr[i] = mdev->hw_addr[i];
+	if (drci_rd_reg(usb_device, DRCI_REG_HW_ADDR_HI, &hi) < 0) {
+		dev_err(dev, "Vendor request 'hw_addr_hi' failed\n");
+		return;
+	}
 
-	if (hdm_update_netinfo(mdev) < 0)
+	if (drci_rd_reg(usb_device, DRCI_REG_HW_ADDR_MI, &mi) < 0) {
+		dev_err(dev, "Vendor request 'hw_addr_mid' failed\n");
 		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])
-		most_deliver_netinfo(&mdev->iface, mdev->link_stat,
-				     &mdev->hw_addr[0]);
+	}
+
+	if (drci_rd_reg(usb_device, DRCI_REG_HW_ADDR_LO, &lo) < 0) {
+		dev_err(dev, "Vendor request 'hw_addr_low' failed\n");
+		return;
+	}
+
+	if (drci_rd_reg(usb_device, DRCI_REG_NI_STATE, &link) < 0) {
+		dev_err(dev, "Vendor request 'link status' failed\n");
+		return;
+	}
+
+	hw_addr[0] = hi >> 8;
+	hw_addr[1] = hi;
+	hw_addr[2] = mi >> 8;
+	hw_addr[3] = mi;
+	hw_addr[4] = lo >> 8;
+	hw_addr[5] = lo;
+
+	most_deliver_netinfo(&mdev->iface, link, hw_addr);
 }
 
 /**
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 09/10] staging: most: hdm-dim2: remove tracing of mac address
  2016-10-04 15:10 [PATCH 00/10] staging: most: fix driver modules Christian Gromm
                   ` (7 preceding siblings ...)
  2016-10-04 15:10 ` [PATCH 08/10] staging: most: hdm-usb: remove filtering of networking state Christian Gromm
@ 2016-10-04 15:10 ` Christian Gromm
  2016-10-04 15:10 ` [PATCH 10/10] staging: most: hdm-usb: fix mbo buffer leak Christian Gromm
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Gromm @ 2016-10-04 15:10 UTC (permalink / raw)
  To: gregkh; +Cc: Andrey Shvetsov, driverdev-devel, Christian Gromm

From: Andrey Shvetsov <andrey.shvetsov@k2l.de>

This patch removes tracing of the MAC address from the DIM2 HDM as it is
already done in the networking AIM.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/hdm-dim2/dim2_hdm.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/most/hdm-dim2/dim2_hdm.c b/drivers/staging/most/hdm-dim2/dim2_hdm.c
index 78b2c3d..35aee9f 100644
--- a/drivers/staging/most/hdm-dim2/dim2_hdm.c
+++ b/drivers/staging/most/hdm-dim2/dim2_hdm.c
@@ -306,14 +306,11 @@ static int deliver_netinfo_thread(void *data)
 static void retrieve_netinfo(struct dim2_hdm *dev, struct mbo *mbo)
 {
 	u8 *data = mbo->virt_address;
-	u8 *mac = dev->mac_addrs;
 
 	pr_info("Node Address: 0x%03x\n", (u16)data[16] << 8 | data[17]);
 	dev->link_state = data[18];
 	pr_info("NIState: %d\n", dev->link_state);
-	memcpy(mac, data + 19, 6);
-	pr_info("MAC address: %02X:%02X:%02X:%02X:%02X:%02X\n",
-		mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]);
+	memcpy(dev->mac_addrs, data + 19, 6);
 	dev->deliver_netinfo++;
 	wake_up_interruptible(&dev->netinfo_waitq);
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 10/10] staging: most: hdm-usb: fix mbo buffer leak
  2016-10-04 15:10 [PATCH 00/10] staging: most: fix driver modules Christian Gromm
                   ` (8 preceding siblings ...)
  2016-10-04 15:10 ` [PATCH 09/10] staging: most: hdm-dim2: remove tracing of mac address Christian Gromm
@ 2016-10-04 15:10 ` Christian Gromm
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Gromm @ 2016-10-04 15:10 UTC (permalink / raw)
  To: gregkh; +Cc: Andrey Shvetsov, driverdev-devel, Christian Gromm

From: Andrey Shvetsov <andrey.shvetsov@k2l.de>

This patch fixes an MBO leak by replacing the proprietary
free_anchored_buffers() function with the usb_kill_anchored_urbs() function
of the USB subsystem and guarantees that the mbo->complete() completion
function is being called for each URB.

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 | 85 ++++++++++------------------------
 1 file changed, 25 insertions(+), 60 deletions(-)

diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c b/drivers/staging/most/hdm-usb/hdm_usb.c
index 70e58c4..1a630e1 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -183,30 +183,6 @@ static inline int drci_wr_reg(struct usb_device *dev, u16 reg, u16 data)
 }
 
 /**
- * free_anchored_buffers - free device's anchored items
- * @mdev: the device
- * @channel: channel ID
- * @status: status of MBO termination
- */
-static void free_anchored_buffers(struct most_dev *mdev, unsigned int channel,
-				  enum mbo_status_flags status)
-{
-	struct mbo *mbo;
-	struct urb *urb;
-
-	while ((urb = usb_get_from_anchor(&mdev->busy_urbs[channel]))) {
-		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);
-	}
-}
-
-/**
  * get_stream_frame_size - calculate frame size of current configuration
  * @cfg: channel configuration
  */
@@ -274,7 +250,7 @@ static int hdm_poison_channel(struct most_interface *iface, int channel)
 	cancel_work_sync(&mdev->clear_work[channel].ws);
 
 	mutex_lock(&mdev->io_mutex);
-	free_anchored_buffers(mdev, channel, MBO_E_CLOSE);
+	usb_kill_anchored_urbs(&mdev->busy_urbs[channel]);
 	if (mdev->padding_active[channel])
 		mdev->padding_active[channel] = false;
 
@@ -373,33 +349,27 @@ static void hdm_write_completion(struct urb *urb)
 	unsigned long flags;
 
 	spin_lock_irqsave(lock, flags);
-	if (urb->status == -ENOENT || urb->status == -ECONNRESET ||
-	    !mdev->is_channel_healthy[channel]) {
-		spin_unlock_irqrestore(lock, flags);
-		return;
-	}
 
-	if (unlikely(urb->status && urb->status != -ESHUTDOWN)) {
-		mbo->processed_length = 0;
+	mbo->processed_length = 0;
+	mbo->status = MBO_E_INVAL;
+	if (likely(mdev->is_channel_healthy[channel])) {
 		switch (urb->status) {
+		case 0:
+		case -ESHUTDOWN:
+			mbo->processed_length = urb->actual_length;
+			mbo->status = MBO_SUCCESS;
+			break;
 		case -EPIPE:
 			dev_warn(dev, "Broken OUT pipe detected\n");
 			mdev->is_channel_healthy[channel] = false;
-			spin_unlock_irqrestore(lock, flags);
 			mdev->clear_work[channel].pipe = urb->pipe;
 			schedule_work(&mdev->clear_work[channel].ws);
-			return;
+			break;
 		case -ENODEV:
 		case -EPROTO:
 			mbo->status = MBO_E_CLOSE;
 			break;
-		default:
-			mbo->status = MBO_E_INVAL;
-			break;
 		}
-	} else {
-		mbo->status = MBO_SUCCESS;
-		mbo->processed_length = urb->actual_length;
 	}
 
 	spin_unlock_irqrestore(lock, flags);
@@ -527,40 +497,35 @@ static void hdm_read_completion(struct urb *urb)
 	unsigned long flags;
 
 	spin_lock_irqsave(lock, flags);
-	if (urb->status == -ENOENT || urb->status == -ECONNRESET ||
-	    !mdev->is_channel_healthy[channel]) {
-		spin_unlock_irqrestore(lock, flags);
-		return;
-	}
 
-	if (unlikely(urb->status && urb->status != -ESHUTDOWN)) {
-		mbo->processed_length = 0;
+	mbo->processed_length = 0;
+	mbo->status = MBO_E_INVAL;
+	if (likely(mdev->is_channel_healthy[channel])) {
 		switch (urb->status) {
+		case 0:
+		case -ESHUTDOWN:
+			mbo->processed_length = urb->actual_length;
+			mbo->status = MBO_SUCCESS;
+			if (mdev->padding_active[channel] &&
+			    hdm_remove_padding(mdev, channel, mbo)) {
+				mbo->processed_length = 0;
+				mbo->status = MBO_E_INVAL;
+			}
+			break;
 		case -EPIPE:
 			dev_warn(dev, "Broken IN pipe detected\n");
 			mdev->is_channel_healthy[channel] = false;
-			spin_unlock_irqrestore(lock, flags);
 			mdev->clear_work[channel].pipe = urb->pipe;
 			schedule_work(&mdev->clear_work[channel].ws);
-			return;
+			break;
 		case -ENODEV:
 		case -EPROTO:
 			mbo->status = MBO_E_CLOSE;
 			break;
 		case -EOVERFLOW:
 			dev_warn(dev, "Babble on IN pipe detected\n");
-		default:
-			mbo->status = MBO_E_INVAL;
 			break;
 		}
-	} else {
-		mbo->processed_length = urb->actual_length;
-		mbo->status = MBO_SUCCESS;
-		if (mdev->padding_active[channel] &&
-		    hdm_remove_padding(mdev, channel, mbo)) {
-			mbo->processed_length = 0;
-			mbo->status = MBO_E_INVAL;
-		}
 	}
 
 	spin_unlock_irqrestore(lock, flags);
@@ -827,7 +792,7 @@ static void wq_clear_halt(struct work_struct *wq_obj)
 
 	mutex_lock(&mdev->io_mutex);
 	most_stop_enqueue(&mdev->iface, channel);
-	free_anchored_buffers(mdev, channel, MBO_E_INVAL);
+	usb_kill_anchored_urbs(&mdev->busy_urbs[channel]);
 	if (usb_clear_halt(mdev->usb_device, pipe))
 		dev_warn(&mdev->usb_device->dev, "Failed to reset endpoint.\n");
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-10-04 15:10 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-04 15:10 [PATCH 00/10] staging: most: fix driver modules Christian Gromm
2016-10-04 15:10 ` [PATCH 01/10] staging: most: core: remove member add_link Christian Gromm
2016-10-04 15:10 ` [PATCH 02/10] staging: most: core: remove read option from remove_link Christian Gromm
2016-10-04 15:10 ` [PATCH 03/10] staging: most: core: remove processing of deprecated names Christian Gromm
2016-10-04 15:10 ` [PATCH 04/10] staging: most: core: update examples on how to link channels Christian Gromm
2016-10-04 15:10 ` [PATCH 05/10] staging: most: aim-network: fix startup scenario Christian Gromm
2016-10-04 15:10 ` [PATCH 06/10] staging: most: aim-network: setup mac address before ifup has finished Christian Gromm
2016-10-04 15:10 ` [PATCH 07/10] staging: most: aim-network: avoid calling netdev_info() Christian Gromm
2016-10-04 15:10 ` [PATCH 08/10] staging: most: hdm-usb: remove filtering of networking state Christian Gromm
2016-10-04 15:10 ` [PATCH 09/10] staging: most: hdm-dim2: remove tracing of mac address Christian Gromm
2016-10-04 15:10 ` [PATCH 10/10] staging: most: hdm-usb: fix mbo buffer leak Christian Gromm

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.