All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/8] staging: most: move USB adapter driver to stable branch
@ 2020-05-14  9:52 Christian Gromm
  2020-05-14  9:52 ` [PATCH v2 1/8] drivers: most: add usb adapter driver Christian Gromm
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Christian Gromm @ 2020-05-14  9:52 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, linux-usb, Christian Gromm

This series moves the USB adapter driver to the stable branch and fixes
the comments/findings on the submitted code.

v2:
	- create a patch set to address the comments on submitted code

Christian Gromm (8):
  drivers: most: add usb adapter driver
  drivers: most: usb: use dev_*() functions to print messages
  drivers: most: usb: remove reference to USB error codes
  drivers: most: usb: check number of reported endpoints
  drivers: most: usb: use dev_dbg function
  drivers: most: fix typo in Kconfig
  drivers: most: usb: use macro ATTRIBUTE_GROUPS
  Documentation: ABI: correct sysfs attribute description of MOST driver

 Documentation/ABI/testing/sysfs-bus-most |  104 +--
 drivers/most/Kconfig                     |   12 +
 drivers/most/Makefile                    |    2 +
 drivers/most/most_usb.c                  | 1252 ++++++++++++++++++++++++++++++
 4 files changed, 1319 insertions(+), 51 deletions(-)
 create mode 100644 drivers/most/most_usb.c

-- 
2.7.4


^ permalink raw reply	[flat|nested] 16+ messages in thread
* [PATCH v2 1/8] drivers: most: add usb adapter driver
  2020-05-14  9:52 [PATCH v2 0/8] staging: most: move USB adapter driver to stable branch Christian Gromm
@ 2020-05-14  9:52 ` Christian Gromm
  2020-05-14 10:25   ` Greg KH
  2020-05-20 13:17   ` Dan Carpenter
  2020-05-14  9:52 ` [PATCH v2 2/8] drivers: most: usb: use dev_*() functions to print messages Christian Gromm
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 16+ messages in thread
From: Christian Gromm @ 2020-05-14  9:52 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, linux-usb, Christian Gromm

This patch adds the usb driver source file most_usb.c and
modifies the Makefile and Kconfig accordingly.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v2:
Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
	- don't remove usb driver from staging area
	- don't touch staging/most/Kconfig
	- remove subdirectory for USB driver and put source file into
	  drivers/most

 drivers/most/Kconfig    |   12 +
 drivers/most/Makefile   |    2 +
 drivers/most/most_usb.c | 1262 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1276 insertions(+)
 create mode 100644 drivers/most/most_usb.c

diff --git a/drivers/most/Kconfig b/drivers/most/Kconfig
index 58d7999..8650683 100644
--- a/drivers/most/Kconfig
+++ b/drivers/most/Kconfig
@@ -13,3 +13,15 @@ menuconfig MOST
 	  module will be called most_core.
 
 	  If in doubt, say N here.
+
+if MOST
+config MOST_USB
+	tristate "USB"
+	depends on USB && NET
+	help
+	  Say Y here if you want to connect via USB to network tranceiver.
+	  This device driver depends on the networking AIM.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called most_usb.
+endif
diff --git a/drivers/most/Makefile b/drivers/most/Makefile
index e810cd3..97ffc06 100644
--- a/drivers/most/Makefile
+++ b/drivers/most/Makefile
@@ -2,3 +2,5 @@
 obj-$(CONFIG_MOST) += most_core.o
 most_core-y :=	core.o \
 		configfs.o
+
+obj-$(CONFIG_MOST_USB) += most_usb.o
diff --git a/drivers/most/most_usb.c b/drivers/most/most_usb.c
new file mode 100644
index 0000000..daa5e4b
--- /dev/null
+++ b/drivers/most/most_usb.c
@@ -0,0 +1,1262 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * usb.c - Hardware dependent module for USB
+ *
+ * Copyright (C) 2013-2015 Microchip Technology Germany II GmbH & Co. KG
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/module.h>
+#include <linux/fs.h>
+#include <linux/usb.h>
+#include <linux/slab.h>
+#include <linux/init.h>
+#include <linux/cdev.h>
+#include <linux/device.h>
+#include <linux/list.h>
+#include <linux/completion.h>
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
+#include <linux/interrupt.h>
+#include <linux/workqueue.h>
+#include <linux/sysfs.h>
+#include <linux/dma-mapping.h>
+#include <linux/etherdevice.h>
+#include <linux/uaccess.h>
+#include <linux/most.h>
+
+#define USB_MTU			512
+#define NO_ISOCHRONOUS_URB	0
+#define AV_PACKETS_PER_XACT	2
+#define BUF_CHAIN_SIZE		0xFFFF
+#define MAX_NUM_ENDPOINTS	30
+#define MAX_SUFFIX_LEN		10
+#define MAX_STRING_LEN		80
+#define MAX_BUF_SIZE		0xFFFF
+
+#define USB_VENDOR_ID_SMSC	0x0424  /* VID: SMSC */
+#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
+#define DRCI_REG_NODE_ADDR	0x0102
+#define DRCI_REG_NODE_POS	0x0103
+#define DRCI_REG_MEP_FILTER	0x0140
+#define DRCI_REG_HASH_TBL0	0x0141
+#define DRCI_REG_HASH_TBL1	0x0142
+#define DRCI_REG_HASH_TBL2	0x0143
+#define DRCI_REG_HASH_TBL3	0x0144
+#define DRCI_REG_HW_ADDR_HI	0x0145
+#define DRCI_REG_HW_ADDR_MI	0x0146
+#define DRCI_REG_HW_ADDR_LO	0x0147
+#define DRCI_REG_BASE		0x1100
+#define DRCI_COMMAND		0x02
+#define DRCI_READ_REQ		0xA0
+#define DRCI_WRITE_REQ		0xA1
+
+/**
+ * struct most_dci_obj - Direct Communication Interface
+ * @kobj:position in sysfs
+ * @usb_device: pointer to the usb device
+ * @reg_addr: register address for arbitrary DCI access
+ */
+struct most_dci_obj {
+	struct device dev;
+	struct usb_device *usb_device;
+	u16 reg_addr;
+};
+
+#define to_dci_obj(p) container_of(p, struct most_dci_obj, dev)
+
+struct most_dev;
+
+struct clear_hold_work {
+	struct work_struct ws;
+	struct most_dev *mdev;
+	unsigned int channel;
+	int pipe;
+};
+
+#define to_clear_hold_work(w) container_of(w, struct clear_hold_work, ws)
+
+/**
+ * struct most_dev - holds all usb interface specific stuff
+ * @usb_device: pointer to usb device
+ * @iface: hardware interface
+ * @cap: channel capabilities
+ * @conf: channel configuration
+ * @dci: direct communication interface of hardware
+ * @ep_address: endpoint address table
+ * @description: device description
+ * @suffix: suffix for channel name
+ * @channel_lock: synchronize channel access
+ * @padding_active: indicates channel uses padding
+ * @is_channel_healthy: health status table of each channel
+ * @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
+ */
+struct most_dev {
+	struct device dev;
+	struct usb_device *usb_device;
+	struct most_interface iface;
+	struct most_channel_capability *cap;
+	struct most_channel_config *conf;
+	struct most_dci_obj *dci;
+	u8 *ep_address;
+	char description[MAX_STRING_LEN];
+	char suffix[MAX_NUM_ENDPOINTS][MAX_SUFFIX_LEN];
+	spinlock_t channel_lock[MAX_NUM_ENDPOINTS]; /* sync channel access */
+	bool padding_active[MAX_NUM_ENDPOINTS];
+	bool is_channel_healthy[MAX_NUM_ENDPOINTS];
+	struct clear_hold_work clear_work[MAX_NUM_ENDPOINTS];
+	struct usb_anchor *busy_urbs;
+	struct mutex io_mutex;
+	struct timer_list link_stat_timer;
+	struct work_struct poll_work_obj;
+	void (*on_netinfo)(struct most_interface *most_iface,
+			   unsigned char link_state, unsigned char *addrs);
+};
+
+#define to_mdev(d) container_of(d, struct most_dev, iface)
+#define to_mdev_from_dev(d) container_of(d, struct most_dev, dev)
+#define to_mdev_from_work(w) container_of(w, struct most_dev, poll_work_obj)
+
+static void wq_clear_halt(struct work_struct *wq_obj);
+static void wq_netinfo(struct work_struct *wq_obj);
+
+/**
+ * drci_rd_reg - read a DCI register
+ * @dev: usb device
+ * @reg: register address
+ * @buf: buffer to store data
+ *
+ * This is reads data from INIC's direct register communication interface
+ */
+static inline int drci_rd_reg(struct usb_device *dev, u16 reg, u16 *buf)
+{
+	int retval;
+	__le16 *dma_buf = kzalloc(sizeof(*dma_buf), GFP_KERNEL);
+	u8 req_type = USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE;
+
+	if (!dma_buf)
+		return -ENOMEM;
+
+	retval = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
+				 DRCI_READ_REQ, req_type,
+				 0x0000,
+				 reg, dma_buf, sizeof(*dma_buf), 5 * HZ);
+	*buf = le16_to_cpu(*dma_buf);
+	kfree(dma_buf);
+
+	return retval;
+}
+
+/**
+ * drci_wr_reg - write a DCI register
+ * @dev: usb device
+ * @reg: register address
+ * @data: data to write
+ *
+ * This is writes data to INIC's direct register communication interface
+ */
+static inline int drci_wr_reg(struct usb_device *dev, u16 reg, u16 data)
+{
+	return usb_control_msg(dev,
+			       usb_sndctrlpipe(dev, 0),
+			       DRCI_WRITE_REQ,
+			       USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+			       data,
+			       reg,
+			       NULL,
+			       0,
+			       5 * HZ);
+}
+
+static inline int start_sync_ep(struct usb_device *usb_dev, u16 ep)
+{
+	return drci_wr_reg(usb_dev, DRCI_REG_BASE + DRCI_COMMAND + ep * 16, 1);
+}
+
+/**
+ * get_stream_frame_size - calculate frame size of current configuration
+ * @cfg: channel configuration
+ */
+static unsigned int get_stream_frame_size(struct most_channel_config *cfg)
+{
+	unsigned int frame_size = 0;
+	unsigned int sub_size = cfg->subbuffer_size;
+
+	if (!sub_size) {
+		pr_warn("Misconfig: Subbuffer size zero.\n");
+		return frame_size;
+	}
+	switch (cfg->data_type) {
+	case MOST_CH_ISOC:
+		frame_size = AV_PACKETS_PER_XACT * sub_size;
+		break;
+	case MOST_CH_SYNC:
+		if (cfg->packets_per_xact == 0) {
+			pr_warn("Misconfig: Packets per XACT zero\n");
+			frame_size = 0;
+		} else if (cfg->packets_per_xact == 0xFF) {
+			frame_size = (USB_MTU / sub_size) * sub_size;
+		} else {
+			frame_size = cfg->packets_per_xact * sub_size;
+		}
+		break;
+	default:
+		pr_warn("Query frame size of non-streaming channel\n");
+		break;
+	}
+	return frame_size;
+}
+
+/**
+ * hdm_poison_channel - mark buffers of this channel as invalid
+ * @iface: pointer to the interface
+ * @channel: channel ID
+ *
+ * This unlinks all URBs submitted to the HCD,
+ * calls the associated completion function of the core and removes
+ * them from the list.
+ *
+ * Returns 0 on success or error code otherwise.
+ */
+static int hdm_poison_channel(struct most_interface *iface, int channel)
+{
+	struct most_dev *mdev = to_mdev(iface);
+	unsigned long flags;
+	spinlock_t *lock; /* temp. lock */
+
+	if (channel < 0 || channel >= iface->num_channels) {
+		dev_warn(&mdev->usb_device->dev, "Channel ID out of range.\n");
+		return -ECHRNG;
+	}
+
+	lock = mdev->channel_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);
+
+	mutex_lock(&mdev->io_mutex);
+	usb_kill_anchored_urbs(&mdev->busy_urbs[channel]);
+	if (mdev->padding_active[channel])
+		mdev->padding_active[channel] = false;
+
+	if (mdev->conf[channel].data_type == MOST_CH_ASYNC) {
+		del_timer_sync(&mdev->link_stat_timer);
+		cancel_work_sync(&mdev->poll_work_obj);
+	}
+	mutex_unlock(&mdev->io_mutex);
+	return 0;
+}
+
+/**
+ * hdm_add_padding - add padding bytes
+ * @mdev: most device
+ * @channel: channel ID
+ * @mbo: buffer object
+ *
+ * This inserts the INIC hardware specific padding bytes into a streaming
+ * channel's buffer
+ */
+static int hdm_add_padding(struct most_dev *mdev, int channel, struct mbo *mbo)
+{
+	struct most_channel_config *conf = &mdev->conf[channel];
+	unsigned int frame_size = get_stream_frame_size(conf);
+	unsigned int j, num_frames;
+
+	if (!frame_size)
+		return -EINVAL;
+	num_frames = mbo->buffer_length / frame_size;
+
+	if (num_frames < 1) {
+		dev_err(&mdev->usb_device->dev,
+			"Missed minimal transfer unit.\n");
+		return -EINVAL;
+	}
+
+	for (j = num_frames - 1; j > 0; j--)
+		memmove(mbo->virt_address + j * USB_MTU,
+			mbo->virt_address + j * frame_size,
+			frame_size);
+	mbo->buffer_length = num_frames * USB_MTU;
+	return 0;
+}
+
+/**
+ * hdm_remove_padding - remove padding bytes
+ * @mdev: most device
+ * @channel: channel ID
+ * @mbo: buffer object
+ *
+ * This takes the INIC hardware specific padding bytes off a streaming
+ * channel's buffer.
+ */
+static int hdm_remove_padding(struct most_dev *mdev, int channel,
+			      struct mbo *mbo)
+{
+	struct most_channel_config *const conf = &mdev->conf[channel];
+	unsigned int frame_size = get_stream_frame_size(conf);
+	unsigned int j, num_frames;
+
+	if (!frame_size)
+		return -EINVAL;
+	num_frames = mbo->processed_length / USB_MTU;
+
+	for (j = 1; j < num_frames; j++)
+		memmove(mbo->virt_address + frame_size * j,
+			mbo->virt_address + USB_MTU * j,
+			frame_size);
+
+	mbo->processed_length = frame_size * num_frames;
+	return 0;
+}
+
+/**
+ * hdm_write_completion - completion function for submitted Tx URBs
+ * @urb: the URB that has been completed
+ *
+ * This checks the status of the completed URB. In case the URB has been
+ * unlinked before, it is immediately freed. On any other error the MBO
+ * transfer flag is set. On success it frees allocated resources and calls
+ * the completion function.
+ *
+ * Context: interrupt!
+ */
+static void hdm_write_completion(struct urb *urb)
+{
+	struct mbo *mbo = urb->context;
+	struct most_dev *mdev = to_mdev(mbo->ifp);
+	unsigned int channel = mbo->hdm_channel_id;
+	spinlock_t *lock = mdev->channel_lock + channel;
+	unsigned long flags;
+
+	spin_lock_irqsave(lock, flags);
+
+	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(&mdev->usb_device->dev,
+				 "Broken pipe on ep%02x\n",
+				 mdev->ep_address[channel]);
+			mdev->is_channel_healthy[channel] = false;
+			mdev->clear_work[channel].pipe = urb->pipe;
+			schedule_work(&mdev->clear_work[channel].ws);
+			break;
+		case -ENODEV:
+		case -EPROTO:
+			mbo->status = MBO_E_CLOSE;
+			break;
+		}
+	}
+
+	spin_unlock_irqrestore(lock, flags);
+
+	if (likely(mbo->complete))
+		mbo->complete(mbo);
+	usb_free_urb(urb);
+}
+
+/**
+ * hdm_read_completion - completion function for submitted Rx URBs
+ * @urb: the URB that has been completed
+ *
+ * This checks the status of the completed URB. In case the URB has been
+ * unlinked before it is immediately freed. On any other error the MBO transfer
+ * flag is set. On success it frees allocated resources, removes
+ * padding bytes -if necessary- and calls the completion function.
+ *
+ * Context: interrupt!
+ *
+ * **************************************************************************
+ *                   Error codes returned by in urb->status
+ *                   or in iso_frame_desc[n].status (for ISO)
+ * *************************************************************************
+ *
+ * USB device drivers may only test urb status values in completion handlers.
+ * This is because otherwise there would be a race between HCDs updating
+ * these values on one CPU, and device drivers testing them on another CPU.
+ *
+ * A transfer's actual_length may be positive even when an error has been
+ * reported.  That's because transfers often involve several packets, so that
+ * one or more packets could finish before an error stops further endpoint I/O.
+ *
+ * For isochronous URBs, the urb status value is non-zero only if the URB is
+ * unlinked, the device is removed, the host controller is disabled or the total
+ * transferred length is less than the requested length and the URB_SHORT_NOT_OK
+ * flag is set.  Completion handlers for isochronous URBs should only see
+ * urb->status set to zero, -ENOENT, -ECONNRESET, -ESHUTDOWN, or -EREMOTEIO.
+ * Individual frame descriptor status fields may report more status codes.
+ *
+ *
+ * 0			Transfer completed successfully
+ *
+ * -ENOENT		URB was synchronously unlinked by usb_unlink_urb
+ *
+ * -EINPROGRESS		URB still pending, no results yet
+ *			(That is, if drivers see this it's a bug.)
+ *
+ * -EPROTO (*, **)	a) bitstuff error
+ *			b) no response packet received within the
+ *			   prescribed bus turn-around time
+ *			c) unknown USB error
+ *
+ * -EILSEQ (*, **)	a) CRC mismatch
+ *			b) no response packet received within the
+ *			   prescribed bus turn-around time
+ *			c) unknown USB error
+ *
+ *			Note that often the controller hardware does not
+ *			distinguish among cases a), b), and c), so a
+ *			driver cannot tell whether there was a protocol
+ *			error, a failure to respond (often caused by
+ *			device disconnect), or some other fault.
+ *
+ * -ETIME (**)		No response packet received within the prescribed
+ *			bus turn-around time.  This error may instead be
+ *			reported as -EPROTO or -EILSEQ.
+ *
+ * -ETIMEDOUT		Synchronous USB message functions use this code
+ *			to indicate timeout expired before the transfer
+ *			completed, and no other error was reported by HC.
+ *
+ * -EPIPE (**)		Endpoint stalled.  For non-control endpoints,
+ *			reset this status with usb_clear_halt().
+ *
+ * -ECOMM		During an IN transfer, the host controller
+ *			received data from an endpoint faster than it
+ *			could be written to system memory
+ *
+ * -ENOSR		During an OUT transfer, the host controller
+ *			could not retrieve data from system memory fast
+ *			enough to keep up with the USB data rate
+ *
+ * -EOVERFLOW (*)	The amount of data returned by the endpoint was
+ *			greater than either the max packet size of the
+ *			endpoint or the remaining buffer size.  "Babble".
+ *
+ * -EREMOTEIO		The data read from the endpoint did not fill the
+ *			specified buffer, and URB_SHORT_NOT_OK was set in
+ *			urb->transfer_flags.
+ *
+ * -ENODEV		Device was removed.  Often preceded by a burst of
+ *			other errors, since the hub driver doesn't detect
+ *			device removal events immediately.
+ *
+ * -EXDEV		ISO transfer only partially completed
+ *			(only set in iso_frame_desc[n].status, not urb->status)
+ *
+ * -EINVAL		ISO madness, if this happens: Log off and go home
+ *
+ * -ECONNRESET		URB was asynchronously unlinked by usb_unlink_urb
+ *
+ * -ESHUTDOWN		The device or host controller has been disabled due
+ *			to some problem that could not be worked around,
+ *			such as a physical disconnect.
+ *
+ *
+ * (*) Error codes like -EPROTO, -EILSEQ and -EOVERFLOW normally indicate
+ * hardware problems such as bad devices (including firmware) or cables.
+ *
+ * (**) This is also one of several codes that different kinds of host
+ * controller use to indicate a transfer has failed because of device
+ * disconnect.  In the interval before the hub driver starts disconnect
+ * processing, devices may receive such fault reports for every request.
+ *
+ * See <https://www.kernel.org/doc/Documentation/driver-api/usb/error-codes.rst>
+ */
+static void hdm_read_completion(struct urb *urb)
+{
+	struct mbo *mbo = urb->context;
+	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->channel_lock + channel;
+	unsigned long flags;
+
+	spin_lock_irqsave(lock, flags);
+
+	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 pipe on ep%02x\n",
+				 mdev->ep_address[channel]);
+			mdev->is_channel_healthy[channel] = false;
+			mdev->clear_work[channel].pipe = urb->pipe;
+			schedule_work(&mdev->clear_work[channel].ws);
+			break;
+		case -ENODEV:
+		case -EPROTO:
+			mbo->status = MBO_E_CLOSE;
+			break;
+		case -EOVERFLOW:
+			dev_warn(dev, "Babble on ep%02x\n",
+				 mdev->ep_address[channel]);
+			break;
+		}
+	}
+
+	spin_unlock_irqrestore(lock, flags);
+
+	if (likely(mbo->complete))
+		mbo->complete(mbo);
+	usb_free_urb(urb);
+}
+
+/**
+ * hdm_enqueue - receive a buffer to be used for data transfer
+ * @iface: interface to enqueue to
+ * @channel: ID of the channel
+ * @mbo: pointer to the buffer object
+ *
+ * This allocates a new URB and fills it according to the channel
+ * that is being used for transmission of data. Before the URB is
+ * submitted it is stored in the private anchor list.
+ *
+ * Returns 0 on success. On any error the URB is freed and a error code
+ * is returned.
+ *
+ * Context: Could in _some_ cases be interrupt!
+ */
+static int hdm_enqueue(struct most_interface *iface, int channel,
+		       struct mbo *mbo)
+{
+	struct most_dev *mdev = to_mdev(iface);
+	struct most_channel_config *conf;
+	int retval = 0;
+	struct urb *urb;
+	unsigned long length;
+	void *virt_address;
+
+	if (!mbo)
+		return -EINVAL;
+	if (iface->num_channels <= channel || channel < 0)
+		return -ECHRNG;
+
+	conf = &mdev->conf[channel];
+
+	mutex_lock(&mdev->io_mutex);
+	if (!mdev->usb_device) {
+		retval = -ENODEV;
+		goto unlock_io_mutex;
+	}
+
+	urb = usb_alloc_urb(NO_ISOCHRONOUS_URB, GFP_ATOMIC);
+	if (!urb) {
+		retval = -ENOMEM;
+		goto unlock_io_mutex;
+	}
+
+	if ((conf->direction & MOST_CH_TX) && mdev->padding_active[channel] &&
+	    hdm_add_padding(mdev, channel, mbo)) {
+		retval = -EINVAL;
+		goto err_free_urb;
+	}
+
+	urb->transfer_dma = mbo->bus_address;
+	virt_address = mbo->virt_address;
+	length = mbo->buffer_length;
+
+	if (conf->direction & MOST_CH_TX) {
+		usb_fill_bulk_urb(urb, mdev->usb_device,
+				  usb_sndbulkpipe(mdev->usb_device,
+						  mdev->ep_address[channel]),
+				  virt_address,
+				  length,
+				  hdm_write_completion,
+				  mbo);
+		if (conf->data_type != MOST_CH_ISOC &&
+		    conf->data_type != MOST_CH_SYNC)
+			urb->transfer_flags |= URB_ZERO_PACKET;
+	} else {
+		usb_fill_bulk_urb(urb, mdev->usb_device,
+				  usb_rcvbulkpipe(mdev->usb_device,
+						  mdev->ep_address[channel]),
+				  virt_address,
+				  length + conf->extra_len,
+				  hdm_read_completion,
+				  mbo);
+	}
+	urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+
+	usb_anchor_urb(urb, &mdev->busy_urbs[channel]);
+
+	retval = usb_submit_urb(urb, GFP_KERNEL);
+	if (retval) {
+		dev_err(&mdev->usb_device->dev,
+			"URB submit failed with error %d.\n", retval);
+		goto err_unanchor_urb;
+	}
+	goto unlock_io_mutex;
+
+err_unanchor_urb:
+	usb_unanchor_urb(urb);
+err_free_urb:
+	usb_free_urb(urb);
+unlock_io_mutex:
+	mutex_unlock(&mdev->io_mutex);
+	return retval;
+}
+
+static void *hdm_dma_alloc(struct mbo *mbo, u32 size)
+{
+	struct most_dev *mdev = to_mdev(mbo->ifp);
+
+	return usb_alloc_coherent(mdev->usb_device, size, GFP_KERNEL,
+				  &mbo->bus_address);
+}
+
+static void hdm_dma_free(struct mbo *mbo, u32 size)
+{
+	struct most_dev *mdev = to_mdev(mbo->ifp);
+
+	usb_free_coherent(mdev->usb_device, size, mbo->virt_address,
+			  mbo->bus_address);
+}
+
+/**
+ * hdm_configure_channel - receive channel configuration from core
+ * @iface: interface
+ * @channel: channel ID
+ * @conf: structure that holds the configuration information
+ *
+ * The attached network interface controller (NIC) supports a padding mode
+ * to avoid short packets on USB, hence increasing the performance due to a
+ * lower interrupt load. This mode is default for synchronous data and can
+ * be switched on for isochronous data. In case padding is active the
+ * driver needs to know the frame size of the payload in order to calculate
+ * the number of bytes it needs to pad when transmitting or to cut off when
+ * receiving data.
+ *
+ */
+static int hdm_configure_channel(struct most_interface *iface, int channel,
+				 struct most_channel_config *conf)
+{
+	unsigned int num_frames;
+	unsigned int frame_size;
+	struct most_dev *mdev = to_mdev(iface);
+	struct device *dev = &mdev->usb_device->dev;
+
+	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);
+
+	if (!conf) {
+		dev_err(dev, "Bad config pointer.\n");
+		return -EINVAL;
+	}
+	if (channel < 0 || channel >= iface->num_channels) {
+		dev_err(dev, "Channel ID out of range.\n");
+		return -EINVAL;
+	}
+	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 &&
+	      conf->packets_per_xact != 0xFF)) {
+		mdev->padding_active[channel] = false;
+		/*
+		 * Since the NIC's padding mode is not going to be
+		 * used, we can skip the frame size calculations and
+		 * move directly on to exit.
+		 */
+		goto exit;
+	}
+
+	mdev->padding_active[channel] = true;
+
+	frame_size = get_stream_frame_size(conf);
+	if (frame_size == 0 || frame_size > USB_MTU) {
+		dev_warn(dev, "Misconfig: frame size wrong\n");
+		return -EINVAL;
+	}
+
+	num_frames = conf->buffer_size / frame_size;
+
+	if (conf->buffer_size % frame_size) {
+		u16 old_size = conf->buffer_size;
+
+		conf->buffer_size = num_frames * frame_size;
+		dev_warn(dev, "%s: fixed buffer size (%d -> %d)\n",
+			 mdev->suffix[channel], old_size, conf->buffer_size);
+	}
+
+	/* calculate extra length to comply w/ HW padding */
+	conf->extra_len = num_frames * (USB_MTU - frame_size);
+
+exit:
+	mdev->conf[channel] = *conf;
+	if (conf->data_type == MOST_CH_ASYNC) {
+		u16 ep = mdev->ep_address[channel];
+
+		if (start_sync_ep(mdev->usb_device, ep) < 0)
+			dev_warn(dev, "sync for ep%02x failed", ep);
+	}
+	return 0;
+}
+
+/**
+ * hdm_request_netinfo - request network information
+ * @iface: pointer to interface
+ * @channel: channel ID
+ *
+ * This is used as trigger to set up the link status timer that
+ * polls for the NI state of the INIC every 2 seconds.
+ *
+ */
+static void hdm_request_netinfo(struct most_interface *iface, int channel,
+				void (*on_netinfo)(struct most_interface *,
+						   unsigned char,
+						   unsigned char *))
+{
+	struct most_dev *mdev = to_mdev(iface);
+
+	mdev->on_netinfo = on_netinfo;
+	if (!on_netinfo)
+		return;
+
+	mdev->link_stat_timer.expires = jiffies + HZ;
+	mod_timer(&mdev->link_stat_timer, mdev->link_stat_timer.expires);
+}
+
+/**
+ * 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
+ * tasks to a work queue.
+ */
+static void link_stat_timer_handler(struct timer_list *t)
+{
+	struct most_dev *mdev = from_timer(mdev, t, link_stat_timer);
+
+	schedule_work(&mdev->poll_work_obj);
+	mdev->link_stat_timer.expires = jiffies + (2 * HZ);
+	add_timer(&mdev->link_stat_timer);
+}
+
+/**
+ * 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
+ */
+static void wq_netinfo(struct work_struct *wq_obj)
+{
+	struct most_dev *mdev = to_mdev_from_work(wq_obj);
+	struct usb_device *usb_device = mdev->usb_device;
+	struct device *dev = &usb_device->dev;
+	u16 hi, mi, lo, link;
+	u8 hw_addr[6];
+
+	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 (drci_rd_reg(usb_device, DRCI_REG_HW_ADDR_MI, &mi) < 0) {
+		dev_err(dev, "Vendor request 'hw_addr_mid' failed\n");
+		return;
+	}
+
+	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;
+
+	if (mdev->on_netinfo)
+		mdev->on_netinfo(&mdev->iface, link, hw_addr);
+}
+
+/**
+ * wq_clear_halt - work queue function
+ * @wq_obj: work_struct object to execute
+ *
+ * This sends a clear_halt to the given USB pipe.
+ */
+static void wq_clear_halt(struct work_struct *wq_obj)
+{
+	struct clear_hold_work *clear_work = to_clear_hold_work(wq_obj);
+	struct most_dev *mdev = clear_work->mdev;
+	unsigned int channel = clear_work->channel;
+	int pipe = clear_work->pipe;
+
+	mutex_lock(&mdev->io_mutex);
+	most_stop_enqueue(&mdev->iface, channel);
+	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");
+
+	/* If the functional Stall condition has been set on an
+	 * asynchronous rx channel, we need to clear the tx channel
+	 * too, since the hardware runs its clean-up sequence on both
+	 * channels, as they are physically one on the network.
+	 *
+	 * The USB interface that exposes the asynchronous channels
+	 * contains always two endpoints, and two only.
+	 */
+	if (mdev->conf[channel].data_type == MOST_CH_ASYNC &&
+	    mdev->conf[channel].direction == MOST_CH_RX) {
+		int peer = 1 - channel;
+		int snd_pipe = usb_sndbulkpipe(mdev->usb_device,
+					       mdev->ep_address[peer]);
+		usb_clear_halt(mdev->usb_device, snd_pipe);
+	}
+	mdev->is_channel_healthy[channel] = true;
+	most_resume_enqueue(&mdev->iface, channel);
+	mutex_unlock(&mdev->io_mutex);
+}
+
+/**
+ * hdm_usb_fops - file operation table for USB driver
+ */
+static const struct file_operations hdm_usb_fops = {
+	.owner = THIS_MODULE,
+};
+
+/**
+ * usb_device_id - ID table for HCD device probing
+ */
+static const 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 */
+};
+
+struct regs {
+	const char *name;
+	u16 reg;
+};
+
+static const struct regs ro_regs[] = {
+	{ "ni_state", DRCI_REG_NI_STATE },
+	{ "packet_bandwidth", DRCI_REG_PACKET_BW },
+	{ "node_address", DRCI_REG_NODE_ADDR },
+	{ "node_position", DRCI_REG_NODE_POS },
+};
+
+static const struct regs rw_regs[] = {
+	{ "mep_filter", DRCI_REG_MEP_FILTER },
+	{ "mep_hash0", DRCI_REG_HASH_TBL0 },
+	{ "mep_hash1", DRCI_REG_HASH_TBL1 },
+	{ "mep_hash2", DRCI_REG_HASH_TBL2 },
+	{ "mep_hash3", DRCI_REG_HASH_TBL3 },
+	{ "mep_eui48_hi", DRCI_REG_HW_ADDR_HI },
+	{ "mep_eui48_mi", DRCI_REG_HW_ADDR_MI },
+	{ "mep_eui48_lo", DRCI_REG_HW_ADDR_LO },
+};
+
+static int get_stat_reg_addr(const struct regs *regs, int size,
+			     const char *name, u16 *reg_addr)
+{
+	int i;
+
+	for (i = 0; i < size; i++) {
+		if (!strcmp(name, regs[i].name)) {
+			*reg_addr = regs[i].reg;
+			return 0;
+		}
+	}
+	return -EFAULT;
+}
+
+#define get_static_reg_addr(regs, name, reg_addr) \
+	get_stat_reg_addr(regs, ARRAY_SIZE(regs), name, reg_addr)
+
+static ssize_t value_show(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	const char *name = attr->attr.name;
+	struct most_dci_obj *dci_obj = to_dci_obj(dev);
+	u16 val;
+	u16 reg_addr;
+	int err;
+
+	if (!strcmp(name, "arb_address"))
+		return snprintf(buf, PAGE_SIZE, "%04x\n", dci_obj->reg_addr);
+
+	if (!strcmp(name, "arb_value"))
+		reg_addr = dci_obj->reg_addr;
+	else if (get_static_reg_addr(ro_regs, name, &reg_addr) &&
+		 get_static_reg_addr(rw_regs, name, &reg_addr))
+		return -EFAULT;
+
+	err = drci_rd_reg(dci_obj->usb_device, reg_addr, &val);
+	if (err < 0)
+		return err;
+
+	return snprintf(buf, PAGE_SIZE, "%04x\n", val);
+}
+
+static ssize_t value_store(struct device *dev, struct device_attribute *attr,
+			   const char *buf, size_t count)
+{
+	u16 val;
+	u16 reg_addr;
+	const char *name = attr->attr.name;
+	struct most_dci_obj *dci_obj = to_dci_obj(dev);
+	struct usb_device *usb_dev = dci_obj->usb_device;
+	int err = kstrtou16(buf, 16, &val);
+
+	if (err)
+		return err;
+
+	if (!strcmp(name, "arb_address")) {
+		dci_obj->reg_addr = val;
+		return count;
+	}
+
+	if (!strcmp(name, "arb_value"))
+		err = drci_wr_reg(usb_dev, dci_obj->reg_addr, val);
+	else if (!strcmp(name, "sync_ep"))
+		err = start_sync_ep(usb_dev, val);
+	else if (!get_static_reg_addr(rw_regs, name, &reg_addr))
+		err = drci_wr_reg(usb_dev, reg_addr, val);
+	else
+		return -EFAULT;
+
+	if (err < 0)
+		return err;
+
+	return count;
+}
+
+static DEVICE_ATTR(ni_state, 0444, value_show, NULL);
+static DEVICE_ATTR(packet_bandwidth, 0444, value_show, NULL);
+static DEVICE_ATTR(node_address, 0444, value_show, NULL);
+static DEVICE_ATTR(node_position, 0444, value_show, NULL);
+static DEVICE_ATTR(sync_ep, 0200, NULL, value_store);
+static DEVICE_ATTR(mep_filter, 0644, value_show, value_store);
+static DEVICE_ATTR(mep_hash0, 0644, value_show, value_store);
+static DEVICE_ATTR(mep_hash1, 0644, value_show, value_store);
+static DEVICE_ATTR(mep_hash2, 0644, value_show, value_store);
+static DEVICE_ATTR(mep_hash3, 0644, value_show, value_store);
+static DEVICE_ATTR(mep_eui48_hi, 0644, value_show, value_store);
+static DEVICE_ATTR(mep_eui48_mi, 0644, value_show, value_store);
+static DEVICE_ATTR(mep_eui48_lo, 0644, value_show, value_store);
+static DEVICE_ATTR(arb_address, 0644, value_show, value_store);
+static DEVICE_ATTR(arb_value, 0644, value_show, value_store);
+
+static struct attribute *dci_attrs[] = {
+	&dev_attr_ni_state.attr,
+	&dev_attr_packet_bandwidth.attr,
+	&dev_attr_node_address.attr,
+	&dev_attr_node_position.attr,
+	&dev_attr_sync_ep.attr,
+	&dev_attr_mep_filter.attr,
+	&dev_attr_mep_hash0.attr,
+	&dev_attr_mep_hash1.attr,
+	&dev_attr_mep_hash2.attr,
+	&dev_attr_mep_hash3.attr,
+	&dev_attr_mep_eui48_hi.attr,
+	&dev_attr_mep_eui48_mi.attr,
+	&dev_attr_mep_eui48_lo.attr,
+	&dev_attr_arb_address.attr,
+	&dev_attr_arb_value.attr,
+	NULL,
+};
+
+static struct attribute_group dci_attr_group = {
+	.attrs = dci_attrs,
+};
+
+static const struct attribute_group *dci_attr_groups[] = {
+	&dci_attr_group,
+	NULL,
+};
+
+static void release_dci(struct device *dev)
+{
+	struct most_dci_obj *dci = to_dci_obj(dev);
+
+	kfree(dci);
+}
+
+static void release_mdev(struct device *dev)
+{
+	struct most_dev *mdev = to_mdev_from_dev(dev);
+
+	kfree(mdev);
+}
+/**
+ * hdm_probe - probe function of USB device driver
+ * @interface: Interface of the attached USB device
+ * @id: Pointer to the USB ID table.
+ *
+ * This allocates and initializes the device instance, adds the new
+ * entry to the internal list, scans the USB descriptors and registers
+ * the interface with the core.
+ * Additionally, the DCI objects are created and the hardware is sync'd.
+ *
+ * Return 0 on success. In case of an error a negative number is returned.
+ */
+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 usb_endpoint_descriptor *ep_desc;
+	int ret = 0;
+
+	if (!mdev)
+		goto err_out_of_memory;
+
+	usb_set_intfdata(interface, mdev);
+	num_endpoints = usb_iface_desc->desc.bNumEndpoints;
+	mutex_init(&mdev->io_mutex);
+	INIT_WORK(&mdev->poll_work_obj, wq_netinfo);
+	timer_setup(&mdev->link_stat_timer, link_stat_timer_handler, 0);
+
+	mdev->usb_device = usb_dev;
+	mdev->link_stat_timer.expires = jiffies + (2 * HZ);
+
+	mdev->iface.mod = hdm_usb_fops.owner;
+	mdev->iface.dev = &mdev->dev;
+	mdev->iface.driver_dev = &interface->dev;
+	mdev->iface.interface = ITYPE_USB;
+	mdev->iface.configure = hdm_configure_channel;
+	mdev->iface.request_netinfo = hdm_request_netinfo;
+	mdev->iface.enqueue = hdm_enqueue;
+	mdev->iface.poison_channel = hdm_poison_channel;
+	mdev->iface.dma_alloc = hdm_dma_alloc;
+	mdev->iface.dma_free = hdm_dma_free;
+	mdev->iface.description = mdev->description;
+	mdev->iface.num_channels = num_endpoints;
+
+	snprintf(mdev->description, sizeof(mdev->description),
+		 "%d-%s:%d.%d",
+		 usb_dev->bus->busnum,
+		 usb_dev->devpath,
+		 usb_dev->config->desc.bConfigurationValue,
+		 usb_iface_desc->desc.bInterfaceNumber);
+
+	mdev->dev.init_name = mdev->description;
+	mdev->dev.parent = &interface->dev;
+	mdev->dev.release = release_mdev;
+	mdev->conf = kcalloc(num_endpoints, sizeof(*mdev->conf), GFP_KERNEL);
+	if (!mdev->conf)
+		goto err_free_mdev;
+
+	mdev->cap = kcalloc(num_endpoints, sizeof(*mdev->cap), GFP_KERNEL);
+	if (!mdev->cap)
+		goto err_free_conf;
+
+	mdev->iface.channel_vector = mdev->cap;
+	mdev->ep_address =
+		kcalloc(num_endpoints, sizeof(*mdev->ep_address), GFP_KERNEL);
+	if (!mdev->ep_address)
+		goto err_free_cap;
+
+	mdev->busy_urbs =
+		kcalloc(num_endpoints, sizeof(*mdev->busy_urbs), GFP_KERNEL);
+	if (!mdev->busy_urbs)
+		goto err_free_ep_address;
+
+	tmp_cap = mdev->cap;
+	for (i = 0; i < num_endpoints; i++) {
+		ep_desc = &usb_iface_desc->endpoint[i].desc;
+		mdev->ep_address[i] = ep_desc->bEndpointAddress;
+		mdev->padding_active[i] = false;
+		mdev->is_channel_healthy[i] = true;
+
+		snprintf(&mdev->suffix[i][0], MAX_SUFFIX_LEN, "ep%02x",
+			 mdev->ep_address[i]);
+
+		tmp_cap->name_suffix = &mdev->suffix[i][0];
+		tmp_cap->buffer_size_packet = MAX_BUF_SIZE;
+		tmp_cap->buffer_size_streaming = MAX_BUF_SIZE;
+		tmp_cap->num_buffers_packet = BUF_CHAIN_SIZE;
+		tmp_cap->num_buffers_streaming = BUF_CHAIN_SIZE;
+		tmp_cap->data_type = MOST_CH_CONTROL | MOST_CH_ASYNC |
+				     MOST_CH_ISOC | MOST_CH_SYNC;
+		if (usb_endpoint_dir_in(ep_desc))
+			tmp_cap->direction = MOST_CH_RX;
+		else
+			tmp_cap->direction = MOST_CH_TX;
+		tmp_cap++;
+		init_usb_anchor(&mdev->busy_urbs[i]);
+		spin_lock_init(&mdev->channel_lock[i]);
+	}
+	dev_notice(dev, "claimed gadget: Vendor=%4.4x ProdID=%4.4x Bus=%02x Device=%02x\n",
+		   le16_to_cpu(usb_dev->descriptor.idVendor),
+		   le16_to_cpu(usb_dev->descriptor.idProduct),
+		   usb_dev->bus->busnum,
+		   usb_dev->devnum);
+
+	dev_notice(dev, "device path: /sys/bus/usb/devices/%d-%s:%d.%d\n",
+		   usb_dev->bus->busnum,
+		   usb_dev->devpath,
+		   usb_dev->config->desc.bConfigurationValue,
+		   usb_iface_desc->desc.bInterfaceNumber);
+
+	ret = most_register_interface(&mdev->iface);
+	if (ret)
+		goto err_free_busy_urbs;
+
+	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_OS81210) {
+		mdev->dci = kzalloc(sizeof(*mdev->dci), GFP_KERNEL);
+		if (!mdev->dci) {
+			mutex_unlock(&mdev->io_mutex);
+			most_deregister_interface(&mdev->iface);
+			ret = -ENOMEM;
+			goto err_free_busy_urbs;
+		}
+
+		mdev->dci->dev.init_name = "dci";
+		mdev->dci->dev.parent = get_device(mdev->iface.dev);
+		mdev->dci->dev.groups = dci_attr_groups;
+		mdev->dci->dev.release = release_dci;
+		if (device_register(&mdev->dci->dev)) {
+			mutex_unlock(&mdev->io_mutex);
+			most_deregister_interface(&mdev->iface);
+			ret = -ENOMEM;
+			goto err_free_dci;
+		}
+		mdev->dci->usb_device = mdev->usb_device;
+	}
+	mutex_unlock(&mdev->io_mutex);
+	return 0;
+err_free_dci:
+	put_device(&mdev->dci->dev);
+err_free_busy_urbs:
+	kfree(mdev->busy_urbs);
+err_free_ep_address:
+	kfree(mdev->ep_address);
+err_free_cap:
+	kfree(mdev->cap);
+err_free_conf:
+	kfree(mdev->conf);
+err_free_mdev:
+	put_device(&mdev->dev);
+err_out_of_memory:
+	if (ret == 0 || ret == -ENOMEM) {
+		ret = -ENOMEM;
+		dev_err(dev, "out of memory\n");
+	}
+	return ret;
+}
+
+/**
+ * hdm_disconnect - disconnect function of USB device driver
+ * @interface: Interface of the attached USB device
+ *
+ * This deregisters the interface with the core, removes the kernel timer
+ * and frees resources.
+ *
+ * Context: hub kernel thread
+ */
+static void hdm_disconnect(struct usb_interface *interface)
+{
+	struct most_dev *mdev = usb_get_intfdata(interface);
+
+	mutex_lock(&mdev->io_mutex);
+	usb_set_intfdata(interface, NULL);
+	mdev->usb_device = NULL;
+	mutex_unlock(&mdev->io_mutex);
+
+	del_timer_sync(&mdev->link_stat_timer);
+	cancel_work_sync(&mdev->poll_work_obj);
+
+	if (mdev->dci)
+		device_unregister(&mdev->dci->dev);
+	most_deregister_interface(&mdev->iface);
+
+	kfree(mdev->busy_urbs);
+	kfree(mdev->cap);
+	kfree(mdev->conf);
+	kfree(mdev->ep_address);
+	put_device(&mdev->dev);
+}
+
+static int hdm_suspend(struct usb_interface *interface, pm_message_t message)
+{
+	struct most_dev *mdev = usb_get_intfdata(interface);
+	int i;
+
+	mutex_lock(&mdev->io_mutex);
+	for (i = 0; i < mdev->iface.num_channels; i++) {
+		most_stop_enqueue(&mdev->iface, i);
+		usb_kill_anchored_urbs(&mdev->busy_urbs[i]);
+	}
+	mutex_unlock(&mdev->io_mutex);
+	return 0;
+}
+
+static int hdm_resume(struct usb_interface *interface)
+{
+	struct most_dev *mdev = usb_get_intfdata(interface);
+	int i;
+
+	mutex_lock(&mdev->io_mutex);
+	for (i = 0; i < mdev->iface.num_channels; i++)
+		most_resume_enqueue(&mdev->iface, i);
+	mutex_unlock(&mdev->io_mutex);
+	return 0;
+}
+
+static struct usb_driver hdm_usb = {
+	.name = "hdm_usb",
+	.id_table = usbid,
+	.probe = hdm_probe,
+	.disconnect = hdm_disconnect,
+	.resume = hdm_resume,
+	.suspend = hdm_suspend,
+};
+
+module_usb_driver(hdm_usb);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Christian Gromm <christian.gromm@microchip.com>");
+MODULE_DESCRIPTION("HDM_4_USB");
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 2/8] drivers: most: usb: use dev_*() functions to print messages
  2020-05-14  9:52 [PATCH v2 0/8] staging: most: move USB adapter driver to stable branch Christian Gromm
  2020-05-14  9:52 ` [PATCH v2 1/8] drivers: most: add usb adapter driver Christian Gromm
@ 2020-05-14  9:52 ` Christian Gromm
  2020-05-19 13:42   ` Dan Carpenter
  2020-05-14  9:52 ` [PATCH v2 3/8] drivers: most: usb: remove reference to USB error codes Christian Gromm
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Christian Gromm @ 2020-05-14  9:52 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, linux-usb, Christian Gromm

This patch removes the pr_*() functions and uses dev_*() instead.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v2:

 drivers/most/most_usb.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/most/most_usb.c b/drivers/most/most_usb.c
index daa5e4b..0846b38 100644
--- a/drivers/most/most_usb.c
+++ b/drivers/most/most_usb.c
@@ -5,7 +5,6 @@
  * Copyright (C) 2013-2015 Microchip Technology Germany II GmbH & Co. KG
  */
 
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 #include <linux/module.h>
 #include <linux/fs.h>
 #include <linux/usb.h>
@@ -186,13 +185,14 @@ static inline int start_sync_ep(struct usb_device *usb_dev, u16 ep)
  * get_stream_frame_size - calculate frame size of current configuration
  * @cfg: channel configuration
  */
-static unsigned int get_stream_frame_size(struct most_channel_config *cfg)
+static unsigned int get_stream_frame_size(struct most_channel_config *cfg,
+					  struct device *dev)
 {
 	unsigned int frame_size = 0;
 	unsigned int sub_size = cfg->subbuffer_size;
 
 	if (!sub_size) {
-		pr_warn("Misconfig: Subbuffer size zero.\n");
+		dev_warn(dev, "Misconfig: Subbuffer size zero.\n");
 		return frame_size;
 	}
 	switch (cfg->data_type) {
@@ -201,7 +201,7 @@ static unsigned int get_stream_frame_size(struct most_channel_config *cfg)
 		break;
 	case MOST_CH_SYNC:
 		if (cfg->packets_per_xact == 0) {
-			pr_warn("Misconfig: Packets per XACT zero\n");
+			dev_warn(dev, "Misconfig: Packets per XACT zero\n");
 			frame_size = 0;
 		} else if (cfg->packets_per_xact == 0xFF) {
 			frame_size = (USB_MTU / sub_size) * sub_size;
@@ -210,7 +210,7 @@ static unsigned int get_stream_frame_size(struct most_channel_config *cfg)
 		}
 		break;
 	default:
-		pr_warn("Query frame size of non-streaming channel\n");
+		dev_warn(dev, "Query frame size of non-streaming channel\n");
 		break;
 	}
 	return frame_size;
@@ -270,7 +270,7 @@ static int hdm_poison_channel(struct most_interface *iface, int channel)
 static int hdm_add_padding(struct most_dev *mdev, int channel, struct mbo *mbo)
 {
 	struct most_channel_config *conf = &mdev->conf[channel];
-	unsigned int frame_size = get_stream_frame_size(conf);
+	unsigned int frame_size = get_stream_frame_size(conf, &mdev->dev);
 	unsigned int j, num_frames;
 
 	if (!frame_size)
@@ -304,7 +304,7 @@ static int hdm_remove_padding(struct most_dev *mdev, int channel,
 			      struct mbo *mbo)
 {
 	struct most_channel_config *const conf = &mdev->conf[channel];
-	unsigned int frame_size = get_stream_frame_size(conf);
+	unsigned int frame_size = get_stream_frame_size(conf, &mdev->dev);
 	unsigned int j, num_frames;
 
 	if (!frame_size)
@@ -696,7 +696,7 @@ static int hdm_configure_channel(struct most_interface *iface, int channel,
 
 	mdev->padding_active[channel] = true;
 
-	frame_size = get_stream_frame_size(conf);
+	frame_size = get_stream_frame_size(conf, &mdev->dev);
 	if (frame_size == 0 || frame_size > USB_MTU) {
 		dev_warn(dev, "Misconfig: frame size wrong\n");
 		return -EINVAL;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 3/8] drivers: most: usb: remove reference to USB error codes
  2020-05-14  9:52 [PATCH v2 0/8] staging: most: move USB adapter driver to stable branch Christian Gromm
  2020-05-14  9:52 ` [PATCH v2 1/8] drivers: most: add usb adapter driver Christian Gromm
  2020-05-14  9:52 ` [PATCH v2 2/8] drivers: most: usb: use dev_*() functions to print messages Christian Gromm
@ 2020-05-14  9:52 ` Christian Gromm
  2020-05-14  9:52 ` [PATCH v2 4/8] drivers: most: usb: check number of reported endpoints Christian Gromm
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Christian Gromm @ 2020-05-14  9:52 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, linux-usb, Christian Gromm

This patch removes the reference to the driver API file for USB error
codes.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v2:

 drivers/most/most_usb.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/most/most_usb.c b/drivers/most/most_usb.c
index 0846b38..49d0b40 100644
--- a/drivers/most/most_usb.c
+++ b/drivers/most/most_usb.c
@@ -477,8 +477,6 @@ static void hdm_write_completion(struct urb *urb)
  * controller use to indicate a transfer has failed because of device
  * disconnect.  In the interval before the hub driver starts disconnect
  * processing, devices may receive such fault reports for every request.
- *
- * See <https://www.kernel.org/doc/Documentation/driver-api/usb/error-codes.rst>
  */
 static void hdm_read_completion(struct urb *urb)
 {
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 4/8] drivers: most: usb: check number of reported endpoints
  2020-05-14  9:52 [PATCH v2 0/8] staging: most: move USB adapter driver to stable branch Christian Gromm
                   ` (2 preceding siblings ...)
  2020-05-14  9:52 ` [PATCH v2 3/8] drivers: most: usb: remove reference to USB error codes Christian Gromm
@ 2020-05-14  9:52 ` Christian Gromm
  2020-05-14 20:53     ` kbuild test robot
  2020-05-14  9:52 ` [PATCH v2 5/8] drivers: most: usb: use dev_dbg function Christian Gromm
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Christian Gromm @ 2020-05-14  9:52 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, linux-usb, Christian Gromm

This patch checks the number of endpoints reported by the USB
interface descriptor and throws an error if the number exceeds
MAX_NUM_ENDPOINTS.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v2:

 drivers/most/most_usb.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/most/most_usb.c b/drivers/most/most_usb.c
index 49d0b40..1655fcd 100644
--- a/drivers/most/most_usb.c
+++ b/drivers/most/most_usb.c
@@ -1044,13 +1044,17 @@ hdm_probe(struct usb_interface *interface, const struct usb_device_id *id)
 	unsigned int num_endpoints;
 	struct most_channel_capability *tmp_cap;
 	struct usb_endpoint_descriptor *ep_desc;
-	int ret = 0;
+	int ret;
 
 	if (!mdev)
-		goto err_out_of_memory;
+		return -ENOMEM;
 
 	usb_set_intfdata(interface, mdev);
 	num_endpoints = usb_iface_desc->desc.bNumEndpoints;
+	if (num_endpoints > MAX_NUM_ENDPOINTS) {
+		kfree(mdev);
+		return -EINVAL;
+	}
 	mutex_init(&mdev->io_mutex);
 	INIT_WORK(&mdev->poll_work_obj, wq_netinfo);
 	timer_setup(&mdev->link_stat_timer, link_stat_timer_handler, 0);
@@ -1179,11 +1183,6 @@ hdm_probe(struct usb_interface *interface, const struct usb_device_id *id)
 	kfree(mdev->conf);
 err_free_mdev:
 	put_device(&mdev->dev);
-err_out_of_memory:
-	if (ret == 0 || ret == -ENOMEM) {
-		ret = -ENOMEM;
-		dev_err(dev, "out of memory\n");
-	}
 	return ret;
 }
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 5/8] drivers: most: usb: use dev_dbg function
  2020-05-14  9:52 [PATCH v2 0/8] staging: most: move USB adapter driver to stable branch Christian Gromm
                   ` (3 preceding siblings ...)
  2020-05-14  9:52 ` [PATCH v2 4/8] drivers: most: usb: check number of reported endpoints Christian Gromm
@ 2020-05-14  9:52 ` Christian Gromm
  2020-05-19 13:43   ` Dan Carpenter
  2020-05-14  9:52 ` [PATCH v2 6/8] drivers: most: fix typo in Kconfig Christian Gromm
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Christian Gromm @ 2020-05-14  9:52 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, linux-usb, Christian Gromm

This patch replaces the functions dev_notice with dev_dbg to silence
the driver during normal operation.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v2:

 drivers/most/most_usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/most/most_usb.c b/drivers/most/most_usb.c
index 1655fcd..35620a1 100644
--- a/drivers/most/most_usb.c
+++ b/drivers/most/most_usb.c
@@ -1129,13 +1129,13 @@ hdm_probe(struct usb_interface *interface, const struct usb_device_id *id)
 		init_usb_anchor(&mdev->busy_urbs[i]);
 		spin_lock_init(&mdev->channel_lock[i]);
 	}
-	dev_notice(dev, "claimed gadget: Vendor=%4.4x ProdID=%4.4x Bus=%02x Device=%02x\n",
+	dev_dbg(dev, "claimed gadget: Vendor=%4.4x ProdID=%4.4x Bus=%02x Device=%02x\n",
 		   le16_to_cpu(usb_dev->descriptor.idVendor),
 		   le16_to_cpu(usb_dev->descriptor.idProduct),
 		   usb_dev->bus->busnum,
 		   usb_dev->devnum);
 
-	dev_notice(dev, "device path: /sys/bus/usb/devices/%d-%s:%d.%d\n",
+	dev_dbg(dev, "device path: /sys/bus/usb/devices/%d-%s:%d.%d\n",
 		   usb_dev->bus->busnum,
 		   usb_dev->devpath,
 		   usb_dev->config->desc.bConfigurationValue,
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 6/8] drivers: most: fix typo in Kconfig
  2020-05-14  9:52 [PATCH v2 0/8] staging: most: move USB adapter driver to stable branch Christian Gromm
                   ` (4 preceding siblings ...)
  2020-05-14  9:52 ` [PATCH v2 5/8] drivers: most: usb: use dev_dbg function Christian Gromm
@ 2020-05-14  9:52 ` Christian Gromm
  2020-05-14  9:52 ` [PATCH v2 7/8] drivers: most: usb: use macro ATTRIBUTE_GROUPS Christian Gromm
  2020-05-14  9:52 ` [PATCH v2 8/8] Documentation: ABI: correct sysfs attribute description of MOST driver Christian Gromm
  7 siblings, 0 replies; 16+ messages in thread
From: Christian Gromm @ 2020-05-14  9:52 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, linux-usb, Christian Gromm

This patch corrects the typo in the Kconfig file where it says
tranceiver instead of transceiver.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
Reported-by: Randy Dunlap <rdunlap@infradead.org>
---
v2:

 drivers/most/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/most/Kconfig b/drivers/most/Kconfig
index 8650683..42f042d 100644
--- a/drivers/most/Kconfig
+++ b/drivers/most/Kconfig
@@ -19,7 +19,7 @@ config MOST_USB
 	tristate "USB"
 	depends on USB && NET
 	help
-	  Say Y here if you want to connect via USB to network tranceiver.
+	  Say Y here if you want to connect via USB to network transceiver.
 	  This device driver depends on the networking AIM.
 
 	  To compile this driver as a module, choose M here: the
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 7/8] drivers: most: usb: use macro ATTRIBUTE_GROUPS
  2020-05-14  9:52 [PATCH v2 0/8] staging: most: move USB adapter driver to stable branch Christian Gromm
                   ` (5 preceding siblings ...)
  2020-05-14  9:52 ` [PATCH v2 6/8] drivers: most: fix typo in Kconfig Christian Gromm
@ 2020-05-14  9:52 ` Christian Gromm
  2020-05-14  9:52 ` [PATCH v2 8/8] Documentation: ABI: correct sysfs attribute description of MOST driver Christian Gromm
  7 siblings, 0 replies; 16+ messages in thread
From: Christian Gromm @ 2020-05-14  9:52 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, linux-usb, Christian Gromm

This patch makes use of the macro ATTRIBUTE_GROUPS to create the groups
instead of defining them manually.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v2:

 drivers/most/most_usb.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/most/most_usb.c b/drivers/most/most_usb.c
index 35620a1..dce64bb 100644
--- a/drivers/most/most_usb.c
+++ b/drivers/most/most_usb.c
@@ -999,14 +999,7 @@ static struct attribute *dci_attrs[] = {
 	NULL,
 };
 
-static struct attribute_group dci_attr_group = {
-	.attrs = dci_attrs,
-};
-
-static const struct attribute_group *dci_attr_groups[] = {
-	&dci_attr_group,
-	NULL,
-};
+ATTRIBUTE_GROUPS(dci);
 
 static void release_dci(struct device *dev)
 {
@@ -1159,7 +1152,7 @@ hdm_probe(struct usb_interface *interface, const struct usb_device_id *id)
 
 		mdev->dci->dev.init_name = "dci";
 		mdev->dci->dev.parent = get_device(mdev->iface.dev);
-		mdev->dci->dev.groups = dci_attr_groups;
+		mdev->dci->dev.groups = dci_groups;
 		mdev->dci->dev.release = release_dci;
 		if (device_register(&mdev->dci->dev)) {
 			mutex_unlock(&mdev->io_mutex);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 8/8] Documentation: ABI: correct sysfs attribute description of MOST driver
  2020-05-14  9:52 [PATCH v2 0/8] staging: most: move USB adapter driver to stable branch Christian Gromm
                   ` (6 preceding siblings ...)
  2020-05-14  9:52 ` [PATCH v2 7/8] drivers: most: usb: use macro ATTRIBUTE_GROUPS Christian Gromm
@ 2020-05-14  9:52 ` Christian Gromm
  7 siblings, 0 replies; 16+ messages in thread
From: Christian Gromm @ 2020-05-14  9:52 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, linux-usb, Christian Gromm

This patch fixes the ABI description file sysfs-bus-most.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v2:

 Documentation/ABI/testing/sysfs-bus-most | 104 ++++++++++++++++---------------
 1 file changed, 53 insertions(+), 51 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-most b/Documentation/ABI/testing/sysfs-bus-most
index 6b1d06e..ec0a603 100644
--- a/Documentation/ABI/testing/sysfs-bus-most
+++ b/Documentation/ABI/testing/sysfs-bus-most
@@ -1,14 +1,14 @@
-What:		/sys/bus/most/devices/.../description
+What:		/sys/bus/most/devices/<dev>/description
 Date:		March 2017
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
 Description:
-		Provides information about the interface type and the physical
-		location of the device. Hardware attached via USB, for instance,
+		Provides information about the physical location of the
+		device. Hardware attached via USB, for instance,
 		might return <1-1.1:1.0>
 Users:
 
-What:		/sys/bus/most/devices/.../interface
+What:		/sys/bus/most/devices/<dev>/interface
 Date:		March 2017
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -16,7 +16,7 @@ Description:
 		Indicates the type of peripheral interface the device uses.
 Users:
 
-What:		/sys/bus/most/devices/.../dci
+What:		/sys/bus/most/devices/<dev>/dci
 Date:		June 2016
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -26,7 +26,7 @@ Description:
 		write the controller's DCI registers.
 Users:
 
-What:		/sys/bus/most/devices/.../dci/arb_address
+What:		/sys/bus/most/devices/<dev>/dci/arb_address
 Date:		June 2016
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -35,7 +35,7 @@ Description:
 		application wants to read from or write to.
 Users:
 
-What:		/sys/bus/most/devices/.../dci/arb_value
+What:		/sys/bus/most/devices/<dev>/dci/arb_value
 Date:		June 2016
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -44,7 +44,7 @@ Description:
 		is stored in arb_address.
 Users:
 
-What:		/sys/bus/most/devices/.../dci/mep_eui48_hi
+What:		/sys/bus/most/devices/<dev>/dci/mep_eui48_hi
 Date:		June 2016
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -52,7 +52,7 @@ Description:
 		This is used to check and configure the MAC address.
 Users:
 
-What:		/sys/bus/most/devices/.../dci/mep_eui48_lo
+What:		/sys/bus/most/devices/<dev>/dci/mep_eui48_lo
 Date:		June 2016
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -60,7 +60,7 @@ Description:
 		This is used to check and configure the MAC address.
 Users:
 
-What:		/sys/bus/most/devices/.../dci/mep_eui48_mi
+What:		/sys/bus/most/devices/<dev>/dci/mep_eui48_mi
 Date:		June 2016
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -68,7 +68,7 @@ Description:
 		This is used to check and configure the MAC address.
 Users:
 
-What:		/sys/bus/most/devices/.../dci/mep_filter
+What:		/sys/bus/most/devices/<dev>/dci/mep_filter
 Date:		June 2016
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -76,7 +76,7 @@ Description:
 		This is used to check and configure the MEP filter address.
 Users:
 
-What:		/sys/bus/most/devices/.../dci/mep_hash0
+What:		/sys/bus/most/devices/<dev>/dci/mep_hash0
 Date:		June 2016
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -84,7 +84,7 @@ Description:
 		This is used to check and configure the MEP hash table.
 Users:
 
-What:		/sys/bus/most/devices/.../dci/mep_hash1
+What:		/sys/bus/most/devices/<dev>/dci/mep_hash1
 Date:		June 2016
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -92,7 +92,7 @@ Description:
 		This is used to check and configure the MEP hash table.
 Users:
 
-What:		/sys/bus/most/devices/.../dci/mep_hash2
+What:		/sys/bus/most/devices/<dev>/dci/mep_hash2
 Date:		June 2016
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -100,7 +100,7 @@ Description:
 		This is used to check and configure the MEP hash table.
 Users:
 
-What:		/sys/bus/most/devices/.../dci/mep_hash3
+What:		/sys/bus/most/devices/<dev>/dci/mep_hash3
 Date:		June 2016
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -108,7 +108,7 @@ Description:
 		This is used to check and configure the MEP hash table.
 Users:
 
-What:		/sys/bus/most/devices/.../dci/ni_state
+What:		/sys/bus/most/devices/<dev>/dci/ni_state
 Date:		June 2016
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -116,7 +116,7 @@ Description:
 		Indicates the current network interface state.
 Users:
 
-What:		/sys/bus/most/devices/.../dci/node_address
+What:		/sys/bus/most/devices/<dev>/dci/node_address
 Date:		June 2016
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -124,7 +124,7 @@ Description:
 		Indicates the current node address.
 Users:
 
-What:		/sys/bus/most/devices/.../dci/node_position
+What:		/sys/bus/most/devices/<dev>/dci/node_position
 Date:		June 2016
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -132,7 +132,7 @@ Description:
 		Indicates the current node position.
 Users:
 
-What:		/sys/bus/most/devices/.../dci/packet_bandwidth
+What:		/sys/bus/most/devices/<dev>/dci/packet_bandwidth
 Date:		June 2016
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -140,7 +140,7 @@ Description:
 		Indicates the configured packet bandwidth.
 Users:
 
-What:		/sys/bus/most/devices/.../dci/sync_ep
+What:		/sys/bus/most/devices/<dev>/dci/sync_ep
 Date:		June 2016
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -149,7 +149,7 @@ Description:
 		endpoint.
 Users:
 
-What:		/sys/bus/most/devices/.../<channel>/
+What:		/sys/bus/most/devices/<dev>/<channel>/
 Date:		March 2017
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -160,91 +160,92 @@ Description:
 		configure it.
 Users:
 
-What:		/sys/bus/most/devices/.../<channel>/available_datatypes
+What:		/sys/bus/most/devices/<dev>/<channel>/available_datatypes
 Date:		March 2017
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
 Description:
-		Indicates the data types the current channel can transport.
+		Indicates the data types the channel can transport.
 Users:
 
-What:		/sys/bus/most/devices/.../<channel>/available_directions
+What:		/sys/bus/most/devices/<dev>/<channel>/available_directions
 Date:		March 2017
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
 Description:
-		Indicates the directions the current channel is capable of.
+		Indicates the directions the channel is capable of.
 Users:
 
-What:		/sys/bus/most/devices/.../<channel>/number_of_packet_buffers
+What:		/sys/bus/most/devices/<dev>/<channel>/number_of_packet_buffers
 Date:		March 2017
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
 Description:
-		Indicates the number of packet buffers the current channel can
+		Indicates the number of packet buffers the channel can
 		handle.
 Users:
 
-What:		/sys/bus/most/devices/.../<channel>/number_of_stream_buffers
+What:		/sys/bus/most/devices/<dev>/<channel>/number_of_stream_buffers
 Date:		March 2017
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
 Description:
-		Indicates the number of streaming buffers the current channel can
+		Indicates the number of streaming buffers the channel can
 		handle.
 Users:
 
-What:		/sys/bus/most/devices/.../<channel>/size_of_packet_buffer
+What:		/sys/bus/most/devices/<dev>/<channel>/size_of_packet_buffer
 Date:		March 2017
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
 Description:
-		Indicates the size of a packet buffer the current channel can
+		Indicates the size of a packet buffer the channel can
 		handle.
 Users:
 
-What:		/sys/bus/most/devices/.../<channel>/size_of_stream_buffer
+What:		/sys/bus/most/devices/<dev>/<channel>/size_of_stream_buffer
 Date:		March 2017
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
 Description:
-		Indicates the size of a streaming buffer the current channel can
+		Indicates the size of a streaming buffer the channel can
 		handle.
 Users:
 
-What:		/sys/bus/most/devices/.../<channel>/set_number_of_buffers
+What:		/sys/bus/most/devices/<dev>/<channel>/set_number_of_buffers
 Date:		March 2017
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
 Description:
-		This is to configure the number of buffers of the current channel.
+		This is to read back the configured number of buffers of
+		the channel.
 Users:
 
-What:		/sys/bus/most/devices/.../<channel>/set_buffer_size
+What:		/sys/bus/most/devices/<dev>/<channel>/set_buffer_size
 Date:		March 2017
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
 Description:
-		This is to configure the size of a buffer of the current channel.
+		This is to read back the configured buffer size of the channel.
 Users:
 
-What:		/sys/bus/most/devices/.../<channel>/set_direction
+What:		/sys/bus/most/devices/<dev>/<channel>/set_direction
 Date:		March 2017
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
 Description:
-		This is to configure the direction of the current channel.
+		This is to read back the configured direction of the channel.
 		The following strings will be accepted:
-			'dir_tx',
-			'dir_rx'
+			'tx',
+			'rx'
 Users:
 
-What:		/sys/bus/most/devices/.../<channel>/set_datatype
+What:		/sys/bus/most/devices/<dev>/<channel>/set_datatype
 Date:		March 2017
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
 Description:
-		This is to configure the data type of the current channel.
+		This is to read back the configured data type of the channel.
 		The following strings will be accepted:
 			'control',
 			'async',
@@ -252,30 +253,31 @@ Description:
 			'isoc_avp'
 Users:
 
-What:		/sys/bus/most/devices/.../<channel>/set_subbuffer_size
+What:		/sys/bus/most/devices/<dev>/<channel>/set_subbuffer_size
 Date:		March 2017
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
 Description:
-		This is to configure the subbuffer size of the current channel.
+		This is to read back the configured subbuffer size of
+		the channel.
 Users:
 
-What:		/sys/bus/most/devices/.../<channel>/set_packets_per_xact
+What:		/sys/bus/most/devices/<dev>/<channel>/set_packets_per_xact
 Date:		March 2017
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
 Description:
-		This is to configure the number of packets per transaction of
-		the current channel. This is only needed network interface
-		controller is attached via USB.
+		This is to read back the configured number of packets per
+		transaction of the channel. This is only applicable when
+		connected via USB.
 Users:
 
-What:		/sys/bus/most/devices/.../<channel>/channel_starving
+What:		/sys/bus/most/devices/<dev>/<channel>/channel_starving
 Date:		March 2017
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
 Description:
-		Indicates whether current channel ran out of buffers.
+		Indicates whether channel ran out of buffers.
 Users:
 
 What:		/sys/bus/most/drivers/most_core/components
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/8] drivers: most: add usb adapter driver
  2020-05-14  9:52 ` [PATCH v2 1/8] drivers: most: add usb adapter driver Christian Gromm
@ 2020-05-14 10:25   ` Greg KH
  2020-05-20 13:17   ` Dan Carpenter
  1 sibling, 0 replies; 16+ messages in thread
From: Greg KH @ 2020-05-14 10:25 UTC (permalink / raw)
  To: Christian Gromm; +Cc: driverdev-devel, linux-usb

On Thu, May 14, 2020 at 11:52:49AM +0200, Christian Gromm wrote:
> This patch adds the usb driver source file most_usb.c and
> modifies the Makefile and Kconfig accordingly.
> 
> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> ---
> v2:
> Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 	- don't remove usb driver from staging area
> 	- don't touch staging/most/Kconfig
> 	- remove subdirectory for USB driver and put source file into
> 	  drivers/most

Hm, no, can you invert this series?  I'll gladly take the "fixes" for
the existing code in staging, and then we can do a new review after that
of the file being added to match what is in staging.  I think you might
have missed at least one change that happened there already :(

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 4/8] drivers: most: usb: check number of reported endpoints
  2020-05-14  9:52 ` [PATCH v2 4/8] drivers: most: usb: check number of reported endpoints Christian Gromm
@ 2020-05-14 20:53     ` kbuild test robot
  0 siblings, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2020-05-14 20:53 UTC (permalink / raw)
  To: Christian Gromm, gregkh
  Cc: kbuild-all, clang-built-linux, driverdev-devel, linux-usb,
	Christian Gromm

[-- Attachment #1: Type: text/plain, Size: 17687 bytes --]

Hi Christian,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on staging/staging-testing]
[also build test WARNING on usb/usb-testing linus/master v5.7-rc5 next-20200514]
[cannot apply to balbi-usb/next peter.chen-usb/ci-for-usb-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Christian-Gromm/staging-most-move-USB-adapter-driver-to-stable-branch/20200514-183525
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 8a01032e02c8a0fb3e9f33791023b62dee73cc03
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project a52f10b5a382c040e7ad1ce933cda6c07a4b3a8d)
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> drivers/most/most_usb.c:1104:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!mdev->busy_urbs)
^~~~~~~~~~~~~~~~
drivers/most/most_usb.c:1186:9: note: uninitialized use occurs here
return ret;
^~~
drivers/most/most_usb.c:1104:2: note: remove the 'if' if its condition is always false
if (!mdev->busy_urbs)
^~~~~~~~~~~~~~~~~~~~~
drivers/most/most_usb.c:1099:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!mdev->ep_address)
^~~~~~~~~~~~~~~~~
drivers/most/most_usb.c:1186:9: note: uninitialized use occurs here
return ret;
^~~
drivers/most/most_usb.c:1099:2: note: remove the 'if' if its condition is always false
if (!mdev->ep_address)
^~~~~~~~~~~~~~~~~~~~~~
drivers/most/most_usb.c:1093:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!mdev->cap)
^~~~~~~~~~
drivers/most/most_usb.c:1186:9: note: uninitialized use occurs here
return ret;
^~~
drivers/most/most_usb.c:1093:2: note: remove the 'if' if its condition is always false
if (!mdev->cap)
^~~~~~~~~~~~~~~
drivers/most/most_usb.c:1089:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!mdev->conf)
^~~~~~~~~~~
drivers/most/most_usb.c:1186:9: note: uninitialized use occurs here
return ret;
^~~
drivers/most/most_usb.c:1089:2: note: remove the 'if' if its condition is always false
if (!mdev->conf)
^~~~~~~~~~~~~~~~
drivers/most/most_usb.c:1047:9: note: initialize the variable 'ret' to silence this warning
int ret;
^
= 0
4 warnings generated.

vim +1104 drivers/most/most_usb.c

56c7d34c835125c6 Christian Gromm 2020-05-14  1017  
56c7d34c835125c6 Christian Gromm 2020-05-14  1018  static void release_mdev(struct device *dev)
56c7d34c835125c6 Christian Gromm 2020-05-14  1019  {
56c7d34c835125c6 Christian Gromm 2020-05-14  1020  	struct most_dev *mdev = to_mdev_from_dev(dev);
56c7d34c835125c6 Christian Gromm 2020-05-14  1021  
56c7d34c835125c6 Christian Gromm 2020-05-14  1022  	kfree(mdev);
56c7d34c835125c6 Christian Gromm 2020-05-14  1023  }
56c7d34c835125c6 Christian Gromm 2020-05-14  1024  /**
56c7d34c835125c6 Christian Gromm 2020-05-14  1025   * hdm_probe - probe function of USB device driver
56c7d34c835125c6 Christian Gromm 2020-05-14  1026   * @interface: Interface of the attached USB device
56c7d34c835125c6 Christian Gromm 2020-05-14  1027   * @id: Pointer to the USB ID table.
56c7d34c835125c6 Christian Gromm 2020-05-14  1028   *
56c7d34c835125c6 Christian Gromm 2020-05-14  1029   * This allocates and initializes the device instance, adds the new
56c7d34c835125c6 Christian Gromm 2020-05-14  1030   * entry to the internal list, scans the USB descriptors and registers
56c7d34c835125c6 Christian Gromm 2020-05-14  1031   * the interface with the core.
56c7d34c835125c6 Christian Gromm 2020-05-14  1032   * Additionally, the DCI objects are created and the hardware is sync'd.
56c7d34c835125c6 Christian Gromm 2020-05-14  1033   *
56c7d34c835125c6 Christian Gromm 2020-05-14  1034   * Return 0 on success. In case of an error a negative number is returned.
56c7d34c835125c6 Christian Gromm 2020-05-14  1035   */
56c7d34c835125c6 Christian Gromm 2020-05-14  1036  static int
56c7d34c835125c6 Christian Gromm 2020-05-14  1037  hdm_probe(struct usb_interface *interface, const struct usb_device_id *id)
56c7d34c835125c6 Christian Gromm 2020-05-14  1038  {
56c7d34c835125c6 Christian Gromm 2020-05-14  1039  	struct usb_host_interface *usb_iface_desc = interface->cur_altsetting;
56c7d34c835125c6 Christian Gromm 2020-05-14  1040  	struct usb_device *usb_dev = interface_to_usbdev(interface);
56c7d34c835125c6 Christian Gromm 2020-05-14  1041  	struct device *dev = &usb_dev->dev;
56c7d34c835125c6 Christian Gromm 2020-05-14  1042  	struct most_dev *mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
56c7d34c835125c6 Christian Gromm 2020-05-14  1043  	unsigned int i;
56c7d34c835125c6 Christian Gromm 2020-05-14  1044  	unsigned int num_endpoints;
56c7d34c835125c6 Christian Gromm 2020-05-14  1045  	struct most_channel_capability *tmp_cap;
56c7d34c835125c6 Christian Gromm 2020-05-14  1046  	struct usb_endpoint_descriptor *ep_desc;
220484e072685b00 Christian Gromm 2020-05-14  1047  	int ret;
56c7d34c835125c6 Christian Gromm 2020-05-14  1048  
56c7d34c835125c6 Christian Gromm 2020-05-14  1049  	if (!mdev)
220484e072685b00 Christian Gromm 2020-05-14  1050  		return -ENOMEM;
56c7d34c835125c6 Christian Gromm 2020-05-14  1051  
56c7d34c835125c6 Christian Gromm 2020-05-14  1052  	usb_set_intfdata(interface, mdev);
56c7d34c835125c6 Christian Gromm 2020-05-14  1053  	num_endpoints = usb_iface_desc->desc.bNumEndpoints;
220484e072685b00 Christian Gromm 2020-05-14  1054  	if (num_endpoints > MAX_NUM_ENDPOINTS) {
220484e072685b00 Christian Gromm 2020-05-14  1055  		kfree(mdev);
220484e072685b00 Christian Gromm 2020-05-14  1056  		return -EINVAL;
220484e072685b00 Christian Gromm 2020-05-14  1057  	}
56c7d34c835125c6 Christian Gromm 2020-05-14  1058  	mutex_init(&mdev->io_mutex);
56c7d34c835125c6 Christian Gromm 2020-05-14  1059  	INIT_WORK(&mdev->poll_work_obj, wq_netinfo);
56c7d34c835125c6 Christian Gromm 2020-05-14  1060  	timer_setup(&mdev->link_stat_timer, link_stat_timer_handler, 0);
56c7d34c835125c6 Christian Gromm 2020-05-14  1061  
56c7d34c835125c6 Christian Gromm 2020-05-14  1062  	mdev->usb_device = usb_dev;
56c7d34c835125c6 Christian Gromm 2020-05-14  1063  	mdev->link_stat_timer.expires = jiffies + (2 * HZ);
56c7d34c835125c6 Christian Gromm 2020-05-14  1064  
56c7d34c835125c6 Christian Gromm 2020-05-14  1065  	mdev->iface.mod = hdm_usb_fops.owner;
56c7d34c835125c6 Christian Gromm 2020-05-14  1066  	mdev->iface.dev = &mdev->dev;
56c7d34c835125c6 Christian Gromm 2020-05-14  1067  	mdev->iface.driver_dev = &interface->dev;
56c7d34c835125c6 Christian Gromm 2020-05-14  1068  	mdev->iface.interface = ITYPE_USB;
56c7d34c835125c6 Christian Gromm 2020-05-14  1069  	mdev->iface.configure = hdm_configure_channel;
56c7d34c835125c6 Christian Gromm 2020-05-14  1070  	mdev->iface.request_netinfo = hdm_request_netinfo;
56c7d34c835125c6 Christian Gromm 2020-05-14  1071  	mdev->iface.enqueue = hdm_enqueue;
56c7d34c835125c6 Christian Gromm 2020-05-14  1072  	mdev->iface.poison_channel = hdm_poison_channel;
56c7d34c835125c6 Christian Gromm 2020-05-14  1073  	mdev->iface.dma_alloc = hdm_dma_alloc;
56c7d34c835125c6 Christian Gromm 2020-05-14  1074  	mdev->iface.dma_free = hdm_dma_free;
56c7d34c835125c6 Christian Gromm 2020-05-14  1075  	mdev->iface.description = mdev->description;
56c7d34c835125c6 Christian Gromm 2020-05-14  1076  	mdev->iface.num_channels = num_endpoints;
56c7d34c835125c6 Christian Gromm 2020-05-14  1077  
56c7d34c835125c6 Christian Gromm 2020-05-14  1078  	snprintf(mdev->description, sizeof(mdev->description),
56c7d34c835125c6 Christian Gromm 2020-05-14  1079  		 "%d-%s:%d.%d",
56c7d34c835125c6 Christian Gromm 2020-05-14  1080  		 usb_dev->bus->busnum,
56c7d34c835125c6 Christian Gromm 2020-05-14  1081  		 usb_dev->devpath,
56c7d34c835125c6 Christian Gromm 2020-05-14  1082  		 usb_dev->config->desc.bConfigurationValue,
56c7d34c835125c6 Christian Gromm 2020-05-14  1083  		 usb_iface_desc->desc.bInterfaceNumber);
56c7d34c835125c6 Christian Gromm 2020-05-14  1084  
56c7d34c835125c6 Christian Gromm 2020-05-14  1085  	mdev->dev.init_name = mdev->description;
56c7d34c835125c6 Christian Gromm 2020-05-14  1086  	mdev->dev.parent = &interface->dev;
56c7d34c835125c6 Christian Gromm 2020-05-14  1087  	mdev->dev.release = release_mdev;
56c7d34c835125c6 Christian Gromm 2020-05-14  1088  	mdev->conf = kcalloc(num_endpoints, sizeof(*mdev->conf), GFP_KERNEL);
56c7d34c835125c6 Christian Gromm 2020-05-14  1089  	if (!mdev->conf)
56c7d34c835125c6 Christian Gromm 2020-05-14  1090  		goto err_free_mdev;
56c7d34c835125c6 Christian Gromm 2020-05-14  1091  
56c7d34c835125c6 Christian Gromm 2020-05-14  1092  	mdev->cap = kcalloc(num_endpoints, sizeof(*mdev->cap), GFP_KERNEL);
56c7d34c835125c6 Christian Gromm 2020-05-14  1093  	if (!mdev->cap)
56c7d34c835125c6 Christian Gromm 2020-05-14  1094  		goto err_free_conf;
56c7d34c835125c6 Christian Gromm 2020-05-14  1095  
56c7d34c835125c6 Christian Gromm 2020-05-14  1096  	mdev->iface.channel_vector = mdev->cap;
56c7d34c835125c6 Christian Gromm 2020-05-14  1097  	mdev->ep_address =
56c7d34c835125c6 Christian Gromm 2020-05-14  1098  		kcalloc(num_endpoints, sizeof(*mdev->ep_address), GFP_KERNEL);
56c7d34c835125c6 Christian Gromm 2020-05-14  1099  	if (!mdev->ep_address)
56c7d34c835125c6 Christian Gromm 2020-05-14  1100  		goto err_free_cap;
56c7d34c835125c6 Christian Gromm 2020-05-14  1101  
56c7d34c835125c6 Christian Gromm 2020-05-14  1102  	mdev->busy_urbs =
56c7d34c835125c6 Christian Gromm 2020-05-14  1103  		kcalloc(num_endpoints, sizeof(*mdev->busy_urbs), GFP_KERNEL);
56c7d34c835125c6 Christian Gromm 2020-05-14 @1104  	if (!mdev->busy_urbs)
56c7d34c835125c6 Christian Gromm 2020-05-14  1105  		goto err_free_ep_address;
56c7d34c835125c6 Christian Gromm 2020-05-14  1106  
56c7d34c835125c6 Christian Gromm 2020-05-14  1107  	tmp_cap = mdev->cap;
56c7d34c835125c6 Christian Gromm 2020-05-14  1108  	for (i = 0; i < num_endpoints; i++) {
56c7d34c835125c6 Christian Gromm 2020-05-14  1109  		ep_desc = &usb_iface_desc->endpoint[i].desc;
56c7d34c835125c6 Christian Gromm 2020-05-14  1110  		mdev->ep_address[i] = ep_desc->bEndpointAddress;
56c7d34c835125c6 Christian Gromm 2020-05-14  1111  		mdev->padding_active[i] = false;
56c7d34c835125c6 Christian Gromm 2020-05-14  1112  		mdev->is_channel_healthy[i] = true;
56c7d34c835125c6 Christian Gromm 2020-05-14  1113  
56c7d34c835125c6 Christian Gromm 2020-05-14  1114  		snprintf(&mdev->suffix[i][0], MAX_SUFFIX_LEN, "ep%02x",
56c7d34c835125c6 Christian Gromm 2020-05-14  1115  			 mdev->ep_address[i]);
56c7d34c835125c6 Christian Gromm 2020-05-14  1116  
56c7d34c835125c6 Christian Gromm 2020-05-14  1117  		tmp_cap->name_suffix = &mdev->suffix[i][0];
56c7d34c835125c6 Christian Gromm 2020-05-14  1118  		tmp_cap->buffer_size_packet = MAX_BUF_SIZE;
56c7d34c835125c6 Christian Gromm 2020-05-14  1119  		tmp_cap->buffer_size_streaming = MAX_BUF_SIZE;
56c7d34c835125c6 Christian Gromm 2020-05-14  1120  		tmp_cap->num_buffers_packet = BUF_CHAIN_SIZE;
56c7d34c835125c6 Christian Gromm 2020-05-14  1121  		tmp_cap->num_buffers_streaming = BUF_CHAIN_SIZE;
56c7d34c835125c6 Christian Gromm 2020-05-14  1122  		tmp_cap->data_type = MOST_CH_CONTROL | MOST_CH_ASYNC |
56c7d34c835125c6 Christian Gromm 2020-05-14  1123  				     MOST_CH_ISOC | MOST_CH_SYNC;
56c7d34c835125c6 Christian Gromm 2020-05-14  1124  		if (usb_endpoint_dir_in(ep_desc))
56c7d34c835125c6 Christian Gromm 2020-05-14  1125  			tmp_cap->direction = MOST_CH_RX;
56c7d34c835125c6 Christian Gromm 2020-05-14  1126  		else
56c7d34c835125c6 Christian Gromm 2020-05-14  1127  			tmp_cap->direction = MOST_CH_TX;
56c7d34c835125c6 Christian Gromm 2020-05-14  1128  		tmp_cap++;
56c7d34c835125c6 Christian Gromm 2020-05-14  1129  		init_usb_anchor(&mdev->busy_urbs[i]);
56c7d34c835125c6 Christian Gromm 2020-05-14  1130  		spin_lock_init(&mdev->channel_lock[i]);
56c7d34c835125c6 Christian Gromm 2020-05-14  1131  	}
56c7d34c835125c6 Christian Gromm 2020-05-14  1132  	dev_notice(dev, "claimed gadget: Vendor=%4.4x ProdID=%4.4x Bus=%02x Device=%02x\n",
56c7d34c835125c6 Christian Gromm 2020-05-14  1133  		   le16_to_cpu(usb_dev->descriptor.idVendor),
56c7d34c835125c6 Christian Gromm 2020-05-14  1134  		   le16_to_cpu(usb_dev->descriptor.idProduct),
56c7d34c835125c6 Christian Gromm 2020-05-14  1135  		   usb_dev->bus->busnum,
56c7d34c835125c6 Christian Gromm 2020-05-14  1136  		   usb_dev->devnum);
56c7d34c835125c6 Christian Gromm 2020-05-14  1137  
56c7d34c835125c6 Christian Gromm 2020-05-14  1138  	dev_notice(dev, "device path: /sys/bus/usb/devices/%d-%s:%d.%d\n",
56c7d34c835125c6 Christian Gromm 2020-05-14  1139  		   usb_dev->bus->busnum,
56c7d34c835125c6 Christian Gromm 2020-05-14  1140  		   usb_dev->devpath,
56c7d34c835125c6 Christian Gromm 2020-05-14  1141  		   usb_dev->config->desc.bConfigurationValue,
56c7d34c835125c6 Christian Gromm 2020-05-14  1142  		   usb_iface_desc->desc.bInterfaceNumber);
56c7d34c835125c6 Christian Gromm 2020-05-14  1143  
56c7d34c835125c6 Christian Gromm 2020-05-14  1144  	ret = most_register_interface(&mdev->iface);
56c7d34c835125c6 Christian Gromm 2020-05-14  1145  	if (ret)
56c7d34c835125c6 Christian Gromm 2020-05-14  1146  		goto err_free_busy_urbs;
56c7d34c835125c6 Christian Gromm 2020-05-14  1147  
56c7d34c835125c6 Christian Gromm 2020-05-14  1148  	mutex_lock(&mdev->io_mutex);
56c7d34c835125c6 Christian Gromm 2020-05-14  1149  	if (le16_to_cpu(usb_dev->descriptor.idProduct) == USB_DEV_ID_OS81118 ||
56c7d34c835125c6 Christian Gromm 2020-05-14  1150  	    le16_to_cpu(usb_dev->descriptor.idProduct) == USB_DEV_ID_OS81119 ||
56c7d34c835125c6 Christian Gromm 2020-05-14  1151  	    le16_to_cpu(usb_dev->descriptor.idProduct) == USB_DEV_ID_OS81210) {
56c7d34c835125c6 Christian Gromm 2020-05-14  1152  		mdev->dci = kzalloc(sizeof(*mdev->dci), GFP_KERNEL);
56c7d34c835125c6 Christian Gromm 2020-05-14  1153  		if (!mdev->dci) {
56c7d34c835125c6 Christian Gromm 2020-05-14  1154  			mutex_unlock(&mdev->io_mutex);
56c7d34c835125c6 Christian Gromm 2020-05-14  1155  			most_deregister_interface(&mdev->iface);
56c7d34c835125c6 Christian Gromm 2020-05-14  1156  			ret = -ENOMEM;
56c7d34c835125c6 Christian Gromm 2020-05-14  1157  			goto err_free_busy_urbs;
56c7d34c835125c6 Christian Gromm 2020-05-14  1158  		}
56c7d34c835125c6 Christian Gromm 2020-05-14  1159  
56c7d34c835125c6 Christian Gromm 2020-05-14  1160  		mdev->dci->dev.init_name = "dci";
56c7d34c835125c6 Christian Gromm 2020-05-14  1161  		mdev->dci->dev.parent = get_device(mdev->iface.dev);
56c7d34c835125c6 Christian Gromm 2020-05-14  1162  		mdev->dci->dev.groups = dci_attr_groups;
56c7d34c835125c6 Christian Gromm 2020-05-14  1163  		mdev->dci->dev.release = release_dci;
56c7d34c835125c6 Christian Gromm 2020-05-14  1164  		if (device_register(&mdev->dci->dev)) {
56c7d34c835125c6 Christian Gromm 2020-05-14  1165  			mutex_unlock(&mdev->io_mutex);
56c7d34c835125c6 Christian Gromm 2020-05-14  1166  			most_deregister_interface(&mdev->iface);
56c7d34c835125c6 Christian Gromm 2020-05-14  1167  			ret = -ENOMEM;
56c7d34c835125c6 Christian Gromm 2020-05-14  1168  			goto err_free_dci;
56c7d34c835125c6 Christian Gromm 2020-05-14  1169  		}
56c7d34c835125c6 Christian Gromm 2020-05-14  1170  		mdev->dci->usb_device = mdev->usb_device;
56c7d34c835125c6 Christian Gromm 2020-05-14  1171  	}
56c7d34c835125c6 Christian Gromm 2020-05-14  1172  	mutex_unlock(&mdev->io_mutex);
56c7d34c835125c6 Christian Gromm 2020-05-14  1173  	return 0;
56c7d34c835125c6 Christian Gromm 2020-05-14  1174  err_free_dci:
56c7d34c835125c6 Christian Gromm 2020-05-14  1175  	put_device(&mdev->dci->dev);
56c7d34c835125c6 Christian Gromm 2020-05-14  1176  err_free_busy_urbs:
56c7d34c835125c6 Christian Gromm 2020-05-14  1177  	kfree(mdev->busy_urbs);
56c7d34c835125c6 Christian Gromm 2020-05-14  1178  err_free_ep_address:
56c7d34c835125c6 Christian Gromm 2020-05-14  1179  	kfree(mdev->ep_address);
56c7d34c835125c6 Christian Gromm 2020-05-14  1180  err_free_cap:
56c7d34c835125c6 Christian Gromm 2020-05-14  1181  	kfree(mdev->cap);
56c7d34c835125c6 Christian Gromm 2020-05-14  1182  err_free_conf:
56c7d34c835125c6 Christian Gromm 2020-05-14  1183  	kfree(mdev->conf);
56c7d34c835125c6 Christian Gromm 2020-05-14  1184  err_free_mdev:
56c7d34c835125c6 Christian Gromm 2020-05-14  1185  	put_device(&mdev->dev);
56c7d34c835125c6 Christian Gromm 2020-05-14  1186  	return ret;
56c7d34c835125c6 Christian Gromm 2020-05-14  1187  }
56c7d34c835125c6 Christian Gromm 2020-05-14  1188  

:::::: The code at line 1104 was first introduced by commit
:::::: 56c7d34c835125c6587fb28f67cddd1d4062975f drivers: most: add usb adapter driver

:::::: TO: Christian Gromm <christian.gromm@microchip.com>
:::::: CC: 0day robot <lkp@intel.com>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 73441 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 4/8] drivers: most: usb: check number of reported endpoints
@ 2020-05-14 20:53     ` kbuild test robot
  0 siblings, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2020-05-14 20:53 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 17943 bytes --]

Hi Christian,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on staging/staging-testing]
[also build test WARNING on usb/usb-testing linus/master v5.7-rc5 next-20200514]
[cannot apply to balbi-usb/next peter.chen-usb/ci-for-usb-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Christian-Gromm/staging-most-move-USB-adapter-driver-to-stable-branch/20200514-183525
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 8a01032e02c8a0fb3e9f33791023b62dee73cc03
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project a52f10b5a382c040e7ad1ce933cda6c07a4b3a8d)
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> drivers/most/most_usb.c:1104:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!mdev->busy_urbs)
^~~~~~~~~~~~~~~~
drivers/most/most_usb.c:1186:9: note: uninitialized use occurs here
return ret;
^~~
drivers/most/most_usb.c:1104:2: note: remove the 'if' if its condition is always false
if (!mdev->busy_urbs)
^~~~~~~~~~~~~~~~~~~~~
drivers/most/most_usb.c:1099:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!mdev->ep_address)
^~~~~~~~~~~~~~~~~
drivers/most/most_usb.c:1186:9: note: uninitialized use occurs here
return ret;
^~~
drivers/most/most_usb.c:1099:2: note: remove the 'if' if its condition is always false
if (!mdev->ep_address)
^~~~~~~~~~~~~~~~~~~~~~
drivers/most/most_usb.c:1093:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!mdev->cap)
^~~~~~~~~~
drivers/most/most_usb.c:1186:9: note: uninitialized use occurs here
return ret;
^~~
drivers/most/most_usb.c:1093:2: note: remove the 'if' if its condition is always false
if (!mdev->cap)
^~~~~~~~~~~~~~~
drivers/most/most_usb.c:1089:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!mdev->conf)
^~~~~~~~~~~
drivers/most/most_usb.c:1186:9: note: uninitialized use occurs here
return ret;
^~~
drivers/most/most_usb.c:1089:2: note: remove the 'if' if its condition is always false
if (!mdev->conf)
^~~~~~~~~~~~~~~~
drivers/most/most_usb.c:1047:9: note: initialize the variable 'ret' to silence this warning
int ret;
^
= 0
4 warnings generated.

vim +1104 drivers/most/most_usb.c

56c7d34c835125c6 Christian Gromm 2020-05-14  1017  
56c7d34c835125c6 Christian Gromm 2020-05-14  1018  static void release_mdev(struct device *dev)
56c7d34c835125c6 Christian Gromm 2020-05-14  1019  {
56c7d34c835125c6 Christian Gromm 2020-05-14  1020  	struct most_dev *mdev = to_mdev_from_dev(dev);
56c7d34c835125c6 Christian Gromm 2020-05-14  1021  
56c7d34c835125c6 Christian Gromm 2020-05-14  1022  	kfree(mdev);
56c7d34c835125c6 Christian Gromm 2020-05-14  1023  }
56c7d34c835125c6 Christian Gromm 2020-05-14  1024  /**
56c7d34c835125c6 Christian Gromm 2020-05-14  1025   * hdm_probe - probe function of USB device driver
56c7d34c835125c6 Christian Gromm 2020-05-14  1026   * @interface: Interface of the attached USB device
56c7d34c835125c6 Christian Gromm 2020-05-14  1027   * @id: Pointer to the USB ID table.
56c7d34c835125c6 Christian Gromm 2020-05-14  1028   *
56c7d34c835125c6 Christian Gromm 2020-05-14  1029   * This allocates and initializes the device instance, adds the new
56c7d34c835125c6 Christian Gromm 2020-05-14  1030   * entry to the internal list, scans the USB descriptors and registers
56c7d34c835125c6 Christian Gromm 2020-05-14  1031   * the interface with the core.
56c7d34c835125c6 Christian Gromm 2020-05-14  1032   * Additionally, the DCI objects are created and the hardware is sync'd.
56c7d34c835125c6 Christian Gromm 2020-05-14  1033   *
56c7d34c835125c6 Christian Gromm 2020-05-14  1034   * Return 0 on success. In case of an error a negative number is returned.
56c7d34c835125c6 Christian Gromm 2020-05-14  1035   */
56c7d34c835125c6 Christian Gromm 2020-05-14  1036  static int
56c7d34c835125c6 Christian Gromm 2020-05-14  1037  hdm_probe(struct usb_interface *interface, const struct usb_device_id *id)
56c7d34c835125c6 Christian Gromm 2020-05-14  1038  {
56c7d34c835125c6 Christian Gromm 2020-05-14  1039  	struct usb_host_interface *usb_iface_desc = interface->cur_altsetting;
56c7d34c835125c6 Christian Gromm 2020-05-14  1040  	struct usb_device *usb_dev = interface_to_usbdev(interface);
56c7d34c835125c6 Christian Gromm 2020-05-14  1041  	struct device *dev = &usb_dev->dev;
56c7d34c835125c6 Christian Gromm 2020-05-14  1042  	struct most_dev *mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
56c7d34c835125c6 Christian Gromm 2020-05-14  1043  	unsigned int i;
56c7d34c835125c6 Christian Gromm 2020-05-14  1044  	unsigned int num_endpoints;
56c7d34c835125c6 Christian Gromm 2020-05-14  1045  	struct most_channel_capability *tmp_cap;
56c7d34c835125c6 Christian Gromm 2020-05-14  1046  	struct usb_endpoint_descriptor *ep_desc;
220484e072685b00 Christian Gromm 2020-05-14  1047  	int ret;
56c7d34c835125c6 Christian Gromm 2020-05-14  1048  
56c7d34c835125c6 Christian Gromm 2020-05-14  1049  	if (!mdev)
220484e072685b00 Christian Gromm 2020-05-14  1050  		return -ENOMEM;
56c7d34c835125c6 Christian Gromm 2020-05-14  1051  
56c7d34c835125c6 Christian Gromm 2020-05-14  1052  	usb_set_intfdata(interface, mdev);
56c7d34c835125c6 Christian Gromm 2020-05-14  1053  	num_endpoints = usb_iface_desc->desc.bNumEndpoints;
220484e072685b00 Christian Gromm 2020-05-14  1054  	if (num_endpoints > MAX_NUM_ENDPOINTS) {
220484e072685b00 Christian Gromm 2020-05-14  1055  		kfree(mdev);
220484e072685b00 Christian Gromm 2020-05-14  1056  		return -EINVAL;
220484e072685b00 Christian Gromm 2020-05-14  1057  	}
56c7d34c835125c6 Christian Gromm 2020-05-14  1058  	mutex_init(&mdev->io_mutex);
56c7d34c835125c6 Christian Gromm 2020-05-14  1059  	INIT_WORK(&mdev->poll_work_obj, wq_netinfo);
56c7d34c835125c6 Christian Gromm 2020-05-14  1060  	timer_setup(&mdev->link_stat_timer, link_stat_timer_handler, 0);
56c7d34c835125c6 Christian Gromm 2020-05-14  1061  
56c7d34c835125c6 Christian Gromm 2020-05-14  1062  	mdev->usb_device = usb_dev;
56c7d34c835125c6 Christian Gromm 2020-05-14  1063  	mdev->link_stat_timer.expires = jiffies + (2 * HZ);
56c7d34c835125c6 Christian Gromm 2020-05-14  1064  
56c7d34c835125c6 Christian Gromm 2020-05-14  1065  	mdev->iface.mod = hdm_usb_fops.owner;
56c7d34c835125c6 Christian Gromm 2020-05-14  1066  	mdev->iface.dev = &mdev->dev;
56c7d34c835125c6 Christian Gromm 2020-05-14  1067  	mdev->iface.driver_dev = &interface->dev;
56c7d34c835125c6 Christian Gromm 2020-05-14  1068  	mdev->iface.interface = ITYPE_USB;
56c7d34c835125c6 Christian Gromm 2020-05-14  1069  	mdev->iface.configure = hdm_configure_channel;
56c7d34c835125c6 Christian Gromm 2020-05-14  1070  	mdev->iface.request_netinfo = hdm_request_netinfo;
56c7d34c835125c6 Christian Gromm 2020-05-14  1071  	mdev->iface.enqueue = hdm_enqueue;
56c7d34c835125c6 Christian Gromm 2020-05-14  1072  	mdev->iface.poison_channel = hdm_poison_channel;
56c7d34c835125c6 Christian Gromm 2020-05-14  1073  	mdev->iface.dma_alloc = hdm_dma_alloc;
56c7d34c835125c6 Christian Gromm 2020-05-14  1074  	mdev->iface.dma_free = hdm_dma_free;
56c7d34c835125c6 Christian Gromm 2020-05-14  1075  	mdev->iface.description = mdev->description;
56c7d34c835125c6 Christian Gromm 2020-05-14  1076  	mdev->iface.num_channels = num_endpoints;
56c7d34c835125c6 Christian Gromm 2020-05-14  1077  
56c7d34c835125c6 Christian Gromm 2020-05-14  1078  	snprintf(mdev->description, sizeof(mdev->description),
56c7d34c835125c6 Christian Gromm 2020-05-14  1079  		 "%d-%s:%d.%d",
56c7d34c835125c6 Christian Gromm 2020-05-14  1080  		 usb_dev->bus->busnum,
56c7d34c835125c6 Christian Gromm 2020-05-14  1081  		 usb_dev->devpath,
56c7d34c835125c6 Christian Gromm 2020-05-14  1082  		 usb_dev->config->desc.bConfigurationValue,
56c7d34c835125c6 Christian Gromm 2020-05-14  1083  		 usb_iface_desc->desc.bInterfaceNumber);
56c7d34c835125c6 Christian Gromm 2020-05-14  1084  
56c7d34c835125c6 Christian Gromm 2020-05-14  1085  	mdev->dev.init_name = mdev->description;
56c7d34c835125c6 Christian Gromm 2020-05-14  1086  	mdev->dev.parent = &interface->dev;
56c7d34c835125c6 Christian Gromm 2020-05-14  1087  	mdev->dev.release = release_mdev;
56c7d34c835125c6 Christian Gromm 2020-05-14  1088  	mdev->conf = kcalloc(num_endpoints, sizeof(*mdev->conf), GFP_KERNEL);
56c7d34c835125c6 Christian Gromm 2020-05-14  1089  	if (!mdev->conf)
56c7d34c835125c6 Christian Gromm 2020-05-14  1090  		goto err_free_mdev;
56c7d34c835125c6 Christian Gromm 2020-05-14  1091  
56c7d34c835125c6 Christian Gromm 2020-05-14  1092  	mdev->cap = kcalloc(num_endpoints, sizeof(*mdev->cap), GFP_KERNEL);
56c7d34c835125c6 Christian Gromm 2020-05-14  1093  	if (!mdev->cap)
56c7d34c835125c6 Christian Gromm 2020-05-14  1094  		goto err_free_conf;
56c7d34c835125c6 Christian Gromm 2020-05-14  1095  
56c7d34c835125c6 Christian Gromm 2020-05-14  1096  	mdev->iface.channel_vector = mdev->cap;
56c7d34c835125c6 Christian Gromm 2020-05-14  1097  	mdev->ep_address =
56c7d34c835125c6 Christian Gromm 2020-05-14  1098  		kcalloc(num_endpoints, sizeof(*mdev->ep_address), GFP_KERNEL);
56c7d34c835125c6 Christian Gromm 2020-05-14  1099  	if (!mdev->ep_address)
56c7d34c835125c6 Christian Gromm 2020-05-14  1100  		goto err_free_cap;
56c7d34c835125c6 Christian Gromm 2020-05-14  1101  
56c7d34c835125c6 Christian Gromm 2020-05-14  1102  	mdev->busy_urbs =
56c7d34c835125c6 Christian Gromm 2020-05-14  1103  		kcalloc(num_endpoints, sizeof(*mdev->busy_urbs), GFP_KERNEL);
56c7d34c835125c6 Christian Gromm 2020-05-14 @1104  	if (!mdev->busy_urbs)
56c7d34c835125c6 Christian Gromm 2020-05-14  1105  		goto err_free_ep_address;
56c7d34c835125c6 Christian Gromm 2020-05-14  1106  
56c7d34c835125c6 Christian Gromm 2020-05-14  1107  	tmp_cap = mdev->cap;
56c7d34c835125c6 Christian Gromm 2020-05-14  1108  	for (i = 0; i < num_endpoints; i++) {
56c7d34c835125c6 Christian Gromm 2020-05-14  1109  		ep_desc = &usb_iface_desc->endpoint[i].desc;
56c7d34c835125c6 Christian Gromm 2020-05-14  1110  		mdev->ep_address[i] = ep_desc->bEndpointAddress;
56c7d34c835125c6 Christian Gromm 2020-05-14  1111  		mdev->padding_active[i] = false;
56c7d34c835125c6 Christian Gromm 2020-05-14  1112  		mdev->is_channel_healthy[i] = true;
56c7d34c835125c6 Christian Gromm 2020-05-14  1113  
56c7d34c835125c6 Christian Gromm 2020-05-14  1114  		snprintf(&mdev->suffix[i][0], MAX_SUFFIX_LEN, "ep%02x",
56c7d34c835125c6 Christian Gromm 2020-05-14  1115  			 mdev->ep_address[i]);
56c7d34c835125c6 Christian Gromm 2020-05-14  1116  
56c7d34c835125c6 Christian Gromm 2020-05-14  1117  		tmp_cap->name_suffix = &mdev->suffix[i][0];
56c7d34c835125c6 Christian Gromm 2020-05-14  1118  		tmp_cap->buffer_size_packet = MAX_BUF_SIZE;
56c7d34c835125c6 Christian Gromm 2020-05-14  1119  		tmp_cap->buffer_size_streaming = MAX_BUF_SIZE;
56c7d34c835125c6 Christian Gromm 2020-05-14  1120  		tmp_cap->num_buffers_packet = BUF_CHAIN_SIZE;
56c7d34c835125c6 Christian Gromm 2020-05-14  1121  		tmp_cap->num_buffers_streaming = BUF_CHAIN_SIZE;
56c7d34c835125c6 Christian Gromm 2020-05-14  1122  		tmp_cap->data_type = MOST_CH_CONTROL | MOST_CH_ASYNC |
56c7d34c835125c6 Christian Gromm 2020-05-14  1123  				     MOST_CH_ISOC | MOST_CH_SYNC;
56c7d34c835125c6 Christian Gromm 2020-05-14  1124  		if (usb_endpoint_dir_in(ep_desc))
56c7d34c835125c6 Christian Gromm 2020-05-14  1125  			tmp_cap->direction = MOST_CH_RX;
56c7d34c835125c6 Christian Gromm 2020-05-14  1126  		else
56c7d34c835125c6 Christian Gromm 2020-05-14  1127  			tmp_cap->direction = MOST_CH_TX;
56c7d34c835125c6 Christian Gromm 2020-05-14  1128  		tmp_cap++;
56c7d34c835125c6 Christian Gromm 2020-05-14  1129  		init_usb_anchor(&mdev->busy_urbs[i]);
56c7d34c835125c6 Christian Gromm 2020-05-14  1130  		spin_lock_init(&mdev->channel_lock[i]);
56c7d34c835125c6 Christian Gromm 2020-05-14  1131  	}
56c7d34c835125c6 Christian Gromm 2020-05-14  1132  	dev_notice(dev, "claimed gadget: Vendor=%4.4x ProdID=%4.4x Bus=%02x Device=%02x\n",
56c7d34c835125c6 Christian Gromm 2020-05-14  1133  		   le16_to_cpu(usb_dev->descriptor.idVendor),
56c7d34c835125c6 Christian Gromm 2020-05-14  1134  		   le16_to_cpu(usb_dev->descriptor.idProduct),
56c7d34c835125c6 Christian Gromm 2020-05-14  1135  		   usb_dev->bus->busnum,
56c7d34c835125c6 Christian Gromm 2020-05-14  1136  		   usb_dev->devnum);
56c7d34c835125c6 Christian Gromm 2020-05-14  1137  
56c7d34c835125c6 Christian Gromm 2020-05-14  1138  	dev_notice(dev, "device path: /sys/bus/usb/devices/%d-%s:%d.%d\n",
56c7d34c835125c6 Christian Gromm 2020-05-14  1139  		   usb_dev->bus->busnum,
56c7d34c835125c6 Christian Gromm 2020-05-14  1140  		   usb_dev->devpath,
56c7d34c835125c6 Christian Gromm 2020-05-14  1141  		   usb_dev->config->desc.bConfigurationValue,
56c7d34c835125c6 Christian Gromm 2020-05-14  1142  		   usb_iface_desc->desc.bInterfaceNumber);
56c7d34c835125c6 Christian Gromm 2020-05-14  1143  
56c7d34c835125c6 Christian Gromm 2020-05-14  1144  	ret = most_register_interface(&mdev->iface);
56c7d34c835125c6 Christian Gromm 2020-05-14  1145  	if (ret)
56c7d34c835125c6 Christian Gromm 2020-05-14  1146  		goto err_free_busy_urbs;
56c7d34c835125c6 Christian Gromm 2020-05-14  1147  
56c7d34c835125c6 Christian Gromm 2020-05-14  1148  	mutex_lock(&mdev->io_mutex);
56c7d34c835125c6 Christian Gromm 2020-05-14  1149  	if (le16_to_cpu(usb_dev->descriptor.idProduct) == USB_DEV_ID_OS81118 ||
56c7d34c835125c6 Christian Gromm 2020-05-14  1150  	    le16_to_cpu(usb_dev->descriptor.idProduct) == USB_DEV_ID_OS81119 ||
56c7d34c835125c6 Christian Gromm 2020-05-14  1151  	    le16_to_cpu(usb_dev->descriptor.idProduct) == USB_DEV_ID_OS81210) {
56c7d34c835125c6 Christian Gromm 2020-05-14  1152  		mdev->dci = kzalloc(sizeof(*mdev->dci), GFP_KERNEL);
56c7d34c835125c6 Christian Gromm 2020-05-14  1153  		if (!mdev->dci) {
56c7d34c835125c6 Christian Gromm 2020-05-14  1154  			mutex_unlock(&mdev->io_mutex);
56c7d34c835125c6 Christian Gromm 2020-05-14  1155  			most_deregister_interface(&mdev->iface);
56c7d34c835125c6 Christian Gromm 2020-05-14  1156  			ret = -ENOMEM;
56c7d34c835125c6 Christian Gromm 2020-05-14  1157  			goto err_free_busy_urbs;
56c7d34c835125c6 Christian Gromm 2020-05-14  1158  		}
56c7d34c835125c6 Christian Gromm 2020-05-14  1159  
56c7d34c835125c6 Christian Gromm 2020-05-14  1160  		mdev->dci->dev.init_name = "dci";
56c7d34c835125c6 Christian Gromm 2020-05-14  1161  		mdev->dci->dev.parent = get_device(mdev->iface.dev);
56c7d34c835125c6 Christian Gromm 2020-05-14  1162  		mdev->dci->dev.groups = dci_attr_groups;
56c7d34c835125c6 Christian Gromm 2020-05-14  1163  		mdev->dci->dev.release = release_dci;
56c7d34c835125c6 Christian Gromm 2020-05-14  1164  		if (device_register(&mdev->dci->dev)) {
56c7d34c835125c6 Christian Gromm 2020-05-14  1165  			mutex_unlock(&mdev->io_mutex);
56c7d34c835125c6 Christian Gromm 2020-05-14  1166  			most_deregister_interface(&mdev->iface);
56c7d34c835125c6 Christian Gromm 2020-05-14  1167  			ret = -ENOMEM;
56c7d34c835125c6 Christian Gromm 2020-05-14  1168  			goto err_free_dci;
56c7d34c835125c6 Christian Gromm 2020-05-14  1169  		}
56c7d34c835125c6 Christian Gromm 2020-05-14  1170  		mdev->dci->usb_device = mdev->usb_device;
56c7d34c835125c6 Christian Gromm 2020-05-14  1171  	}
56c7d34c835125c6 Christian Gromm 2020-05-14  1172  	mutex_unlock(&mdev->io_mutex);
56c7d34c835125c6 Christian Gromm 2020-05-14  1173  	return 0;
56c7d34c835125c6 Christian Gromm 2020-05-14  1174  err_free_dci:
56c7d34c835125c6 Christian Gromm 2020-05-14  1175  	put_device(&mdev->dci->dev);
56c7d34c835125c6 Christian Gromm 2020-05-14  1176  err_free_busy_urbs:
56c7d34c835125c6 Christian Gromm 2020-05-14  1177  	kfree(mdev->busy_urbs);
56c7d34c835125c6 Christian Gromm 2020-05-14  1178  err_free_ep_address:
56c7d34c835125c6 Christian Gromm 2020-05-14  1179  	kfree(mdev->ep_address);
56c7d34c835125c6 Christian Gromm 2020-05-14  1180  err_free_cap:
56c7d34c835125c6 Christian Gromm 2020-05-14  1181  	kfree(mdev->cap);
56c7d34c835125c6 Christian Gromm 2020-05-14  1182  err_free_conf:
56c7d34c835125c6 Christian Gromm 2020-05-14  1183  	kfree(mdev->conf);
56c7d34c835125c6 Christian Gromm 2020-05-14  1184  err_free_mdev:
56c7d34c835125c6 Christian Gromm 2020-05-14  1185  	put_device(&mdev->dev);
56c7d34c835125c6 Christian Gromm 2020-05-14  1186  	return ret;
56c7d34c835125c6 Christian Gromm 2020-05-14  1187  }
56c7d34c835125c6 Christian Gromm 2020-05-14  1188  

:::::: The code at line 1104 was first introduced by commit
:::::: 56c7d34c835125c6587fb28f67cddd1d4062975f drivers: most: add usb adapter driver

:::::: TO: Christian Gromm <christian.gromm@microchip.com>
:::::: CC: 0day robot <lkp@intel.com>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 73441 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/8] drivers: most: usb: use dev_*() functions to print messages
  2020-05-14  9:52 ` [PATCH v2 2/8] drivers: most: usb: use dev_*() functions to print messages Christian Gromm
@ 2020-05-19 13:42   ` Dan Carpenter
  0 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2020-05-19 13:42 UTC (permalink / raw)
  To: Christian Gromm; +Cc: gregkh, driverdev-devel, linux-usb

On Thu, May 14, 2020 at 11:52:50AM +0200, Christian Gromm wrote:
> @@ -186,13 +185,14 @@ static inline int start_sync_ep(struct usb_device *usb_dev, u16 ep)
>   * get_stream_frame_size - calculate frame size of current configuration
>   * @cfg: channel configuration
>   */
> -static unsigned int get_stream_frame_size(struct most_channel_config *cfg)
> +static unsigned int get_stream_frame_size(struct most_channel_config *cfg,
> +					  struct device *dev)

I feel like normally "dev" would be the first pointer instead of the
second.  It goes from permanent --> temporary.  Central --> outer.

That rule of thumb basically works for this file.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 5/8] drivers: most: usb: use dev_dbg function
  2020-05-14  9:52 ` [PATCH v2 5/8] drivers: most: usb: use dev_dbg function Christian Gromm
@ 2020-05-19 13:43   ` Dan Carpenter
  0 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2020-05-19 13:43 UTC (permalink / raw)
  To: Christian Gromm; +Cc: gregkh, driverdev-devel, linux-usb

On Thu, May 14, 2020 at 11:52:53AM +0200, Christian Gromm wrote:
> This patch replaces the functions dev_notice with dev_dbg to silence
> the driver during normal operation.
> 
> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> v2:
> 
>  drivers/most/most_usb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/most/most_usb.c b/drivers/most/most_usb.c
> index 1655fcd..35620a1 100644
> --- a/drivers/most/most_usb.c
> +++ b/drivers/most/most_usb.c
> @@ -1129,13 +1129,13 @@ hdm_probe(struct usb_interface *interface, const struct usb_device_id *id)
>  		init_usb_anchor(&mdev->busy_urbs[i]);
>  		spin_lock_init(&mdev->channel_lock[i]);
>  	}
> -	dev_notice(dev, "claimed gadget: Vendor=%4.4x ProdID=%4.4x Bus=%02x Device=%02x\n",
> +	dev_dbg(dev, "claimed gadget: Vendor=%4.4x ProdID=%4.4x Bus=%02x Device=%02x\n",
>  		   le16_to_cpu(usb_dev->descriptor.idVendor),
>  		   le16_to_cpu(usb_dev->descriptor.idProduct),
>  		   usb_dev->bus->busnum,
>  		   usb_dev->devnum);

All the parameters aren't aligned now.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/8] drivers: most: add usb adapter driver
  2020-05-14  9:52 ` [PATCH v2 1/8] drivers: most: add usb adapter driver Christian Gromm
  2020-05-14 10:25   ` Greg KH
@ 2020-05-20 13:17   ` Dan Carpenter
  2020-05-20 13:54     ` Christian.Gromm
  1 sibling, 1 reply; 16+ messages in thread
From: Dan Carpenter @ 2020-05-20 13:17 UTC (permalink / raw)
  To: Christian Gromm; +Cc: gregkh, driverdev-devel, linux-usb

On Thu, May 14, 2020 at 11:52:49AM +0200, Christian Gromm wrote:
> This patch adds the usb driver source file most_usb.c and
> modifies the Makefile and Kconfig accordingly.
> 
> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> ---
> v2:
> Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 	- don't remove usb driver from staging area
> 	- don't touch staging/most/Kconfig
> 	- remove subdirectory for USB driver and put source file into
> 	  drivers/most
> 
>  drivers/most/Kconfig    |   12 +
>  drivers/most/Makefile   |    2 +
>  drivers/most/most_usb.c | 1262 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1276 insertions(+)
>  create mode 100644 drivers/most/most_usb.c
> 
> diff --git a/drivers/most/Kconfig b/drivers/most/Kconfig
> index 58d7999..8650683 100644
> --- a/drivers/most/Kconfig
> +++ b/drivers/most/Kconfig
> @@ -13,3 +13,15 @@ menuconfig MOST
>  	  module will be called most_core.
>  
>  	  If in doubt, say N here.
> +
> +if MOST
> +config MOST_USB
> +	tristate "USB"
> +	depends on USB && NET
> +	help
> +	  Say Y here if you want to connect via USB to network tranceiver.
> +	  This device driver depends on the networking AIM.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called most_usb.
> +endif
> diff --git a/drivers/most/Makefile b/drivers/most/Makefile
> index e810cd3..97ffc06 100644
> --- a/drivers/most/Makefile
> +++ b/drivers/most/Makefile
> @@ -2,3 +2,5 @@
>  obj-$(CONFIG_MOST) += most_core.o
>  most_core-y :=	core.o \
>  		configfs.o
> +
> +obj-$(CONFIG_MOST_USB) += most_usb.o
> diff --git a/drivers/most/most_usb.c b/drivers/most/most_usb.c
> new file mode 100644
> index 0000000..daa5e4b
> --- /dev/null
> +++ b/drivers/most/most_usb.c
> @@ -0,0 +1,1262 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * usb.c - Hardware dependent module for USB
> + *
> + * Copyright (C) 2013-2015 Microchip Technology Germany II GmbH & Co. KG
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +#include <linux/module.h>
> +#include <linux/fs.h>
> +#include <linux/usb.h>
> +#include <linux/slab.h>
> +#include <linux/init.h>
> +#include <linux/cdev.h>
> +#include <linux/device.h>
> +#include <linux/list.h>
> +#include <linux/completion.h>
> +#include <linux/mutex.h>
> +#include <linux/spinlock.h>
> +#include <linux/interrupt.h>
> +#include <linux/workqueue.h>
> +#include <linux/sysfs.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/etherdevice.h>
> +#include <linux/uaccess.h>
> +#include <linux/most.h>
> +
> +#define USB_MTU			512
> +#define NO_ISOCHRONOUS_URB	0
> +#define AV_PACKETS_PER_XACT	2
> +#define BUF_CHAIN_SIZE		0xFFFF
> +#define MAX_NUM_ENDPOINTS	30
> +#define MAX_SUFFIX_LEN		10
> +#define MAX_STRING_LEN		80
> +#define MAX_BUF_SIZE		0xFFFF
> +
> +#define USB_VENDOR_ID_SMSC	0x0424  /* VID: SMSC */
> +#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
> +#define DRCI_REG_NODE_ADDR	0x0102
> +#define DRCI_REG_NODE_POS	0x0103
> +#define DRCI_REG_MEP_FILTER	0x0140
> +#define DRCI_REG_HASH_TBL0	0x0141
> +#define DRCI_REG_HASH_TBL1	0x0142
> +#define DRCI_REG_HASH_TBL2	0x0143
> +#define DRCI_REG_HASH_TBL3	0x0144
> +#define DRCI_REG_HW_ADDR_HI	0x0145
> +#define DRCI_REG_HW_ADDR_MI	0x0146
> +#define DRCI_REG_HW_ADDR_LO	0x0147
> +#define DRCI_REG_BASE		0x1100
> +#define DRCI_COMMAND		0x02
> +#define DRCI_READ_REQ		0xA0
> +#define DRCI_WRITE_REQ		0xA1
> +
> +/**
> + * struct most_dci_obj - Direct Communication Interface
> + * @kobj:position in sysfs
> + * @usb_device: pointer to the usb device
> + * @reg_addr: register address for arbitrary DCI access
> + */
> +struct most_dci_obj {
> +	struct device dev;
> +	struct usb_device *usb_device;
> +	u16 reg_addr;
> +};
> +
> +#define to_dci_obj(p) container_of(p, struct most_dci_obj, dev)
> +
> +struct most_dev;
> +
> +struct clear_hold_work {
> +	struct work_struct ws;
> +	struct most_dev *mdev;
> +	unsigned int channel;
> +	int pipe;
> +};
> +
> +#define to_clear_hold_work(w) container_of(w, struct clear_hold_work, ws)
> +
> +/**
> + * struct most_dev - holds all usb interface specific stuff
> + * @usb_device: pointer to usb device
> + * @iface: hardware interface
> + * @cap: channel capabilities
> + * @conf: channel configuration
> + * @dci: direct communication interface of hardware
> + * @ep_address: endpoint address table
> + * @description: device description
> + * @suffix: suffix for channel name
> + * @channel_lock: synchronize channel access
> + * @padding_active: indicates channel uses padding
> + * @is_channel_healthy: health status table of each channel
> + * @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
> + */
> +struct most_dev {
> +	struct device dev;
> +	struct usb_device *usb_device;
> +	struct most_interface iface;
> +	struct most_channel_capability *cap;
> +	struct most_channel_config *conf;
> +	struct most_dci_obj *dci;
> +	u8 *ep_address;
> +	char description[MAX_STRING_LEN];
> +	char suffix[MAX_NUM_ENDPOINTS][MAX_SUFFIX_LEN];
> +	spinlock_t channel_lock[MAX_NUM_ENDPOINTS]; /* sync channel access */
> +	bool padding_active[MAX_NUM_ENDPOINTS];
> +	bool is_channel_healthy[MAX_NUM_ENDPOINTS];
> +	struct clear_hold_work clear_work[MAX_NUM_ENDPOINTS];
> +	struct usb_anchor *busy_urbs;
> +	struct mutex io_mutex;
> +	struct timer_list link_stat_timer;
> +	struct work_struct poll_work_obj;
> +	void (*on_netinfo)(struct most_interface *most_iface,
> +			   unsigned char link_state, unsigned char *addrs);
> +};
> +
> +#define to_mdev(d) container_of(d, struct most_dev, iface)
> +#define to_mdev_from_dev(d) container_of(d, struct most_dev, dev)
> +#define to_mdev_from_work(w) container_of(w, struct most_dev, poll_work_obj)
> +
> +static void wq_clear_halt(struct work_struct *wq_obj);
> +static void wq_netinfo(struct work_struct *wq_obj);
> +
> +/**
> + * drci_rd_reg - read a DCI register
> + * @dev: usb device
> + * @reg: register address
> + * @buf: buffer to store data
> + *
> + * This is reads data from INIC's direct register communication interface
> + */
> +static inline int drci_rd_reg(struct usb_device *dev, u16 reg, u16 *buf)
> +{
> +	int retval;
> +	__le16 *dma_buf = kzalloc(sizeof(*dma_buf), GFP_KERNEL);

I really hate allocations hidden in the declaration block.  :/

> +	u8 req_type = USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE;
> +
> +	if (!dma_buf)
> +		return -ENOMEM;
> +
> +	retval = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
> +				 DRCI_READ_REQ, req_type,
> +				 0x0000,
> +				 reg, dma_buf, sizeof(*dma_buf), 5 * HZ);
> +	*buf = le16_to_cpu(*dma_buf);
> +	kfree(dma_buf);
> +
> +	return retval;

Why bother returning positive values here when none of the callers use
that?

> +}
> +
> +/**
> + * drci_wr_reg - write a DCI register
> + * @dev: usb device
> + * @reg: register address
> + * @data: data to write
> + *
> + * This is writes data to INIC's direct register communication interface
> + */
> +static inline int drci_wr_reg(struct usb_device *dev, u16 reg, u16 data)
> +{
> +	return usb_control_msg(dev,
> +			       usb_sndctrlpipe(dev, 0),
> +			       DRCI_WRITE_REQ,
> +			       USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> +			       data,
> +			       reg,
> +			       NULL,
> +			       0,
> +			       5 * HZ);
> +}
> +
> +static inline int start_sync_ep(struct usb_device *usb_dev, u16 ep)
> +{
> +	return drci_wr_reg(usb_dev, DRCI_REG_BASE + DRCI_COMMAND + ep * 16, 1);
> +}
> +
> +/**
> + * get_stream_frame_size - calculate frame size of current configuration
> + * @cfg: channel configuration
> + */
> +static unsigned int get_stream_frame_size(struct most_channel_config *cfg)
> +{
> +	unsigned int frame_size = 0;
> +	unsigned int sub_size = cfg->subbuffer_size;
> +
> +	if (!sub_size) {
> +		pr_warn("Misconfig: Subbuffer size zero.\n");
> +		return frame_size;

I wish this were a "return 0;"

> +	}
> +	switch (cfg->data_type) {
> +	case MOST_CH_ISOC:
> +		frame_size = AV_PACKETS_PER_XACT * sub_size;
> +		break;
> +	case MOST_CH_SYNC:
> +		if (cfg->packets_per_xact == 0) {
> +			pr_warn("Misconfig: Packets per XACT zero\n");
> +			frame_size = 0;
> +		} else if (cfg->packets_per_xact == 0xFF) {
> +			frame_size = (USB_MTU / sub_size) * sub_size;
> +		} else {
> +			frame_size = cfg->packets_per_xact * sub_size;
> +		}
> +		break;
> +	default:
> +		pr_warn("Query frame size of non-streaming channel\n");
> +		break;
> +	}
> +	return frame_size;
> +}
> +
> +/**
> + * hdm_poison_channel - mark buffers of this channel as invalid
> + * @iface: pointer to the interface
> + * @channel: channel ID
> + *
> + * This unlinks all URBs submitted to the HCD,
> + * calls the associated completion function of the core and removes
> + * them from the list.
> + *
> + * Returns 0 on success or error code otherwise.
> + */
> +static int hdm_poison_channel(struct most_interface *iface, int channel)
> +{
> +	struct most_dev *mdev = to_mdev(iface);
> +	unsigned long flags;
> +	spinlock_t *lock; /* temp. lock */
> +
> +	if (channel < 0 || channel >= iface->num_channels) {
> +		dev_warn(&mdev->usb_device->dev, "Channel ID out of range.\n");
> +		return -ECHRNG;
> +	}
> +
> +	lock = mdev->channel_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);
> +
> +	mutex_lock(&mdev->io_mutex);
> +	usb_kill_anchored_urbs(&mdev->busy_urbs[channel]);
> +	if (mdev->padding_active[channel])
> +		mdev->padding_active[channel] = false;
> +
> +	if (mdev->conf[channel].data_type == MOST_CH_ASYNC) {
> +		del_timer_sync(&mdev->link_stat_timer);
> +		cancel_work_sync(&mdev->poll_work_obj);
> +	}
> +	mutex_unlock(&mdev->io_mutex);
> +	return 0;
> +}
> +
> +/**
> + * hdm_add_padding - add padding bytes
> + * @mdev: most device
> + * @channel: channel ID
> + * @mbo: buffer object
> + *
> + * This inserts the INIC hardware specific padding bytes into a streaming
> + * channel's buffer
> + */
> +static int hdm_add_padding(struct most_dev *mdev, int channel, struct mbo *mbo)
> +{
> +	struct most_channel_config *conf = &mdev->conf[channel];
> +	unsigned int frame_size = get_stream_frame_size(conf);
> +	unsigned int j, num_frames;
> +
> +	if (!frame_size)
> +		return -EINVAL;
> +	num_frames = mbo->buffer_length / frame_size;
> +
> +	if (num_frames < 1) {
> +		dev_err(&mdev->usb_device->dev,
> +			"Missed minimal transfer unit.\n");
> +		return -EINVAL;
> +	}
> +
> +	for (j = num_frames - 1; j > 0; j--)
> +		memmove(mbo->virt_address + j * USB_MTU,
> +			mbo->virt_address + j * frame_size,
> +			frame_size);
> +	mbo->buffer_length = num_frames * USB_MTU;
> +	return 0;
> +}
> +
> +/**
> + * hdm_remove_padding - remove padding bytes
> + * @mdev: most device
> + * @channel: channel ID
> + * @mbo: buffer object
> + *
> + * This takes the INIC hardware specific padding bytes off a streaming
> + * channel's buffer.
> + */
> +static int hdm_remove_padding(struct most_dev *mdev, int channel,
> +			      struct mbo *mbo)
> +{
> +	struct most_channel_config *const conf = &mdev->conf[channel];
> +	unsigned int frame_size = get_stream_frame_size(conf);
> +	unsigned int j, num_frames;
> +
> +	if (!frame_size)
> +		return -EINVAL;
> +	num_frames = mbo->processed_length / USB_MTU;
> +
> +	for (j = 1; j < num_frames; j++)
> +		memmove(mbo->virt_address + frame_size * j,
> +			mbo->virt_address + USB_MTU * j,
> +			frame_size);
> +
> +	mbo->processed_length = frame_size * num_frames;
> +	return 0;
> +}
> +
> +/**
> + * hdm_write_completion - completion function for submitted Tx URBs
> + * @urb: the URB that has been completed
> + *
> + * This checks the status of the completed URB. In case the URB has been
> + * unlinked before, it is immediately freed. On any other error the MBO
> + * transfer flag is set. On success it frees allocated resources and calls
> + * the completion function.
> + *
> + * Context: interrupt!
> + */
> +static void hdm_write_completion(struct urb *urb)
> +{
> +	struct mbo *mbo = urb->context;
> +	struct most_dev *mdev = to_mdev(mbo->ifp);
> +	unsigned int channel = mbo->hdm_channel_id;
> +	spinlock_t *lock = mdev->channel_lock + channel;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(lock, flags);
> +
> +	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(&mdev->usb_device->dev,
> +				 "Broken pipe on ep%02x\n",
> +				 mdev->ep_address[channel]);
> +			mdev->is_channel_healthy[channel] = false;
> +			mdev->clear_work[channel].pipe = urb->pipe;
> +			schedule_work(&mdev->clear_work[channel].ws);
> +			break;
> +		case -ENODEV:
> +		case -EPROTO:
> +			mbo->status = MBO_E_CLOSE;
> +			break;
> +		}
> +	}
> +
> +	spin_unlock_irqrestore(lock, flags);
> +
> +	if (likely(mbo->complete))
> +		mbo->complete(mbo);
> +	usb_free_urb(urb);
> +}
> +
> +/**
> + * hdm_read_completion - completion function for submitted Rx URBs
> + * @urb: the URB that has been completed
> + *
> + * This checks the status of the completed URB. In case the URB has been
> + * unlinked before it is immediately freed. On any other error the MBO transfer
> + * flag is set. On success it frees allocated resources, removes
> + * padding bytes -if necessary- and calls the completion function.
> + *
> + * Context: interrupt!
> + *
> + * **************************************************************************
> + *                   Error codes returned by in urb->status
> + *                   or in iso_frame_desc[n].status (for ISO)
> + * *************************************************************************
> + *
> + * USB device drivers may only test urb status values in completion handlers.
> + * This is because otherwise there would be a race between HCDs updating
> + * these values on one CPU, and device drivers testing them on another CPU.
> + *
> + * A transfer's actual_length may be positive even when an error has been
> + * reported.  That's because transfers often involve several packets, so that
> + * one or more packets could finish before an error stops further endpoint I/O.
> + *
> + * For isochronous URBs, the urb status value is non-zero only if the URB is
> + * unlinked, the device is removed, the host controller is disabled or the total
> + * transferred length is less than the requested length and the URB_SHORT_NOT_OK
> + * flag is set.  Completion handlers for isochronous URBs should only see
> + * urb->status set to zero, -ENOENT, -ECONNRESET, -ESHUTDOWN, or -EREMOTEIO.
> + * Individual frame descriptor status fields may report more status codes.
> + *
> + *
> + * 0			Transfer completed successfully
> + *
> + * -ENOENT		URB was synchronously unlinked by usb_unlink_urb
> + *
> + * -EINPROGRESS		URB still pending, no results yet
> + *			(That is, if drivers see this it's a bug.)
> + *
> + * -EPROTO (*, **)	a) bitstuff error
> + *			b) no response packet received within the
> + *			   prescribed bus turn-around time
> + *			c) unknown USB error
> + *
> + * -EILSEQ (*, **)	a) CRC mismatch
> + *			b) no response packet received within the
> + *			   prescribed bus turn-around time
> + *			c) unknown USB error
> + *
> + *			Note that often the controller hardware does not
> + *			distinguish among cases a), b), and c), so a
> + *			driver cannot tell whether there was a protocol
> + *			error, a failure to respond (often caused by
> + *			device disconnect), or some other fault.
> + *
> + * -ETIME (**)		No response packet received within the prescribed
> + *			bus turn-around time.  This error may instead be
> + *			reported as -EPROTO or -EILSEQ.
> + *
> + * -ETIMEDOUT		Synchronous USB message functions use this code
> + *			to indicate timeout expired before the transfer
> + *			completed, and no other error was reported by HC.
> + *
> + * -EPIPE (**)		Endpoint stalled.  For non-control endpoints,
> + *			reset this status with usb_clear_halt().
> + *
> + * -ECOMM		During an IN transfer, the host controller
> + *			received data from an endpoint faster than it
> + *			could be written to system memory
> + *
> + * -ENOSR		During an OUT transfer, the host controller
> + *			could not retrieve data from system memory fast
> + *			enough to keep up with the USB data rate
> + *
> + * -EOVERFLOW (*)	The amount of data returned by the endpoint was
> + *			greater than either the max packet size of the
> + *			endpoint or the remaining buffer size.  "Babble".
> + *
> + * -EREMOTEIO		The data read from the endpoint did not fill the
> + *			specified buffer, and URB_SHORT_NOT_OK was set in
> + *			urb->transfer_flags.
> + *
> + * -ENODEV		Device was removed.  Often preceded by a burst of
> + *			other errors, since the hub driver doesn't detect
> + *			device removal events immediately.
> + *
> + * -EXDEV		ISO transfer only partially completed
> + *			(only set in iso_frame_desc[n].status, not urb->status)
> + *
> + * -EINVAL		ISO madness, if this happens: Log off and go home
> + *
> + * -ECONNRESET		URB was asynchronously unlinked by usb_unlink_urb
> + *
> + * -ESHUTDOWN		The device or host controller has been disabled due
> + *			to some problem that could not be worked around,
> + *			such as a physical disconnect.
> + *
> + *
> + * (*) Error codes like -EPROTO, -EILSEQ and -EOVERFLOW normally indicate
> + * hardware problems such as bad devices (including firmware) or cables.
> + *
> + * (**) This is also one of several codes that different kinds of host
> + * controller use to indicate a transfer has failed because of device
> + * disconnect.  In the interval before the hub driver starts disconnect
> + * processing, devices may receive such fault reports for every request.
> + *
> + * See <https://www.kernel.org/doc/Documentation/driver-api/usb/error-codes.rst>
> + */
> +static void hdm_read_completion(struct urb *urb)
> +{
> +	struct mbo *mbo = urb->context;
> +	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->channel_lock + channel;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(lock, flags);
> +
> +	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 pipe on ep%02x\n",
> +				 mdev->ep_address[channel]);
> +			mdev->is_channel_healthy[channel] = false;
> +			mdev->clear_work[channel].pipe = urb->pipe;
> +			schedule_work(&mdev->clear_work[channel].ws);
> +			break;
> +		case -ENODEV:
> +		case -EPROTO:
> +			mbo->status = MBO_E_CLOSE;
> +			break;
> +		case -EOVERFLOW:
> +			dev_warn(dev, "Babble on ep%02x\n",
> +				 mdev->ep_address[channel]);
> +			break;
> +		}
> +	}
> +
> +	spin_unlock_irqrestore(lock, flags);
> +
> +	if (likely(mbo->complete))
> +		mbo->complete(mbo);
> +	usb_free_urb(urb);
> +}
> +
> +/**
> + * hdm_enqueue - receive a buffer to be used for data transfer
> + * @iface: interface to enqueue to
> + * @channel: ID of the channel
> + * @mbo: pointer to the buffer object
> + *
> + * This allocates a new URB and fills it according to the channel
> + * that is being used for transmission of data. Before the URB is
> + * submitted it is stored in the private anchor list.
> + *
> + * Returns 0 on success. On any error the URB is freed and a error code
> + * is returned.
> + *
> + * Context: Could in _some_ cases be interrupt!
> + */
> +static int hdm_enqueue(struct most_interface *iface, int channel,
> +		       struct mbo *mbo)
> +{
> +	struct most_dev *mdev = to_mdev(iface);
> +	struct most_channel_config *conf;
> +	int retval = 0;
> +	struct urb *urb;
> +	unsigned long length;
> +	void *virt_address;
> +
> +	if (!mbo)
> +		return -EINVAL;
> +	if (iface->num_channels <= channel || channel < 0)
> +		return -ECHRNG;
> +
> +	conf = &mdev->conf[channel];
> +
> +	mutex_lock(&mdev->io_mutex);
> +	if (!mdev->usb_device) {
> +		retval = -ENODEV;
> +		goto unlock_io_mutex;
> +	}
> +
> +	urb = usb_alloc_urb(NO_ISOCHRONOUS_URB, GFP_ATOMIC);

You could move this before the mutex_lock().  Would GFP_ATOMIC still be
required?

> +	if (!urb) {
> +		retval = -ENOMEM;
> +		goto unlock_io_mutex;
> +	}
> +
> +	if ((conf->direction & MOST_CH_TX) && mdev->padding_active[channel] &&
> +	    hdm_add_padding(mdev, channel, mbo)) {
> +		retval = -EINVAL;
> +		goto err_free_urb;
> +	}
> +
> +	urb->transfer_dma = mbo->bus_address;
> +	virt_address = mbo->virt_address;
> +	length = mbo->buffer_length;
> +
> +	if (conf->direction & MOST_CH_TX) {
> +		usb_fill_bulk_urb(urb, mdev->usb_device,
> +				  usb_sndbulkpipe(mdev->usb_device,
> +						  mdev->ep_address[channel]),
> +				  virt_address,
> +				  length,
> +				  hdm_write_completion,
> +				  mbo);
> +		if (conf->data_type != MOST_CH_ISOC &&
> +		    conf->data_type != MOST_CH_SYNC)
> +			urb->transfer_flags |= URB_ZERO_PACKET;
> +	} else {
> +		usb_fill_bulk_urb(urb, mdev->usb_device,
> +				  usb_rcvbulkpipe(mdev->usb_device,
> +						  mdev->ep_address[channel]),
> +				  virt_address,
> +				  length + conf->extra_len,
> +				  hdm_read_completion,
> +				  mbo);
> +	}
> +	urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> +
> +	usb_anchor_urb(urb, &mdev->busy_urbs[channel]);
> +
> +	retval = usb_submit_urb(urb, GFP_KERNEL);
> +	if (retval) {
> +		dev_err(&mdev->usb_device->dev,
> +			"URB submit failed with error %d.\n", retval);
> +		goto err_unanchor_urb;
> +	}
> +	goto unlock_io_mutex;

Replace the goto with:

	mutex_unlock(&mdev->io_mutex);
	return 0;

It lets you move the lock around more easily.

> +
> +err_unanchor_urb:
> +	usb_unanchor_urb(urb);
> +err_free_urb:
> +	usb_free_urb(urb);
> +unlock_io_mutex:
> +	mutex_unlock(&mdev->io_mutex);
> +	return retval;
> +}
> +
> +static void *hdm_dma_alloc(struct mbo *mbo, u32 size)
> +{
> +	struct most_dev *mdev = to_mdev(mbo->ifp);
> +
> +	return usb_alloc_coherent(mdev->usb_device, size, GFP_KERNEL,
> +				  &mbo->bus_address);
> +}
> +
> +static void hdm_dma_free(struct mbo *mbo, u32 size)
> +{
> +	struct most_dev *mdev = to_mdev(mbo->ifp);
> +
> +	usb_free_coherent(mdev->usb_device, size, mbo->virt_address,
> +			  mbo->bus_address);
> +}
> +
> +/**
> + * hdm_configure_channel - receive channel configuration from core
> + * @iface: interface
> + * @channel: channel ID
> + * @conf: structure that holds the configuration information
> + *
> + * The attached network interface controller (NIC) supports a padding mode
> + * to avoid short packets on USB, hence increasing the performance due to a
> + * lower interrupt load. This mode is default for synchronous data and can
> + * be switched on for isochronous data. In case padding is active the
> + * driver needs to know the frame size of the payload in order to calculate
> + * the number of bytes it needs to pad when transmitting or to cut off when
> + * receiving data.
> + *
> + */
> +static int hdm_configure_channel(struct most_interface *iface, int channel,
> +				 struct most_channel_config *conf)
> +{
> +	unsigned int num_frames;
> +	unsigned int frame_size;
> +	struct most_dev *mdev = to_mdev(iface);
> +	struct device *dev = &mdev->usb_device->dev;
> +
> +	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);
> +
> +	if (!conf) {
> +		dev_err(dev, "Bad config pointer.\n");
> +		return -EINVAL;
> +	}
> +	if (channel < 0 || channel >= iface->num_channels) {
> +		dev_err(dev, "Channel ID out of range.\n");
> +		return -EINVAL;
> +	}
> +	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 &&
> +	      conf->packets_per_xact != 0xFF)) {
> +		mdev->padding_active[channel] = false;
> +		/*
> +		 * Since the NIC's padding mode is not going to be
> +		 * used, we can skip the frame size calculations and
> +		 * move directly on to exit.
> +		 */

I absolutely hate that we're skipping the other checks...  At least
zero out the frame size information so that we don't save invalid data.

> +		goto exit;
> +	}
> +
> +	mdev->padding_active[channel] = true;
> +
> +	frame_size = get_stream_frame_size(conf);
> +	if (frame_size == 0 || frame_size > USB_MTU) {
> +		dev_warn(dev, "Misconfig: frame size wrong\n");
> +		return -EINVAL;
> +	}
> +
> +	num_frames = conf->buffer_size / frame_size;
> +
> +	if (conf->buffer_size % frame_size) {
> +		u16 old_size = conf->buffer_size;
> +
> +		conf->buffer_size = num_frames * frame_size;
> +		dev_warn(dev, "%s: fixed buffer size (%d -> %d)\n",
> +			 mdev->suffix[channel], old_size, conf->buffer_size);
> +	}
> +
> +	/* calculate extra length to comply w/ HW padding */
> +	conf->extra_len = num_frames * (USB_MTU - frame_size);
> +
> +exit:
> +	mdev->conf[channel] = *conf;
> +	if (conf->data_type == MOST_CH_ASYNC) {
> +		u16 ep = mdev->ep_address[channel];
> +
> +		if (start_sync_ep(mdev->usb_device, ep) < 0)
> +			dev_warn(dev, "sync for ep%02x failed", ep);
> +	}
> +	return 0;
> +}
> +
> +/**
> + * hdm_request_netinfo - request network information
> + * @iface: pointer to interface
> + * @channel: channel ID
> + *
> + * This is used as trigger to set up the link status timer that
> + * polls for the NI state of the INIC every 2 seconds.
> + *
> + */
> +static void hdm_request_netinfo(struct most_interface *iface, int channel,
> +				void (*on_netinfo)(struct most_interface *,
> +						   unsigned char,
> +						   unsigned char *))
> +{
> +	struct most_dev *mdev = to_mdev(iface);
> +
> +	mdev->on_netinfo = on_netinfo;
> +	if (!on_netinfo)
> +		return;
> +
> +	mdev->link_stat_timer.expires = jiffies + HZ;
> +	mod_timer(&mdev->link_stat_timer, mdev->link_stat_timer.expires);
> +}
> +
> +/**
> + * 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
> + * tasks to a work queue.
> + */
> +static void link_stat_timer_handler(struct timer_list *t)
> +{
> +	struct most_dev *mdev = from_timer(mdev, t, link_stat_timer);
> +
> +	schedule_work(&mdev->poll_work_obj);
> +	mdev->link_stat_timer.expires = jiffies + (2 * HZ);
> +	add_timer(&mdev->link_stat_timer);
> +}
> +
> +/**
> + * 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
> + */
> +static void wq_netinfo(struct work_struct *wq_obj)
> +{
> +	struct most_dev *mdev = to_mdev_from_work(wq_obj);
> +	struct usb_device *usb_device = mdev->usb_device;
> +	struct device *dev = &usb_device->dev;
> +	u16 hi, mi, lo, link;
> +	u8 hw_addr[6];
> +
> +	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 (drci_rd_reg(usb_device, DRCI_REG_HW_ADDR_MI, &mi) < 0) {
> +		dev_err(dev, "Vendor request 'hw_addr_mid' failed\n");
> +		return;
> +	}
> +
> +	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;
> +
> +	if (mdev->on_netinfo)
> +		mdev->on_netinfo(&mdev->iface, link, hw_addr);
> +}
> +
> +/**
> + * wq_clear_halt - work queue function
> + * @wq_obj: work_struct object to execute
> + *
> + * This sends a clear_halt to the given USB pipe.
> + */
> +static void wq_clear_halt(struct work_struct *wq_obj)
> +{
> +	struct clear_hold_work *clear_work = to_clear_hold_work(wq_obj);
> +	struct most_dev *mdev = clear_work->mdev;
> +	unsigned int channel = clear_work->channel;
> +	int pipe = clear_work->pipe;
> +
> +	mutex_lock(&mdev->io_mutex);
> +	most_stop_enqueue(&mdev->iface, channel);
> +	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");
> +
> +	/* If the functional Stall condition has been set on an
> +	 * asynchronous rx channel, we need to clear the tx channel
> +	 * too, since the hardware runs its clean-up sequence on both
> +	 * channels, as they are physically one on the network.
> +	 *
> +	 * The USB interface that exposes the asynchronous channels
> +	 * contains always two endpoints, and two only.
> +	 */
> +	if (mdev->conf[channel].data_type == MOST_CH_ASYNC &&
> +	    mdev->conf[channel].direction == MOST_CH_RX) {
> +		int peer = 1 - channel;
                ^^^^^^^^^^^^^^^^^^^^^^
This is too tricky.  At the start of the function channel seems to be a
number between 0-30 so this looks like "peer" can be negative.
Presumably, only the first two channels are MOST_CH_ASYNC and MOST_CH_RX?

But could we just write it like:

		if (channel == 0)
			peer = 1;
		else
			peer = 0;


> +		int snd_pipe = usb_sndbulkpipe(mdev->usb_device,
> +					       mdev->ep_address[peer]);
> +		usb_clear_halt(mdev->usb_device, snd_pipe);
> +	}
> +	mdev->is_channel_healthy[channel] = true;
> +	most_resume_enqueue(&mdev->iface, channel);
> +	mutex_unlock(&mdev->io_mutex);
> +}
> +
> +/**
> + * hdm_usb_fops - file operation table for USB driver
> + */
> +static const struct file_operations hdm_usb_fops = {
> +	.owner = THIS_MODULE,
> +};
> +
> +/**
> + * usb_device_id - ID table for HCD device probing
> + */
> +static const 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 */
> +};
> +
> +struct regs {
> +	const char *name;
> +	u16 reg;
> +};
> +
> +static const struct regs ro_regs[] = {
> +	{ "ni_state", DRCI_REG_NI_STATE },
> +	{ "packet_bandwidth", DRCI_REG_PACKET_BW },
> +	{ "node_address", DRCI_REG_NODE_ADDR },
> +	{ "node_position", DRCI_REG_NODE_POS },
> +};
> +
> +static const struct regs rw_regs[] = {
> +	{ "mep_filter", DRCI_REG_MEP_FILTER },
> +	{ "mep_hash0", DRCI_REG_HASH_TBL0 },
> +	{ "mep_hash1", DRCI_REG_HASH_TBL1 },
> +	{ "mep_hash2", DRCI_REG_HASH_TBL2 },
> +	{ "mep_hash3", DRCI_REG_HASH_TBL3 },
> +	{ "mep_eui48_hi", DRCI_REG_HW_ADDR_HI },
> +	{ "mep_eui48_mi", DRCI_REG_HW_ADDR_MI },
> +	{ "mep_eui48_lo", DRCI_REG_HW_ADDR_LO },
> +};
> +
> +static int get_stat_reg_addr(const struct regs *regs, int size,
> +			     const char *name, u16 *reg_addr)
> +{
> +	int i;
> +
> +	for (i = 0; i < size; i++) {
> +		if (!strcmp(name, regs[i].name)) {
> +			*reg_addr = regs[i].reg;
> +			return 0;
> +		}
> +	}
> +	return -EFAULT;

This should be -EINVAL

> +}
> +
> +#define get_static_reg_addr(regs, name, reg_addr) \
> +	get_stat_reg_addr(regs, ARRAY_SIZE(regs), name, reg_addr)
> +
> +static ssize_t value_show(struct device *dev, struct device_attribute *attr,
> +			  char *buf)
> +{
> +	const char *name = attr->attr.name;
> +	struct most_dci_obj *dci_obj = to_dci_obj(dev);
> +	u16 val;
> +	u16 reg_addr;
> +	int err;
> +
> +	if (!strcmp(name, "arb_address"))

I feel like most of these strcmp() should be sysfs_streq().

> +		return snprintf(buf, PAGE_SIZE, "%04x\n", dci_obj->reg_addr);
> +
> +	if (!strcmp(name, "arb_value"))
> +		reg_addr = dci_obj->reg_addr;
> +	else if (get_static_reg_addr(ro_regs, name, &reg_addr) &&
> +		 get_static_reg_addr(rw_regs, name, &reg_addr))
> +		return -EFAULT;

-EINVAL

> +
> +	err = drci_rd_reg(dci_obj->usb_device, reg_addr, &val);
> +	if (err < 0)
> +		return err;
> +
> +	return snprintf(buf, PAGE_SIZE, "%04x\n", val);
> +}
> +
> +static ssize_t value_store(struct device *dev, struct device_attribute *attr,
> +			   const char *buf, size_t count)
> +{
> +	u16 val;
> +	u16 reg_addr;
> +	const char *name = attr->attr.name;
> +	struct most_dci_obj *dci_obj = to_dci_obj(dev);
> +	struct usb_device *usb_dev = dci_obj->usb_device;
> +	int err = kstrtou16(buf, 16, &val);

Could we move function calls which can fail out of the declaration block?
Otherwise there is a blank or worse between the call and the error
check.

> +
> +	if (err)
> +		return err;
> +
> +	if (!strcmp(name, "arb_address")) {

Use sysfs_streq()?

> +		dci_obj->reg_addr = val;
> +		return count;
> +	}
> +
> +	if (!strcmp(name, "arb_value"))
> +		err = drci_wr_reg(usb_dev, dci_obj->reg_addr, val);
> +	else if (!strcmp(name, "sync_ep"))
> +		err = start_sync_ep(usb_dev, val);
> +	else if (!get_static_reg_addr(rw_regs, name, &reg_addr))
> +		err = drci_wr_reg(usb_dev, reg_addr, val);
> +	else
> +		return -EFAULT;

A lot of -EFAULT should be changed to -EINVAL.  Search for it everywhere,
please.

> +
> +	if (err < 0)
> +		return err;
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR(ni_state, 0444, value_show, NULL);
> +static DEVICE_ATTR(packet_bandwidth, 0444, value_show, NULL);
> +static DEVICE_ATTR(node_address, 0444, value_show, NULL);
> +static DEVICE_ATTR(node_position, 0444, value_show, NULL);
> +static DEVICE_ATTR(sync_ep, 0200, NULL, value_store);
> +static DEVICE_ATTR(mep_filter, 0644, value_show, value_store);
> +static DEVICE_ATTR(mep_hash0, 0644, value_show, value_store);
> +static DEVICE_ATTR(mep_hash1, 0644, value_show, value_store);
> +static DEVICE_ATTR(mep_hash2, 0644, value_show, value_store);
> +static DEVICE_ATTR(mep_hash3, 0644, value_show, value_store);
> +static DEVICE_ATTR(mep_eui48_hi, 0644, value_show, value_store);
> +static DEVICE_ATTR(mep_eui48_mi, 0644, value_show, value_store);
> +static DEVICE_ATTR(mep_eui48_lo, 0644, value_show, value_store);
> +static DEVICE_ATTR(arb_address, 0644, value_show, value_store);
> +static DEVICE_ATTR(arb_value, 0644, value_show, value_store);
> +
> +static struct attribute *dci_attrs[] = {
> +	&dev_attr_ni_state.attr,
> +	&dev_attr_packet_bandwidth.attr,
> +	&dev_attr_node_address.attr,
> +	&dev_attr_node_position.attr,
> +	&dev_attr_sync_ep.attr,
> +	&dev_attr_mep_filter.attr,
> +	&dev_attr_mep_hash0.attr,
> +	&dev_attr_mep_hash1.attr,
> +	&dev_attr_mep_hash2.attr,
> +	&dev_attr_mep_hash3.attr,
> +	&dev_attr_mep_eui48_hi.attr,
> +	&dev_attr_mep_eui48_mi.attr,
> +	&dev_attr_mep_eui48_lo.attr,
> +	&dev_attr_arb_address.attr,
> +	&dev_attr_arb_value.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group dci_attr_group = {
> +	.attrs = dci_attrs,
> +};
> +
> +static const struct attribute_group *dci_attr_groups[] = {
> +	&dci_attr_group,
> +	NULL,
> +};
> +
> +static void release_dci(struct device *dev)
> +{
> +	struct most_dci_obj *dci = to_dci_obj(dev);
> +
> +	kfree(dci);
> +}
> +
> +static void release_mdev(struct device *dev)
> +{
> +	struct most_dev *mdev = to_mdev_from_dev(dev);
> +
> +	kfree(mdev);
> +}
> +/**
> + * hdm_probe - probe function of USB device driver
> + * @interface: Interface of the attached USB device
> + * @id: Pointer to the USB ID table.
> + *
> + * This allocates and initializes the device instance, adds the new
> + * entry to the internal list, scans the USB descriptors and registers
> + * the interface with the core.
> + * Additionally, the DCI objects are created and the hardware is sync'd.
> + *
> + * Return 0 on success. In case of an error a negative number is returned.
> + */
> +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 usb_endpoint_descriptor *ep_desc;
> +	int ret = 0;
> +
> +	if (!mdev)
> +		goto err_out_of_memory;

The "goto" checks for if err isn't set an defaults to -ENOMEM.  Please
just set the error code.  But actually here the appropriate thing is
to just return directly:

	mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
	if (!mdev)
		return -ENOMEM;

> +
> +	usb_set_intfdata(interface, mdev);
> +	num_endpoints = usb_iface_desc->desc.bNumEndpoints;
> +	mutex_init(&mdev->io_mutex);
> +	INIT_WORK(&mdev->poll_work_obj, wq_netinfo);
> +	timer_setup(&mdev->link_stat_timer, link_stat_timer_handler, 0);
> +
> +	mdev->usb_device = usb_dev;
> +	mdev->link_stat_timer.expires = jiffies + (2 * HZ);
> +
> +	mdev->iface.mod = hdm_usb_fops.owner;
> +	mdev->iface.dev = &mdev->dev;
> +	mdev->iface.driver_dev = &interface->dev;
> +	mdev->iface.interface = ITYPE_USB;
> +	mdev->iface.configure = hdm_configure_channel;
> +	mdev->iface.request_netinfo = hdm_request_netinfo;
> +	mdev->iface.enqueue = hdm_enqueue;
> +	mdev->iface.poison_channel = hdm_poison_channel;
> +	mdev->iface.dma_alloc = hdm_dma_alloc;
> +	mdev->iface.dma_free = hdm_dma_free;
> +	mdev->iface.description = mdev->description;
> +	mdev->iface.num_channels = num_endpoints;
> +
> +	snprintf(mdev->description, sizeof(mdev->description),
> +		 "%d-%s:%d.%d",
> +		 usb_dev->bus->busnum,
> +		 usb_dev->devpath,
> +		 usb_dev->config->desc.bConfigurationValue,
> +		 usb_iface_desc->desc.bInterfaceNumber);
> +
> +	mdev->dev.init_name = mdev->description;
> +	mdev->dev.parent = &interface->dev;
> +	mdev->dev.release = release_mdev;
> +	mdev->conf = kcalloc(num_endpoints, sizeof(*mdev->conf), GFP_KERNEL);
> +	if (!mdev->conf)
> +		goto err_free_mdev;
> +
> +	mdev->cap = kcalloc(num_endpoints, sizeof(*mdev->cap), GFP_KERNEL);
> +	if (!mdev->cap)
> +		goto err_free_conf;
> +
> +	mdev->iface.channel_vector = mdev->cap;
> +	mdev->ep_address = kcalloc(num_endpoints, sizeof(*mdev->ep_address), GFP_KERNEL);
> +	if (!mdev->ep_address)
> +		goto err_free_cap;
> +
> +	mdev->busy_urbs = kcalloc(num_endpoints, sizeof(*mdev->busy_urbs), GFP_KERNEL);
> +	if (!mdev->busy_urbs)
> +		goto err_free_ep_address;
> +
> +	tmp_cap = mdev->cap;
> +	for (i = 0; i < num_endpoints; i++) {
> +		ep_desc = &usb_iface_desc->endpoint[i].desc;
> +		mdev->ep_address[i] = ep_desc->bEndpointAddress;
> +		mdev->padding_active[i] = false;
> +		mdev->is_channel_healthy[i] = true;
> +
> +		snprintf(&mdev->suffix[i][0], MAX_SUFFIX_LEN, "ep%02x",
> +			 mdev->ep_address[i]);
> +
> +		tmp_cap->name_suffix = &mdev->suffix[i][0];
> +		tmp_cap->buffer_size_packet = MAX_BUF_SIZE;
> +		tmp_cap->buffer_size_streaming = MAX_BUF_SIZE;
> +		tmp_cap->num_buffers_packet = BUF_CHAIN_SIZE;
> +		tmp_cap->num_buffers_streaming = BUF_CHAIN_SIZE;
> +		tmp_cap->data_type = MOST_CH_CONTROL | MOST_CH_ASYNC |
> +				     MOST_CH_ISOC | MOST_CH_SYNC;
> +		if (usb_endpoint_dir_in(ep_desc))
> +			tmp_cap->direction = MOST_CH_RX;
> +		else
> +			tmp_cap->direction = MOST_CH_TX;
> +		tmp_cap++;
> +		init_usb_anchor(&mdev->busy_urbs[i]);
> +		spin_lock_init(&mdev->channel_lock[i]);
> +	}
> +	dev_notice(dev, "claimed gadget: Vendor=%4.4x ProdID=%4.4x Bus=%02x Device=%02x\n",
> +		   le16_to_cpu(usb_dev->descriptor.idVendor),
> +		   le16_to_cpu(usb_dev->descriptor.idProduct),
> +		   usb_dev->bus->busnum,
> +		   usb_dev->devnum);
> +
> +	dev_notice(dev, "device path: /sys/bus/usb/devices/%d-%s:%d.%d\n",
> +		   usb_dev->bus->busnum,
> +		   usb_dev->devpath,
> +		   usb_dev->config->desc.bConfigurationValue,
> +		   usb_iface_desc->desc.bInterfaceNumber);
> +
> +	ret = most_register_interface(&mdev->iface);
> +	if (ret)
> +		goto err_free_busy_urbs;
> +
> +	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_OS81210) {
> +		mdev->dci = kzalloc(sizeof(*mdev->dci), GFP_KERNEL);
> +		if (!mdev->dci) {
> +			mutex_unlock(&mdev->io_mutex);
> +			most_deregister_interface(&mdev->iface);

Free iface after the goto.

> +			ret = -ENOMEM;
> +			goto err_free_busy_urbs;
> +		}
> +
> +		mdev->dci->dev.init_name = "dci";
> +		mdev->dci->dev.parent = get_device(mdev->iface.dev);
                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^
Does this get_device() need a matching put_device() somewhere?  I'm not
totally sure how the ->parent stuff works...

> +		mdev->dci->dev.groups = dci_attr_groups;
> +		mdev->dci->dev.release = release_dci;
> +		if (device_register(&mdev->dci->dev)) {
> +			mutex_unlock(&mdev->io_mutex);
> +			most_deregister_interface(&mdev->iface);
> +			ret = -ENOMEM;
> +			goto err_free_dci;
> +		}
> +		mdev->dci->usb_device = mdev->usb_device;
> +	}
> +	mutex_unlock(&mdev->io_mutex);
> +	return 0;
> +err_free_dci:
> +	put_device(&mdev->dci->dev);
> +err_free_busy_urbs:
> +	kfree(mdev->busy_urbs);
> +err_free_ep_address:
> +	kfree(mdev->ep_address);
> +err_free_cap:
> +	kfree(mdev->cap);
> +err_free_conf:
> +	kfree(mdev->conf);
> +err_free_mdev:
> +	put_device(&mdev->dev);
> +err_out_of_memory:
> +	if (ret == 0 || ret == -ENOMEM) {
> +		ret = -ENOMEM;
> +		dev_err(dev, "out of memory\n");
> +	}
> +	return ret;
> +}

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/8] drivers: most: add usb adapter driver
  2020-05-20 13:17   ` Dan Carpenter
@ 2020-05-20 13:54     ` Christian.Gromm
  0 siblings, 0 replies; 16+ messages in thread
From: Christian.Gromm @ 2020-05-20 13:54 UTC (permalink / raw)
  To: dan.carpenter; +Cc: driverdev-devel, gregkh, linux-usb

On Wed, 2020-05-20 at 16:17 +0300, Dan Carpenter wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Thu, May 14, 2020 at 11:52:49AM +0200, Christian Gromm wrote:
> > This patch adds the usb driver source file most_usb.c and
> > modifies the Makefile and Kconfig accordingly.
> > 
> > Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> > ---
> > v2:
> > Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >       - don't remove usb driver from staging area
> >       - don't touch staging/most/Kconfig
> >       - remove subdirectory for USB driver and put source file into
> >         drivers/most
> > 
> >  drivers/most/Kconfig    |   12 +
> >  drivers/most/Makefile   |    2 +
> >  drivers/most/most_usb.c | 1262
> > +++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 1276 insertions(+)
> >  create mode 100644 drivers/most/most_usb.c
> > 
> > 

There is already a v3 out there that has some of your concerns
already addressed. I'll take your comments and fix them in the
next version of the patch.

thanks,
Chris





^ permalink raw reply	[flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-05-20 13:54 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-14  9:52 [PATCH v2 0/8] staging: most: move USB adapter driver to stable branch Christian Gromm
2020-05-14  9:52 ` [PATCH v2 1/8] drivers: most: add usb adapter driver Christian Gromm
2020-05-14 10:25   ` Greg KH
2020-05-20 13:17   ` Dan Carpenter
2020-05-20 13:54     ` Christian.Gromm
2020-05-14  9:52 ` [PATCH v2 2/8] drivers: most: usb: use dev_*() functions to print messages Christian Gromm
2020-05-19 13:42   ` Dan Carpenter
2020-05-14  9:52 ` [PATCH v2 3/8] drivers: most: usb: remove reference to USB error codes Christian Gromm
2020-05-14  9:52 ` [PATCH v2 4/8] drivers: most: usb: check number of reported endpoints Christian Gromm
2020-05-14 20:53   ` kbuild test robot
2020-05-14 20:53     ` kbuild test robot
2020-05-14  9:52 ` [PATCH v2 5/8] drivers: most: usb: use dev_dbg function Christian Gromm
2020-05-19 13:43   ` Dan Carpenter
2020-05-14  9:52 ` [PATCH v2 6/8] drivers: most: fix typo in Kconfig Christian Gromm
2020-05-14  9:52 ` [PATCH v2 7/8] drivers: most: usb: use macro ATTRIBUTE_GROUPS Christian Gromm
2020-05-14  9:52 ` [PATCH v2 8/8] Documentation: ABI: correct sysfs attribute description of MOST driver 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.