alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: Mark Brown <broonie@kernel.org>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
	Jean-Francois Moine <moinejf@free.fr>,
	alsa-devel@alsa-project.org, Jason Cooper <jason@lakedaemon.net>,
	Liam Girdwood <lgirdwood@gmail.com>,
	linux-arm-kernel@lists.infradead.org,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Subject: Re: [PATCH v2] ASoC: kirkwood: add S/PDIF support
Date: Sun, 20 Oct 2013 18:56:27 +0100	[thread overview]
Message-ID: <20131020175627.GP25034@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <20131020171255.GP2443@sirena.org.uk>

On Sun, Oct 20, 2013 at 06:12:55PM +0100, Mark Brown wrote:
> On Fri, Oct 18, 2013 at 08:37:50PM +0200, Jean-Francois Moine wrote:
> > This patch adds S/PDIF input/output for mvebu DT boards.
> 
> This looks basically good, thanks.  A couple of things though:
> 
> > +	if (dai->name[0] == 'i')
> > +		ctl &= ~KIRKWOOD_PLAYCTL_SPDIF_EN;	/* i2s */
> > +	else
> > +		ctl &= ~KIRKWOOD_PLAYCTL_I2S_EN;	/* spdif */
> > +
> 
> This works and isn't actively a problem but it's not really normal
> either - the normal thing would be to use dai->id.
> 
> > +static struct snd_soc_dai_driver kirkwood_i2s_dai[2] = {
> > +    {
> > +	.name = "i2s",
> > +	.playback = {
> 
> This should cause problems for the existing (non-DT) machine drivers?
> They're using cpu_dai_name to bind the CPU DAI and you're now setting an
> explicit name instead of letting the dev_name() be used.  The patch
> should be updating those machine drivers too unless I'm missing
> something.

For your information, I intend to nobble you and Liam at the kernel
summit and knock your two heads together over this.  You both have
been dysfunctional over this driver and its about time someone banged
some sense into you both.

  reply	other threads:[~2013-10-20 17:57 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-18 18:37 [PATCH v2] ASoC: kirkwood: add S/PDIF support Jean-Francois Moine
2013-10-20 17:12 ` Mark Brown
2013-10-20 17:56   ` Russell King - ARM Linux [this message]
  -- strict thread matches above, loose matches on Subject: below --
2013-10-21  8:50 Jean-Francois Moine
2013-10-24 11:06 ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131020175627.GP25034@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=jason@lakedaemon.net \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=moinejf@free.fr \
    --cc=sebastian.hesselbarth@gmail.com \
    --cc=thomas.petazzoni@free-electrons.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).