Skip to contentSkip to conversation

N900 Modem Speech Support

PATCH Superseded Sebastian Reichel 5 participants 12 messages Mar 3, 2015 – Mar 6, 2015

Subsystems (2)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Sebastian Reichel

author maintainer
#0 lore raw

v2 cover N900 Modem Speech Support

Hi,

This patchset contains the missing speech data support for the Nokia N900 modem.

Userland access goes via /dev/cmt_speech. The API is implemented in libcmtspeechdata, which is used by ofono and the freesmartphone.org project. Apart from that the device is also used by the phone binaries distributed with Maemo. So while this is a new userland ABI for the mainline kernel it has been tested in the wild for some years.

Simple Testing of the API can be done by checking out libcmtspeechdata [0], building the test tool and executing it. The tool will loop back audio data received from the caller.

I have prepared a kernel branch for this patchset, which can be found at [1].

Changes since PATCHv1 [2]:
* Squash cmt-speech patches together
* cs_alloc_cmds(): GFP_ATOMIC -> GFP_KERNEL
* CS_SET_WAKELINE ioctl: Add sanity check

[0] https://lkml.org/lkml/2015/2/11/526
[1] git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-hsi.git branch/cmt-speech-2
[2] https://lkml.org/lkml/2015/3/1/282

-- Sebastian

Kai Vehmanen (1):
HSI: cmt_speech: Add cmt-speech driver

Sebastian Reichel (1):
HSI: nokia-modem: Add cmt-speech support

 drivers/hsi/clients/Kconfig          |   11 +-
 drivers/hsi/clients/Makefile         |    1 +
 drivers/hsi/clients/cmt_speech.c     | 1457 ++++++++++++++++++++++++++++++++++
 drivers/hsi/clients/nokia-modem.c    |   31 +-
 include/uapi/linux/hsi/Kbuild        |    2 +-
 include/uapi/linux/hsi/cs-protocol.h |  113 +++
 6 files changed, 1612 insertions(+), 3 deletions(-)
 create mode 100644 drivers/hsi/clients/cmt_speech.c
 create mode 100644 include/uapi/linux/hsi/cs-protocol.h
Signature · 1 line
2.1.4
author maintainer
#1 lore raw

PATCH v2 1/2 HSI: cmt_speech: Add cmt-speech driver

From: Kai Vehmanen <[email protected]>

Introduces the cmt-speech driver, which implements a character device interface for transferring speech data frames over HSI/SSI.

The driver is used to exchange voice/speech data between the Nokia N900/N950/N9's modem and its cpu.

Since the original driver has been written for 2.6.28 some build fixes and general cleanups have been added by me:

* fix build for 4.0 kernel
* replace GFP_ATOMIC with GFP_KERNEL in cs_alloc_cmds()
* add sanity check for CS_SET_WAKELINE ioctl
* cleanup driver initialisation
* rename driver to cmt-speech to be consistent with ssi-protocol driver
* move cs-protocol.h to include/uapi/linux/hsi, since it describes a userspace API
* replace hardcoded channels numbers with values provided via the HSI framework (e.g. coming from DT)

---
 drivers/hsi/clients/Kconfig          |    9 +
 drivers/hsi/clients/Makefile         |    1 +
 drivers/hsi/clients/cmt_speech.c     | 1457 ++++++++++++++++++++++++++++++++++
 include/uapi/linux/hsi/Kbuild        |    2 +-
 include/uapi/linux/hsi/cs-protocol.h |  113 +++
 5 files changed, 1581 insertions(+), 1 deletion(-)
 create mode 100644 drivers/hsi/clients/cmt_speech.c
 create mode 100644 include/uapi/linux/hsi/cs-protocol.h
drivers/hsi/clients/Kconfig +9 -0
index bc60dec..26f62f8 100644
@@ -13,6 +13,15 @@ config NOKIA_MODEM
 
 	If unsure, say N.
 
+config CMT_SPEECH
+	tristate "CMT speech"
+	depends on HSI && PHONET && OMAP_SSI
+	help
+	If you say Y here, you will enable the CMT speech protocol used
+	by Nokia modems.
+
+	If unsure, say N.
+
 config SSI_PROTOCOL
 	tristate "SSI protocol"
 	depends on HSI && PHONET && OMAP_SSI
drivers/hsi/clients/Makefile +1 -0
index 4d5bc0e..2607232 100644
@@ -4,4 +4,5 @@
 
 obj-$(CONFIG_NOKIA_MODEM)	+= nokia-modem.o
 obj-$(CONFIG_SSI_PROTOCOL)	+= ssi_protocol.o
+obj-$(CONFIG_CMT_SPEECH)	+= cmt_speech.o
 obj-$(CONFIG_HSI_CHAR)		+= hsi_char.o
drivers/hsi/clients/cmt_speech.c new file +1457 -0

1,457 lines not on this page · raw message

include/uapi/linux/hsi/Kbuild +1 -1
index 30ab3cd..a16a005 100644
@@ -1,2 +1,2 @@
 # UAPI Header export list
-header-y += hsi_char.h
+header-y += hsi_char.h cs-protocol.h
include/uapi/linux/hsi/cs-protocol.h new file +113 -0

113 lines not on this page · raw message

Signature · 1 line
2.1.4
#2 lore raw

Re: [PATCHv2 1/2] HSI: cmt_speech: Add cmt-speech driver

A license nit only.

Sebastian Reichel schreef op di 03-03-2015 om 01:44 [+0100]:

Quoted 15 lines
drivers/hsi/clients/cmt_speech.c new file +12 -0
@@ -0,0 +1,1457 @@
+/*
+ * cmt_speech.c - HSI CMT speech driver
+ *
+ * Copyright (C) 2008,2009,2010 Nokia Corporation. All rights reserved.
+ *
+ * Contact: Kai Vehmanen <[email protected]>
+ * Original author: Peter Ujfalusi <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *

drivers/hsi/clients/cmt_speech.c line 22

Quoted 10 lines
drivers/hsi/clients/cmt_speech.c new file +10 -0
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */

This states the license is GPL v2.

drivers/hsi/clients/cmt_speech.c line 1457

Quoted 1 line
drivers/hsi/clients/cmt_speech.c new file +1 -0
+MODULE_LICENSE("GPL");

So you probably want

    MODULE_LICENSE("GPL v2");

here.

Paul Bolle

author maintainer
#3 lore raw

PATCH v2 2/2 HSI: nokia-modem: Add cmt-speech support

In reply to Sebastian Reichel

Register cmt-speech driver in nokia-modem driver and forward hsi channel information.

---
 drivers/hsi/clients/Kconfig       |  2 +-
 drivers/hsi/clients/nokia-modem.c | 31 ++++++++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 2 deletions(-)
drivers/hsi/clients/Kconfig +1 -1
index 26f62f8..77ee7bc 100644
@@ -6,7 +6,7 @@ comment "HSI clients"
 
 config NOKIA_MODEM
 	tristate "Nokia Modem"
-	depends on HSI && SSI_PROTOCOL
+	depends on HSI && SSI_PROTOCOL && CMT_SPEECH
 	help
 	Say Y here if you want to add support for the modem on Nokia
 	N900 (Nokia RX-51) hardware.
drivers/hsi/clients/nokia-modem.c +30 -1
index 9be4867..ef6ebda 100644
@@ -46,6 +46,7 @@ struct nokia_modem_device {
 	struct nokia_modem_gpio	*gpios;
 	int			gpio_amount;
 	struct hsi_client	*ssi_protocol;
+	struct hsi_client	*cmt_speech;
 };
 
 static void do_nokia_modem_rst_ind_tasklet(unsigned long data)
@@ -149,6 +150,7 @@ static int nokia_modem_probe(struct device *dev)
 	struct hsi_port *port = hsi_get_port(cl);
 	int irq, pflags, err;
 	struct hsi_board_info ssip;
+	struct hsi_board_info cmtspeech;
 
 	np = dev->of_node;
 	if (!np) {
@@ -214,12 +216,34 @@ static int nokia_modem_probe(struct device *dev)
 		goto error3;
 	}
 
-	/* TODO: register cmt-speech hsi client */
+	cmtspeech.name = "cmt-speech";
+	cmtspeech.tx_cfg = cl->tx_cfg;
+	cmtspeech.rx_cfg = cl->rx_cfg;
+	cmtspeech.platform_data = NULL;
+	cmtspeech.archdata = NULL;
+
+	modem->cmt_speech = hsi_new_client(port, &cmtspeech);
+	if (!modem->cmt_speech) {
+		dev_err(dev, "Could not register cmt-speech device\n");
+		goto error3;
+	}
+
+	err = device_attach(&modem->cmt_speech->device);
+	if (err == 0) {
+		dev_err(dev, "Missing cmt-speech driver\n");
+		err = -EPROBE_DEFER;
+		goto error4;
+	} else if (err < 0) {
+		dev_err(dev, "Could not load cmt-speech driver (%d)\n", err);
+		goto error4;
+	}
 
 	dev_info(dev, "Registered Nokia HSI modem\n");
 
 	return 0;
 
+error4:
+	hsi_remove_client(&modem->cmt_speech->device, NULL);
 error3:
 	hsi_remove_client(&modem->ssi_protocol->device, NULL);
 error2:
@@ -238,6 +262,11 @@ static int nokia_modem_remove(struct device *dev)
 	if (!modem)
 		return 0;
 
+	if (modem->cmt_speech) {
+		hsi_remove_client(&modem->cmt_speech->device, NULL);
+		modem->cmt_speech = NULL;
+	}
+
 	if (modem->ssi_protocol) {
 		hsi_remove_client(&modem->ssi_protocol->device, NULL);
 		modem->ssi_protocol = NULL;
Signature · 1 line
2.1.4
#4 lore raw
In reply to Sebastian Reichel

Hi!

Quoted 13 lines

This patchset contains the missing speech data support for the Nokia N900 modem.

Userland access goes via /dev/cmt_speech. The API is implemented in libcmtspeechdata, which is used by ofono and the freesmartphone.org project. Apart from that the device is also used by the phone binaries distributed with Maemo. So while this is a new userland ABI for the mainline kernel it has been tested in the wild for some years.

Simple Testing of the API can be done by checking out libcmtspeechdata [0], building the test tool and executing it. The tool will loop back audio data received from the caller.

Quoted 6 lines

I have prepared a kernel branch for this patchset, which can be found at [1].

Changes since PATCHv1 [2]:
* Squash cmt-speech patches together
* cs_alloc_cmds(): GFP_ATOMIC -> GFP_KERNEL
* CS_SET_WAKELINE ioctl: Add sanity check

commit fd3492869b9387f7bd97821badb491e9c0be463f

Author: Pavel <[email protected]>
Date:   Wed Mar 4 20:22:32 2015 +0100

Simple cleanups, improve consistency.

drivers/hsi/clients/cmt_speech.c +13 -18
index a017292..9609766 100644
@@ -115,6 +115,7 @@ struct cs_hsi_iface {
 	unsigned int			rx_ptr_boundary;
 	unsigned int			rx_offsets[CS_MAX_BUFFERS];
 	unsigned int			tx_offsets[CS_MAX_BUFFERS];
+
 	/* size of aligned memory blocks */
 	unsigned int			slot_size;
 	unsigned int			flags;
@@ -194,32 +195,27 @@ static void cs_notify_data(u32 message, int maxlength)
 	cs_notify(message, &cs_char_data.dataind_queue);
 
 	spin_lock(&cs_char_data.lock);
-	++cs_char_data.dataind_pending;
+	cs_char_data.dataind_pending++;
 	while (cs_char_data.dataind_pending > maxlength &&
 				!list_empty(&cs_char_data.dataind_queue)) {
 		dev_dbg(&cs_char_data.cl->device, "data notification "
 		"queue overrun (%u entries)\n", cs_char_data.dataind_pending);
 
 		cs_pop_entry(&cs_char_data.dataind_queue);
-		--cs_char_data.dataind_pending;
+		cs_char_data.dataind_pending--;
 	}
 	spin_unlock(&cs_char_data.lock);
 }
 
 static inline void cs_set_cmd(struct hsi_msg *msg, u32 cmd)
 {
-	u32 *data;
-
-	data = sg_virt(msg->sgt.sgl);
+	u32 *data = sg_virt(msg->sgt.sgl);
 	*data = cmd;
 }
 
 static inline u32 cs_get_cmd(struct hsi_msg *msg)
 {
-	u32 *data;
-
-	data = sg_virt(msg->sgt.sgl);
-
+	u32 *data = sg_virt(msg->sgt.sgl);
 	return *data;
 }
 
@@ -641,7 +637,7 @@ static void cs_hsi_peek_on_data_complete(struct hsi_msg *msg)
 		cs_hsi_data_read_error(hi, msg);
 }
 
-/**
+/*
  * Read/write transaction is ongoing. Returns false if in
  * SSI_CHANNEL_STATE_POLL state.
  */
@@ -651,7 +647,7 @@ static inline int cs_state_xfer_active(unsigned int state)
 		(state & SSI_CHANNEL_STATE_READING);
 }
 
-/**
+/*
  * No pending read/writes
  */
 static inline int cs_state_idle(unsigned int state)
@@ -1117,10 +1113,9 @@ static int cs_char_fasync(int fd, struct file *file, int on)
 {
 	struct cs_char *csdata = file->private_data;
 
-	if (fasync_helper(fd, file, on, &csdata->async_queue) >= 0)
-		return 0;
-	else
+	if (fasync_helper(fd, file, on, &csdata->async_queue) < 0)
 		return -EIO;
+	return 0;
 }
 
 static unsigned int cs_char_poll(struct file *file, poll_table *wait)
@@ -1149,7 +1144,7 @@ static ssize_t cs_char_read(struct file *file, char __user *buf, size_t count,
 	if (count < sizeof(data))
 		return -EINVAL;
 
-	for ( ; ; ) {
+	for (;;) {
 		DEFINE_WAIT(wait);
 
 		spin_lock_bh(&csdata->lock);
@@ -1157,7 +1152,7 @@ static ssize_t cs_char_read(struct file *file, char __user *buf, size_t count,
 			data = cs_pop_entry(&csdata->chardev_queue);
 		} else if (!list_empty(&csdata->dataind_queue)) {
 			data = cs_pop_entry(&csdata->dataind_queue);
-			--csdata->dataind_pending;
+			csdata->dataind_pending--;
 
 		} else {
 			data = 0;
@@ -1239,8 +1234,8 @@ static long cs_char_ioctl(struct file *file, unsigned int cmd,
 
 		if (copy_to_user((void __user *)arg, &ifver, sizeof(ifver)))
 			r = -EFAULT;
-		break;
 	}
+		break;
 	case CS_CONFIG_BUFS: {
 		struct cs_buffer_config buf_cfg;
 
@@ -1249,8 +1244,8 @@ static long cs_char_ioctl(struct file *file, unsigned int cmd,
 			r = -EFAULT;
 		else
 			r = cs_hsi_buf_config(csdata->hi, &buf_cfg);
-		break;
 	}
+		break;
 	default:
 		r = -ENOTTY;
 		break;
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#5 lore raw
Quoted 11 lines · Pavel Machek
drivers/hsi/clients/cmt_speech.c +1 -1
@@ -1239,8 +1234,8 @@ static long cs_char_ioctl(struct file *file, unsigned int cmd,
 
 		if (copy_to_user((void __user *)arg, &ifver, sizeof(ifver)))
 			r = -EFAULT;
-		break;
 	}
+		break;
 	case CS_CONFIG_BUFS: {
 		struct cs_buffer_config buf_cfg;
 

drivers/hsi/clients/cmt_speech.c static long cs_char_ioctl(struct file *file, unsigned int cmd, · line 1248

Quoted 6 lines
drivers/hsi/clients/cmt_speech.c +1 -1
@@ -1249,8 +1244,8 @@ static long cs_char_ioctl(struct file *file, unsigned int cmd,
 			r = -EFAULT;
 		else
 			r = cs_hsi_buf_config(csdata->hi, &buf_cfg);
-		break;
 	}
+		break;

I think having the break inside the block is more readable. Now the indendation is mess.

A.

#6 lore raw
Quoted 11 lines · Aaro Koskinen
Quoted 10 lines · Pavel Machek
drivers/hsi/clients/cmt_speech.c +1 -1
@@ -1239,8 +1234,8 @@ static long cs_char_ioctl(struct file *file, unsigned int cmd,
 
 		if (copy_to_user((void __user *)arg, &ifver, sizeof(ifver)))
 			r = -EFAULT;
-		break;
 	}
+		break;
 	case CS_CONFIG_BUFS: {
 		struct cs_buffer_config buf_cfg;
 
Quoted 10 lines

drivers/hsi/clients/cmt_speech.c static long cs_char_ioctl(struct file *file, unsigned int cmd, · line 1248

Quoted 7 lines
drivers/hsi/clients/cmt_speech.c +1 -1
@@ -1249,8 +1244,8 @@ static long cs_char_ioctl(struct file *file, unsigned int cmd,
 			r = -EFAULT;
 		else
 			r = cs_hsi_buf_config(csdata->hi, &buf_cfg);
-		break;
 	}
+		break;

I think having the break inside the block is more readable. Now the indendation is mess.

Half the function does it one way, one the other way. I don't care which way, but it should be consistent.
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#7 lore raw
In reply to Pavel Machek

Hi!

CMT speech (CMT_SPEECH) [N/m/y/?] (NEW) ?

CONFIG_CMT_SPEECH:

If you say Y here, you will enable the CMT speech protocol used by Nokia modems.

If unsure, say N.

Normally, we also specify name of the module in config text. .. which is strange, because I selected "m" and can't find cmt_speech.ko anywhere.
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#8 lore raw
In reply to Sebastian Reichel

Hi!

Quoted 5 lines

Userland access goes via /dev/cmt_speech. The API is implemented in libcmtspeechdata, which is used by ofono and the freesmartphone.org project. Apart from that the device is also used by the phone binaries distributed with Maemo. So while this is a new userland ABI for the mainline kernel it has been tested in the wild for some years.

I'm sorry, Dave. I can't let you do that.

Yes, the ABI is "tested" for some years, but it is not documented, and it is very wrong ABI.

I'm not sure what they do with the "read()". I was assuming it is meant for passing voice data, but it can return at most 4 bytes, AFAICT.

We already have perfectly good ABI for passing voice data around. It is called "ALSA". libcmtspeech will then become unneccessary, and the daemon routing voice data will be as simple as "read sample from ALSA/modem, write sample to ALSA/rx-51_soundcard" & "read sample from ALSA/rx-51_soundcard, write sample to ALSA/modem" & .

Should this driver be merged to drivers/staging while the interface is fixed? Big part of driver should stay the same, userspace interface is only small part of the driver...

Sorry about that,
Pavel

drivers/hsi/clients/cmt_speech.c +191 -14
index 5dbbc67..06dc81c 100644
@@ -21,6 +21,8 @@
  * 02110-1301 USA
  */
 
+/* Thanks to http://ben-collins.blogspot.cz/2010/05/writing-alsa-driver-basics.html */
+
 #include <linux/errno.h>
 #include <linux/module.h>
 #include <linux/types.h>
@@ -39,6 +41,11 @@
 #include <linux/hsi/ssi_protocol.h>
 #include <linux/hsi/cs-protocol.h>
 
+#include <sound/initval.h>
+#include <sound/core.h>
+#include <sound/memalloc.h>
+#include <sound/pcm.h>
+
 #define CS_MMAP_SIZE	PAGE_SIZE
 
 struct char_queue {
@@ -62,8 +69,12 @@ struct cs_char {
 	/* hsi channel ids */
 	int                     channel_id_cmd;
 	int                     channel_id_data;
+	/* alsa */
+	struct snd_card *card;
 };
 
+#define cs_char cs_char
+
 #define SSI_CHANNEL_STATE_READING	1
 #define SSI_CHANNEL_STATE_WRITING	(1 << 1)
 #define SSI_CHANNEL_STATE_POLL		(1 << 2)
@@ -168,6 +179,8 @@ static void cs_notify(u32 message, struct list_head *head)
 	wake_up_interruptible(&cs_char_data.wait);
 	kill_fasync(&cs_char_data.async_queue, SIGIO, POLL_IN);
 
+	/* snd_pcm_period_elapsed(my_dev->ss); ?? FIXME */
+
 out:
 	return;
 }
@@ -1134,10 +1147,8 @@ static unsigned int cs_char_poll(struct file *file, poll_table *wait)
 	return ret;
 }
 
-static ssize_t cs_char_read(struct file *file, char __user *buf, size_t count,
-								loff_t *unused)
+static ssize_t __cs_char_read(struct cs_char *csdata, char __user *buf, size_t count, int nonblock)
 {
-	struct cs_char *csdata = file->private_data;
 	u32 data;
 	ssize_t retval;
 
@@ -1161,7 +1172,7 @@ static ssize_t cs_char_read(struct file *file, char __user *buf, size_t count,
 
 		if (data)
 			break;
-		if (file->f_flags & O_NONBLOCK) {
+		if (nonblock) {
 			retval = -EAGAIN;
 			goto out;
 		} else if (signal_pending(current)) {
@@ -1182,10 +1193,17 @@ out:
 	return retval;
 }
 
-static ssize_t cs_char_write(struct file *file, const char __user *buf,
-						size_t count, loff_t *unused)
+
+static ssize_t cs_char_read(struct file *file, char __user *buf, size_t count,
+								loff_t *unused)
 {
 	struct cs_char *csdata = file->private_data;
+	return __cs_char_read(csdata, buf, count, file->f_flags & O_NONBLOCK);
+}
+
+static ssize_t __cs_char_write(struct cs_char *csdata, const char __user *buf,
+						size_t count)
+{
 	u32 data;
 	int err;
 	ssize_t	retval;
@@ -1205,6 +1223,13 @@ static ssize_t cs_char_write(struct file *file, const char __user *buf,
 	return retval;
 }
 
+static ssize_t cs_char_write(struct file *file, const char __user *buf,
+						size_t count, loff_t *unused)
+{
+	struct cs_char *csdata = file->private_data;
+	return __cs_char_write(csdata, buf, count);
+}
+
 static long cs_char_ioctl(struct file *file, unsigned int cmd,
 				unsigned long arg)
 {
@@ -1269,7 +1294,7 @@ static int cs_char_mmap(struct file *file, struct vm_area_struct *vma)
 	return 0;
 }
 
-static int cs_char_open(struct inode *unused, struct file *file)
+static int __cs_char_open(void)
 {
 	int ret = 0;
 	unsigned long p;
@@ -1300,8 +1325,6 @@ static int cs_char_open(struct inode *unused, struct file *file)
 	cs_char_data.mmap_base = p;
 	cs_char_data.mmap_size = CS_MMAP_SIZE;
 
-	file->private_data = &cs_char_data;
-
 	return 0;
 
 out3:
@@ -1314,6 +1337,13 @@ out1:
 	return ret;
 }
 
+static int cs_char_open(struct inode *unused, struct file *file)
+{
+	file->private_data = &cs_char_data;
+
+	return __cs_char_open();
+}
+
 static void cs_free_char_queue(struct list_head *head)
 {
 	struct char_queue *entry;
@@ -1329,10 +1359,8 @@ static void cs_free_char_queue(struct list_head *head)
 
 }
 
-static int cs_char_release(struct inode *unused, struct file *file)
+static int __cs_char_release(struct cs_char *csdata)
 {
-	struct cs_char *csdata = file->private_data;
-
 	cs_hsi_stop(csdata->hi);
 	spin_lock_bh(&csdata->lock);
 	csdata->hi = NULL;
@@ -1345,6 +1373,13 @@ static int cs_char_release(struct inode *unused, struct file *file)
 	return 0;
 }
 
+static int cs_char_release(struct inode *unused, struct file *file)
+{
+	struct cs_char *csdata = file->private_data;
+
+	return __cs_char_release(csdata);
+}
+
 static const struct file_operations cs_char_fops = {
 	.owner		= THIS_MODULE,
 	.read		= cs_char_read,
@@ -1363,6 +1398,112 @@ static struct miscdevice cs_char_miscdev = {
 	.fops	= &cs_char_fops
 };
 
+static struct snd_pcm_hardware my_pcm_hw = {
+	.info = (SNDRV_PCM_INFO_MMAP |
+		 SNDRV_PCM_INFO_INTERLEAVED |
+		 SNDRV_PCM_INFO_BLOCK_TRANSFER |
+		 SNDRV_PCM_INFO_MMAP_VALID),
+	.formats          = SNDRV_PCM_FMTBIT_U8,
+	.rates            = SNDRV_PCM_RATE_8000,
+	.rate_min         = 8000,
+	.rate_max         = 8000,
+	.channels_min     = 1,
+	.channels_max     = 1,
+	.buffer_bytes_max = (32 * 48),
+	.period_bytes_min = 48,
+	.period_bytes_max = 48,
+	.periods_min      = 1,
+	.periods_max      = 32,
+};
+#define MAX_BUFFER 1024
+
+static int my_pcm_open(struct snd_pcm_substream *ss)
+{
+	ss->runtime->hw = my_pcm_hw;
+	ss->private_data = &cs_char_data;
+
+	printk("my_pcm_open\n");
+
+	return 0;
+}
+
+static int my_pcm_close(struct snd_pcm_substream *ss)
+{
+	ss->private_data = NULL;
+
+	printk("my_pcm_close\n");	
+
+	return 0;
+}
+
+static int my_hw_params(struct snd_pcm_substream *ss,
+			struct snd_pcm_hw_params *hw_params)
+{
+	return snd_pcm_lib_malloc_pages(ss,
+					params_buffer_bytes(hw_params));
+}
+
+static int my_hw_free(struct snd_pcm_substream *ss)
+{
+	return snd_pcm_lib_free_pages(ss);
+}
+
+static int my_pcm_prepare(struct snd_pcm_substream *ss)
+{
+	return 0;
+}
+
+static int my_pcm_trigger(struct snd_pcm_substream *ss,
+			  int cmd)
+{
+	struct cs_char *my_dev = snd_pcm_substream_chip(ss);
+	int ret = 0;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		// Start the hardware capture
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+		// Stop the hardware capture
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static snd_pcm_uframes_t my_pcm_pointer(struct snd_pcm_substream *ss)
+{
+	struct cs_char *my_dev = snd_pcm_substream_chip(ss);
+
+//	return my_dev->hw_idx;
+	return 0;
+}
+
+static int my_pcm_copy(struct snd_pcm_substream *ss,
+		       int channel, snd_pcm_uframes_t pos,
+		       void __user *dst,
+		       snd_pcm_uframes_t count)
+{
+	struct cs_char *my_dev = snd_pcm_substream_chip(ss);
+
+	//return copy_to_user(dst, my_dev->buffer + pos, count);
+	return -EFAULT;
+}
+
+static struct snd_pcm_ops my_pcm_ops = {
+	.open      = my_pcm_open,
+	.close     = my_pcm_close,
+	.ioctl     = snd_pcm_lib_ioctl,
+	.hw_params = my_hw_params,
+	.hw_free   = my_hw_free,
+	.prepare   = my_pcm_prepare,
+	.trigger   = my_pcm_trigger,
+	.pointer   = my_pcm_pointer,
+	.copy      = my_pcm_copy,
+};
+
 static int cs_hsi_client_probe(struct device *dev)
 {
 	int err = 0;
@@ -1398,14 +1539,48 @@ static int cs_hsi_client_probe(struct device *dev)
 		dev_err(dev, "Failed to register: %d\n", err);
 
 	printk("Registering sound card\n");
-#if 0
+#if 1
 	{
 	struct snd_card *card;
 	int ret;
-	ret = snd_card_create(SNDRV_DEFAULT_IDX1, "Nokia HSI modem",
+	ret = snd_card_new(dev, SNDRV_DEFAULT_IDX1, "Nokia HSI modem",
 			      THIS_MODULE, 0, &card);
 	if (ret < 0)
 		return ret;
+
+	strcpy(card->driver, "cmt_speech");
+	strcpy(card->shortname, "HSI modem");
+	sprintf(card->longname, "Nokia HSI modem");
+	snd_card_set_dev(card, dev);
+
+	static struct snd_device_ops ops = { NULL };
+	ret = snd_device_new(card, SNDRV_DEV_LOWLEVEL, dev, &ops);
+	if (ret < 0)
+		return ret;
+
+	struct snd_pcm *pcm;
+	ret = snd_pcm_new(card, card->driver, 0, 0, 1,
+			  &pcm);
+	if (ret < 0)
+		return ret;
+
+	snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE,
+			&my_pcm_ops);
+	pcm->private_data = dev;
+	pcm->info_flags = 0;
+	strcpy(pcm->name, card->shortname);
+
+	ret = snd_pcm_lib_preallocate_pages_for_all(pcm,
+						    SNDRV_DMA_TYPE_CONTINUOUS,
+						    snd_dma_continuous_data(GFP_KERNEL),
+						    MAX_BUFFER, MAX_BUFFER);
+	if (ret < 0)
+		return ret;
+	
+	if ((ret = snd_card_register(card)) < 0)
+		return ret;
+
+	cs_char_data.card = card;
 	}
 #endif
 
@@ -1417,6 +1592,8 @@ static int cs_hsi_client_remove(struct device *dev)
 	struct cs_hsi_iface *hi;
 
 	dev_dbg(dev, "hsi_client_remove\n");
+
+	snd_card_free(cs_char_data.card);
 	misc_deregister(&cs_char_miscdev);
 	spin_lock_bh(&cs_char_data.lock);
 	hi = cs_char_data.hi;
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#9 lore raw

Hi,

Quoted 5 lines · Pavel Machek
Quoted 2 lines

Userland access goes via /dev/cmt_speech. The API is implemented in libcmtspeechdata, which is used by ofono and the freesmartphone.org project.

Yes, the ABI is "tested" for some years, but it is not documented, and it is very wrong ABI.

Quoted 7 lines

I'm not sure what they do with the "read()". I was assuming it is meant for passing voice data, but it can return at most 4 bytes, AFAICT.

We already have perfectly good ABI for passing voice data around. It is called "ALSA". libcmtspeech will then become unneccessary, and the daemon routing voice data will be as simple as "read sample from

I'm no longer involved with cmt_speech (with this driver nor modems in general), but let me clarify some bits about the design.

First, the team that designed the driver and the stack above had a lot of folks working also with ALSA (and the ALSA drivers have been merged to mainline long ago) and we considered ALSA on multiple occasions as the interface for this as well.

Our take was that ALSA is not the right interface for cmt_speech. The cmt_speech interface in the modem is _not_ a PCM interface as modelled by ALSA. Specifically:

- the interface is lossy in both directions
- data is sent in packets, not a stream of samples (could be other things than PCM samples), with timing and meta-data
- timing of uplink is of utmost importance

Some definite similarities:
- the mmap interface to manage the PCM buffers (that is on purpose similar to that of ALSA)

The interface was designed so that the audio mixer (e.g. Pulseaudio) is run with a soft real-time SCHED_FIFO/RR user-space thread that has full control over _when_ voice _packets_ are sent, and can receive packets with meta-data (see libcmtspeechdata interface, cmtspeech.h), and can detect and handle gaps in the received packets.

This is very different from modems that offer an actual PCM voice link for example over I2S to the application processor (there are lots of these on the market). When you walk out of coverage during a call with these modems, you'll still get samples over I2S, but not so with cmt_speech, so ALSA is not the right interface.

Now, I'm not saying the interface is perfect, but just to give a bit of background, why a custom char-device interface was chosen.

PS Not saying it's enough for mainline inclusion, but libcmtspeechdata [1]
was released and documented to enable the driver to be used by other software than the closed pulseaudio modules. You Pavel of course know this as you've been maintaining the library, but FYI for others.

[1] https://www.gitorious.org/libcmtspeechdata

Br, Kai

#10 lore raw

Right interface for cellphone modem audio (was Re: [PATCHv2 0/2] N900 Modem Speech Support)

Hi!

Quoted 9 lines
Quoted 9 lines
Quoted 2 lines

Userland access goes via /dev/cmt_speech. The API is implemented in libcmtspeechdata, which is used by ofono and the freesmartphone.org project.

Yes, the ABI is "tested" for some years, but it is not documented, and it is very wrong ABI.

I'm not sure what they do with the "read()". I was assuming it is meant for passing voice data, but it can return at most 4 bytes, AFAICT.

Quoted 6 lines
Quoted 3 lines

We already have perfectly good ABI for passing voice data around. It is called "ALSA". libcmtspeech will then become unneccessary, and the daemon routing voice data will be as simple as "read sample from

I'm no longer involved with cmt_speech (with this driver nor modems in general), but let me clarify some bits about the design.

Thanks a lot for your insights; high level design decisions are quite hard to understand from C code.

Quoted 5 lines

First, the team that designed the driver and the stack above had a lot of folks working also with ALSA (and the ALSA drivers have been merged to mainline long ago) and we considered ALSA on multiple occasions as the interface for this as well.

Quoted 8 lines

Our take was that ALSA is not the right interface for cmt_speech. The cmt_speech interface in the modem is _not_ a PCM interface as modelled by ALSA. Specifically:

- the interface is lossy in both directions
- data is sent in packets, not a stream of samples (could be other things than PCM samples), with timing and meta-data
- timing of uplink is of utmost importance

I see that you may not have data available in "downlink" scenario, but how is it lossy in "uplink" scenario? Phone should always try to fill the uplink, no? (Or do you detect silence and not transmit in this case?) (Actually, I guess applications should be ready for "data not ready" case even on "normal" hardware due to differing clocks.)

Packets vs. stream of samples... does userland need to know about the packets? Could we simply hide it from the userland? As userland daemon is (supposed to be) realtime, do we really need extra set of timestamps? What other metadata are there?

Uplink timing... As the daemon is realtime, can it just send the data at the right time? Also normally uplink would be filled, no?

Quoted 9 lines

Some definite similarities:
- the mmap interface to manage the PCM buffers (that is on purpose similar to that of ALSA)

The interface was designed so that the audio mixer (e.g. Pulseaudio) is run with a soft real-time SCHED_FIFO/RR user-space thread that has full control over _when_ voice _packets_ are sent, and can receive packets with meta-data (see libcmtspeechdata interface, cmtspeech.h), and can detect and handle gaps in the received packets.

Well, packets are of fixed size, right? So the userland can simply supply the right size in the common case. As for sending at the right time... well... if the userspace is already real-time, that should be easy.

Now, there's a difference in the downlink. Maybe ALSA people have an idea what to do in this case? Perhaps we can just provide artificial "zero" data?

Quoted 5 lines

This is very different from modems that offer an actual PCM voice link for example over I2S to the application processor (there are lots of these on the market). When you walk out of coverage during a call with these modems, you'll still get samples over I2S, but not so with cmt_speech, so ALSA is not the right interface.

Yes, understood.

Quoted 2 lines

Now, I'm not saying the interface is perfect, but just to give a bit of background, why a custom char-device interface was chosen.

Thanks and best regards,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#11 lore raw

Re: Right interface for cellphone modem audio (was Re: [PATCHv2 0/2] N900 Modem Speech Support)

Hi,

Quoted 4 lines · Pavel Machek
Quoted 4 lines

Our take was that ALSA is not the right interface for cmt_speech. The cmt_speech interface in the modem is _not_ a PCM interface as modelled by ALSA. Specifically:

Quoted 8 lines
Quoted 4 lines

- the interface is lossy in both directions
- data is sent in packets, not a stream of samples (could be other things than PCM samples), with timing and meta-data
- timing of uplink is of utmost importance

I see that you may not have data available in "downlink" scenario, but how is it lossy in "uplink" scenario? Phone should always try to fill the uplink, no? (Or do you detect silence and not transmit in this

Lossy was perhaps not the best choice of words, non-continuous would be a better choice in the uplink case. To adjust timing, some samples from the continuous locally recorded PCM stream need to be skipped and/or duplicated. This would normally be done between speech bursts to avoid artifacts.

Quoted 4 lines

Packets vs. stream of samples... does userland need to know about the packets? Could we simply hide it from the userland? As userland daemon is (supposed to be) realtime, do we really need extra set of timestamps? What other metadata are there?

Yes, we need flags that tell about the frame. Please see docs for 'frame_flags' and 'spc_flags' in libcmtspeechdata cmtspeech.h:
https://www.gitorious.org/libcmtspeechdata/libcmtspeechdata/source/9206835ea3c96815840a80ccba9eaeb16ff7e294:cmtspeech.h

Kernel space does not have enough info to handle these flags as the audio mixer is not implemented in kernel, so they have to be passed to/from user-space.

And some further info in libcmtspeechdata/docs/ https://www.gitorious.org/libcmtspeechdata/libcmtspeechdata/source/9206835ea3c96815840a80ccba9eaeb16ff7e294:doc/libcmtspeechdata_api_docs_main.txt

Quoted 2 lines

Uplink timing... As the daemon is realtime, can it just send the data at the right time? Also normally uplink would be filled, no?

But how would you implement that via the ALSA API? With cmt_speech, a speech packet is prepared in a mmap'ed buffer, flags are set to describe the buffer, and at the correct time, write() is called to trigger transmission in HW (see cmtspeech_ul_buffer_release() in libcmtspeechdata() -> compare this to snd_pcm_mmap_commit() in ALSA). In ALSA, the mmap commit and PCM write variants just add data to the ringbuffer and update the appl pointer. Only initial start (and stop) on stream have the "do something now" semantics in ALSA.

The ALSA compressed offload API did not exist back when we were working on cmt_speech, but that's still not a good fit, although adds some of the concepts (notably frames).

Quoted 3 lines

Well, packets are of fixed size, right? So the userland can simply supply the right size in the common case. As for sending at the right time... well... if the userspace is already real-time, that should be easy

See above, ALSA just doesn't work like that, there's no syscall for "send these samples now", the model is different.

Br, Kai