All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: alsa-devel@alsa-project.org, rdunlap@infradead.org,
	YueHaibing <yuehaibing@huawei.com>,
	linux-kernel@vger.kernel.org, lgirdwood@gmail.com,
	broonie@kernel.org
Subject: Re: [alsa-devel] [PATCH] ASoC: SOF: Fix build error with CONFIG_SND_SOC_SOF_NOCODEC=m
Date: Fri, 10 May 2019 20:05:54 +0200	[thread overview]
Message-ID: <s5hy33ekwvx.wl-tiwai@suse.de> (raw)
In-Reply-To: <75c3efb7-8057-0d4c-a53d-45f4527d90a1@linux.intel.com>

On Fri, 10 May 2019 19:56:51 +0200,
Pierre-Louis Bossart wrote:
> 
> 
> >>>> Yes, that would work.  OTOH, I see no merit to build an extra module
> >>>> for nocodec.  nocodec.c can be built together with sof-core stuff.
> >>
> >> the module has its benefits. Today nocodec includes all possible DAIs,
> >> I wanted to add module parameters to restrict things a bit for
> >> tests/debug. It'll be e.g. very helpful for SoundWire to avoid
> >> exposing the SSP DAIs.
> >>
> >> Also nocodec is incompatible with hdaudio/hdmi support at the moment,
> >> we had all sorts of issues with suspend/resume.
> >
> > Well, in the case of SOF, the core code calls directly
> > soc_nocodec_setup(), hence it's rather a direct link.  So it makes
> > little sense to make the nocodec code split from sof-core, unless the
> > nocodec code is used / linked by components other than SOF.  I doubt
> > the possibility because the current DAI is clearly only for SOF...
> >
> > The module option can be still be there; it'll be applied just to
> > sof-core instead of sof-nocodec.
> 
> I see your point and this SOF core/nocodec dependency is a conceptual
> miss on our side. Thanks for bringing our attention on this.
> 
> The core is really supposed to be about the DSP side of things. It
> shouldn't be burdened with machine driver stuff, but it unfortunately
> is at two levels.
> 
> Initially the nocodec code was handled at the soc-acpi-dev or
> soc-pci-dev level, and it's still there that the FORCE_CODEC mode is
> handled, along with the calls to check the codec ACPI IDs. Now when we
> enabled the HDaudio case, we somehow ended-up moving parts of the
> nocodec support in the SOF core to simplify our life but created a
> dependency that wasn't intentional at all. we collectively missed it
> while we were struggling with nocodec/hdaudio compatibility.
> 
> The second issue is that we create a platform_device for the machine
> driver in the SOF core. This is a shortcut that we took and that works
> for Intel, but for DeviceTree-based platforms this will have to
> change.
> 
> So long story short, I'd rather have a simple Kconfig fix to avoid
> compilation issues for now and revisit all the machine driver support,
> e.g. when the i.MX patches show up, than strengthen a dependency that
> we introduced by accident rather than by design.

OK, thanks, that's convincing enough.  My proposal was just some minor
optimization, and as long as there is a planned change in future, it's
fine to leave in the current or similar form.


thanks,

Takashi

  reply	other threads:[~2019-05-10 18:05 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-10  2:36 [PATCH] ASoC: SOF: Fix build error with CONFIG_SND_SOC_SOF_NOCODEC=m YueHaibing
2019-05-10  2:36 ` YueHaibing
2019-05-10  7:12 ` Takashi Iwai
2019-05-10  7:12   ` Takashi Iwai
2019-05-10 12:56   ` Pierre-Louis Bossart
2019-05-10 13:04     ` Takashi Iwai
2019-05-10 13:34       ` [alsa-devel] " Pierre-Louis Bossart
2019-05-10 13:41         ` Takashi Iwai
2019-05-10 13:56           ` Takashi Iwai
2019-05-10 15:29             ` Pierre-Louis Bossart
2019-05-10 16:57               ` Takashi Iwai
2019-05-10 17:56                 ` Pierre-Louis Bossart
2019-05-10 18:05                   ` Takashi Iwai [this message]
2019-05-10 13:09   ` YueHaibing
2019-05-10 13:09     ` YueHaibing
2019-05-10 13:29 ` [PATCH V2] " YueHaibing
2019-05-10 13:29   ` YueHaibing
2019-05-10 13:36   ` Pierre-Louis Bossart
2019-05-10 13:50     ` YueHaibing
2019-05-10 13:50       ` YueHaibing
2019-05-10 16:24       ` Pierre-Louis Bossart

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=s5hy33ekwvx.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=rdunlap@infradead.org \
    --cc=yuehaibing@huawei.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.