Re: [alsa-devel] [RFC PATCH 3/5] ASoC: SOF: Intel: hda: add SoundWire IP support

[Vinod Koul <vkoul@kernel.org>]

On 04-09-19, 08:25, Pierre-Louis Bossart wrote:
> On 9/4/19 2:21 AM, Vinod Koul wrote:
> > On 21-08-19, 15:17, Pierre-Louis Bossart wrote:
> > > The Core0 needs to be powered before the SoundWire IP is initialized.
> > > 
> > > Call sdw_intel_init/exit and store the context. We only have one
> > > context, but depending on the hardware capabilities and BIOS settings
> > > may enable multiple SoundWire links.
> > > 
> > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > ---
> > >   sound/soc/sof/intel/hda.c | 40 +++++++++++++++++++++++++++++++++------
> > >   sound/soc/sof/intel/hda.h |  5 +++++
> > >   2 files changed, 39 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
> > > index a968890d0754..e754058e3679 100644
> > > --- a/sound/soc/sof/intel/hda.c
> > > +++ b/sound/soc/sof/intel/hda.c
> > > @@ -57,6 +57,8 @@ static int hda_sdw_init(struct snd_sof_dev *sdev)
> > >   {
> > >   	acpi_handle handle;
> > >   	struct sdw_intel_res res;
> > > +	struct sof_intel_hda_dev *hdev;
> > > +	void *sdw;
> > >   	handle = ACPI_HANDLE(sdev->dev);
> > > @@ -66,23 +68,32 @@ static int hda_sdw_init(struct snd_sof_dev *sdev)
> > >   	res.irq = sdev->ipc_irq;
> > >   	res.parent = sdev->dev;
> > > -	hda_sdw_int_enable(sdev, true);
> > > -
> > > -	sdev->sdw = sdw_intel_init(handle, &res);
> > > -	if (!sdev->sdw) {
> > > +	sdw = sdw_intel_init(handle, &res);
> > 
> > should this be called for platforms without sdw, I was hoping that some
> > checks would be performed.. For example how would skl deal with this?
> 
> Good point. For now we rely on CONFIG_SOUNDWIRE_INTEL to use a fallback, but
> if the kernel defines this config and we run on an older platform the only
> safety would be the hardware capabilities and BIOS dependencies, I need to
> test if it works.

Yes I am not sure given the experience with BIOS relying on that is a
great idea ! But if that works, that would be better.


> > > diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
> > > index c8f93317aeb4..48e09b7daf0a 100644
> > > --- a/sound/soc/sof/intel/hda.h
> > > +++ b/sound/soc/sof/intel/hda.h
> > > @@ -399,6 +399,11 @@ struct sof_intel_hda_dev {
> > >   	/* DMIC device */
> > >   	struct platform_device *dmic_dev;
> > > +
> > > +#if IS_ENABLED(CONFIG_SOUNDWIRE_INTEL)
> > 
> > is this really required, context is a void pointer

??

> > > +	/* sdw context */
> > > +	void *sdw;
> > 
> > > +#endif

-- 
~Vinod