Patchwork ASoC: kirkwood: add S/PDIF support

login
register
mail settings
Submitter Jean-Francois Moine
Date Sept. 24, 2013, 10:41 a.m.
Message ID <20130924124122.64834b2a@armhf>
Download mbox | patch
Permalink /patch/277428/
State New
Headers show

Comments

Jean-Francois Moine - Sept. 24, 2013, 10:41 a.m.
This patch adds S/PDIF input/output for mvebu DT boards.

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 Documentation/devicetree/bindings/sound/mvebu-audio.txt |  23 +++++
 sound/soc/kirkwood/kirkwood-i2s.c                       | 128 ++++++++++++++++----------
 sound/soc/kirkwood/kirkwood.h                           |   2 +
 3 files changed, 102 insertions(+), 51 deletions(-)
Russell King - ARM Linux - Sept. 24, 2013, 10:51 a.m.
On Tue, Sep 24, 2013 at 12:41:22PM +0200, Jean-Francois Moine wrote:
> This patch adds S/PDIF input/output for mvebu DT boards.
> 
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>

Right, so if this goes in, then we're completely fucked for doing this
the way that Mark apparantly wants it to be done, because once these
DT properties go in, they're there to stay.
Mark Brown - Sept. 24, 2013, 3:37 p.m.
On Tue, Sep 24, 2013 at 11:51:27AM +0100, Russell King - ARM Linux wrote:
> On Tue, Sep 24, 2013 at 12:41:22PM +0200, Jean-Francois Moine wrote:
> > This patch adds S/PDIF input/output for mvebu DT boards.

> Right, so if this goes in, then we're completely fucked for doing this
> the way that Mark apparantly wants it to be done, because once these
> DT properties go in, they're there to stay.

I'm not sure that this is in particular conflict with any other way of
writing things, though it would be redundant.  The best thing would be
to key this off connecting to DAIs so that people just need to make the
connections that exist in their system in the device tree.
Jean-Francois Moine - Sept. 24, 2013, 4:32 p.m.
On Tue, 24 Sep 2013 11:51:27 +0100
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Tue, Sep 24, 2013 at 12:41:22PM +0200, Jean-Francois Moine wrote:
> > This patch adds S/PDIF input/output for mvebu DT boards.
> > 
> > Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
> 
> Right, so if this goes in, then we're completely fucked for doing this
> the way that Mark apparantly wants it to be done, because once these
> DT properties go in, they're there to stay.

Indeed!

The DT properties I propose just reflect the hardware connections of
the board. There are only i2s-out and spdif-out in the Cubox, while
there may be kirkwood boards with, say, only spdif-in and spdif-out.

These definitions do not interfere with your DPCM problem: you assume
i2s-out and spdif-out for the i2s1 dove device, and that is what I
assume too, but from the DT. Then, the driver does not need to know the
type of the audio device (Kirkwood or Dove), nor to know if it is the
first or the second i2s device of the Dove chip.
Mark Brown - Oct. 18, 2013, 12:34 a.m.
On Tue, Sep 24, 2013 at 12:41:22PM +0200, Jean-Francois Moine wrote:

> This patch adds S/PDIF input/output for mvebu DT boards.

This got less discussion than I was expecting and then slipped under a
lot of other patches in my queue while that was timing out, sorry...  in
any case, this still has the issue that it used a single DAI and a
single DAI link to represent two physical DAIs and their links.

Please as covered in the prior threads on this either implement DPCM or
implement two traditional DAIs which can't be used simultaneously (which
should be simpler and can then be built on incrementally).

The last set of DPCMish patches was reported to work so should be a
fairly small refactoring away from being OK - the main thing is to
create back end DAIs for the physical links and then put the widgets
that manage the enable of the S/PDIF and I2S interfaces between the FE
and BE DAIs instead of directly connecting them to widgets on the CODEC.

For two traditional DAIs it should just be a case of duplicating the DAI
with a differnet ID then setting the enable bits based on the ID of the
DAI in use and ideally adding some code to check that they're not both
active (or that limit can be enforced by only having machine drivers
that use one or the other).
Russell King - ARM Linux - Oct. 18, 2013, 10:30 a.m.
On Fri, Oct 18, 2013 at 01:34:39AM +0100, Mark Brown wrote:
> The last set of DPCMish patches was reported to work so should be a
> fairly small refactoring away from being OK - the main thing is to
> create back end DAIs for the physical links and then put the widgets
> that manage the enable of the S/PDIF and I2S interfaces between the FE
> and BE DAIs instead of directly connecting them to widgets on the CODEC.

ROTFL.  You're now telling JF to do exactly what I did in my last patch.
Your two-faced-ness is utterly astounding.

"widgets that manage the enable of the S/PDIF and I2S interfaces between
the FE and BE DAIs" is exactly what I did and you told me that was wrong.
Make up your fscking mind.
Mark Brown - Oct. 18, 2013, 12:48 p.m.
On Fri, Oct 18, 2013 at 11:30:19AM +0100, Russell King - ARM Linux wrote:
> On Fri, Oct 18, 2013 at 01:34:39AM +0100, Mark Brown wrote:

> > The last set of DPCMish patches was reported to work so should be a
> > fairly small refactoring away from being OK - the main thing is to
> > create back end DAIs for the physical links and then put the widgets
> > that manage the enable of the S/PDIF and I2S interfaces between the FE
> > and BE DAIs instead of directly connecting them to widgets on the CODEC.

> ROTFL.  You're now telling JF to do exactly what I did in my last patch.
> Your two-faced-ness is utterly astounding.

> "widgets that manage the enable of the S/PDIF and I2S interfaces between
> the FE and BE DAIs" is exactly what I did and you told me that was wrong.
> Make up your fscking mind.

No, this is not the case.  The major problem with your last set of
patches was that they did not create any back end DAIs, instead they
created purely DAPM routes from the single traditional DAI out to
widgets in the CODEC.  What I'm asking Jean-Francois to do above is
create and use back end DAIs.

Please let me also remind you that confrontational behaviour such as the
ad hominem remarks above is not at all constructive or helpful, please
keep the discussion civil.
Russell King - ARM Linux - Oct. 18, 2013, 12:56 p.m.
On Fri, Oct 18, 2013 at 01:48:32PM +0100, Mark Brown wrote:
> On Fri, Oct 18, 2013 at 11:30:19AM +0100, Russell King - ARM Linux wrote:
> > On Fri, Oct 18, 2013 at 01:34:39AM +0100, Mark Brown wrote:
> 
> > > The last set of DPCMish patches was reported to work so should be a
> > > fairly small refactoring away from being OK - the main thing is to
> > > create back end DAIs for the physical links and then put the widgets
> > > that manage the enable of the S/PDIF and I2S interfaces between the FE
> > > and BE DAIs instead of directly connecting them to widgets on the CODEC.
> 
> > ROTFL.  You're now telling JF to do exactly what I did in my last patch.
> > Your two-faced-ness is utterly astounding.
> 
> > "widgets that manage the enable of the S/PDIF and I2S interfaces between
> > the FE and BE DAIs" is exactly what I did and you told me that was wrong.
> > Make up your fscking mind.
> 
> No, this is not the case.  The major problem with your last set of
> patches was that they did not create any back end DAIs, instead they
> created purely DAPM routes from the single traditional DAI out to
> widgets in the CODEC.  What I'm asking Jean-Francois to do above is
> create and use back end DAIs.
> 
> Please let me also remind you that confrontational behaviour such as the
> ad hominem remarks above is not at all constructive or helpful, please
> keep the discussion civil.

Let me remind you that I had a definition of what a front end and a back
end DAI was from Liam, and your definition is at odds with that.

I am still of the opinion that you don't know what you're talking about
most of the time, and nothing has changed in that regard, and I still
regard you as a completely  obstructive and unhelpful person.

And I think you're just wrong on what you've said above.  But... I've
basically given up any hope of progressing that code after months of
pissing around with your obstructive attitude, and I've elected (and
others) to maintain it completely out of mainline for the forseeable
future - hopefully until you get replaced by someone who can be more
assistive and helpful.
Mark Brown - Oct. 18, 2013, 1:48 p.m.
On Fri, Oct 18, 2013 at 01:56:38PM +0100, Russell King - ARM Linux wrote:

> Let me remind you that I had a definition of what a front end and a back
> end DAI was from Liam, and your definition is at odds with that.

Could you please provide some detail on this?  I was aware that you were
for a time confused about this but had not been aware of the source of
the confusion.

> I am still of the opinion that you don't know what you're talking about
> most of the time, and nothing has changed in that regard, and I still
> regard you as a completely  obstructive and unhelpful person.

> And I think you're just wrong on what you've said above.  But... I've
> basically given up any hope of progressing that code after months of
> pissing around with your obstructive attitude, and I've elected (and
> others) to maintain it completely out of mainline for the forseeable
> future - hopefully until you get replaced by someone who can be more
> assistive and helpful.

Once more, please be civil and constructive - a confrontational approach
has not been helping here.  If you are intent on behaving in this
fashion please at least try to avoid disrupting others who are
interested in working in mainline.
Russell King - ARM Linux - Oct. 18, 2013, 2:27 p.m.
On Fri, Oct 18, 2013 at 02:48:29PM +0100, Mark Brown wrote:
> On Fri, Oct 18, 2013 at 01:56:38PM +0100, Russell King - ARM Linux wrote:
> 
> > Let me remind you that I had a definition of what a front end and a back
> > end DAI was from Liam, and your definition is at odds with that.
> 
> Could you please provide some detail on this?  I was aware that you were
> for a time confused about this but had not been aware of the source of
> the confusion.

Let's see:

16:10 < rmk> broonie: can you explain this pcm frontend/backend stuff in
	asoc. what exactly is a frontend and backend pcm, and how do they
	relate to the CPU hardware and codec links?  Which are the ones
	which are given ALSA PCMs?
17:22 < broonie> rmk: A front end is the bit that does DMA from memory, a
	back end is a physical interface.
17:22 < broonie> The bit in the middle is usually a DSP.
17:23 < broonie> To be frank I've not actually *used* this stuff myself
	except in bolting a new CODEC on the edge of it during system
	integrations - going to be rectifying that very soon with the
	Samsung drivers I hope.
17:24 < broonie> But the general idea is that the front end is what the
	application sees, the back end is what comes out of the hardware
	at the other end.
17:25 < broonie> And the bit in the middle does any routing an rewriting
	of formats between the two.
17:26 < broonie> (The Samsung picture should be very similar to Kirkwood
	but the opposite way around - two front ends mixed togther to a
	single back end)
...
16:49 < rmk> oh, another question
16:49 < rmk> .dynamic in the dai link structure, that's for backends only,
	right?
16:50 < broonie> Think so, yes.
16:52 < rmk> hmm
16:52 < rmk>         if (rtd->dai_link->dynamic) {
16:52 < rmk>                 rtd->ops.open           = dpcm_fe_dai_open;
16:52 < rmk> probably needed for frontends
Russell King - ARM Linux - Oct. 18, 2013, 2:33 p.m.
On Fri, Oct 18, 2013 at 02:48:29PM +0100, Mark Brown wrote:
> On Fri, Oct 18, 2013 at 01:56:38PM +0100, Russell King - ARM Linux wrote:
> > And I think you're just wrong on what you've said above.  But... I've
> > basically given up any hope of progressing that code after months of
> > pissing around with your obstructive attitude, and I've elected (and
> > others) to maintain it completely out of mainline for the forseeable
> > future - hopefully until you get replaced by someone who can be more
> > assistive and helpful.
> 
> Once more, please be civil and constructive - a confrontational approach
> has not been helping here.  If you are intent on behaving in this
> fashion please at least try to avoid disrupting others who are
> interested in working in mainline.

Well, you say that but that's actually a total lie.  It is you who have
done far more than anyone else to disrupt progress on this issue through
your purposely misleading and unhelpful comments.  Many I remind you that
it was _me_ who spent months implementing the suggestions you came out
with only to be told time and time again that it was wrong.
Jean-Francois Moine - Oct. 18, 2013, 3:38 p.m.
On Fri, 18 Oct 2013 13:48:32 +0100
Mark Brown <broonie@kernel.org> wrote:

> > "widgets that manage the enable of the S/PDIF and I2S interfaces between
> > the FE and BE DAIs" is exactly what I did and you told me that was wrong.
> > Make up your fscking mind.  
> 
> No, this is not the case.  The major problem with your last set of
> patches was that they did not create any back end DAIs, instead they
> created purely DAPM routes from the single traditional DAI out to
> widgets in the CODEC.  What I'm asking Jean-Francois to do above is
> create and use back end DAIs.

Mark,

I think I understood what you mean. I am testing my system with this
new approach and I will submit a new patch as soon as it works.

Patch

diff --git a/Documentation/devicetree/bindings/sound/mvebu-audio.txt b/Documentation/devicetree/bindings/sound/mvebu-audio.txt
index f0062c5..c19a5d3 100644
--- a/Documentation/devicetree/bindings/sound/mvebu-audio.txt
+++ b/Documentation/devicetree/bindings/sound/mvebu-audio.txt
@@ -22,6 +22,27 @@  Required properties:
 	"internal" for the internal clock
 	"extclk" for the external clock
 
+Optional properties:
+
+- i2s-out: This is a boolean property. If present, the transmitting
+  function of I2S will be enabled, indicating the I2S is connected
+  to some IP block, such as an HDMI encoder/display-controller.
+
+- i2s-in: This is a boolean property. If present, the receiving
+  function of I2S will be enabled, indicating the I2S is connected
+  to some IP block, such as an HDMI encoder/display-controller.
+
+- spdif-out: This is a boolean property. If present, the transmitting
+  function of S/PDIF will be enabled, indicating there's a physical
+  S/PDIF out connector/jack on the board or it's connecting to some
+  other IP block, such as an HDMI encoder/display-controller.
+
+- spdif-in: This is a boolean property. If present, the receiving
+  function of S/PDIF will be enabled, indicating there's a physical
+  S/PDIF in connector/jack on the board.
+
+* Note: At least one of these four properties should be set in the DT binding.
+
 Example:
 
 i2s1: audio-controller@b4000 {
@@ -30,4 +51,6 @@  i2s1: audio-controller@b4000 {
 	interrupts = <21>, <22>;
 	clocks = <&gate_clk 13>;
 	clock-names = "internal";
+	i2s-out;
+	spdif-out;
 };
diff --git a/sound/soc/kirkwood/kirkwood-i2s.c b/sound/soc/kirkwood/kirkwood-i2s.c
index 0f3d73d..c62c7fa 100644
--- a/sound/soc/kirkwood/kirkwood-i2s.c
+++ b/sound/soc/kirkwood/kirkwood-i2s.c
@@ -160,9 +160,11 @@  static int kirkwood_i2s_hw_params(struct snd_pcm_substream *substream,
 	case SNDRV_PCM_FORMAT_S16_LE:
 		i2s_value |= KIRKWOOD_I2S_CTL_SIZE_16;
 		ctl_play = KIRKWOOD_PLAYCTL_SIZE_16_C |
-			   KIRKWOOD_PLAYCTL_I2S_EN;
+			   KIRKWOOD_PLAYCTL_I2S_EN |
+			   KIRKWOOD_PLAYCTL_SPDIF_EN;
 		ctl_rec = KIRKWOOD_RECCTL_SIZE_16_C |
-			  KIRKWOOD_RECCTL_I2S_EN;
+			  KIRKWOOD_RECCTL_I2S_EN |
+			  KIRKWOOD_RECCTL_SPDIF_EN;
 		break;
 	/*
 	 * doesn't work... S20_3LE != kirkwood 20bit format ?
@@ -178,9 +180,11 @@  static int kirkwood_i2s_hw_params(struct snd_pcm_substream *substream,
 	case SNDRV_PCM_FORMAT_S24_LE:
 		i2s_value |= KIRKWOOD_I2S_CTL_SIZE_24;
 		ctl_play = KIRKWOOD_PLAYCTL_SIZE_24 |
-			   KIRKWOOD_PLAYCTL_I2S_EN;
+			   KIRKWOOD_PLAYCTL_I2S_EN |
+			   KIRKWOOD_PLAYCTL_SPDIF_EN;
 		ctl_rec = KIRKWOOD_RECCTL_SIZE_24 |
-			  KIRKWOOD_RECCTL_I2S_EN;
+			  KIRKWOOD_RECCTL_I2S_EN |
+			  KIRKWOOD_PLAYCTL_I2S_EN;
 		break;
 	case SNDRV_PCM_FORMAT_S32_LE:
 		i2s_value |= KIRKWOOD_I2S_CTL_SIZE_32;
@@ -253,12 +257,14 @@  static int kirkwood_i2s_play_trigger(struct snd_pcm_substream *substream,
 		writel(value, priv->io + KIRKWOOD_INT_MASK);
 
 		/* enable playback */
+		ctl &= priv->ctl_play_mask;
 		writel(ctl, priv->io + KIRKWOOD_PLAYCTL);
 		break;
 
 	case SNDRV_PCM_TRIGGER_STOP:
 		/* stop audio, disable interrupts */
-		ctl |= KIRKWOOD_PLAYCTL_PAUSE | KIRKWOOD_PLAYCTL_I2S_MUTE;
+		ctl |= KIRKWOOD_PLAYCTL_PAUSE | KIRKWOOD_PLAYCTL_I2S_MUTE |
+				KIRKWOOD_PLAYCTL_SPDIF_MUTE;
 		writel(ctl, priv->io + KIRKWOOD_PLAYCTL);
 
 		value = readl(priv->io + KIRKWOOD_INT_MASK);
@@ -272,13 +278,15 @@  static int kirkwood_i2s_play_trigger(struct snd_pcm_substream *substream,
 
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
-		ctl |= KIRKWOOD_PLAYCTL_PAUSE | KIRKWOOD_PLAYCTL_I2S_MUTE;
+		ctl |= KIRKWOOD_PLAYCTL_PAUSE | KIRKWOOD_PLAYCTL_I2S_MUTE |
+				KIRKWOOD_PLAYCTL_SPDIF_MUTE;
 		writel(ctl, priv->io + KIRKWOOD_PLAYCTL);
 		break;
 
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		ctl &= ~(KIRKWOOD_PLAYCTL_PAUSE | KIRKWOOD_PLAYCTL_I2S_MUTE);
+		ctl &= ~(KIRKWOOD_PLAYCTL_PAUSE | KIRKWOOD_PLAYCTL_I2S_MUTE |
+				KIRKWOOD_PLAYCTL_SPDIF_MUTE);
 		writel(ctl, priv->io + KIRKWOOD_PLAYCTL);
 		break;
 
@@ -301,7 +309,8 @@  static int kirkwood_i2s_rec_trigger(struct snd_pcm_substream *substream,
 	case SNDRV_PCM_TRIGGER_START:
 		/* configure */
 		ctl = priv->ctl_rec;
-		value = ctl & ~KIRKWOOD_RECCTL_I2S_EN;
+		value = ctl & ~(KIRKWOOD_RECCTL_I2S_EN |
+				KIRKWOOD_RECCTL_SPDIF_EN);
 		writel(value, priv->io + KIRKWOOD_RECCTL);
 
 		/* enable interrupts */
@@ -310,6 +319,7 @@  static int kirkwood_i2s_rec_trigger(struct snd_pcm_substream *substream,
 		writel(value, priv->io + KIRKWOOD_INT_MASK);
 
 		/* enable record */
+		ctl &= priv->ctl_rec_mask;
 		writel(ctl, priv->io + KIRKWOOD_RECCTL);
 		break;
 
@@ -404,55 +414,75 @@  static const struct snd_soc_dai_ops kirkwood_i2s_dai_ops = {
 	.set_fmt        = kirkwood_i2s_set_fmt,
 };
 
+static const struct snd_soc_component_driver kirkwood_i2s_component = {
+	.name		= DRV_NAME,
+};
 
-static struct snd_soc_dai_driver kirkwood_i2s_dai = {
-	.probe = kirkwood_i2s_probe,
-	.playback = {
-		.channels_min = 1,
-		.channels_max = 2,
-		.rates = SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000 |
-				SNDRV_PCM_RATE_96000,
-		.formats = KIRKWOOD_I2S_FORMATS,
-	},
-	.capture = {
+static int kirkwood_register_component(struct device *dev,
+					struct kirkwood_dma_data *priv)
+{
+	struct device_node *np = dev->of_node;
+	uint32_t ctl_play_mask = ~(KIRKWOOD_PLAYCTL_I2S_EN |
+				KIRKWOOD_PLAYCTL_SPDIF_EN);
+	uint32_t ctl_rec_mask = ~(KIRKWOOD_RECCTL_I2S_EN |
+				KIRKWOOD_RECCTL_SPDIF_EN);
+	int ret;
+
+	static const struct snd_soc_pcm_stream stream = {
 		.channels_min = 1,
 		.channels_max = 2,
 		.rates = SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000 |
 				SNDRV_PCM_RATE_96000,
 		.formats = KIRKWOOD_I2S_FORMATS,
-	},
-	.ops = &kirkwood_i2s_dai_ops,
-};
+	};
 
-static struct snd_soc_dai_driver kirkwood_i2s_dai_extclk = {
-	.probe = kirkwood_i2s_probe,
-	.playback = {
-		.channels_min = 1,
-		.channels_max = 2,
-		.rates = SNDRV_PCM_RATE_8000_192000 |
-			 SNDRV_PCM_RATE_CONTINUOUS |
-			 SNDRV_PCM_RATE_KNOT,
-		.formats = KIRKWOOD_I2S_FORMATS,
-	},
-	.capture = {
-		.channels_min = 1,
-		.channels_max = 2,
-		.rates = SNDRV_PCM_RATE_8000_192000 |
-			 SNDRV_PCM_RATE_CONTINUOUS |
-			 SNDRV_PCM_RATE_KNOT,
-		.formats = KIRKWOOD_I2S_FORMATS,
-	},
-	.ops = &kirkwood_i2s_dai_ops,
-};
+	if (np) {
+		if (of_property_read_bool(np, "i2s-in"))
+			ctl_rec_mask |= KIRKWOOD_RECCTL_I2S_EN;
+		if (of_property_read_bool(np, "i2s-out"))
+			ctl_play_mask |= KIRKWOOD_PLAYCTL_I2S_EN;
+		if (of_property_read_bool(np, "spdif-in"))
+			ctl_rec_mask |= KIRKWOOD_RECCTL_SPDIF_EN;
+		if (of_property_read_bool(np, "spdif-out"))
+			ctl_play_mask |= KIRKWOOD_PLAYCTL_SPDIF_EN;
+	} else {
+		ctl_rec_mask |= KIRKWOOD_RECCTL_I2S_EN;
+		ctl_play_mask |= KIRKWOOD_PLAYCTL_I2S_EN;
+	}
+	priv->ctl_play_mask = ctl_play_mask;
+	priv->ctl_rec_mask = ctl_rec_mask;
+
+	priv->soc_dai.probe = kirkwood_i2s_probe;
+	priv->soc_dai.ops = &kirkwood_i2s_dai_ops;
+	if (ctl_play_mask & (KIRKWOOD_PLAYCTL_I2S_EN |
+				KIRKWOOD_PLAYCTL_SPDIF_EN)) {
+		memcpy(&priv->soc_dai.playback, &stream,
+			sizeof priv->soc_dai.playback);
+		if (!IS_ERR(priv->extclk))
+			priv->soc_dai.playback.rates = SNDRV_PCM_RATE_8000_192000 |
+						SNDRV_PCM_RATE_CONTINUOUS |
+						SNDRV_PCM_RATE_KNOT;
+	}
+	if (ctl_rec_mask & (KIRKWOOD_RECCTL_I2S_EN |
+				KIRKWOOD_RECCTL_SPDIF_EN)) {
+		memcpy(&priv->soc_dai.capture, &stream,
+			sizeof priv->soc_dai.capture);
+		if (!IS_ERR(priv->extclk))
+			priv->soc_dai.capture.rates = SNDRV_PCM_RATE_8000_192000 |
+						SNDRV_PCM_RATE_CONTINUOUS |
+						SNDRV_PCM_RATE_KNOT;
+	}
 
-static const struct snd_soc_component_driver kirkwood_i2s_component = {
-	.name		= DRV_NAME,
-};
+	ret = snd_soc_register_component(dev, &kirkwood_i2s_component,
+					 &priv->soc_dai, 1);
+	if (ret)
+		dev_err(dev, "snd_soc_register_component failed\n");
+	return ret;
+}
 
 static int kirkwood_i2s_dev_probe(struct platform_device *pdev)
 {
 	struct kirkwood_asoc_platform_data *data = pdev->dev.platform_data;
-	struct snd_soc_dai_driver *soc_dai = &kirkwood_i2s_dai;
 	struct kirkwood_dma_data *priv;
 	struct resource *mem;
 	struct device_node *np = pdev->dev.of_node;
@@ -503,7 +533,6 @@  static int kirkwood_i2s_dev_probe(struct platform_device *pdev)
 		} else {
 			dev_info(&pdev->dev, "found external clock\n");
 			clk_prepare_enable(priv->extclk);
-			soc_dai = &kirkwood_i2s_dai_extclk;
 		}
 	}
 
@@ -520,12 +549,9 @@  static int kirkwood_i2s_dev_probe(struct platform_device *pdev)
 		priv->ctl_rec |= KIRKWOOD_RECCTL_BURST_128;
 	}
 
-	err = snd_soc_register_component(&pdev->dev, &kirkwood_i2s_component,
-					 soc_dai, 1);
-	if (err) {
-		dev_err(&pdev->dev, "snd_soc_register_component failed\n");
+	err = kirkwood_register_component(&pdev->dev, priv);
+	if (err)
 		goto err_component;
-	}
 
 	err = snd_soc_register_platform(&pdev->dev, &kirkwood_soc_platform);
 	if (err) {
diff --git a/sound/soc/kirkwood/kirkwood.h b/sound/soc/kirkwood/kirkwood.h
index f8e1ccc..8f11e03 100644
--- a/sound/soc/kirkwood/kirkwood.h
+++ b/sound/soc/kirkwood/kirkwood.h
@@ -134,6 +134,8 @@  struct kirkwood_dma_data {
 	struct clk *extclk;
 	uint32_t ctl_play;
 	uint32_t ctl_rec;
+	uint32_t ctl_play_mask, ctl_rec_mask;
+	struct snd_soc_dai_driver soc_dai;
 	struct snd_pcm_substream *substream_play;
 	struct snd_pcm_substream *substream_rec;
 	int irq;