diff mbox

[26/44] fireworks: Add PCM interface

Message ID 1395400229-22957-27-git-send-email-o-takashi@sakamocchi.jp (mailing list archive)
State Superseded
Headers show

Commit Message

Takashi Sakamoto March 21, 2014, 11:10 a.m. UTC
This commit adds a functionality to capture/playback PCM samples.

When AMDTP stream is already running for PCM or the source of clock is not
internal, available sampling rate is limited at current one.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/Kconfig                      |   1 +
 sound/firewire/fireworks/Makefile           |   2 +-
 sound/firewire/fireworks/fireworks.c        |   4 +
 sound/firewire/fireworks/fireworks.h        |   4 +
 sound/firewire/fireworks/fireworks_pcm.c    | 468 ++++++++++++++++++++++++++++
 sound/firewire/fireworks/fireworks_stream.c |  33 --
 6 files changed, 478 insertions(+), 34 deletions(-)
 create mode 100644 sound/firewire/fireworks/fireworks_pcm.c

Comments

Clemens Ladisch April 4, 2014, 8:48 a.m. UTC | #1
Takashi Sakamoto wrote:
> This commit adds a functionality to capture/playback PCM samples.
>
> +++ b/sound/firewire/fireworks/fireworks_pcm.c
> +static unsigned int freq_table[] = {

This table is never changed; it can be made const.

> +static int
> +hw_rule_xxxxx(struct snd_pcm_hw_params *params,
> +	     struct snd_pcm_hw_rule *rule,
> +	     struct snd_efw *efw, unsigned int *channels)

The channels parameters can be made const, too.

> +	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
> +		substream->runtime->hw.formats = SNDRV_PCM_FMTBIT_S32;
> +	} else {
> +		substream->runtime->hw.formats = AMDTP_OUT_PCM_FORMAT_BITS;

The should have been a similar symbol for AMDTP capture streams.

> +	/*
> +	 * AMDTP functionality in firewire-lib require periods to be aligned to
> +	 * 16 bit, or 24bit inner 32bit.
> +	 */
> +	err = snd_pcm_hw_constraint_step(substream->runtime, 0,
> +					 SNDRV_PCM_HW_PARAM_PERIOD_BYTES, 32);
> +	if (err < 0)
> +		goto end;
> +	err = snd_pcm_hw_constraint_step(substream->runtime, 0,
> +					 SNDRV_PCM_HW_PARAM_BUFFER_BYTES, 32);
> +	if (err < 0)
> +		goto end;

The comment is not correct.

Period and buffer sizes are always aligned to the frame size, because
they are measured in frames.

The DICE driver aligns the period and buffer sizes to a multiple of
32 *frames* because the dual-wire sample transfer functions did not
handle buffer boundaries in the middle of a packet, and period
interrupts are more accurate if the period boundaries are at the exact
end a packet.  (The alignment could be reduced to 16 or 8 frames at
lower sample rates, but I was too lazy.)

This alignment makes sense only when the driver uses blocking mode, so
the snd-firewire-speakers driver does not use any alignment.

Aligning to 32 *bytes* instead of *frames* does not make sense, because
there is no such constraint in the hardware or the driver.

This driver uses blocking mode, so aligning to packets makes sense.
Replace _BYTES with _SIZE, and change the comment to something like this
(I should have written a comment like this in dice.c in the first place):

/*
 * Align the period size to SYT_INTERVAL to align the period interrupts
 * with the packet boundaries.  Align the buffer size to SYT_INTERVAL to
 * avoid having a buffer boundary inside a packet.
 */

> +static int pcm_open(struct snd_pcm_substream *substream)
> +{
> +	...
> +	/*
> +	 * When source of clock is not internal or any PCM streams are running,
> +	 * available sampling rate is limited at current sampling rate.
> +	 */
> +	if ((clock_source != SND_EFW_CLOCK_SOURCE_INTERNAL) ||
> +	    amdtp_stream_pcm_running(&efw->tx_stream) ||
> +	    amdtp_stream_pcm_running(&efw->rx_stream)) {

Opening the playback and capture streams of a single PCM device is
protected with the same mutex, but this does not help against races with
the MIDI callbacks.

This substream management code must be protected with a mutex.
(Also in hw_params and hw_free.)


Regards,
Clemens
Takashi Sakamoto April 6, 2014, 12:44 p.m. UTC | #2
Hi Clemens,

(Apr 4 2014 17:48), Clemens Ladisch wrote:
> Takashi Sakamoto wrote:
>> This commit adds a functionality to capture/playback PCM samples.
>>
>> +++ b/sound/firewire/fireworks/fireworks_pcm.c
>> +static unsigned int freq_table[] = {
>
> This table is never changed; it can be made const.

OK.

>> +static int
>> +hw_rule_xxxxx(struct snd_pcm_hw_params *params,
>> +	     struct snd_pcm_hw_rule *rule,
>> +	     struct snd_efw *efw, unsigned int *channels)
>
> The channels parameters can be made const, too.

OK.

>> +	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
>> +		substream->runtime->hw.formats = SNDRV_PCM_FMTBIT_S32;
>> +	} else {
>> +		substream->runtime->hw.formats = AMDTP_OUT_PCM_FORMAT_BITS;
>
> The should have been a similar symbol for AMDTP capture streams.

Do you suggest to add AMDTP_IN_PCM_FORMAT_BITS?

>> +	/*
>> +	 * AMDTP functionality in firewire-lib require periods to be aligned to
>> +	 * 16 bit, or 24bit inner 32bit.
>> +	 */
>> +	err = snd_pcm_hw_constraint_step(substream->runtime, 0,
>> +					 SNDRV_PCM_HW_PARAM_PERIOD_BYTES, 32);
>> +	if (err < 0)
>> +		goto end;
>> +	err = snd_pcm_hw_constraint_step(substream->runtime, 0,
>> +					 SNDRV_PCM_HW_PARAM_BUFFER_BYTES, 32);
>> +	if (err < 0)
>> +		goto end;
>
> The comment is not correct.
...
> This driver uses blocking mode, so aligning to packets makes sense.
> Replace _BYTES with _SIZE, and change the comment to something like this
> (I should have written a comment like this in dice.c in the first place):
>
> /*
>   * Align the period size to SYT_INTERVAL to align the period interrupts
>   * with the packet boundaries.  Align the buffer size to SYT_INTERVAL to
>   * avoid having a buffer boundary inside a packet.
>   */

Now I realize to misunderstand these codes.

Currently firewire-lib exports a table for SYT_INTERVAL so we can make 
better rules for this, can't we?

>> +static int pcm_open(struct snd_pcm_substream *substream)
>> +{
>> +	...
>> +	/*
>> +	 * When source of clock is not internal or any PCM streams are running,
>> +	 * available sampling rate is limited at current sampling rate.
>> +	 */
>> +	if ((clock_source != SND_EFW_CLOCK_SOURCE_INTERNAL) ||
>> +	    amdtp_stream_pcm_running(&efw->tx_stream) ||
>> +	    amdtp_stream_pcm_running(&efw->rx_stream)) {
>
> Opening the playback and capture streams of a single PCM device is
> protected with the same mutex, but this does not help against races with
> the MIDI callbacks.
>
> This substream management code must be protected with a mutex.
> (Also in hw_params and hw_free.)

Hm. I have no ideas for such races, except for substream counter. Can I 
request you more explaination? What race state can appears between 
PCM/MIDI functionalities?

For hw_params/hw_free, please see my reply to [24/44].


Thank you

Takashi Sakamoto
o-takashi@sakamocchi.jp
Clemens Ladisch April 6, 2014, 2:52 p.m. UTC | #3
Takashi Sakamoto wrote:
> (Apr 4 2014 17:48), Clemens Ladisch wrote:
>> Takashi Sakamoto wrote:
>>> +    if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
>>> +        substream->runtime->hw.formats = SNDRV_PCM_FMTBIT_S32;
>>> +    } else {
>>> +        substream->runtime->hw.formats = AMDTP_OUT_PCM_FORMAT_BITS;
>>
>> The should have been a similar symbol for AMDTP capture streams.
>
> Do you suggest to add AMDTP_IN_PCM_FORMAT_BITS?

Yes; in the ideal case, this driver does not need to know which formats
are actually supported.

>> /*
>>  * Align the period size to SYT_INTERVAL to align the period interrupts
>>  * with the packet boundaries.  Align the buffer size to SYT_INTERVAL to
>>  * avoid having a buffer boundary inside a packet.
>>  */
>
> Currently firewire-lib exports a table for SYT_INTERVAL so we can make
> better rules for this, can't we?

Yes, we could make better rules, if we wanted to.

>>> +static int pcm_open(struct snd_pcm_substream *substream)
>>> +{
>>> +    ...
>>> +    /*
>>> +     * When source of clock is not internal or any PCM streams are running,
>>> +     * available sampling rate is limited at current sampling rate.
>>> +     */
>>> +    if ((clock_source != SND_EFW_CLOCK_SOURCE_INTERNAL) ||
>>> +        amdtp_stream_pcm_running(&efw->tx_stream) ||
>>> +        amdtp_stream_pcm_running(&efw->rx_stream)) {
>>
>> Opening the playback and capture streams of a single PCM device is
>> protected with the same mutex, but this does not help against races with
>> the MIDI callbacks.
>>
>> This substream management code must be protected with a mutex.
>> (Also in hw_params and hw_free.)
>
> Hm. I have no ideas for such races, except for substream counter.

There is already a mutex for everything in start_duplex(), so the only
unprotected piece of data is this counter.


Regards,
Clemens
Takashi Sakamoto April 7, 2014, 7:20 a.m. UTC | #4
Hi Clemens,

(Apr 6 2014 23:52), Clemens Ladisch wrote:
>>>> +static int pcm_open(struct snd_pcm_substream *substream)
>>>> +{
>>>> +    ...
>>>> +    /*
>>>> +     * When source of clock is not internal or any PCM streams are running,
>>>> +     * available sampling rate is limited at current sampling rate.
>>>> +     */
>>>> +    if ((clock_source != SND_EFW_CLOCK_SOURCE_INTERNAL) ||
>>>> +        amdtp_stream_pcm_running(&efw->tx_stream) ||
>>>> +        amdtp_stream_pcm_running(&efw->rx_stream)) {
>>>
> There is already a mutex for everything in start_duplex(), so the only
> unprotected piece of data is this counter.

Yes. I think atomic_t mostly solves the issue.


But these codes (in PCM .open) includes another issue which brings a 
great headache to me.

The driver uses SNDRV_PCM_INFO_JOINT_DUPLEX. So sampling rate for 
playback/capture should be the same. But there is a case to allow 
applications to break this rule.

Let's assume that two applications try to use PCM functionality. 
Application 1 is for 48.0kHz playback, and Application 2 is for 88.2kHz 
capture. In this case, Application 1 write 48.0kHz samples on 88.2kHz 
streams:

Application 1: snd_pcm_open() for playback
Application 1: snd_pcm_hw_params() at 48.0kHz
(snd_pcm_prepare() and AMDTP streams start)
Application 2: snd_pcm_open() for capture
Application 2: snd_pcm_hw_params() at 88.2kHz
(snd_pcm_prepare() and AMDTP streams re-start)
Application 2: read PCM samples
Application 1: write PCM samples

The reason to allow snd_pcm_prepare() to restart streams at different 
sampling rate is to continue MIDI substreams at different sampling rate.

I've been considering about this issue for several months but still have 
no good ideas...

But I think this is actually too rare. I assume most applications 
write/read samples after snd_pcm_hw_params() as soon as possible and 
most sound server uses the same sampling rate for its capturing/playbacking.

So I hope this issue pending for my future work.


Thanks

Takashi Sakamoto
o-takashi@sakamocchi.jp
diff mbox

Patch

diff --git a/sound/firewire/Kconfig b/sound/firewire/Kconfig
index 36a1212..fb39f9c 100644
--- a/sound/firewire/Kconfig
+++ b/sound/firewire/Kconfig
@@ -65,6 +65,7 @@  config SND_FIREWORKS
 	tristate "Echo Fireworks board module support"
 	select SND_FIREWIRE_LIB
 	select SND_RAWMIDI
+	select SND_PCM
 	help
 	  Say Y here to include support for FireWire devices based
 	  on Echo Digital Audio Fireworks board:
diff --git a/sound/firewire/fireworks/Makefile b/sound/firewire/fireworks/Makefile
index a2cecc6..d7ebf83 100644
--- a/sound/firewire/fireworks/Makefile
+++ b/sound/firewire/fireworks/Makefile
@@ -1,4 +1,4 @@ 
 snd-fireworks-objs := fireworks_transaction.o fireworks_command.o \
 		      fireworks_stream.o fireworks_proc.o fireworks_midi.o \
-		      fireworks.o
+		      fireworks_pcm.o fireworks.o
 obj-m += snd-fireworks.o
diff --git a/sound/firewire/fireworks/fireworks.c b/sound/firewire/fireworks/fireworks.c
index ca3cf41..3c4351f 100644
--- a/sound/firewire/fireworks/fireworks.c
+++ b/sound/firewire/fireworks/fireworks.c
@@ -214,6 +214,10 @@  efw_probe(struct fw_unit *unit,
 			goto error;
 	}
 
+	err = snd_efw_create_pcm_devices(efw);
+	if (err < 0)
+		goto error;
+
 	err = snd_card_register(card);
 	if (err < 0)
 		goto error;
diff --git a/sound/firewire/fireworks/fireworks.h b/sound/firewire/fireworks/fireworks.h
index 426cbeb..86d6cfb 100644
--- a/sound/firewire/fireworks/fireworks.h
+++ b/sound/firewire/fireworks/fireworks.h
@@ -23,6 +23,7 @@ 
 #include <sound/pcm.h>
 #include <sound/info.h>
 #include <sound/rawmidi.h>
+#include <sound/pcm_params.h>
 
 #include "../packets-buffer.h"
 #include "../iso-resources.h"
@@ -196,6 +197,9 @@  void snd_efw_proc_init(struct snd_efw *efw);
 
 int snd_efw_create_midi_devices(struct snd_efw *efw);
 
+int snd_efw_create_pcm_devices(struct snd_efw *efw);
+int snd_efw_get_multiplier_mode(unsigned int sampling_rate, unsigned int *mode);
+
 #define SND_EFW_DEV_ENTRY(vendor, model) \
 { \
 	.match_flags	= IEEE1394_MATCH_VENDOR_ID | \
diff --git a/sound/firewire/fireworks/fireworks_pcm.c b/sound/firewire/fireworks/fireworks_pcm.c
new file mode 100644
index 0000000..88c1d53
--- /dev/null
+++ b/sound/firewire/fireworks/fireworks_pcm.c
@@ -0,0 +1,468 @@ 
+/*
+ * fireworks_pcm.c - a part of driver for Fireworks based devices
+ *
+ * Copyright (c) 2009-2010 Clemens Ladisch
+ * Copyright (c) 2013 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+#include "./fireworks.h"
+
+/*
+ * NOTE:
+ * Fireworks changes its AMDTP channels for PCM data according to its sampling
+ * rate. There are three modes. Here _XX is either _rx or _tx.
+ *  0:  32.0- 48.0 kHz then snd_efw_hwinfo.amdtp_XX_pcm_channels applied
+ *  1:  88.2- 96.0 kHz then snd_efw_hwinfo.amdtp_XX_pcm_channels_2x applied
+ *  2: 176.4-192.0 kHz then snd_efw_hwinfo.amdtp_XX_pcm_channels_4x applied
+ *
+ * The number of PCM channels for analog input and output are always fixed but
+ * the number of PCM channels for digital input and output are differed.
+ *
+ * Additionally, according to "AudioFire Owner's Manual Version 2.2", in some
+ * model, the number of PCM channels for digital input has more restriction
+ * depending on which digital interface is selected.
+ *  - S/PDIF coaxial and optical	: use input 1-2
+ *  - ADAT optical at 32.0-48.0 kHz	: use input 1-8
+ *  - ADAT optical at 88.2-96.0 kHz	: use input 1-4 (S/MUX format)
+ *
+ * The data in AMDTP channels for blank PCM channels are zero.
+ */
+static unsigned int freq_table[] = {
+	/* multiplier mode 0 */
+	[0] = 32000,
+	[1] = 44100,
+	[2] = 48000,
+	/* multiplier mode 1 */
+	[3] = 88200,
+	[4] = 96000,
+	/* multiplier mode 2 */
+	[5] = 176400,
+	[6] = 192000,
+};
+
+static inline unsigned int
+get_multiplier_mode_with_index(unsigned int index)
+{
+	return ((int)index - 1) / 2;
+}
+
+int snd_efw_get_multiplier_mode(unsigned int sampling_rate, unsigned int *mode)
+{
+	unsigned int i;
+
+	for (i = 0; i < sizeof(freq_table); i++) {
+		if (freq_table[i] == sampling_rate) {
+			*mode = get_multiplier_mode_with_index(i);
+			return 0;
+		}
+	}
+
+	return -EINVAL;
+}
+
+static int
+hw_rule_rate(struct snd_pcm_hw_params *params,
+	     struct snd_pcm_hw_rule *rule,
+	     struct snd_efw *efw, unsigned int *channels)
+{
+	struct snd_interval *r =
+		hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
+	const struct snd_interval *c =
+		hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_CHANNELS);
+	struct snd_interval t = {
+		.min = UINT_MAX, .max = 0, .integer = 1
+	};
+	unsigned int i, mode, rate_bit;
+
+	for (i = 0; i < ARRAY_SIZE(freq_table); i++) {
+		/* skip unsupported sampling rate */
+		rate_bit = snd_pcm_rate_to_rate_bit(freq_table[i]);
+		if (!(efw->supported_sampling_rate & rate_bit))
+			continue;
+
+		mode = get_multiplier_mode_with_index(i);
+		if (!snd_interval_test(c, channels[mode]))
+			continue;
+
+		t.min = min(t.min, freq_table[i]);
+		t.max = max(t.max, freq_table[i]);
+
+	}
+
+	return snd_interval_refine(r, &t);
+}
+
+static int
+hw_rule_channels(struct snd_pcm_hw_params *params,
+		 struct snd_pcm_hw_rule *rule,
+		 struct snd_efw *efw, unsigned int *channels)
+{
+	struct snd_interval *c =
+		hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS);
+	const struct snd_interval *r =
+		hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_RATE);
+	struct snd_interval t = {
+		.min = UINT_MAX, .max = 0, .integer = 1
+	};
+	unsigned int i, mode, rate_bit;
+
+	for (i = 0; i < ARRAY_SIZE(freq_table); i++) {
+		/* skip unsupported sampling rate */
+		rate_bit = snd_pcm_rate_to_rate_bit(freq_table[i]);
+		if (!(efw->supported_sampling_rate & rate_bit))
+			continue;
+
+		mode = get_multiplier_mode_with_index(i);
+		if (!snd_interval_test(r, freq_table[i]))
+			continue;
+
+		t.min = min(t.min, channels[mode]);
+		t.max = max(t.max, channels[mode]);
+
+	}
+
+	return snd_interval_refine(c, &t);
+}
+
+static inline int
+hw_rule_capture_rate(struct snd_pcm_hw_params *params,
+		     struct snd_pcm_hw_rule *rule)
+{
+	struct snd_efw *efw = rule->private;
+	return hw_rule_rate(params, rule, efw,
+				efw->pcm_capture_channels);
+}
+
+static inline int
+hw_rule_playback_rate(struct snd_pcm_hw_params *params,
+		      struct snd_pcm_hw_rule *rule)
+{
+	struct snd_efw *efw = rule->private;
+	return hw_rule_rate(params, rule, efw,
+				efw->pcm_playback_channels);
+}
+
+static inline int
+hw_rule_capture_channels(struct snd_pcm_hw_params *params,
+			 struct snd_pcm_hw_rule *rule)
+{
+	struct snd_efw *efw = rule->private;
+	return hw_rule_channels(params, rule, efw,
+				efw->pcm_capture_channels);
+}
+
+static inline int
+hw_rule_playback_channels(struct snd_pcm_hw_params *params,
+			  struct snd_pcm_hw_rule *rule)
+{
+	struct snd_efw *efw = rule->private;
+	return hw_rule_channels(params, rule, efw,
+				efw->pcm_playback_channels);
+}
+
+static int
+pcm_init_hw_params(struct snd_efw *efw,
+		   struct snd_pcm_substream *substream)
+{
+	unsigned int i, *pcm_channels, rate_bit, mode;
+	int err;
+
+	struct snd_pcm_hardware hardware = {
+		.info = SNDRV_PCM_INFO_BATCH |
+			SNDRV_PCM_INFO_BLOCK_TRANSFER |
+			SNDRV_PCM_INFO_INTERLEAVED |
+			SNDRV_PCM_INFO_JOINT_DUPLEX |
+			SNDRV_PCM_INFO_MMAP |
+			SNDRV_PCM_INFO_MMAP_VALID,
+		.rates = efw->supported_sampling_rate,
+		/* set up later */
+		.rate_min = UINT_MAX,
+		.rate_max = 0,
+		/* set up later */
+		.channels_min = UINT_MAX,
+		.channels_max = 0,
+		.buffer_bytes_max = 4 * 34 * 2048 * 2,
+		.period_bytes_min = 1,
+		.period_bytes_max = 4 * 34 * 2048,
+		.periods_min = 2,
+		.periods_max = UINT_MAX,
+	};
+
+	substream->runtime->hw = hardware;
+
+	/* add rule between channels and sampling rate */
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
+		substream->runtime->hw.formats = SNDRV_PCM_FMTBIT_S32;
+		snd_pcm_hw_rule_add(substream->runtime, 0,
+				    SNDRV_PCM_HW_PARAM_CHANNELS,
+				    hw_rule_capture_channels, efw,
+				    SNDRV_PCM_HW_PARAM_RATE, -1);
+		snd_pcm_hw_rule_add(substream->runtime, 0,
+				    SNDRV_PCM_HW_PARAM_RATE,
+				    hw_rule_capture_rate, efw,
+				    SNDRV_PCM_HW_PARAM_CHANNELS, -1);
+		pcm_channels = efw->pcm_capture_channels;
+	} else {
+		substream->runtime->hw.formats = AMDTP_OUT_PCM_FORMAT_BITS;
+		snd_pcm_hw_rule_add(substream->runtime, 0,
+				    SNDRV_PCM_HW_PARAM_CHANNELS,
+				    hw_rule_playback_channels, efw,
+				    SNDRV_PCM_HW_PARAM_RATE, -1);
+		snd_pcm_hw_rule_add(substream->runtime, 0,
+				    SNDRV_PCM_HW_PARAM_RATE,
+				    hw_rule_playback_rate, efw,
+				    SNDRV_PCM_HW_PARAM_CHANNELS, -1);
+		pcm_channels = efw->pcm_playback_channels;
+	}
+
+	/* limitation for min/max sampling rate */
+	snd_pcm_limit_hw_rates(substream->runtime);
+
+	/* limitation for the number of channels */
+	for (i = 0; i < ARRAY_SIZE(freq_table); i++) {
+		/* skip unsupported sampling rate */
+		rate_bit = snd_pcm_rate_to_rate_bit(freq_table[i]);
+		if (!(efw->supported_sampling_rate & rate_bit))
+			continue;
+
+		mode = get_multiplier_mode_with_index(i);
+		if (pcm_channels[mode] == 0)
+			continue;
+		substream->runtime->hw.channels_min =
+				min(substream->runtime->hw.channels_min,
+				    pcm_channels[mode]);
+		substream->runtime->hw.channels_max =
+				max(substream->runtime->hw.channels_max,
+				    pcm_channels[mode]);
+	}
+
+	/* AM824 in IEC 61883-6 can deliver 24bit data */
+	err = snd_pcm_hw_constraint_msbits(substream->runtime, 0, 32, 24);
+	if (err < 0)
+		goto end;
+
+	/*
+	 * AMDTP functionality in firewire-lib require periods to be aligned to
+	 * 16 bit, or 24bit inner 32bit.
+	 */
+	err = snd_pcm_hw_constraint_step(substream->runtime, 0,
+					 SNDRV_PCM_HW_PARAM_PERIOD_BYTES, 32);
+	if (err < 0)
+		goto end;
+	err = snd_pcm_hw_constraint_step(substream->runtime, 0,
+					 SNDRV_PCM_HW_PARAM_BUFFER_BYTES, 32);
+	if (err < 0)
+		goto end;
+
+	/*
+	 * Currently INTERRUPT_INTERVAL in amdtp.c is 16.
+	 * So snd_pcm_period_elapsed() can be called every 2m sec.
+	 */
+	err = snd_pcm_hw_constraint_minmax(substream->runtime,
+					   SNDRV_PCM_HW_PARAM_PERIOD_TIME,
+					   2000, UINT_MAX);
+end:
+	return err;
+}
+
+static int pcm_open(struct snd_pcm_substream *substream)
+{
+	struct snd_efw *efw = substream->private_data;
+	int sampling_rate;
+	unsigned int clock_source;
+	int err;
+
+	err = pcm_init_hw_params(efw, substream);
+	if (err < 0)
+		goto end;
+
+	err = snd_efw_command_get_clock_source(efw, &clock_source);
+	if (err < 0)
+		goto end;
+
+	/*
+	 * When source of clock is not internal or any PCM streams are running,
+	 * available sampling rate is limited at current sampling rate.
+	 */
+	if ((clock_source != SND_EFW_CLOCK_SOURCE_INTERNAL) ||
+	    amdtp_stream_pcm_running(&efw->tx_stream) ||
+	    amdtp_stream_pcm_running(&efw->rx_stream)) {
+		err = snd_efw_command_get_sampling_rate(efw, &sampling_rate);
+		if (err < 0)
+			goto end;
+		substream->runtime->hw.rate_min = sampling_rate;
+		substream->runtime->hw.rate_max = sampling_rate;
+	}
+
+	snd_pcm_set_sync(substream);
+end:
+	return err;
+}
+
+static int pcm_close(struct snd_pcm_substream *substream)
+{
+	return 0;
+}
+
+static int pcm_capture_hw_params(struct snd_pcm_substream *substream,
+				 struct snd_pcm_hw_params *hw_params)
+{
+	struct snd_efw *efw = substream->private_data;
+
+	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN)
+		efw->capture_substreams++;
+	amdtp_stream_set_pcm_format(&efw->tx_stream, params_format(hw_params));
+
+	return snd_pcm_lib_alloc_vmalloc_buffer(substream,
+						params_buffer_bytes(hw_params));
+}
+static int pcm_playback_hw_params(struct snd_pcm_substream *substream,
+				  struct snd_pcm_hw_params *hw_params)
+{
+	struct snd_efw *efw = substream->private_data;
+
+	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN)
+		efw->playback_substreams++;
+	amdtp_stream_set_pcm_format(&efw->rx_stream, params_format(hw_params));
+
+	return snd_pcm_lib_alloc_vmalloc_buffer(substream,
+						params_buffer_bytes(hw_params));
+}
+
+static int pcm_capture_hw_free(struct snd_pcm_substream *substream)
+{
+	struct snd_efw *efw = substream->private_data;
+
+	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
+		efw->capture_substreams--;
+
+	snd_efw_stream_stop_duplex(efw);
+
+	return snd_pcm_lib_free_vmalloc_buffer(substream);
+}
+static int pcm_playback_hw_free(struct snd_pcm_substream *substream)
+{
+	struct snd_efw *efw = substream->private_data;
+
+	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
+		efw->playback_substreams--;
+
+	snd_efw_stream_stop_duplex(efw);
+
+	return snd_pcm_lib_free_vmalloc_buffer(substream);
+}
+
+static int pcm_capture_prepare(struct snd_pcm_substream *substream)
+{
+	struct snd_efw *efw = substream->private_data;
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	int err;
+
+	err = snd_efw_stream_start_duplex(efw, &efw->tx_stream, runtime->rate);
+	if (err >= 0)
+		amdtp_stream_pcm_prepare(&efw->tx_stream);
+
+	return err;
+}
+static int pcm_playback_prepare(struct snd_pcm_substream *substream)
+{
+	struct snd_efw *efw = substream->private_data;
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	int err;
+
+	err = snd_efw_stream_start_duplex(efw, &efw->rx_stream, runtime->rate);
+	if (err >= 0)
+		amdtp_stream_pcm_prepare(&efw->rx_stream);
+
+	return err;
+}
+
+static int pcm_capture_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	struct snd_efw *efw = substream->private_data;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		amdtp_stream_pcm_trigger(&efw->tx_stream, substream);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+		amdtp_stream_pcm_trigger(&efw->tx_stream, NULL);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+static int pcm_playback_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	struct snd_efw *efw = substream->private_data;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		amdtp_stream_pcm_trigger(&efw->rx_stream, substream);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+		amdtp_stream_pcm_trigger(&efw->rx_stream, NULL);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static snd_pcm_uframes_t pcm_capture_pointer(struct snd_pcm_substream *sbstrm)
+{
+	struct snd_efw *efw = sbstrm->private_data;
+	return amdtp_stream_pcm_pointer(&efw->tx_stream);
+}
+static snd_pcm_uframes_t pcm_playback_pointer(struct snd_pcm_substream *sbstrm)
+{
+	struct snd_efw *efw = sbstrm->private_data;
+	return amdtp_stream_pcm_pointer(&efw->rx_stream);
+}
+
+static const struct snd_pcm_ops pcm_capture_ops = {
+	.open		= pcm_open,
+	.close		= pcm_close,
+	.ioctl		= snd_pcm_lib_ioctl,
+	.hw_params	= pcm_capture_hw_params,
+	.hw_free	= pcm_capture_hw_free,
+	.prepare	= pcm_capture_prepare,
+	.trigger	= pcm_capture_trigger,
+	.pointer	= pcm_capture_pointer,
+	.page		= snd_pcm_lib_get_vmalloc_page,
+};
+
+static const struct snd_pcm_ops pcm_playback_ops = {
+	.open		= pcm_open,
+	.close		= pcm_close,
+	.ioctl		= snd_pcm_lib_ioctl,
+	.hw_params	= pcm_playback_hw_params,
+	.hw_free	= pcm_playback_hw_free,
+	.prepare	= pcm_playback_prepare,
+	.trigger	= pcm_playback_trigger,
+	.pointer	= pcm_playback_pointer,
+	.page		= snd_pcm_lib_get_vmalloc_page,
+	.mmap		= snd_pcm_lib_mmap_vmalloc,
+};
+
+int snd_efw_create_pcm_devices(struct snd_efw *efw)
+{
+	struct snd_pcm *pcm;
+	int err;
+
+	err = snd_pcm_new(efw->card, efw->card->driver, 0, 1, 1, &pcm);
+	if (err < 0)
+		goto end;
+
+	pcm->private_data = efw;
+	snprintf(pcm->name, sizeof(pcm->name), "%s PCM", efw->card->shortname);
+	snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &pcm_playback_ops);
+	snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &pcm_capture_ops);
+end:
+	return err;
+}
+
diff --git a/sound/firewire/fireworks/fireworks_stream.c b/sound/firewire/fireworks/fireworks_stream.c
index ee9a038..d6adb22 100644
--- a/sound/firewire/fireworks/fireworks_stream.c
+++ b/sound/firewire/fireworks/fireworks_stream.c
@@ -9,39 +9,6 @@ 
 
 #define CALLBACK_TIMEOUT	100
 
-static unsigned int freq_table[] = {
-	/* multiplier mode 0 */
-	[0] = 32000,
-	[1] = 44100,
-	[2] = 48000,
-	/* multiplier mode 1 */
-	[3] = 88200,
-	[4] = 96000,
-	/* multiplier mode 2 */
-	[5] = 176400,
-	[6] = 192000,
-};
-
-static inline unsigned int
-get_multiplier_mode_with_index(unsigned int index)
-{
-	return ((int)index - 1) / 2;
-}
-
-int snd_efw_get_multiplier_mode(unsigned int sampling_rate, unsigned int *mode)
-{
-	unsigned int i;
-
-	for (i = 0; i < sizeof(freq_table); i++) {
-		if (freq_table[i] == sampling_rate) {
-			*mode = get_multiplier_mode_with_index(i);
-			return 0;
-		}
-	}
-
-	return -EINVAL;
-}
-
 static int
 init_stream(struct snd_efw *efw, struct amdtp_stream *stream)
 {