[alsa-devel] [PATCH] ARM: kirkwood: enable S/PDIF
This patch enables S/PDIF.
Signed-off-by: Jean-Francois Moine moinejf@free.fr --- sound/soc/kirkwood/kirkwood-i2s.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/sound/soc/kirkwood/kirkwood-i2s.c b/sound/soc/kirkwood/kirkwood-i2s.c index 4c9dad3..ad94c50 100644 --- a/sound/soc/kirkwood/kirkwood-i2s.c +++ b/sound/soc/kirkwood/kirkwood-i2s.c @@ -159,6 +159,7 @@ 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_SPDIF_EN | KIRKWOOD_PLAYCTL_I2S_EN; ctl_rec = KIRKWOOD_RECCTL_SIZE_16_C | KIRKWOOD_RECCTL_I2S_EN; @@ -169,6 +170,7 @@ static int kirkwood_i2s_hw_params(struct snd_pcm_substream *substream, case SNDRV_PCM_FORMAT_S20_3LE: i2s_value |= KIRKWOOD_I2S_CTL_SIZE_20; ctl_play = KIRKWOOD_PLAYCTL_SIZE_20 | + KIRKWOOD_PLAYCTL_SPDIF_EN | KIRKWOOD_PLAYCTL_I2S_EN; ctl_rec = KIRKWOOD_RECCTL_SIZE_20 | KIRKWOOD_RECCTL_I2S_EN; @@ -177,6 +179,7 @@ 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_SPDIF_EN | KIRKWOOD_PLAYCTL_I2S_EN; ctl_rec = KIRKWOOD_RECCTL_SIZE_24 | KIRKWOOD_RECCTL_I2S_EN;
On Tue, Jul 23, 2013 at 10:23:20AM +0200, Jean-Francois Moine wrote:
This patch enables S/PDIF.
Signed-off-by: Jean-Francois Moine moinejf@free.fr
I'm not submitting my patch to do this because:
(a) we don't know what effect this has on other hardware. (b) Mark suggested that we use the slave PCM stuff to deal with this. As yet, that's something which I haven't been able to get to grips with because ASoC is soo damned complicated and undocumented.
On Tue, Jul 23, 2013 at 09:43:47AM +0100, Russell King - ARM Linux wrote:
On Tue, Jul 23, 2013 at 10:23:20AM +0200, Jean-Francois Moine wrote:
This patch enables S/PDIF.
Signed-off-by: Jean-Francois Moine moinejf@free.fr
I'm not submitting my patch to do this because:
(a) we don't know what effect this has on other hardware.
This patch will do absolutely nothing unless it's used in a machine driver which connects a S/PDIF CODEC to it. I see no reason not to apply it, someone with hardware with more complex needs can always build on it later.
On 07/23/13 15:06, Mark Brown wrote:
On Tue, Jul 23, 2013 at 09:43:47AM +0100, Russell King - ARM Linux wrote:
On Tue, Jul 23, 2013 at 10:23:20AM +0200, Jean-Francois Moine wrote:
This patch enables S/PDIF.
Signed-off-by: Jean-Francois Moine moinejf@free.fr
I'm not submitting my patch to do this because:
(a) we don't know what effect this has on other hardware.
This patch will do absolutely nothing unless it's used in a machine driver which connects a S/PDIF CODEC to it. I see no reason not to apply it, someone with hardware with more complex needs can always build on it later.
Mark,
the mask that is changed in the patch is what will be written into i2s controller's registers. So, if there is no S/PDIF in that specific controller that bit can possibly have a different meaning. Also, enabling both I2S playback and SPDIF playback can cause the controller to behave differently.
I share Russell's concern about it and would rather like to use multiple codecs per DAI (DPCM) for that. I see Daniel Mack picked that up again, maybe he submits something soon.
Sebastian
On Tue, Jul 23, 2013 at 03:12:56PM +0200, Sebastian Hesselbarth wrote:
the mask that is changed in the patch is what will be written into i2s controller's registers. So, if there is no S/PDIF in that specific controller that bit can possibly have a different meaning. Also, enabling both I2S playback and SPDIF playback can cause the controller to behave differently.
Oh, so it will - I glanced through it and misread, sorry. If it just makes the enabling of S/PDIF mode conditional on DAI format that'd cover it.
I share Russell's concern about it and would rather like to use multiple codecs per DAI (DPCM) for that. I see Daniel Mack picked that up again, maybe he submits something soon.
Well, that'd be ideal and is going to be needed for any hardware which has both wired up in parallel but a simpler either/or thing doesn't seem like a problem.
On Tue, Jul 23, 2013 at 03:12:56PM +0200, Sebastian Hesselbarth wrote:
On 07/23/13 15:06, Mark Brown wrote:
On Tue, Jul 23, 2013 at 09:43:47AM +0100, Russell King - ARM Linux wrote:
On Tue, Jul 23, 2013 at 10:23:20AM +0200, Jean-Francois Moine wrote:
This patch enables S/PDIF.
Signed-off-by: Jean-Francois Moine moinejf@free.fr
I'm not submitting my patch to do this because:
(a) we don't know what effect this has on other hardware.
This patch will do absolutely nothing unless it's used in a machine driver which connects a S/PDIF CODEC to it. I see no reason not to apply it, someone with hardware with more complex needs can always build on it later.
Mark,
the mask that is changed in the patch is what will be written into i2s controller's registers. So, if there is no S/PDIF in that specific controller that bit can possibly have a different meaning. Also, enabling both I2S playback and SPDIF playback can cause the controller to behave differently.
Right, so the SPDIF output on Dove isn't multiplexed, so I was wrong on that _for_ _dove_, but that may not necessarily be the case for the Kirkwood SoCs - the pin may be multiplexed there.
However, Dove does have two I2S/SPDIF controllers which are otherwise identical, apart from one having SPDIF support and the other not. Setting the SPDIF enable bit on the one without is unspecified what the behaviour would be, so it should not be set.
On Tue, Jul 23, 2013 at 02:06:22PM +0100, Mark Brown wrote:
On Tue, Jul 23, 2013 at 09:43:47AM +0100, Russell King - ARM Linux wrote:
On Tue, Jul 23, 2013 at 10:23:20AM +0200, Jean-Francois Moine wrote:
This patch enables S/PDIF.
Signed-off-by: Jean-Francois Moine moinejf@free.fr
I'm not submitting my patch to do this because:
(a) we don't know what effect this has on other hardware.
This patch will do absolutely nothing unless it's used in a machine driver which connects a S/PDIF CODEC to it. I see no reason not to apply it, someone with hardware with more complex needs can always build on it later.
So... what if setting this bit causes the SoC to start wiggling a pin with the SPDIF signal which has been used for a different purpose?
On Tue, Jul 23, 2013 at 02:21:04PM +0100, Russell King - ARM Linux wrote:
On Tue, Jul 23, 2013 at 02:06:22PM +0100, Mark Brown wrote:
This patch will do absolutely nothing unless it's used in a machine driver which connects a S/PDIF CODEC to it. I see no reason not to apply it, someone with hardware with more complex needs can always build on it later.
So... what if setting this bit causes the SoC to start wiggling a pin with the SPDIF signal which has been used for a different purpose?
Right, yup - didn't read it fully. Like I say doing it conditional on the DAI format should be fine though.
participants (4)
-
Jean-Francois Moine -
Mark Brown -
Russell King - ARM Linux -
Sebastian Hesselbarth