[PATCH v2 0/7] Qualcomm WCNSS HCI support

[PATCH v2 0/7] Qualcomm WCNSS HCI support

[Bjorn Andersson]

After trying to avoid implementing multi-channel support in SMD in v1 of
the HCI driver for Qualcomm WCNSS BT, this new version includes the
necessary SMD refactoring and additon of an API that allows SMD devices
to call back into the SMD core to acquire additonal channels.

The additional channels are tied to the existing SMD device and the life
cycle of the new channel will be tied to, and affect, the original
channel.

With this in place the btqcomsmd driver is refactored into being a
single driver, without global state.

Bjorn Andersson (7):
  soc: qcom: smd: Introduce callback setter
  soc: qcom: smd: Split discovery and state change work
  soc: qcom: smd: Refactor channel open and close handling
  soc: qcom: smd: Support multiple channels per sdev
  soc: qcom: smd: Support opening additional channels
  Bluetooth: Add HCI device identifier for Qualcomm SMD
  Bluetooth: hci_smd: Qualcomm WCNSS HCI driver

 drivers/bluetooth/Kconfig     |  11 ++
 drivers/bluetooth/Makefile    |   1 +
 drivers/bluetooth/btqcomsmd.c | 198 ++++++++++++++++++++++++++++++++++++
 drivers/soc/qcom/smd.c        | 228 +++++++++++++++++++++++++++++++-----------
 include/linux/soc/qcom/smd.h  |   8 +-
 include/net/bluetooth/hci.h   |   1 +
 6 files changed, 387 insertions(+), 60 deletions(-)
 create mode 100644 drivers/bluetooth/btqcomsmd.c

-- 
2.4.2

[PATCH v2 1/7] soc: qcom: smd: Introduce callback setter

[Bjorn Andersson]

Introduce a setter for the callback function pointer to clarify the
locking around the operation and to reduce some duplication.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---

Changes since v1:
- New patch

 drivers/soc/qcom/smd.c       | 25 +++++++++++++++++--------
 include/linux/soc/qcom/smd.h |  4 +++-
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/soc/qcom/smd.c b/drivers/soc/qcom/smd.c
index 18964f154383..8b401d89b0d0 100644
--- a/drivers/soc/qcom/smd.c
+++ b/drivers/soc/qcom/smd.c
@@ -186,7 +186,7 @@ struct qcom_smd_channel {
 	int fifo_size;
 
 	void *bounce_buffer;
-	int (*cb)(struct qcom_smd_device *, const void *, size_t);
+	qcom_smd_cb_t cb;
 
 	spinlock_t recv_lock;
 
@@ -378,6 +378,19 @@ static void qcom_smd_channel_reset(struct qcom_smd_channel *channel)
 }
 
 /*
+ * Set the callback for a channel, with appropriate locking
+ */
+static void qcom_smd_channel_set_callback(struct qcom_smd_channel *channel,
+					  qcom_smd_cb_t cb)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&channel->recv_lock, flags);
+	channel->cb = cb;
+	spin_unlock_irqrestore(&channel->recv_lock, flags);
+};
+
+/*
  * Calculate the amount of data available in the rx fifo
  */
 static size_t qcom_smd_channel_get_rx_avail(struct qcom_smd_channel *channel)
@@ -815,8 +828,7 @@ static int qcom_smd_dev_probe(struct device *dev)
 	if (!channel->bounce_buffer)
 		return -ENOMEM;
 
-	channel->cb = qsdrv->callback;
-
+	qcom_smd_channel_set_callback(channel, qsdrv->callback);
 	qcom_smd_channel_set_state(channel, SMD_CHANNEL_OPENING);
 
 	qcom_smd_channel_set_state(channel, SMD_CHANNEL_OPENED);
@@ -832,7 +844,7 @@ static int qcom_smd_dev_probe(struct device *dev)
 err:
 	dev_err(&qsdev->dev, "probe failed\n");
 
-	channel->cb = NULL;
+	qcom_smd_channel_set_callback(channel, NULL);
 	kfree(channel->bounce_buffer);
 	channel->bounce_buffer = NULL;
 
@@ -851,16 +863,13 @@ static int qcom_smd_dev_remove(struct device *dev)
 	struct qcom_smd_device *qsdev = to_smd_device(dev);
 	struct qcom_smd_driver *qsdrv = to_smd_driver(dev);
 	struct qcom_smd_channel *channel = qsdev->channel;
-	unsigned long flags;
 
 	qcom_smd_channel_set_state(channel, SMD_CHANNEL_CLOSING);
 
 	/*
 	 * Make sure we don't race with the code receiving data.
 	 */
-	spin_lock_irqsave(&channel->recv_lock, flags);
-	channel->cb = NULL;
-	spin_unlock_irqrestore(&channel->recv_lock, flags);
+	qcom_smd_channel_set_callback(channel, NULL);
 
 	/* Wake up any sleepers in qcom_smd_send() */
 	wake_up_interruptible(&channel->fblockread_event);
diff --git a/include/linux/soc/qcom/smd.h b/include/linux/soc/qcom/smd.h
index d0cb6d189a0a..65a64fcdb1aa 100644
--- a/include/linux/soc/qcom/smd.h
+++ b/include/linux/soc/qcom/smd.h
@@ -26,6 +26,8 @@ struct qcom_smd_device {
 	struct qcom_smd_channel *channel;
 };
 
+typedef int (*qcom_smd_cb_t)(struct qcom_smd_device *, const void *, size_t);
+
 /**
  * struct qcom_smd_driver - smd driver struct
  * @driver:	underlying device driver
@@ -42,7 +44,7 @@ struct qcom_smd_driver {
 
 	int (*probe)(struct qcom_smd_device *dev);
 	void (*remove)(struct qcom_smd_device *dev);
-	int (*callback)(struct qcom_smd_device *, const void *, size_t);
+	qcom_smd_cb_t callback;
 };
 
 int qcom_smd_driver_register(struct qcom_smd_driver *drv);
-- 
2.4.2

[PATCH v2 2/7] soc: qcom: smd: Split discovery and state change work

[Bjorn Andersson]

Split the two steps of channel discovery and state change handling into
two different workers. This allows for new channels to be found while
we're are probing, which is required as we introduce multi-channel
support.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---

Changes since v1:
- New patch

 drivers/soc/qcom/smd.c | 58 +++++++++++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/soc/qcom/smd.c b/drivers/soc/qcom/smd.c
index 8b401d89b0d0..fb5f91efd0da 100644
--- a/drivers/soc/qcom/smd.c
+++ b/drivers/soc/qcom/smd.c
@@ -106,9 +106,9 @@ static const struct {
  * @channels:		list of all channels detected on this edge
  * @channels_lock:	guard for modifications of @channels
  * @allocated:		array of bitmaps representing already allocated channels
- * @need_rescan:	flag that the @work needs to scan smem for new channels
  * @smem_available:	last available amount of smem triggering a channel scan
- * @work:		work item for edge house keeping
+ * @scan_work:		work item for discovering new channels
+ * @state_work:		work item for edge state changes
  */
 struct qcom_smd_edge {
 	struct qcom_smd *smd;
@@ -123,14 +123,14 @@ struct qcom_smd_edge {
 	int ipc_bit;
 
 	struct list_head channels;
-	spinlock_t channels_lock;
+	rwlock_t channels_lock;
 
 	DECLARE_BITMAP(allocated[SMD_ALLOC_TBL_COUNT], SMD_ALLOC_TBL_SIZE);
 
-	bool need_rescan;
 	unsigned smem_available;
 
-	struct work_struct work;
+	struct work_struct scan_work;
+	struct work_struct state_work;
 };
 
 /*
@@ -624,13 +624,13 @@ static irqreturn_t qcom_smd_edge_intr(int irq, void *data)
 	/*
 	 * Handle state changes or data on each of the channels on this edge
 	 */
-	spin_lock(&edge->channels_lock);
+	read_lock(&edge->channels_lock);
 	list_for_each_entry(channel, &edge->channels, list) {
 		spin_lock(&channel->recv_lock);
 		kick_worker |= qcom_smd_channel_intr(channel);
 		spin_unlock(&channel->recv_lock);
 	}
-	spin_unlock(&edge->channels_lock);
+	read_unlock(&edge->channels_lock);
 
 	/*
 	 * Creating a new channel requires allocating an smem entry, so we only
@@ -640,12 +640,11 @@ static irqreturn_t qcom_smd_edge_intr(int irq, void *data)
 	available = qcom_smem_get_free_space(edge->remote_pid);
 	if (available != edge->smem_available) {
 		edge->smem_available = available;
-		edge->need_rescan = true;
 		kick_worker = true;
 	}
 
 	if (kick_worker)
-		schedule_work(&edge->work);
+		schedule_work(&edge->scan_work);
 
 	return IRQ_HANDLED;
 }
@@ -1101,8 +1100,9 @@ free_name_and_channel:
  * qcom_smd_create_channel() to create representations of these and add
  * them to the edge's list of channels.
  */
-static void qcom_discover_channels(struct qcom_smd_edge *edge)
+static void qcom_channel_scan_worker(struct work_struct *work)
 {
+	struct qcom_smd_edge *edge = container_of(work, struct qcom_smd_edge, scan_work);
 	struct qcom_smd_alloc_entry *alloc_tbl;
 	struct qcom_smd_alloc_entry *entry;
 	struct qcom_smd_channel *channel;
@@ -1146,16 +1146,16 @@ static void qcom_discover_channels(struct qcom_smd_edge *edge)
 			if (IS_ERR(channel))
 				continue;
 
-			spin_lock_irqsave(&edge->channels_lock, flags);
+			write_lock_irqsave(&edge->channels_lock, flags);
 			list_add(&channel->list, &edge->channels);
-			spin_unlock_irqrestore(&edge->channels_lock, flags);
+			write_unlock_irqrestore(&edge->channels_lock, flags);
 
 			dev_dbg(smd->dev, "new channel found: '%s'\n", channel->name);
 			set_bit(i, edge->allocated[tbl]);
 		}
 	}
 
-	schedule_work(&edge->work);
+	schedule_work(&edge->state_work);
 }
 
 /*
@@ -1163,29 +1163,22 @@ static void qcom_discover_channels(struct qcom_smd_edge *edge)
  * then scans all registered channels for state changes that should be handled
  * by creating or destroying smd client devices for the registered channels.
  *
- * LOCKING: edge->channels_lock is not needed to be held during the traversal
- * of the channels list as it's done synchronously with the only writer.
+ * LOCKING: edge->channels_lock only needs to cover the list operations, as the
+ * worker is killed before any channels are deallocated
  */
 static void qcom_channel_state_worker(struct work_struct *work)
 {
 	struct qcom_smd_channel *channel;
 	struct qcom_smd_edge *edge = container_of(work,
 						  struct qcom_smd_edge,
-						  work);
+						  state_work);
 	unsigned remote_state;
 
 	/*
-	 * Rescan smem if we have reason to belive that there are new channels.
-	 */
-	if (edge->need_rescan) {
-		edge->need_rescan = false;
-		qcom_discover_channels(edge);
-	}
-
-	/*
 	 * Register a device for any closed channel where the remote processor
 	 * is showing interest in opening the channel.
 	 */
+	read_lock(&edge->channels_lock);
 	list_for_each_entry(channel, &edge->channels, list) {
 		if (channel->state != SMD_CHANNEL_CLOSED)
 			continue;
@@ -1195,7 +1188,9 @@ static void qcom_channel_state_worker(struct work_struct *work)
 		    remote_state != SMD_CHANNEL_OPENED)
 			continue;
 
+		read_unlock(&edge->channels_lock);
 		qcom_smd_create_device(channel);
+		read_lock(&edge->channels_lock);
 	}
 
 	/*
@@ -1212,8 +1207,11 @@ static void qcom_channel_state_worker(struct work_struct *work)
 		    remote_state == SMD_CHANNEL_OPENED)
 			continue;
 
+		read_unlock(&edge->channels_lock);
 		qcom_smd_destroy_device(channel);
+		read_lock(&edge->channels_lock);
 	}
+	read_unlock(&edge->channels_lock);
 }
 
 /*
@@ -1229,9 +1227,10 @@ static int qcom_smd_parse_edge(struct device *dev,
 	int ret;
 
 	INIT_LIST_HEAD(&edge->channels);
-	spin_lock_init(&edge->channels_lock);
+	rwlock_init(&edge->channels_lock);
 
-	INIT_WORK(&edge->work, qcom_channel_state_worker);
+	INIT_WORK(&edge->scan_work, qcom_channel_scan_worker);
+	INIT_WORK(&edge->state_work, qcom_channel_state_worker);
 
 	edge->of_node = of_node_get(node);
 
@@ -1320,8 +1319,7 @@ static int qcom_smd_probe(struct platform_device *pdev)
 		if (ret)
 			continue;
 
-		edge->need_rescan = true;
-		schedule_work(&edge->work);
+		schedule_work(&edge->scan_work);
 	}
 
 	platform_set_drvdata(pdev, smd);
@@ -1344,8 +1342,10 @@ static int qcom_smd_remove(struct platform_device *pdev)
 		edge = &smd->edges[i];
 
 		disable_irq(edge->irq);
-		cancel_work_sync(&edge->work);
+		cancel_work_sync(&edge->scan_work);
+		cancel_work_sync(&edge->state_work);
 
+		/* No need to lock here, because the writer is gone */
 		list_for_each_entry(channel, &edge->channels, list) {
 			if (!channel->qsdev)
 				continue;
-- 
2.4.2

[PATCH v2 3/7] soc: qcom: smd: Refactor channel open and close handling

[Bjorn Andersson]

Refactor opening and closing of channels into two separate functions
instead of open coding this in the various places.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---

Changes since v1:
- New patch

 drivers/soc/qcom/smd.c | 62 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 40 insertions(+), 22 deletions(-)

diff --git a/drivers/soc/qcom/smd.c b/drivers/soc/qcom/smd.c
index fb5f91efd0da..669bda585dad 100644
--- a/drivers/soc/qcom/smd.c
+++ b/drivers/soc/qcom/smd.c
@@ -806,18 +806,12 @@ static int qcom_smd_dev_match(struct device *dev, struct device_driver *drv)
 }
 
 /*
- * Probe the smd client.
- *
- * The remote side have indicated that it want the channel to be opened, so
- * complete the state handshake and probe our client driver.
+ * Helper for opening a channel
  */
-static int qcom_smd_dev_probe(struct device *dev)
+static int qcom_smd_channel_open(struct qcom_smd_channel *channel,
+				 qcom_smd_cb_t cb)
 {
-	struct qcom_smd_device *qsdev = to_smd_device(dev);
-	struct qcom_smd_driver *qsdrv = to_smd_driver(dev);
-	struct qcom_smd_channel *channel = qsdev->channel;
 	size_t bb_size;
-	int ret;
 
 	/*
 	 * Packets are maximum 4k, but reduce if the fifo is smaller
@@ -827,11 +821,44 @@ static int qcom_smd_dev_probe(struct device *dev)
 	if (!channel->bounce_buffer)
 		return -ENOMEM;
 
-	qcom_smd_channel_set_callback(channel, qsdrv->callback);
+	qcom_smd_channel_set_callback(channel, cb);
 	qcom_smd_channel_set_state(channel, SMD_CHANNEL_OPENING);
-
 	qcom_smd_channel_set_state(channel, SMD_CHANNEL_OPENED);
 
+	return 0;
+}
+
+/*
+ * Helper for closing and resetting a channel
+ */
+static void qcom_smd_channel_close(struct qcom_smd_channel *channel)
+{
+	qcom_smd_channel_set_callback(channel, NULL);
+
+	kfree(channel->bounce_buffer);
+	channel->bounce_buffer = NULL;
+
+	qcom_smd_channel_set_state(channel, SMD_CHANNEL_CLOSED);
+	qcom_smd_channel_reset(channel);
+}
+
+/*
+ * Probe the smd client.
+ *
+ * The remote side have indicated that it want the channel to be opened, so
+ * complete the state handshake and probe our client driver.
+ */
+static int qcom_smd_dev_probe(struct device *dev)
+{
+	struct qcom_smd_device *qsdev = to_smd_device(dev);
+	struct qcom_smd_driver *qsdrv = to_smd_driver(dev);
+	struct qcom_smd_channel *channel = qsdev->channel;
+	int ret;
+
+	ret = qcom_smd_channel_open(channel, qsdrv->callback);
+	if (ret)
+		return ret;
+
 	ret = qsdrv->probe(qsdev);
 	if (ret)
 		goto err;
@@ -843,11 +870,7 @@ static int qcom_smd_dev_probe(struct device *dev)
 err:
 	dev_err(&qsdev->dev, "probe failed\n");
 
-	qcom_smd_channel_set_callback(channel, NULL);
-	kfree(channel->bounce_buffer);
-	channel->bounce_buffer = NULL;
-
-	qcom_smd_channel_set_state(channel, SMD_CHANNEL_CLOSED);
+	qcom_smd_channel_close(channel);
 	return ret;
 }
 
@@ -884,12 +907,7 @@ static int qcom_smd_dev_remove(struct device *dev)
 	 * The client is now gone, cleanup and reset the channel state.
 	 */
 	channel->qsdev = NULL;
-	kfree(channel->bounce_buffer);
-	channel->bounce_buffer = NULL;
-
-	qcom_smd_channel_set_state(channel, SMD_CHANNEL_CLOSED);
-
-	qcom_smd_channel_reset(channel);
+	qcom_smd_channel_close(channel);
 
 	return 0;
 }
-- 
2.4.2

[PATCH v2 4/7] soc: qcom: smd: Support multiple channels per sdev

[Bjorn Andersson]

This patch allows chaining additional channels to a SMD device, enabling
implementation of multi-channel SMD devies - like Bluetooth.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---

Changes since v1:
- New patch

 drivers/soc/qcom/smd.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/qcom/smd.c b/drivers/soc/qcom/smd.c
index 669bda585dad..4c55708aac50 100644
--- a/drivers/soc/qcom/smd.c
+++ b/drivers/soc/qcom/smd.c
@@ -193,6 +193,7 @@ struct qcom_smd_channel {
 	int pkt_size;
 
 	struct list_head list;
+	struct list_head dev_list;
 };
 
 /**
@@ -885,6 +886,8 @@ static int qcom_smd_dev_remove(struct device *dev)
 	struct qcom_smd_device *qsdev = to_smd_device(dev);
 	struct qcom_smd_driver *qsdrv = to_smd_driver(dev);
 	struct qcom_smd_channel *channel = qsdev->channel;
+	struct qcom_smd_channel *tmp;
+	struct qcom_smd_channel *ch;
 
 	qcom_smd_channel_set_state(channel, SMD_CHANNEL_CLOSING);
 
@@ -904,10 +907,14 @@ static int qcom_smd_dev_remove(struct device *dev)
 		qsdrv->remove(qsdev);
 
 	/*
-	 * The client is now gone, cleanup and reset the channel state.
+	 * The client is now gone, close and release all channels associated
+	 * with this sdev
 	 */
-	channel->qsdev = NULL;
-	qcom_smd_channel_close(channel);
+	list_for_each_entry_safe(ch, tmp, &channel->dev_list, dev_list) {
+		qcom_smd_channel_close(ch);
+		list_del(&ch->dev_list);
+		ch->qsdev = NULL;
+	}
 
 	return 0;
 }
@@ -1056,6 +1063,7 @@ static struct qcom_smd_channel *qcom_smd_create_channel(struct qcom_smd_edge *ed
 	if (!channel)
 		return ERR_PTR(-ENOMEM);
 
+	INIT_LIST_HEAD(&channel->dev_list);
 	channel->edge = edge;
 	channel->name = devm_kstrdup(smd->dev, name, GFP_KERNEL);
 	if (!channel->name)
-- 
2.4.2

[PATCH v2 5/7] soc: qcom: smd: Support opening additional channels

[Bjorn Andersson]

With the qcom_smd_open_channel() API we allow SMD devices to open
additional SMD channels, to allow implementation of multi-channel SMD
devices - like Bluetooth.

Channels are opened from the same edge as the calling SMD device is tied
to.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---

Changes since v1:
- New patch

 drivers/soc/qcom/smd.c       | 75 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/soc/qcom/smd.h |  4 +++
 2 files changed, 79 insertions(+)

diff --git a/drivers/soc/qcom/smd.c b/drivers/soc/qcom/smd.c
index 4c55708aac50..3257ba488d3d 100644
--- a/drivers/soc/qcom/smd.c
+++ b/drivers/soc/qcom/smd.c
@@ -129,6 +129,8 @@ struct qcom_smd_edge {
 
 	unsigned smem_available;
 
+	wait_queue_head_t new_channel_event;
+
 	struct work_struct scan_work;
 	struct work_struct state_work;
 };
@@ -1042,6 +1044,76 @@ void qcom_smd_driver_unregister(struct qcom_smd_driver *qsdrv)
 }
 EXPORT_SYMBOL(qcom_smd_driver_unregister);
 
+static struct qcom_smd_channel *
+qcom_smd_find_channel(struct qcom_smd_edge *edge, const char *name)
+{
+	struct qcom_smd_channel *channel;
+	struct qcom_smd_channel *ret = NULL;
+	unsigned state;
+
+	read_lock(&edge->channels_lock);
+	list_for_each_entry(channel, &edge->channels, list) {
+		if (strcmp(channel->name, name))
+			continue;
+
+		state = GET_RX_CHANNEL_INFO(channel, state);
+		if (state != SMD_CHANNEL_OPENING &&
+		    state != SMD_CHANNEL_OPENED)
+			continue;
+
+		ret = channel;
+		break;
+	}
+	read_unlock(&edge->channels_lock);
+
+	return ret;
+}
+
+/**
+ * qcom_smd_open_channel() - claim additional channels on the same edge
+ * @sdev:	smd_device handle
+ * @name:	channel name
+ * @cb:		callback method to use for incoming data
+ *
+ * Returns a channel handle on success, or -EPROBE_DEFER if the channel isn't
+ * ready.
+ */
+struct qcom_smd_channel *qcom_smd_open_channel(struct qcom_smd_device *sdev,
+					       const char *name,
+					       qcom_smd_cb_t cb)
+{
+	struct qcom_smd_channel *channel;
+	struct qcom_smd_edge *edge = sdev->channel->edge;
+	int ret;
+
+	/* Wait up to HZ for the channel to appear */
+	ret = wait_event_interruptible_timeout(edge->new_channel_event,
+			(channel = qcom_smd_find_channel(edge, name)) != NULL,
+			HZ);
+	if (!ret)
+		return ERR_PTR(-ETIMEDOUT);
+
+	if (channel->state != SMD_CHANNEL_CLOSED) {
+		dev_err(&sdev->dev, "channel %s is busy\n", channel->name);
+		return ERR_PTR(-EBUSY);
+	}
+
+	channel->qsdev = sdev;
+	ret = qcom_smd_channel_open(channel, cb);
+	if (ret) {
+		channel->qsdev = NULL;
+		return ERR_PTR(ret);
+	}
+
+	/*
+	 * Append the list of channel to the channels associated with the sdev
+	 */
+	list_add_tail(&channel->dev_list, &sdev->channel->dev_list);
+
+	return channel;
+}
+EXPORT_SYMBOL(qcom_smd_open_channel);
+
 /*
  * Allocate the qcom_smd_channel object for a newly found smd channel,
  * retrieving and validating the smem items involved.
@@ -1178,6 +1250,8 @@ static void qcom_channel_scan_worker(struct work_struct *work)
 
 			dev_dbg(smd->dev, "new channel found: '%s'\n", channel->name);
 			set_bit(i, edge->allocated[tbl]);
+
+			wake_up_interruptible(&edge->new_channel_event);
 		}
 	}
 
@@ -1340,6 +1414,7 @@ static int qcom_smd_probe(struct platform_device *pdev)
 	for_each_available_child_of_node(pdev->dev.of_node, node) {
 		edge = &smd->edges[i++];
 		edge->smd = smd;
+		init_waitqueue_head(&edge->new_channel_event);
 
 		ret = qcom_smd_parse_edge(&pdev->dev, node, edge);
 		if (ret)
diff --git a/include/linux/soc/qcom/smd.h b/include/linux/soc/qcom/smd.h
index 65a64fcdb1aa..bd51c8a9d807 100644
--- a/include/linux/soc/qcom/smd.h
+++ b/include/linux/soc/qcom/smd.h
@@ -56,4 +56,8 @@ void qcom_smd_driver_unregister(struct qcom_smd_driver *drv);
 
 int qcom_smd_send(struct qcom_smd_channel *channel, const void *data, int len);
 
+struct qcom_smd_channel *qcom_smd_open_channel(struct qcom_smd_device *sdev,
+					       const char *name,
+					       qcom_smd_cb_t cb);
+
 #endif
-- 
2.4.2

[PATCH v2 6/7] Bluetooth: Add HCI device identifier for Qualcomm SMD

[Bjorn Andersson]

This patch assigns the next free HCI device identifier to Bluetooth
devices based on the Qualcomm Shared Memory channels.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---

Changes since v1:
- Split out this from the btqcomsmd patch

 include/net/bluetooth/hci.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index e7f938cac7c6..adfb371a19f9 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -60,6 +60,7 @@
 #define HCI_RS232	4
 #define HCI_PCI		5
 #define HCI_SDIO	6
+#define HCI_SMD		7
 
 /* HCI controller types */
 #define HCI_BREDR	0x00
-- 
2.4.2

[PATCH v2 7/7] Bluetooth: btqcomsmd: Qualcomm WCNSS HCI driver

[Bjorn Andersson]

The Qualcomm WCNSS chip provides two SMD channels to the BT core; one
for command and one for event packets. This driver exposes the two
channels as a hci device.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---

Changes since v1:
- With the introduction of qcom_smd_open_channel() the two drivers are now one
- No more global state
- Corrected memory management of sk_buffs
- Reverted to __hci_cmd_sync_ev() for set_bdaddr
- Renamed the driver to btqcomsmd
- Split out the addition of HCI_SMD to separate patch

 drivers/bluetooth/Kconfig     |  11 +++
 drivers/bluetooth/Makefile    |   1 +
 drivers/bluetooth/btqcomsmd.c | 198 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 210 insertions(+)
 create mode 100644 drivers/bluetooth/btqcomsmd.c

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 62999546a301..1652997e59cc 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -169,6 +169,17 @@ config BT_HCIUART_QCA
 
 	  Say Y here to compile support for QCA protocol.
 
+config BT_QCOMSMD
+	tristate "Qualcomm SMD based HCI support"
+	depends on QCOM_SMD
+	help
+	  Qualcomm SMD based HCI driver.
+	  This driver is used to bridge HCI data onto the shared memory
+	  channels to the WCNSS core.
+
+	  Say Y here to compile support for HCI over Qualcomm SMD into the
+	  kernelor say M to compile as a module.
+
 config BT_HCIBCM203X
 	tristate "HCI BCM203x USB driver"
 	depends on USB
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index 07c9cf381e5a..19e313bf8c39 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_BT_WILINK)		+= btwilink.o
 obj-$(CONFIG_BT_BCM)		+= btbcm.o
 obj-$(CONFIG_BT_RTL)		+= btrtl.o
 obj-$(CONFIG_BT_QCA)		+= btqca.o
+obj-$(CONFIG_BT_QCOMSMD)	+= btqcomsmd.o
 
 btmrvl-y			:= btmrvl_main.o
 btmrvl-$(CONFIG_DEBUG_FS)	+= btmrvl_debugfs.o
diff --git a/drivers/bluetooth/btqcomsmd.c b/drivers/bluetooth/btqcomsmd.c
new file mode 100644
index 000000000000..4b91c830531e
--- /dev/null
+++ b/drivers/bluetooth/btqcomsmd.c
@@ -0,0 +1,198 @@
+/*
+ * Copyright (c) 2015, Sony Mobile Communications Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/soc/qcom/smd.h>
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+#include <net/bluetooth/hci.h>
+
+#define EDL_NVM_ACCESS_SET_REQ_CMD	0x01
+#define EDL_NVM_ACCESS_OPCODE		0xfc0b
+
+struct btqcomsmd {
+	struct qcom_smd_channel *acl_channel;
+	struct qcom_smd_channel *cmd_channel;
+};
+
+static int btqcomsmd_recv(struct hci_dev *hdev,
+			  unsigned type,
+			  const void *data,
+			  size_t count)
+{
+	struct sk_buff *skb;
+	void *buf;
+
+	/* Use GFP_ATOMIC as we're in IRQ context */
+	skb = bt_skb_alloc(count, GFP_ATOMIC);
+	if (!skb)
+		return -ENOMEM;
+
+	bt_cb(skb)->pkt_type = type;
+
+	/* Use io accessor as data might be ioremapped */
+	buf = skb_put(skb, count);
+	memcpy_fromio(buf, data, count);
+
+	return hci_recv_frame(hdev, skb);
+}
+
+static int btqcomsmd_acl_callback(struct qcom_smd_device *qsdev,
+				  const void *data,
+				  size_t count)
+{
+	struct hci_dev *hdev = dev_get_drvdata(&qsdev->dev);
+
+	return btqcomsmd_recv(hdev, HCI_ACLDATA_PKT, data, count);
+}
+
+static int btqcomsmd_cmd_callback(struct qcom_smd_device *qsdev,
+				  const void *data,
+				  size_t count)
+{
+	struct hci_dev *hdev = dev_get_drvdata(&qsdev->dev);
+
+	return btqcomsmd_recv(hdev, HCI_EVENT_PKT, data, count);
+}
+
+static int btqcomsmd_send(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct btqcomsmd *btq = hci_get_drvdata(hdev);
+	int ret;
+
+	switch (bt_cb(skb)->pkt_type) {
+	case HCI_ACLDATA_PKT:
+	case HCI_SCODATA_PKT:
+		ret = qcom_smd_send(btq->acl_channel, skb->data, skb->len);
+		break;
+	case HCI_COMMAND_PKT:
+		ret = qcom_smd_send(btq->cmd_channel, skb->data, skb->len);
+		break;
+	default:
+		ret = -ENODEV;
+		break;
+	}
+
+	return ret;
+}
+
+static int btqcomsmd_open(struct hci_dev *hdev)
+{
+	set_bit(HCI_RUNNING, &hdev->flags);
+	return 0;
+}
+
+static int btqcomsmd_close(struct hci_dev *hdev)
+{
+	clear_bit(HCI_RUNNING, &hdev->flags);
+	return 0;
+}
+
+static int btqcomsmd_set_bdaddr(struct hci_dev *hdev,
+			      const bdaddr_t *bdaddr)
+{
+	struct sk_buff *skb;
+	u8 cmd[9];
+	int err;
+
+	cmd[0] = EDL_NVM_ACCESS_SET_REQ_CMD;
+	cmd[1] = 0x02;			/* TAG ID */
+	cmd[2] = sizeof(bdaddr_t);	/* size */
+	memcpy(cmd + 3, bdaddr, sizeof(bdaddr_t));
+	skb = __hci_cmd_sync_ev(hdev,
+				EDL_NVM_ACCESS_OPCODE,
+				sizeof(cmd), cmd,
+				HCI_VENDOR_PKT, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		err = PTR_ERR(skb);
+		BT_ERR("%s: Change address command failed (%d)",
+		       hdev->name, err);
+		return err;
+	}
+
+	kfree_skb(skb);
+
+	return 0;
+}
+
+static int btqcomsmd_probe(struct qcom_smd_device *sdev)
+{
+	struct qcom_smd_channel *acl;
+	struct btqcomsmd *btq;
+	struct hci_dev *hdev;
+	int ret;
+
+	acl = qcom_smd_open_channel(sdev,
+				    "APPS_RIVA_BT_ACL",
+				    btqcomsmd_acl_callback);
+	if (IS_ERR(acl))
+		return PTR_ERR(acl);
+
+	btq = devm_kzalloc(&sdev->dev, sizeof(*btq), GFP_KERNEL);
+	if (!btq)
+		return -ENOMEM;
+
+	btq->acl_channel = acl;
+	btq->cmd_channel = sdev->channel;
+
+	hdev = hci_alloc_dev();
+	if (!hdev)
+		return -ENOMEM;
+
+	hdev->bus = HCI_SMD;
+	hdev->open = btqcomsmd_open;
+	hdev->close = btqcomsmd_close;
+	hdev->send = btqcomsmd_send;
+	hdev->set_bdaddr = btqcomsmd_set_bdaddr;
+
+	ret = hci_register_dev(hdev);
+	if (ret < 0) {
+		hci_free_dev(hdev);
+		return ret;
+	}
+
+	hci_set_drvdata(hdev, btq);
+	dev_set_drvdata(&sdev->dev, hdev);
+
+	return 0;
+}
+
+static void btqcomsmd_remove(struct qcom_smd_device *sdev)
+{
+	struct hci_dev *hdev = dev_get_drvdata(&sdev->dev);;
+
+	hci_unregister_dev(hdev);
+	hci_free_dev(hdev);
+}
+
+static const struct qcom_smd_id btqcomsmd_match[] = {
+	{ .name = "APPS_RIVA_BT_CMD" },
+	{}
+};
+
+static struct qcom_smd_driver btqcomsmd_cmd_driver = {
+	.probe = btqcomsmd_probe,
+	.remove = btqcomsmd_remove,
+	.callback = btqcomsmd_cmd_callback,
+	.smd_match_table = btqcomsmd_match,
+	.driver  = {
+		.name  = "btqcomsmd",
+		.owner = THIS_MODULE,
+	},
+};
+
+module_qcom_smd_driver(btqcomsmd_cmd_driver);
+
+MODULE_DESCRIPTION("Qualcomm SMD HCI driver");
+MODULE_LICENSE("GPL v2");
-- 
2.4.2

Re: [PATCH v2 7/7] Bluetooth: btqcomsmd: Qualcomm WCNSS HCI driver

[Marcel Holtmann]

Hi Bjorn,

> The Qualcomm WCNSS chip provides two SMD channels to the BT core; one
> for command and one for event packets. This driver exposes the two
> channels as a hci device.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---
> 
> Changes since v1:
> - With the introduction of qcom_smd_open_channel() the two drivers are now one
> - No more global state
> - Corrected memory management of sk_buffs
> - Reverted to __hci_cmd_sync_ev() for set_bdaddr
> - Renamed the driver to btqcomsmd
> - Split out the addition of HCI_SMD to separate patch
> 
> drivers/bluetooth/Kconfig     |  11 +++
> drivers/bluetooth/Makefile    |   1 +
> drivers/bluetooth/btqcomsmd.c | 198 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 210 insertions(+)
> create mode 100644 drivers/bluetooth/btqcomsmd.c
> 
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 62999546a301..1652997e59cc 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -169,6 +169,17 @@ config BT_HCIUART_QCA
> 
> 	  Say Y here to compile support for QCA protocol.
> 
> +config BT_QCOMSMD
> +	tristate "Qualcomm SMD based HCI support"
> +	depends on QCOM_SMD
> +	help
> +	  Qualcomm SMD based HCI driver.
> +	  This driver is used to bridge HCI data onto the shared memory
> +	  channels to the WCNSS core.
> +
> +	  Say Y here to compile support for HCI over Qualcomm SMD into the
> +	  kernelor say M to compile as a module.
> +
> config BT_HCIBCM203X
> 	tristate "HCI BCM203x USB driver"
> 	depends on USB
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index 07c9cf381e5a..19e313bf8c39 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_BT_WILINK)		+= btwilink.o
> obj-$(CONFIG_BT_BCM)		+= btbcm.o
> obj-$(CONFIG_BT_RTL)		+= btrtl.o
> obj-$(CONFIG_BT_QCA)		+= btqca.o
> +obj-$(CONFIG_BT_QCOMSMD)	+= btqcomsmd.o
> 
> btmrvl-y			:= btmrvl_main.o
> btmrvl-$(CONFIG_DEBUG_FS)	+= btmrvl_debugfs.o
> diff --git a/drivers/bluetooth/btqcomsmd.c b/drivers/bluetooth/btqcomsmd.c
> new file mode 100644
> index 000000000000..4b91c830531e
> --- /dev/null
> +++ b/drivers/bluetooth/btqcomsmd.c
> @@ -0,0 +1,198 @@
> +/*
> + * Copyright (c) 2015, Sony Mobile Communications Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/soc/qcom/smd.h>
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +#include <net/bluetooth/hci.h>
> +
> +#define EDL_NVM_ACCESS_SET_REQ_CMD	0x01
> +#define EDL_NVM_ACCESS_OPCODE		0xfc0b
> +
> +struct btqcomsmd {
> +	struct qcom_smd_channel *acl_channel;
> +	struct qcom_smd_channel *cmd_channel;
> +};
> +
> +static int btqcomsmd_recv(struct hci_dev *hdev,
> +			  unsigned type,
> +			  const void *data,
> +			  size_t count)

you are overdoing it here. Place as many parameters in one line as long as they are below 80 chars. Only then put them on the next line. One param per line is too much.

> +{
> +	struct sk_buff *skb;
> +	void *buf;
> +
> +	/* Use GFP_ATOMIC as we're in IRQ context */
> +	skb = bt_skb_alloc(count, GFP_ATOMIC);
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	bt_cb(skb)->pkt_type = type;
> +
> +	/* Use io accessor as data might be ioremapped */
> +	buf = skb_put(skb, count);
> +	memcpy_fromio(buf, data, count);

memcpy_fromio(skb_put(skb, count), data, count);

Avoid the extra buf variable.

> +
> +	return hci_recv_frame(hdev, skb);
> +}
> +
> +static int btqcomsmd_acl_callback(struct qcom_smd_device *qsdev,
> +				  const void *data,
> +				  size_t count)
> +{
> +	struct hci_dev *hdev = dev_get_drvdata(&qsdev->dev);
> +
> +	return btqcomsmd_recv(hdev, HCI_ACLDATA_PKT, data, count);
> +}
> +
> +static int btqcomsmd_cmd_callback(struct qcom_smd_device *qsdev,
> +				  const void *data,
> +				  size_t count)
> +{
> +	struct hci_dev *hdev = dev_get_drvdata(&qsdev->dev);
> +
> +	return btqcomsmd_recv(hdev, HCI_EVENT_PKT, data, count);
> +}
> +
> +static int btqcomsmd_send(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +	struct btqcomsmd *btq = hci_get_drvdata(hdev);
> +	int ret;
> +
> +	switch (bt_cb(skb)->pkt_type) {
> +	case HCI_ACLDATA_PKT:
> +	case HCI_SCODATA_PKT:
> +		ret = qcom_smd_send(btq->acl_channel, skb->data, skb->len);
> +		break;
> +	case HCI_COMMAND_PKT:
> +		ret = qcom_smd_send(btq->cmd_channel, skb->data, skb->len);
> +		break;

You need to update hdev->stat for each packet type.

> +	default:
> +		ret = -ENODEV;

Please use -EILSEQ here.

> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static int btqcomsmd_open(struct hci_dev *hdev)
> +{
> +	set_bit(HCI_RUNNING, &hdev->flags);

You didn't look at bluetooth-next changes and the HCI_RUNNING is now handling in the core. You can turn open and close into empty functions.

> +	return 0;
> +}
> +
> +static int btqcomsmd_close(struct hci_dev *hdev)
> +{
> +	clear_bit(HCI_RUNNING, &hdev->flags);
> +	return 0;
> +}
> +
> +static int btqcomsmd_set_bdaddr(struct hci_dev *hdev,
> +			      const bdaddr_t *bdaddr)
> +{
> +	struct sk_buff *skb;
> +	u8 cmd[9];
> +	int err;
> +
> +	cmd[0] = EDL_NVM_ACCESS_SET_REQ_CMD;
> +	cmd[1] = 0x02;			/* TAG ID */
> +	cmd[2] = sizeof(bdaddr_t);	/* size */
> +	memcpy(cmd + 3, bdaddr, sizeof(bdaddr_t));
> +	skb = __hci_cmd_sync_ev(hdev,
> +				EDL_NVM_ACCESS_OPCODE,
> +				sizeof(cmd), cmd,
> +				HCI_VENDOR_PKT, HCI_INIT_TIMEOUT);
> +	if (IS_ERR(skb)) {
> +		err = PTR_ERR(skb);
> +		BT_ERR("%s: Change address command failed (%d)",
> +		       hdev->name, err);
> +		return err;
> +	}
> +
> +	kfree_skb(skb);
> +
> +	return 0;
> +}

I would actually prefer that you not copy the function here. And instead just #include "btqca.h" and just use qca_set_bdaddr_rome instead. Remember to add select BT_QCA to your Kconfig change.

> +
> +static int btqcomsmd_probe(struct qcom_smd_device *sdev)
> +{
> +	struct qcom_smd_channel *acl;
> +	struct btqcomsmd *btq;
> +	struct hci_dev *hdev;
> +	int ret;
> +
> +	acl = qcom_smd_open_channel(sdev,
> +				    "APPS_RIVA_BT_ACL",
> +				    btqcomsmd_acl_callback);

Same here. Do not overdo the splitting lines. If the second parameter fits into previous line, put it there.

> +	if (IS_ERR(acl))
> +		return PTR_ERR(acl);
> +
> +	btq = devm_kzalloc(&sdev->dev, sizeof(*btq), GFP_KERNEL);
> +	if (!btq)
> +		return -ENOMEM;
> +
> +	btq->acl_channel = acl;
> +	btq->cmd_channel = sdev->channel;
> +
> +	hdev = hci_alloc_dev();
> +	if (!hdev)
> +		return -ENOMEM;
> +
> +	hdev->bus = HCI_SMD;

I prefer if this looks like this:

	hdev->bus = HCI_SMD;
	hci_set_drvdata(dev, btq);

	btq->hdev = hdev;

	SET_HCIDEV_DEV(hdev, &sdev->dev);

The SET_HCIDEV_DEV is actually important so that it gets its right place in sysfs.

> +	hdev->open = btqcomsmd_open;
> +	hdev->close = btqcomsmd_close;
> +	hdev->send = btqcomsmd_send;
> +	hdev->set_bdaddr = btqcomsmd_set_bdaddr;
> +
> +	ret = hci_register_dev(hdev);
> +	if (ret < 0) {
> +		hci_free_dev(hdev);
> +		return ret;
> +	}
> +
> +	hci_set_drvdata(hdev, btq);
> +	dev_set_drvdata(&sdev->dev, hdev);

And here you just set drvdata of sdev to btq.

This is more in line how the other drivers are written. So it would be best if we keep them that way and not try to do too many things differently.

> +
> +	return 0;
> +}
> +
> +static void btqcomsmd_remove(struct qcom_smd_device *sdev)
> +{
> +	struct hci_dev *hdev = dev_get_drvdata(&sdev->dev);;
> +

I think you really want to do this here:

	dev_set_drvdata(&sdev, NULL);

The btsdio.c has most likely the most simple version of a remove function.

> +	hci_unregister_dev(hdev);
> +	hci_free_dev(hdev);
> +}
> +
> +static const struct qcom_smd_id btqcomsmd_match[] = {
> +	{ .name = "APPS_RIVA_BT_CMD" },
> +	{}
> +};
> +
> +static struct qcom_smd_driver btqcomsmd_cmd_driver = {
> +	.probe = btqcomsmd_probe,
> +	.remove = btqcomsmd_remove,
> +	.callback = btqcomsmd_cmd_callback,
> +	.smd_match_table = btqcomsmd_match,
> +	.driver  = {
> +		.name  = "btqcomsmd",
> +		.owner = THIS_MODULE,
> +	},
> +};
> +
> +module_qcom_smd_driver(btqcomsmd_cmd_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm SMD HCI driver");
> +MODULE_LICENSE("GPL v2");

Can we get MODULE_VERSION and MODULE_AUTHOR added here as well. See the other drivers on how we do the version thing. Start with 1.0 since it looks pretty simple. However it is nice to have that in case anybody ever needs to figure out which version we have here.

Regards

Marcel

Re: [PATCH v2 5/7] soc: qcom: smd: Support opening additional channels

[yfw]

Hi Bjorn,

On 2015/10/10 4:48, Bjorn Andersson wrote:
> With the qcom_smd_open_channel() API we allow SMD devices to open
> additional SMD channels, to allow implementation of multi-channel SMD
> devices - like Bluetooth.
>
> Channels are opened from the same edge as the calling SMD device is tied
> to.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---
>
> Changes since v1:
> - New patch
>
>   drivers/soc/qcom/smd.c       | 75 ++++++++++++++++++++++++++++++++++++++++++++
>   include/linux/soc/qcom/smd.h |  4 +++
>   2 files changed, 79 insertions(+)
>
> diff --git a/drivers/soc/qcom/smd.c b/drivers/soc/qcom/smd.c
> index 4c55708aac50..3257ba488d3d 100644
> --- a/drivers/soc/qcom/smd.c
> +++ b/drivers/soc/qcom/smd.c
> @@ -129,6 +129,8 @@ struct qcom_smd_edge {
>
>   	unsigned smem_available;
>
> +	wait_queue_head_t new_channel_event;
> +
>   	struct work_struct scan_work;
>   	struct work_struct state_work;
>   };
> @@ -1042,6 +1044,76 @@ void qcom_smd_driver_unregister(struct qcom_smd_driver *qsdrv)
>   }
>   EXPORT_SYMBOL(qcom_smd_driver_unregister);
>
> +static struct qcom_smd_channel *
> +qcom_smd_find_channel(struct qcom_smd_edge *edge, const char *name)
> +{
> +	struct qcom_smd_channel *channel;
> +	struct qcom_smd_channel *ret = NULL;
> +	unsigned state;
> +
> +	read_lock(&edge->channels_lock);
> +	list_for_each_entry(channel, &edge->channels, list) {
> +		if (strcmp(channel->name, name))
> +			continue;
> +
> +		state = GET_RX_CHANNEL_INFO(channel, state);
> +		if (state != SMD_CHANNEL_OPENING &&
> +		    state != SMD_CHANNEL_OPENED)
> +			continue;
> +
> +		ret = channel;
> +		break;
> +	}
> +	read_unlock(&edge->channels_lock);
> +
> +	return ret;
> +}
> +
> +/**
> + * qcom_smd_open_channel() - claim additional channels on the same edge
> + * @sdev:	smd_device handle
> + * @name:	channel name
> + * @cb:		callback method to use for incoming data
> + *
> + * Returns a channel handle on success, or -EPROBE_DEFER if the channel isn't
> + * ready.
> + */
> +struct qcom_smd_channel *qcom_smd_open_channel(struct qcom_smd_device *sdev,
> +					       const char *name,
> +					       qcom_smd_cb_t cb)
> +{
> +	struct qcom_smd_channel *channel;
> +	struct qcom_smd_edge *edge = sdev->channel->edge;
> +	int ret;
> +
> +	/* Wait up to HZ for the channel to appear */
> +	ret = wait_event_interruptible_timeout(edge->new_channel_event,
> +			(channel = qcom_smd_find_channel(edge, name)) != NULL,
> +			HZ);
> +	if (!ret)
> +		return ERR_PTR(-ETIMEDOUT);
> +
> +	if (channel->state != SMD_CHANNEL_CLOSED) {
> +		dev_err(&sdev->dev, "channel %s is busy\n", channel->name);
> +		return ERR_PTR(-EBUSY);
> +	}
> +
> +	channel->qsdev = sdev;
> +	ret = qcom_smd_channel_open(channel, cb);
> +	if (ret) {
> +		channel->qsdev = NULL;
> +		return ERR_PTR(ret);
> +	}
> +
> +	/*
> +	 * Append the list of channel to the channels associated with the sdev
> +	 */
> +	list_add_tail(&channel->dev_list, &sdev->channel->dev_list);
> +
> +	return channel;
> +}
> +EXPORT_SYMBOL(qcom_smd_open_channel);
> +
Do we need qcom_smd_close_channel API here?

Regards
Yin, Fengwei

>   /*
>    * Allocate the qcom_smd_channel object for a newly found smd channel,
>    * retrieving and validating the smem items involved.
> @@ -1178,6 +1250,8 @@ static void qcom_channel_scan_worker(struct work_struct *work)
>
>   			dev_dbg(smd->dev, "new channel found: '%s'\n", channel->name);
>   			set_bit(i, edge->allocated[tbl]);
> +
> +			wake_up_interruptible(&edge->new_channel_event);
>   		}
>   	}
>
> @@ -1340,6 +1414,7 @@ static int qcom_smd_probe(struct platform_device *pdev)
>   	for_each_available_child_of_node(pdev->dev.of_node, node) {
>   		edge = &smd->edges[i++];
>   		edge->smd = smd;
> +		init_waitqueue_head(&edge->new_channel_event);
>
>   		ret = qcom_smd_parse_edge(&pdev->dev, node, edge);
>   		if (ret)
> diff --git a/include/linux/soc/qcom/smd.h b/include/linux/soc/qcom/smd.h
> index 65a64fcdb1aa..bd51c8a9d807 100644
> --- a/include/linux/soc/qcom/smd.h
> +++ b/include/linux/soc/qcom/smd.h
> @@ -56,4 +56,8 @@ void qcom_smd_driver_unregister(struct qcom_smd_driver *drv);
>
>   int qcom_smd_send(struct qcom_smd_channel *channel, const void *data, int len);
>
> +struct qcom_smd_channel *qcom_smd_open_channel(struct qcom_smd_device *sdev,
> +					       const char *name,
> +					       qcom_smd_cb_t cb);
> +
>   #endif
>

Re: [PATCH v2 7/7] Bluetooth: btqcomsmd: Qualcomm WCNSS HCI driver

[yfw]

On 2015/10/10 4:48, Bjorn Andersson wrote:
> The Qualcomm WCNSS chip provides two SMD channels to the BT core; one
> for command and one for event packets. This driver exposes the two
> channels as a hci device.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---
>
> Changes since v1:
> - With the introduction of qcom_smd_open_channel() the two drivers are now one
> - No more global state
> - Corrected memory management of sk_buffs
> - Reverted to __hci_cmd_sync_ev() for set_bdaddr
> - Renamed the driver to btqcomsmd
> - Split out the addition of HCI_SMD to separate patch
>
>   drivers/bluetooth/Kconfig     |  11 +++
>   drivers/bluetooth/Makefile    |   1 +
>   drivers/bluetooth/btqcomsmd.c | 198 ++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 210 insertions(+)
>   create mode 100644 drivers/bluetooth/btqcomsmd.c
>
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 62999546a301..1652997e59cc 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -169,6 +169,17 @@ config BT_HCIUART_QCA
>
>   	  Say Y here to compile support for QCA protocol.
>
> +config BT_QCOMSMD
> +	tristate "Qualcomm SMD based HCI support"
> +	depends on QCOM_SMD
> +	help
> +	  Qualcomm SMD based HCI driver.
> +	  This driver is used to bridge HCI data onto the shared memory
> +	  channels to the WCNSS core.
> +
> +	  Say Y here to compile support for HCI over Qualcomm SMD into the
> +	  kernelor say M to compile as a module.
> +
>   config BT_HCIBCM203X
>   	tristate "HCI BCM203x USB driver"
>   	depends on USB
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index 07c9cf381e5a..19e313bf8c39 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_BT_WILINK)		+= btwilink.o
>   obj-$(CONFIG_BT_BCM)		+= btbcm.o
>   obj-$(CONFIG_BT_RTL)		+= btrtl.o
>   obj-$(CONFIG_BT_QCA)		+= btqca.o
> +obj-$(CONFIG_BT_QCOMSMD)	+= btqcomsmd.o
>
>   btmrvl-y			:= btmrvl_main.o
>   btmrvl-$(CONFIG_DEBUG_FS)	+= btmrvl_debugfs.o
> diff --git a/drivers/bluetooth/btqcomsmd.c b/drivers/bluetooth/btqcomsmd.c
> new file mode 100644
> index 000000000000..4b91c830531e
> --- /dev/null
> +++ b/drivers/bluetooth/btqcomsmd.c
> @@ -0,0 +1,198 @@
> +/*
> + * Copyright (c) 2015, Sony Mobile Communications Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/soc/qcom/smd.h>
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +#include <net/bluetooth/hci.h>
> +
> +#define EDL_NVM_ACCESS_SET_REQ_CMD	0x01
> +#define EDL_NVM_ACCESS_OPCODE		0xfc0b
> +
> +struct btqcomsmd {
> +	struct qcom_smd_channel *acl_channel;
> +	struct qcom_smd_channel *cmd_channel;
> +};
> +
> +static int btqcomsmd_recv(struct hci_dev *hdev,
> +			  unsigned type,
> +			  const void *data,
> +			  size_t count)
> +{
> +	struct sk_buff *skb;
> +	void *buf;
> +
> +	/* Use GFP_ATOMIC as we're in IRQ context */
> +	skb = bt_skb_alloc(count, GFP_ATOMIC);
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	bt_cb(skb)->pkt_type = type;
> +
> +	/* Use io accessor as data might be ioremapped */
> +	buf = skb_put(skb, count);
> +	memcpy_fromio(buf, data, count);
> +
> +	return hci_recv_frame(hdev, skb);
> +}
> +
> +static int btqcomsmd_acl_callback(struct qcom_smd_device *qsdev,
> +				  const void *data,
> +				  size_t count)
> +{
> +	struct hci_dev *hdev = dev_get_drvdata(&qsdev->dev);
> +
> +	return btqcomsmd_recv(hdev, HCI_ACLDATA_PKT, data, count);
> +}
> +
> +static int btqcomsmd_cmd_callback(struct qcom_smd_device *qsdev,
> +				  const void *data,
> +				  size_t count)
> +{
> +	struct hci_dev *hdev = dev_get_drvdata(&qsdev->dev);
> +
> +	return btqcomsmd_recv(hdev, HCI_EVENT_PKT, data, count);
> +}
> +
> +static int btqcomsmd_send(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +	struct btqcomsmd *btq = hci_get_drvdata(hdev);
> +	int ret;
> +
> +	switch (bt_cb(skb)->pkt_type) {
> +	case HCI_ACLDATA_PKT:
> +	case HCI_SCODATA_PKT:
> +		ret = qcom_smd_send(btq->acl_channel, skb->data, skb->len);
> +		break;
> +	case HCI_COMMAND_PKT:
> +		ret = qcom_smd_send(btq->cmd_channel, skb->data, skb->len);
> +		break;
> +	default:
> +		ret = -ENODEV;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static int btqcomsmd_open(struct hci_dev *hdev)
> +{
> +	set_bit(HCI_RUNNING, &hdev->flags);
> +	return 0;
> +}
> +
> +static int btqcomsmd_close(struct hci_dev *hdev)
> +{
> +	clear_bit(HCI_RUNNING, &hdev->flags);
> +	return 0;
> +}
> +
> +static int btqcomsmd_set_bdaddr(struct hci_dev *hdev,
> +			      const bdaddr_t *bdaddr)
> +{
> +	struct sk_buff *skb;
> +	u8 cmd[9];
> +	int err;
> +
> +	cmd[0] = EDL_NVM_ACCESS_SET_REQ_CMD;
> +	cmd[1] = 0x02;			/* TAG ID */
> +	cmd[2] = sizeof(bdaddr_t);	/* size */
> +	memcpy(cmd + 3, bdaddr, sizeof(bdaddr_t));
> +	skb = __hci_cmd_sync_ev(hdev,
> +				EDL_NVM_ACCESS_OPCODE,
> +				sizeof(cmd), cmd,
> +				HCI_VENDOR_PKT, HCI_INIT_TIMEOUT);
> +	if (IS_ERR(skb)) {
> +		err = PTR_ERR(skb);
> +		BT_ERR("%s: Change address command failed (%d)",
> +		       hdev->name, err);
> +		return err;
> +	}
> +
> +	kfree_skb(skb);
> +
> +	return 0;
> +}
> +
> +static int btqcomsmd_probe(struct qcom_smd_device *sdev)
> +{
> +	struct qcom_smd_channel *acl;
> +	struct btqcomsmd *btq;
> +	struct hci_dev *hdev;
> +	int ret;
> +
> +	acl = qcom_smd_open_channel(sdev,
> +				    "APPS_RIVA_BT_ACL",
> +				    btqcomsmd_acl_callback);
> +	if (IS_ERR(acl))
> +		return PTR_ERR(acl);
Do we need to close acl channel in following failure path and
btqcomsmd_remove?

Regards
Yin, Fengwei

> +
> +	btq = devm_kzalloc(&sdev->dev, sizeof(*btq), GFP_KERNEL);
> +	if (!btq)
> +		return -ENOMEM;
> +
> +	btq->acl_channel = acl;
> +	btq->cmd_channel = sdev->channel;
> +
> +	hdev = hci_alloc_dev();
> +	if (!hdev)
> +		return -ENOMEM;
> +
> +	hdev->bus = HCI_SMD;
> +	hdev->open = btqcomsmd_open;
> +	hdev->close = btqcomsmd_close;
> +	hdev->send = btqcomsmd_send;
> +	hdev->set_bdaddr = btqcomsmd_set_bdaddr;
> +
> +	ret = hci_register_dev(hdev);
> +	if (ret < 0) {
> +		hci_free_dev(hdev);
> +		return ret;
> +	}
> +
> +	hci_set_drvdata(hdev, btq);
> +	dev_set_drvdata(&sdev->dev, hdev);
> +
> +	return 0;
> +}
> +
> +static void btqcomsmd_remove(struct qcom_smd_device *sdev)
> +{
> +	struct hci_dev *hdev = dev_get_drvdata(&sdev->dev);;
> +
> +	hci_unregister_dev(hdev);
> +	hci_free_dev(hdev);
> +}
> +
> +static const struct qcom_smd_id btqcomsmd_match[] = {
> +	{ .name = "APPS_RIVA_BT_CMD" },
> +	{}
> +};
> +
> +static struct qcom_smd_driver btqcomsmd_cmd_driver = {
> +	.probe = btqcomsmd_probe,
> +	.remove = btqcomsmd_remove,
> +	.callback = btqcomsmd_cmd_callback,
> +	.smd_match_table = btqcomsmd_match,
> +	.driver  = {
> +		.name  = "btqcomsmd",
> +		.owner = THIS_MODULE,
> +	},
> +};
> +
> +module_qcom_smd_driver(btqcomsmd_cmd_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm SMD HCI driver");
> +MODULE_LICENSE("GPL v2");
>

Re: [PATCH v2 5/7] soc: qcom: smd: Support opening additional channels

[Bjorn Andersson]

On Wed 14 Oct 08:09 PDT 2015, yfw wrote:

> Hi Bjorn,
> 
> On 2015/10/10 4:48, Bjorn Andersson wrote:
> > With the qcom_smd_open_channel() API we allow SMD devices to open
> > additional SMD channels, to allow implementation of multi-channel SMD
> > devices - like Bluetooth.
> >
> > Channels are opened from the same edge as the calling SMD device is tied
> > to.
> >
[..]
> > +/**
> > + * qcom_smd_open_channel() - claim additional channels on the same edge
> > + * @sdev:	smd_device handle
> > + * @name:	channel name
> > + * @cb:		callback method to use for incoming data
> > + *
> > + * Returns a channel handle on success, or -EPROBE_DEFER if the channel isn't
> > + * ready.
> > + */
> > +struct qcom_smd_channel *qcom_smd_open_channel(struct qcom_smd_device *sdev,
> > +					       const char *name,
> > +					       qcom_smd_cb_t cb)
> > +{
> > +	struct qcom_smd_channel *channel;
> > +	struct qcom_smd_edge *edge = sdev->channel->edge;
> > +	int ret;
> > +
> > +	/* Wait up to HZ for the channel to appear */
> > +	ret = wait_event_interruptible_timeout(edge->new_channel_event,
> > +			(channel = qcom_smd_find_channel(edge, name)) != NULL,
> > +			HZ);
> > +	if (!ret)
> > +		return ERR_PTR(-ETIMEDOUT);
> > +
> > +	if (channel->state != SMD_CHANNEL_CLOSED) {
> > +		dev_err(&sdev->dev, "channel %s is busy\n", channel->name);
> > +		return ERR_PTR(-EBUSY);
> > +	}
> > +
> > +	channel->qsdev = sdev;
> > +	ret = qcom_smd_channel_open(channel, cb);
> > +	if (ret) {
> > +		channel->qsdev = NULL;
> > +		return ERR_PTR(ret);
> > +	}
> > +
> > +	/*
> > +	 * Append the list of channel to the channels associated with the sdev
> > +	 */
> > +	list_add_tail(&channel->dev_list, &sdev->channel->dev_list);
> > +
> > +	return channel;
> > +}
> > +EXPORT_SYMBOL(qcom_smd_open_channel);
> > +
> Do we need qcom_smd_close_channel API here?
> 

On success the channel is associated with the qcom_smd_device, which
tears down all associated channels on destruction.

I have not yet seen any reason for decoupling the life cycle of a
channel further from the device (in most cases it's very must 1:1).

But I will update the comment above to clarify this fact, thanks!

Regards,
Bjorn

Re: [PATCH v2 7/7] Bluetooth: btqcomsmd: Qualcomm WCNSS HCI driver

[Bjorn Andersson]

On Wed 14 Oct 08:10 PDT 2015, yfw wrote:

[..]
> > diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
[..]
> > +static int btqcomsmd_probe(struct qcom_smd_device *sdev)
> > +{
> > +	struct qcom_smd_channel *acl;
> > +	struct btqcomsmd *btq;
> > +	struct hci_dev *hdev;
> > +	int ret;
> > +
> > +	acl = qcom_smd_open_channel(sdev,
> > +				    "APPS_RIVA_BT_ACL",
> > +				    btqcomsmd_acl_callback);
> > +	if (IS_ERR(acl))
> > +		return PTR_ERR(acl);
> Do we need to close acl channel in following failure path and
> btqcomsmd_remove?
> 

As the life cycle of the channels implicitly follow the life cycle of
the qcom_smd_device the channel is supposed to be released by the
caller of probe upon failure.

I'll make sure this is documented in qcom_smd_open_channel()


Thanks for the review.

Regards,
Bjorn