diff mbox series

[7/7] soundwire: intel: remove PDM support

Message ID 20211213054634.30088-8-yung-chuan.liao@linux.intel.com
State Superseded
Headers show
Series ASoC/SoundWire: improve suspend flows and use set_stream() instead of set_tdm_slots() for HDAudio | expand

Commit Message

Liao, Bard Dec. 13, 2021, 5:46 a.m. UTC
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

While the hardware supports PDM streams, this capability has never
been tested or enabled on any product, so this is dead-code. Let's
remove all this.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/cadence_master.c |  36 +--------
 drivers/soundwire/cadence_master.h |  12 +--
 drivers/soundwire/intel.c          | 123 +++++++----------------------
 3 files changed, 31 insertions(+), 140 deletions(-)

Comments

Liao, Bard Dec. 23, 2021, 7:46 a.m. UTC | #1
> -----Original Message-----
> From: Vinod Koul <vkoul@kernel.org>
> Sent: Thursday, December 23, 2021 2:59 PM
> To: Bard Liao <yung-chuan.liao@linux.intel.com>
> Cc: alsa-devel@alsa-project.org; linux-kernel@vger.kernel.org;
> tiwai@suse.de; broonie@kernel.org; gregkh@linuxfoundation.org;
> srinivas.kandagatla@linaro.org; pierre-louis.bossart@linux.intel.com; Kale,
> Sanyog R <sanyog.r.kale@intel.com>; Liao, Bard <bard.liao@intel.com>
> Subject: Re: [PATCH 7/7] soundwire: intel: remove PDM support
> 
> On 13-12-21, 13:46, Bard Liao wrote:
> > From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> >
> > While the hardware supports PDM streams, this capability has never
> > been tested or enabled on any product, so this is dead-code. Let's
> > remove all this.
> 
> So no plans to test and enable this? Do the DMICs not use PDM?

The point is that this code is unused/untested. We can re-add it after
it was tested.

> 
> Again this should not be in this series...

Agree, but since this patche depends on the previous patches, I sent them
together to avoid conflict.

> 
> >
> > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
> > Reviewed-by: Rander Wang <rander.wang@intel.com>
> > Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> > ---
> >  drivers/soundwire/cadence_master.c |  36 +--------
> >  drivers/soundwire/cadence_master.h |  12 +--
> >  drivers/soundwire/intel.c          | 123 +++++++----------------------
> >  3 files changed, 31 insertions(+), 140 deletions(-)
> >
> > diff --git a/drivers/soundwire/cadence_master.c
> b/drivers/soundwire/cadence_master.c
> > index 4fcc3ba93004..558390af44b6 100644
> > --- a/drivers/soundwire/cadence_master.c
> > +++ b/drivers/soundwire/cadence_master.c
> > @@ -1178,9 +1178,6 @@ int sdw_cdns_pdi_init(struct sdw_cdns *cdns,
> >  	cdns->pcm.num_bd = config.pcm_bd;
> >  	cdns->pcm.num_in = config.pcm_in;
> >  	cdns->pcm.num_out = config.pcm_out;
> > -	cdns->pdm.num_bd = config.pdm_bd;
> > -	cdns->pdm.num_in = config.pdm_in;
> > -	cdns->pdm.num_out = config.pdm_out;
> >
> >  	/* Allocate PDIs for PCMs */
> >  	stream = &cdns->pcm;
> > @@ -1211,32 +1208,6 @@ int sdw_cdns_pdi_init(struct sdw_cdns *cdns,
> >  	stream->num_pdi = stream->num_bd + stream->num_in + stream-
> >num_out;
> >  	cdns->num_ports = stream->num_pdi;
> >
> > -	/* Allocate PDIs for PDMs */
> > -	stream = &cdns->pdm;
> > -	ret = cdns_allocate_pdi(cdns, &stream->bd,
> > -				stream->num_bd, offset);
> > -	if (ret)
> > -		return ret;
> > -
> > -	offset += stream->num_bd;
> > -
> > -	ret = cdns_allocate_pdi(cdns, &stream->in,
> > -				stream->num_in, offset);
> > -	if (ret)
> > -		return ret;
> > -
> > -	offset += stream->num_in;
> > -
> > -	ret = cdns_allocate_pdi(cdns, &stream->out,
> > -				stream->num_out, offset);
> > -
> > -	if (ret)
> > -		return ret;
> > -
> > -	/* Update total number of PDM PDIs */
> > -	stream->num_pdi = stream->num_bd + stream->num_in + stream-
> >num_out;
> > -	cdns->num_ports += stream->num_pdi;
> > -
> >  	return 0;
> >  }
> >  EXPORT_SYMBOL(sdw_cdns_pdi_init);
> > @@ -1681,7 +1652,7 @@ int sdw_cdns_probe(struct sdw_cdns *cdns)
> >  EXPORT_SYMBOL(sdw_cdns_probe);
> >
> >  int cdns_set_sdw_stream(struct snd_soc_dai *dai,
> > -			void *stream, bool pcm, int direction)
> > +			void *stream, int direction)
> >  {
> >  	struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai);
> >  	struct sdw_cdns_dma_data *dma;
> > @@ -1705,10 +1676,7 @@ int cdns_set_sdw_stream(struct snd_soc_dai
> *dai,
> >  		if (!dma)
> >  			return -ENOMEM;
> >
> > -		if (pcm)
> > -			dma->stream_type = SDW_STREAM_PCM;
> > -		else
> > -			dma->stream_type = SDW_STREAM_PDM;
> > +		dma->stream_type = SDW_STREAM_PCM;
> >
> >  		dma->bus = &cdns->bus;
> >  		dma->link_id = cdns->instance;
> > diff --git a/drivers/soundwire/cadence_master.h
> b/drivers/soundwire/cadence_master.h
> > index aa4b9b0eb2a8..595d72c15d97 100644
> > --- a/drivers/soundwire/cadence_master.h
> > +++ b/drivers/soundwire/cadence_master.h
> > @@ -17,7 +17,7 @@
> >   * @h_ch_num: high channel for PDI
> >   * @ch_count: total channel count for PDI
> >   * @dir: data direction
> > - * @type: stream type, PDM or PCM
> > + * @type: stream type, (only PCM supported)
> >   */
> >  struct sdw_cdns_pdi {
> >  	int num;
> > @@ -62,17 +62,11 @@ struct sdw_cdns_streams {
> >   * @pcm_bd: number of bidirectional PCM streams supported
> >   * @pcm_in: number of input PCM streams supported
> >   * @pcm_out: number of output PCM streams supported
> > - * @pdm_bd: number of bidirectional PDM streams supported
> > - * @pdm_in: number of input PDM streams supported
> > - * @pdm_out: number of output PDM streams supported
> >   */
> >  struct sdw_cdns_stream_config {
> >  	unsigned int pcm_bd;
> >  	unsigned int pcm_in;
> >  	unsigned int pcm_out;
> > -	unsigned int pdm_bd;
> > -	unsigned int pdm_in;
> > -	unsigned int pdm_out;
> >  };
> >
> >  /**
> > @@ -111,7 +105,6 @@ struct sdw_cdns_dma_data {
> >   * @ports: Data ports
> >   * @num_ports: Total number of data ports
> >   * @pcm: PCM streams
> > - * @pdm: PDM streams
> >   * @registers: Cadence registers
> >   * @link_up: Link status
> >   * @msg_count: Messages sent on bus
> > @@ -129,7 +122,6 @@ struct sdw_cdns {
> >  	int num_ports;
> >
> >  	struct sdw_cdns_streams pcm;
> > -	struct sdw_cdns_streams pdm;
> >
> >  	int pdi_loopback_source;
> >  	int pdi_loopback_target;
> > @@ -188,7 +180,7 @@ cdns_xfer_msg_defer(struct sdw_bus *bus,
> >  int cdns_bus_conf(struct sdw_bus *bus, struct sdw_bus_params
> *params);
> >
> >  int cdns_set_sdw_stream(struct snd_soc_dai *dai,
> > -			void *stream, bool pcm, int direction);
> > +			void *stream, int direction);
> >
> >  void sdw_cdns_check_self_clearing_bits(struct sdw_cdns *cdns, const
> char *string,
> >  				       bool initial_delay, int reset_iterations);
> > diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
> > index 45ea55a7d0c8..79ba0e3f6dac 100644
> > --- a/drivers/soundwire/intel.c
> > +++ b/drivers/soundwire/intel.c
> > @@ -564,7 +564,7 @@ static void intel_pdi_init(struct sdw_intel *sdw,
> >  {
> >  	void __iomem *shim = sdw->link_res->shim;
> >  	unsigned int link_id = sdw->instance;
> > -	int pcm_cap, pdm_cap;
> > +	int pcm_cap;
> >
> >  	/* PCM Stream Capability */
> >  	pcm_cap = intel_readw(shim, SDW_SHIM_PCMSCAP(link_id));
> > @@ -575,41 +575,25 @@ static void intel_pdi_init(struct sdw_intel *sdw,
> >
> >  	dev_dbg(sdw->cdns.dev, "PCM cap bd:%d in:%d out:%d\n",
> >  		config->pcm_bd, config->pcm_in, config->pcm_out);
> > -
> > -	/* PDM Stream Capability */
> > -	pdm_cap = intel_readw(shim, SDW_SHIM_PDMSCAP(link_id));
> > -
> > -	config->pdm_bd = FIELD_GET(SDW_SHIM_PDMSCAP_BSS,
> pdm_cap);
> > -	config->pdm_in = FIELD_GET(SDW_SHIM_PDMSCAP_ISS, pdm_cap);
> > -	config->pdm_out = FIELD_GET(SDW_SHIM_PDMSCAP_OSS,
> pdm_cap);
> > -
> > -	dev_dbg(sdw->cdns.dev, "PDM cap bd:%d in:%d out:%d\n",
> > -		config->pdm_bd, config->pdm_in, config->pdm_out);
> >  }
> >
> >  static int
> > -intel_pdi_get_ch_cap(struct sdw_intel *sdw, unsigned int pdi_num, bool
> pcm)
> > +intel_pdi_get_ch_cap(struct sdw_intel *sdw, unsigned int pdi_num)
> >  {
> >  	void __iomem *shim = sdw->link_res->shim;
> >  	unsigned int link_id = sdw->instance;
> >  	int count;
> >
> > -	if (pcm) {
> > -		count = intel_readw(shim, SDW_SHIM_PCMSYCHC(link_id,
> pdi_num));
> > +	count = intel_readw(shim, SDW_SHIM_PCMSYCHC(link_id,
> pdi_num));
> >
> > -		/*
> > -		 * WORKAROUND: on all existing Intel controllers, pdi
> > -		 * number 2 reports channel count as 1 even though it
> > -		 * supports 8 channels. Performing hardcoding for pdi
> > -		 * number 2.
> > -		 */
> > -		if (pdi_num == 2)
> > -			count = 7;
> > -
> > -	} else {
> > -		count = intel_readw(shim, SDW_SHIM_PDMSCAP(link_id));
> > -		count = FIELD_GET(SDW_SHIM_PDMSCAP_CPSS, count);
> > -	}
> > +	/*
> > +	 * WORKAROUND: on all existing Intel controllers, pdi
> > +	 * number 2 reports channel count as 1 even though it
> > +	 * supports 8 channels. Performing hardcoding for pdi
> > +	 * number 2.
> > +	 */
> > +	if (pdi_num == 2)
> > +		count = 7;
> >
> >  	/* zero based values for channel count in register */
> >  	count++;
> > @@ -620,12 +604,12 @@ intel_pdi_get_ch_cap(struct sdw_intel *sdw,
> unsigned int pdi_num, bool pcm)
> >  static int intel_pdi_get_ch_update(struct sdw_intel *sdw,
> >  				   struct sdw_cdns_pdi *pdi,
> >  				   unsigned int num_pdi,
> > -				   unsigned int *num_ch, bool pcm)
> > +				   unsigned int *num_ch)
> >  {
> >  	int i, ch_count = 0;
> >
> >  	for (i = 0; i < num_pdi; i++) {
> > -		pdi->ch_count = intel_pdi_get_ch_cap(sdw, pdi->num, pcm);
> > +		pdi->ch_count = intel_pdi_get_ch_cap(sdw, pdi->num);
> >  		ch_count += pdi->ch_count;
> >  		pdi++;
> >  	}
> > @@ -635,25 +619,23 @@ static int intel_pdi_get_ch_update(struct
> sdw_intel *sdw,
> >  }
> >
> >  static int intel_pdi_stream_ch_update(struct sdw_intel *sdw,
> > -				      struct sdw_cdns_streams *stream, bool
> pcm)
> > +				      struct sdw_cdns_streams *stream)
> >  {
> >  	intel_pdi_get_ch_update(sdw, stream->bd, stream->num_bd,
> > -				&stream->num_ch_bd, pcm);
> > +				&stream->num_ch_bd);
> >
> >  	intel_pdi_get_ch_update(sdw, stream->in, stream->num_in,
> > -				&stream->num_ch_in, pcm);
> > +				&stream->num_ch_in);
> >
> >  	intel_pdi_get_ch_update(sdw, stream->out, stream->num_out,
> > -				&stream->num_ch_out, pcm);
> > +				&stream->num_ch_out);
> >
> >  	return 0;
> >  }
> >
> >  static int intel_pdi_ch_update(struct sdw_intel *sdw)
> >  {
> > -	/* First update PCM streams followed by PDM streams */
> > -	intel_pdi_stream_ch_update(sdw, &sdw->cdns.pcm, true);
> > -	intel_pdi_stream_ch_update(sdw, &sdw->cdns.pdm, false);
> > +	intel_pdi_stream_ch_update(sdw, &sdw->cdns.pcm);
> >
> >  	return 0;
> >  }
> > @@ -840,7 +822,6 @@ static int intel_hw_params(struct
> snd_pcm_substream *substream,
> >  	struct sdw_port_config *pconfig;
> >  	int ch, dir;
> >  	int ret;
> > -	bool pcm = true;
> >
> >  	dma = snd_soc_dai_get_dma_data(dai, substream);
> >  	if (!dma)
> > @@ -852,13 +833,7 @@ static int intel_hw_params(struct
> snd_pcm_substream *substream,
> >  	else
> >  		dir = SDW_DATA_DIR_TX;
> >
> > -	if (dma->stream_type == SDW_STREAM_PDM)
> > -		pcm = false;
> > -
> > -	if (pcm)
> > -		pdi = sdw_cdns_alloc_pdi(cdns, &cdns->pcm, ch, dir, dai->id);
> > -	else
> > -		pdi = sdw_cdns_alloc_pdi(cdns, &cdns->pdm, ch, dir, dai->id);
> > +	pdi = sdw_cdns_alloc_pdi(cdns, &cdns->pcm, ch, dir, dai->id);
> >
> >  	if (!pdi) {
> >  		ret = -EINVAL;
> > @@ -888,12 +863,7 @@ static int intel_hw_params(struct
> snd_pcm_substream *substream,
> >  	sconfig.frame_rate = params_rate(params);
> >  	sconfig.type = dma->stream_type;
> >
> > -	if (dma->stream_type == SDW_STREAM_PDM) {
> > -		sconfig.frame_rate *= 50;
> > -		sconfig.bps = 1;
> > -	} else {
> > -		sconfig.bps =
> snd_pcm_format_width(params_format(params));
> > -	}
> > +	sconfig.bps = snd_pcm_format_width(params_format(params));
> >
> >  	/* Port configuration */
> >  	pconfig = kzalloc(sizeof(*pconfig), GFP_KERNEL);
> > @@ -1012,13 +982,7 @@ static void intel_shutdown(struct
> snd_pcm_substream *substream,
> >  static int intel_pcm_set_sdw_stream(struct snd_soc_dai *dai,
> >  				    void *stream, int direction)
> >  {
> > -	return cdns_set_sdw_stream(dai, stream, true, direction);
> > -}
> > -
> > -static int intel_pdm_set_sdw_stream(struct snd_soc_dai *dai,
> > -				    void *stream, int direction)
> > -{
> > -	return cdns_set_sdw_stream(dai, stream, false, direction);
> > +	return cdns_set_sdw_stream(dai, stream, direction);
> >  }
> >
> >  static void *intel_get_sdw_stream(struct snd_soc_dai *dai,
> > @@ -1133,16 +1097,6 @@ static const struct snd_soc_dai_ops
> intel_pcm_dai_ops = {
> >  	.get_stream = intel_get_sdw_stream,
> >  };
> >
> > -static const struct snd_soc_dai_ops intel_pdm_dai_ops = {
> > -	.startup = intel_startup,
> > -	.hw_params = intel_hw_params,
> > -	.prepare = intel_prepare,
> > -	.hw_free = intel_hw_free,
> > -	.shutdown = intel_shutdown,
> > -	.set_sdw_stream = intel_pdm_set_sdw_stream,
> > -	.get_sdw_stream = intel_get_sdw_stream,
> > -};
> > -
> >  static const struct snd_soc_component_driver dai_component = {
> >  	.name           = "soundwire",
> >  	.suspend	= intel_component_dais_suspend
> > @@ -1151,7 +1105,7 @@ static const struct snd_soc_component_driver
> dai_component = {
> >  static int intel_create_dai(struct sdw_cdns *cdns,
> >  			    struct snd_soc_dai_driver *dais,
> >  			    enum intel_pdi_type type,
> > -			    u32 num, u32 off, u32 max_ch, bool pcm)
> > +			    u32 num, u32 off, u32 max_ch)
> >  {
> >  	int i;
> >
> > @@ -1180,10 +1134,7 @@ static int intel_create_dai(struct sdw_cdns *cdns,
> >  			dais[i].capture.formats =
> SNDRV_PCM_FMTBIT_S16_LE;
> >  		}
> >
> > -		if (pcm)
> > -			dais[i].ops = &intel_pcm_dai_ops;
> > -		else
> > -			dais[i].ops = &intel_pdm_dai_ops;
> > +		dais[i].ops = &intel_pcm_dai_ops;
> >  	}
> >
> >  	return 0;
> > @@ -1197,7 +1148,7 @@ static int intel_register_dai(struct sdw_intel *sdw)
> >  	int num_dai, ret, off = 0;
> >
> >  	/* DAIs are created based on total number of PDIs supported */
> > -	num_dai = cdns->pcm.num_pdi + cdns->pdm.num_pdi;
> > +	num_dai = cdns->pcm.num_pdi;
> >
> >  	dais = devm_kcalloc(cdns->dev, num_dai, sizeof(*dais),
> GFP_KERNEL);
> >  	if (!dais)
> > @@ -1207,39 +1158,19 @@ static int intel_register_dai(struct sdw_intel
> *sdw)
> >  	stream = &cdns->pcm;
> >
> >  	ret = intel_create_dai(cdns, dais, INTEL_PDI_IN, cdns->pcm.num_in,
> > -			       off, stream->num_ch_in, true);
> > +			       off, stream->num_ch_in);
> >  	if (ret)
> >  		return ret;
> >
> >  	off += cdns->pcm.num_in;
> >  	ret = intel_create_dai(cdns, dais, INTEL_PDI_OUT, cdns-
> >pcm.num_out,
> > -			       off, stream->num_ch_out, true);
> > +			       off, stream->num_ch_out);
> >  	if (ret)
> >  		return ret;
> >
> >  	off += cdns->pcm.num_out;
> >  	ret = intel_create_dai(cdns, dais, INTEL_PDI_BD, cdns->pcm.num_bd,
> > -			       off, stream->num_ch_bd, true);
> > -	if (ret)
> > -		return ret;
> > -
> > -	/* Create PDM DAIs */
> > -	stream = &cdns->pdm;
> > -	off += cdns->pcm.num_bd;
> > -	ret = intel_create_dai(cdns, dais, INTEL_PDI_IN, cdns->pdm.num_in,
> > -			       off, stream->num_ch_in, false);
> > -	if (ret)
> > -		return ret;
> > -
> > -	off += cdns->pdm.num_in;
> > -	ret = intel_create_dai(cdns, dais, INTEL_PDI_OUT, cdns-
> >pdm.num_out,
> > -			       off, stream->num_ch_out, false);
> > -	if (ret)
> > -		return ret;
> > -
> > -	off += cdns->pdm.num_out;
> > -	ret = intel_create_dai(cdns, dais, INTEL_PDI_BD, cdns-
> >pdm.num_bd,
> > -			       off, stream->num_ch_bd, false);
> > +			       off, stream->num_ch_bd);
> >  	if (ret)
> >  		return ret;
> >
> > --
> > 2.17.1
> 
> --
> ~Vinod
Liao, Bard Dec. 23, 2021, 12:29 p.m. UTC | #2
> -----Original Message-----
> From: Vinod Koul <vkoul@kernel.org>
> Sent: Thursday, December 23, 2021 7:05 PM
> To: Liao, Bard <bard.liao@intel.com>
> Cc: Bard Liao <yung-chuan.liao@linux.intel.com>; alsa-devel@alsa-
> project.org; linux-kernel@vger.kernel.org; tiwai@suse.de;
> broonie@kernel.org; gregkh@linuxfoundation.org;
> srinivas.kandagatla@linaro.org; pierre-louis.bossart@linux.intel.com; Kale,
> Sanyog R <sanyog.r.kale@intel.com>
> Subject: Re: [PATCH 7/7] soundwire: intel: remove PDM support
> 
> On 23-12-21, 07:46, Liao, Bard wrote:
> > > -----Original Message-----
> > > From: Vinod Koul <vkoul@kernel.org>
> > > Sent: Thursday, December 23, 2021 2:59 PM
> > > To: Bard Liao <yung-chuan.liao@linux.intel.com>
> > > Cc: alsa-devel@alsa-project.org; linux-kernel@vger.kernel.org;
> > > tiwai@suse.de; broonie@kernel.org; gregkh@linuxfoundation.org;
> > > srinivas.kandagatla@linaro.org; pierre-louis.bossart@linux.intel.com; Kale,
> > > Sanyog R <sanyog.r.kale@intel.com>; Liao, Bard <bard.liao@intel.com>
> > > Subject: Re: [PATCH 7/7] soundwire: intel: remove PDM support
> > >
> > > On 13-12-21, 13:46, Bard Liao wrote:
> > > > From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > >
> > > > While the hardware supports PDM streams, this capability has never
> > > > been tested or enabled on any product, so this is dead-code. Let's
> > > > remove all this.
> > >
> > > So no plans to test and enable this? Do the DMICs not use PDM?
> >
> > The point is that this code is unused/untested. We can re-add it after
> > it was tested.
> 
> That does not answer my question. Do the DMICs not use PDM?

I will leave the question to Pierre. 😊
But he is already on vacation now.

> 
> >
> > >
> > > Again this should not be in this series...
> >
> > Agree, but since this patche depends on the previous patches, I sent them
> > together to avoid conflict.
> 
> There are ways to handle that...

Sure, I will never do it again.

> 
> >
> > >
> > > >
> > > > Signed-off-by: Pierre-Louis Bossart <pierre-
> louis.bossart@linux.intel.com>
> > > > Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
> > > > Reviewed-by: Rander Wang <rander.wang@intel.com>
> > > > Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> > > > ---
> > > >  drivers/soundwire/cadence_master.c |  36 +--------
> > > >  drivers/soundwire/cadence_master.h |  12 +--
> > > >  drivers/soundwire/intel.c          | 123 +++++++----------------------
> > > >  3 files changed, 31 insertions(+), 140 deletions(-)
> > > >
> > > > diff --git a/drivers/soundwire/cadence_master.c
> > > b/drivers/soundwire/cadence_master.c
> > > > index 4fcc3ba93004..558390af44b6 100644
> > > > --- a/drivers/soundwire/cadence_master.c
> > > > +++ b/drivers/soundwire/cadence_master.c
> > > > @@ -1178,9 +1178,6 @@ int sdw_cdns_pdi_init(struct sdw_cdns *cdns,
> > > >  	cdns->pcm.num_bd = config.pcm_bd;
> > > >  	cdns->pcm.num_in = config.pcm_in;
> > > >  	cdns->pcm.num_out = config.pcm_out;
> > > > -	cdns->pdm.num_bd = config.pdm_bd;
> > > > -	cdns->pdm.num_in = config.pdm_in;
> > > > -	cdns->pdm.num_out = config.pdm_out;
> > > >
> > > >  	/* Allocate PDIs for PCMs */
> > > >  	stream = &cdns->pcm;
> > > > @@ -1211,32 +1208,6 @@ int sdw_cdns_pdi_init(struct sdw_cdns
> *cdns,
> > > >  	stream->num_pdi = stream->num_bd + stream->num_in + stream-
> > > >num_out;
> > > >  	cdns->num_ports = stream->num_pdi;
> > > >
> > > > -	/* Allocate PDIs for PDMs */
> > > > -	stream = &cdns->pdm;
> > > > -	ret = cdns_allocate_pdi(cdns, &stream->bd,
> > > > -				stream->num_bd, offset);
> > > > -	if (ret)
> > > > -		return ret;
> > > > -
> > > > -	offset += stream->num_bd;
> > > > -
> > > > -	ret = cdns_allocate_pdi(cdns, &stream->in,
> > > > -				stream->num_in, offset);
> > > > -	if (ret)
> > > > -		return ret;
> > > > -
> > > > -	offset += stream->num_in;
> > > > -
> > > > -	ret = cdns_allocate_pdi(cdns, &stream->out,
> > > > -				stream->num_out, offset);
> > > > -
> > > > -	if (ret)
> > > > -		return ret;
> > > > -
> > > > -	/* Update total number of PDM PDIs */
> > > > -	stream->num_pdi = stream->num_bd + stream->num_in + stream-
> > > >num_out;
> > > > -	cdns->num_ports += stream->num_pdi;
> > > > -
> > > >  	return 0;
> > > >  }
> > > >  EXPORT_SYMBOL(sdw_cdns_pdi_init);
> > > > @@ -1681,7 +1652,7 @@ int sdw_cdns_probe(struct sdw_cdns *cdns)
> > > >  EXPORT_SYMBOL(sdw_cdns_probe);
> > > >
> > > >  int cdns_set_sdw_stream(struct snd_soc_dai *dai,
> > > > -			void *stream, bool pcm, int direction)
> > > > +			void *stream, int direction)
> > > >  {
> > > >  	struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai);
> > > >  	struct sdw_cdns_dma_data *dma;
> > > > @@ -1705,10 +1676,7 @@ int cdns_set_sdw_stream(struct
> snd_soc_dai
> > > *dai,
> > > >  		if (!dma)
> > > >  			return -ENOMEM;
> > > >
> > > > -		if (pcm)
> > > > -			dma->stream_type = SDW_STREAM_PCM;
> > > > -		else
> > > > -			dma->stream_type = SDW_STREAM_PDM;
> > > > +		dma->stream_type = SDW_STREAM_PCM;
> > > >
> > > >  		dma->bus = &cdns->bus;
> > > >  		dma->link_id = cdns->instance;
> > > > diff --git a/drivers/soundwire/cadence_master.h
> > > b/drivers/soundwire/cadence_master.h
> > > > index aa4b9b0eb2a8..595d72c15d97 100644
> > > > --- a/drivers/soundwire/cadence_master.h
> > > > +++ b/drivers/soundwire/cadence_master.h
> > > > @@ -17,7 +17,7 @@
> > > >   * @h_ch_num: high channel for PDI
> > > >   * @ch_count: total channel count for PDI
> > > >   * @dir: data direction
> > > > - * @type: stream type, PDM or PCM
> > > > + * @type: stream type, (only PCM supported)
> > > >   */
> > > >  struct sdw_cdns_pdi {
> > > >  	int num;
> > > > @@ -62,17 +62,11 @@ struct sdw_cdns_streams {
> > > >   * @pcm_bd: number of bidirectional PCM streams supported
> > > >   * @pcm_in: number of input PCM streams supported
> > > >   * @pcm_out: number of output PCM streams supported
> > > > - * @pdm_bd: number of bidirectional PDM streams supported
> > > > - * @pdm_in: number of input PDM streams supported
> > > > - * @pdm_out: number of output PDM streams supported
> > > >   */
> > > >  struct sdw_cdns_stream_config {
> > > >  	unsigned int pcm_bd;
> > > >  	unsigned int pcm_in;
> > > >  	unsigned int pcm_out;
> > > > -	unsigned int pdm_bd;
> > > > -	unsigned int pdm_in;
> > > > -	unsigned int pdm_out;
> > > >  };
> > > >
> > > >  /**
> > > > @@ -111,7 +105,6 @@ struct sdw_cdns_dma_data {
> > > >   * @ports: Data ports
> > > >   * @num_ports: Total number of data ports
> > > >   * @pcm: PCM streams
> > > > - * @pdm: PDM streams
> > > >   * @registers: Cadence registers
> > > >   * @link_up: Link status
> > > >   * @msg_count: Messages sent on bus
> > > > @@ -129,7 +122,6 @@ struct sdw_cdns {
> > > >  	int num_ports;
> > > >
> > > >  	struct sdw_cdns_streams pcm;
> > > > -	struct sdw_cdns_streams pdm;
> > > >
> > > >  	int pdi_loopback_source;
> > > >  	int pdi_loopback_target;
> > > > @@ -188,7 +180,7 @@ cdns_xfer_msg_defer(struct sdw_bus *bus,
> > > >  int cdns_bus_conf(struct sdw_bus *bus, struct sdw_bus_params
> > > *params);
> > > >
> > > >  int cdns_set_sdw_stream(struct snd_soc_dai *dai,
> > > > -			void *stream, bool pcm, int direction);
> > > > +			void *stream, int direction);
> > > >
> > > >  void sdw_cdns_check_self_clearing_bits(struct sdw_cdns *cdns, const
> > > char *string,
> > > >  				       bool initial_delay, int reset_iterations);
> > > > diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
> > > > index 45ea55a7d0c8..79ba0e3f6dac 100644
> > > > --- a/drivers/soundwire/intel.c
> > > > +++ b/drivers/soundwire/intel.c
> > > > @@ -564,7 +564,7 @@ static void intel_pdi_init(struct sdw_intel *sdw,
> > > >  {
> > > >  	void __iomem *shim = sdw->link_res->shim;
> > > >  	unsigned int link_id = sdw->instance;
> > > > -	int pcm_cap, pdm_cap;
> > > > +	int pcm_cap;
> > > >
> > > >  	/* PCM Stream Capability */
> > > >  	pcm_cap = intel_readw(shim, SDW_SHIM_PCMSCAP(link_id));
> > > > @@ -575,41 +575,25 @@ static void intel_pdi_init(struct sdw_intel
> *sdw,
> > > >
> > > >  	dev_dbg(sdw->cdns.dev, "PCM cap bd:%d in:%d out:%d\n",
> > > >  		config->pcm_bd, config->pcm_in, config->pcm_out);
> > > > -
> > > > -	/* PDM Stream Capability */
> > > > -	pdm_cap = intel_readw(shim, SDW_SHIM_PDMSCAP(link_id));
> > > > -
> > > > -	config->pdm_bd = FIELD_GET(SDW_SHIM_PDMSCAP_BSS,
> > > pdm_cap);
> > > > -	config->pdm_in = FIELD_GET(SDW_SHIM_PDMSCAP_ISS, pdm_cap);
> > > > -	config->pdm_out = FIELD_GET(SDW_SHIM_PDMSCAP_OSS,
> > > pdm_cap);
> > > > -
> > > > -	dev_dbg(sdw->cdns.dev, "PDM cap bd:%d in:%d out:%d\n",
> > > > -		config->pdm_bd, config->pdm_in, config->pdm_out);
> > > >  }
> > > >
> > > >  static int
> > > > -intel_pdi_get_ch_cap(struct sdw_intel *sdw, unsigned int pdi_num,
> bool
> > > pcm)
> > > > +intel_pdi_get_ch_cap(struct sdw_intel *sdw, unsigned int pdi_num)
> > > >  {
> > > >  	void __iomem *shim = sdw->link_res->shim;
> > > >  	unsigned int link_id = sdw->instance;
> > > >  	int count;
> > > >
> > > > -	if (pcm) {
> > > > -		count = intel_readw(shim, SDW_SHIM_PCMSYCHC(link_id,
> > > pdi_num));
> > > > +	count = intel_readw(shim, SDW_SHIM_PCMSYCHC(link_id,
> > > pdi_num));
> > > >
> > > > -		/*
> > > > -		 * WORKAROUND: on all existing Intel controllers, pdi
> > > > -		 * number 2 reports channel count as 1 even though it
> > > > -		 * supports 8 channels. Performing hardcoding for pdi
> > > > -		 * number 2.
> > > > -		 */
> > > > -		if (pdi_num == 2)
> > > > -			count = 7;
> > > > -
> > > > -	} else {
> > > > -		count = intel_readw(shim, SDW_SHIM_PDMSCAP(link_id));
> > > > -		count = FIELD_GET(SDW_SHIM_PDMSCAP_CPSS, count);
> > > > -	}
> > > > +	/*
> > > > +	 * WORKAROUND: on all existing Intel controllers, pdi
> > > > +	 * number 2 reports channel count as 1 even though it
> > > > +	 * supports 8 channels. Performing hardcoding for pdi
> > > > +	 * number 2.
> > > > +	 */
> > > > +	if (pdi_num == 2)
> > > > +		count = 7;
> > > >
> > > >  	/* zero based values for channel count in register */
> > > >  	count++;
> > > > @@ -620,12 +604,12 @@ intel_pdi_get_ch_cap(struct sdw_intel *sdw,
> > > unsigned int pdi_num, bool pcm)
> > > >  static int intel_pdi_get_ch_update(struct sdw_intel *sdw,
> > > >  				   struct sdw_cdns_pdi *pdi,
> > > >  				   unsigned int num_pdi,
> > > > -				   unsigned int *num_ch, bool pcm)
> > > > +				   unsigned int *num_ch)
> > > >  {
> > > >  	int i, ch_count = 0;
> > > >
> > > >  	for (i = 0; i < num_pdi; i++) {
> > > > -		pdi->ch_count = intel_pdi_get_ch_cap(sdw, pdi->num, pcm);
> > > > +		pdi->ch_count = intel_pdi_get_ch_cap(sdw, pdi->num);
> > > >  		ch_count += pdi->ch_count;
> > > >  		pdi++;
> > > >  	}
> > > > @@ -635,25 +619,23 @@ static int intel_pdi_get_ch_update(struct
> > > sdw_intel *sdw,
> > > >  }
> > > >
> > > >  static int intel_pdi_stream_ch_update(struct sdw_intel *sdw,
> > > > -				      struct sdw_cdns_streams *stream, bool
> > > pcm)
> > > > +				      struct sdw_cdns_streams *stream)
> > > >  {
> > > >  	intel_pdi_get_ch_update(sdw, stream->bd, stream->num_bd,
> > > > -				&stream->num_ch_bd, pcm);
> > > > +				&stream->num_ch_bd);
> > > >
> > > >  	intel_pdi_get_ch_update(sdw, stream->in, stream->num_in,
> > > > -				&stream->num_ch_in, pcm);
> > > > +				&stream->num_ch_in);
> > > >
> > > >  	intel_pdi_get_ch_update(sdw, stream->out, stream->num_out,
> > > > -				&stream->num_ch_out, pcm);
> > > > +				&stream->num_ch_out);
> > > >
> > > >  	return 0;
> > > >  }
> > > >
> > > >  static int intel_pdi_ch_update(struct sdw_intel *sdw)
> > > >  {
> > > > -	/* First update PCM streams followed by PDM streams */
> > > > -	intel_pdi_stream_ch_update(sdw, &sdw->cdns.pcm, true);
> > > > -	intel_pdi_stream_ch_update(sdw, &sdw->cdns.pdm, false);
> > > > +	intel_pdi_stream_ch_update(sdw, &sdw->cdns.pcm);
> > > >
> > > >  	return 0;
> > > >  }
> > > > @@ -840,7 +822,6 @@ static int intel_hw_params(struct
> > > snd_pcm_substream *substream,
> > > >  	struct sdw_port_config *pconfig;
> > > >  	int ch, dir;
> > > >  	int ret;
> > > > -	bool pcm = true;
> > > >
> > > >  	dma = snd_soc_dai_get_dma_data(dai, substream);
> > > >  	if (!dma)
> > > > @@ -852,13 +833,7 @@ static int intel_hw_params(struct
> > > snd_pcm_substream *substream,
> > > >  	else
> > > >  		dir = SDW_DATA_DIR_TX;
> > > >
> > > > -	if (dma->stream_type == SDW_STREAM_PDM)
> > > > -		pcm = false;
> > > > -
> > > > -	if (pcm)
> > > > -		pdi = sdw_cdns_alloc_pdi(cdns, &cdns->pcm, ch, dir, dai->id);
> > > > -	else
> > > > -		pdi = sdw_cdns_alloc_pdi(cdns, &cdns->pdm, ch, dir, dai->id);
> > > > +	pdi = sdw_cdns_alloc_pdi(cdns, &cdns->pcm, ch, dir, dai->id);
> > > >
> > > >  	if (!pdi) {
> > > >  		ret = -EINVAL;
> > > > @@ -888,12 +863,7 @@ static int intel_hw_params(struct
> > > snd_pcm_substream *substream,
> > > >  	sconfig.frame_rate = params_rate(params);
> > > >  	sconfig.type = dma->stream_type;
> > > >
> > > > -	if (dma->stream_type == SDW_STREAM_PDM) {
> > > > -		sconfig.frame_rate *= 50;
> > > > -		sconfig.bps = 1;
> > > > -	} else {
> > > > -		sconfig.bps =
> > > snd_pcm_format_width(params_format(params));
> > > > -	}
> > > > +	sconfig.bps = snd_pcm_format_width(params_format(params));
> > > >
> > > >  	/* Port configuration */
> > > >  	pconfig = kzalloc(sizeof(*pconfig), GFP_KERNEL);
> > > > @@ -1012,13 +982,7 @@ static void intel_shutdown(struct
> > > snd_pcm_substream *substream,
> > > >  static int intel_pcm_set_sdw_stream(struct snd_soc_dai *dai,
> > > >  				    void *stream, int direction)
> > > >  {
> > > > -	return cdns_set_sdw_stream(dai, stream, true, direction);
> > > > -}
> > > > -
> > > > -static int intel_pdm_set_sdw_stream(struct snd_soc_dai *dai,
> > > > -				    void *stream, int direction)
> > > > -{
> > > > -	return cdns_set_sdw_stream(dai, stream, false, direction);
> > > > +	return cdns_set_sdw_stream(dai, stream, direction);
> > > >  }
> > > >
> > > >  static void *intel_get_sdw_stream(struct snd_soc_dai *dai,
> > > > @@ -1133,16 +1097,6 @@ static const struct snd_soc_dai_ops
> > > intel_pcm_dai_ops = {
> > > >  	.get_stream = intel_get_sdw_stream,
> > > >  };
> > > >
> > > > -static const struct snd_soc_dai_ops intel_pdm_dai_ops = {
> > > > -	.startup = intel_startup,
> > > > -	.hw_params = intel_hw_params,
> > > > -	.prepare = intel_prepare,
> > > > -	.hw_free = intel_hw_free,
> > > > -	.shutdown = intel_shutdown,
> > > > -	.set_sdw_stream = intel_pdm_set_sdw_stream,
> > > > -	.get_sdw_stream = intel_get_sdw_stream,
> > > > -};
> > > > -
> > > >  static const struct snd_soc_component_driver dai_component = {
> > > >  	.name           = "soundwire",
> > > >  	.suspend	= intel_component_dais_suspend
> > > > @@ -1151,7 +1105,7 @@ static const struct snd_soc_component_driver
> > > dai_component = {
> > > >  static int intel_create_dai(struct sdw_cdns *cdns,
> > > >  			    struct snd_soc_dai_driver *dais,
> > > >  			    enum intel_pdi_type type,
> > > > -			    u32 num, u32 off, u32 max_ch, bool pcm)
> > > > +			    u32 num, u32 off, u32 max_ch)
> > > >  {
> > > >  	int i;
> > > >
> > > > @@ -1180,10 +1134,7 @@ static int intel_create_dai(struct sdw_cdns
> *cdns,
> > > >  			dais[i].capture.formats =
> > > SNDRV_PCM_FMTBIT_S16_LE;
> > > >  		}
> > > >
> > > > -		if (pcm)
> > > > -			dais[i].ops = &intel_pcm_dai_ops;
> > > > -		else
> > > > -			dais[i].ops = &intel_pdm_dai_ops;
> > > > +		dais[i].ops = &intel_pcm_dai_ops;
> > > >  	}
> > > >
> > > >  	return 0;
> > > > @@ -1197,7 +1148,7 @@ static int intel_register_dai(struct sdw_intel
> *sdw)
> > > >  	int num_dai, ret, off = 0;
> > > >
> > > >  	/* DAIs are created based on total number of PDIs supported */
> > > > -	num_dai = cdns->pcm.num_pdi + cdns->pdm.num_pdi;
> > > > +	num_dai = cdns->pcm.num_pdi;
> > > >
> > > >  	dais = devm_kcalloc(cdns->dev, num_dai, sizeof(*dais),
> > > GFP_KERNEL);
> > > >  	if (!dais)
> > > > @@ -1207,39 +1158,19 @@ static int intel_register_dai(struct sdw_intel
> > > *sdw)
> > > >  	stream = &cdns->pcm;
> > > >
> > > >  	ret = intel_create_dai(cdns, dais, INTEL_PDI_IN, cdns->pcm.num_in,
> > > > -			       off, stream->num_ch_in, true);
> > > > +			       off, stream->num_ch_in);
> > > >  	if (ret)
> > > >  		return ret;
> > > >
> > > >  	off += cdns->pcm.num_in;
> > > >  	ret = intel_create_dai(cdns, dais, INTEL_PDI_OUT, cdns-
> > > >pcm.num_out,
> > > > -			       off, stream->num_ch_out, true);
> > > > +			       off, stream->num_ch_out);
> > > >  	if (ret)
> > > >  		return ret;
> > > >
> > > >  	off += cdns->pcm.num_out;
> > > >  	ret = intel_create_dai(cdns, dais, INTEL_PDI_BD, cdns->pcm.num_bd,
> > > > -			       off, stream->num_ch_bd, true);
> > > > -	if (ret)
> > > > -		return ret;
> > > > -
> > > > -	/* Create PDM DAIs */
> > > > -	stream = &cdns->pdm;
> > > > -	off += cdns->pcm.num_bd;
> > > > -	ret = intel_create_dai(cdns, dais, INTEL_PDI_IN, cdns->pdm.num_in,
> > > > -			       off, stream->num_ch_in, false);
> > > > -	if (ret)
> > > > -		return ret;
> > > > -
> > > > -	off += cdns->pdm.num_in;
> > > > -	ret = intel_create_dai(cdns, dais, INTEL_PDI_OUT, cdns-
> > > >pdm.num_out,
> > > > -			       off, stream->num_ch_out, false);
> > > > -	if (ret)
> > > > -		return ret;
> > > > -
> > > > -	off += cdns->pdm.num_out;
> > > > -	ret = intel_create_dai(cdns, dais, INTEL_PDI_BD, cdns-
> > > >pdm.num_bd,
> > > > -			       off, stream->num_ch_bd, false);
> > > > +			       off, stream->num_ch_bd);
> > > >  	if (ret)
> > > >  		return ret;
> > > >
> > > > --
> > > > 2.17.1
> > >
> > > --
> > > ~Vinod
> 
> --
> ~Vinod
Vinod Koul Dec. 23, 2021, 4:35 p.m. UTC | #3
On 13-12-21, 13:46, Bard Liao wrote:
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
> While the hardware supports PDM streams, this capability has never
> been tested or enabled on any product, so this is dead-code. Let's
> remove all this.

Acked-By: Vinod Koul <vkoul@kernel.org>
Mark Brown Dec. 23, 2021, 6:26 p.m. UTC | #4
On Mon, Dec 13, 2021 at 01:46:34PM +0800, Bard Liao wrote:
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
> While the hardware supports PDM streams, this capability has never
> been tested or enabled on any product, so this is dead-code. Let's
> remove all this.

This doesn't build with current code, I'm guessing due to a Soundwire
dependency I don't have:

/mnt/kernel/drivers/soundwire/intel.c:1142:3: error: 'const struct snd_soc_dai_ops' has no member named 'set_sdw_stream'; did you mean 'set_stream'?
 1142 |  .set_sdw_stream = intel_pdm_set_sdw_stream,
      |   ^~~~~~~~~~~~~~
      |   set_stream
/mnt/kernel/drivers/soundwire/intel.c:1142:20: error: initialization of 'int (*)(struct snd_pcm_substream *, struct snd_pcm_hw_params *, struct snd_soc_dai *)' from incompatible pointer type 'int (*)(struct snd_soc_dai *, void *, int)' [-Werror=incompatible-pointer-types]
 1142 |  .set_sdw_stream = intel_pdm_set_sdw_stream,
      |                    ^~~~~~~~~~~~~~~~~~~~~~~~
/mnt/kernel/drivers/soundwire/intel.c:1142:20: note: (near initialization for 'intel_pdm_dai_ops.hw_params')
/mnt/kernel/drivers/soundwire/intel.c:1143:3: error: 'const struct snd_soc_dai_ops' has no member named 'get_sdw_stream'; did you mean 'get_stream'?
 1143 |  .get_sdw_stream = intel_get_sdw_stream,
      |   ^~~~~~~~~~~~~~
      |   get_stream
/mnt/kernel/drivers/soundwire/intel.c:1143:20: error: initialization of 'int (*)(struct snd_pcm_substream *, struct snd_soc_dai *)' from incompatible pointer type 'void * (*)(struct snd_soc_dai *, int)' [-Werror=incompatible-pointer-types]
 1143 |  .get_sdw_stream = intel_get_sdw_stream,
      |                    ^~~~~~~~~~~~~~~~~~~~
/mnt/kernel/drivers/soundwire/intel.c:1143:20: note: (near initialization for 'intel_pdm_dai_ops.hw_free')
cc1: all warnings being treated as errors
make[3]: *** [/mnt/kernel/scripts/Makefile.build:287: drivers/soundwire/intel.o] Error 1
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [/mnt/kernel/scripts/Makefile.build:549: drivers/soundwire] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/mnt/kernel/Makefile:1846: drivers] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:219: __sub-make] Error 2
Liao, Bard Dec. 24, 2021, 1:38 a.m. UTC | #5
> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Friday, December 24, 2021 2:27 AM
> To: Bard Liao <yung-chuan.liao@linux.intel.com>
> Cc: alsa-devel@alsa-project.org; vkoul@kernel.org; vinod.koul@linaro.org;
> linux-kernel@vger.kernel.org; tiwai@suse.de; gregkh@linuxfoundation.org;
> srinivas.kandagatla@linaro.org; pierre-louis.bossart@linux.intel.com; Kale,
> Sanyog R <sanyog.r.kale@intel.com>; Liao, Bard <bard.liao@intel.com>
> Subject: Re: [PATCH 7/7] soundwire: intel: remove PDM support
> 
> On Mon, Dec 13, 2021 at 01:46:34PM +0800, Bard Liao wrote:
> > From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> >
> > While the hardware supports PDM streams, this capability has never
> > been tested or enabled on any product, so this is dead-code. Let's
> > remove all this.
> 
> This doesn't build with current code, I'm guessing due to a Soundwire
> dependency I don't have:

Hi Mark,

Sorry, it is my fault.  "[PATCH 7/7] soundwire: intel: remove PDM support"
was before "[PATCH 4/7] ASoC/SoundWire: dai: expand 'stream' concept
beyond". So there was no "intel_pdm_dai_ops" stuff in the "ASoC/SoundWire: 
dai: expand 'stream' concept beyond" patch.
But when I re-order the patches order, the intel_pdm_dai_ops stuff was
still in the "ASoC/SoundWire:  dai: expand 'stream' concept beyond" patch
and I forgot rename set_sdw_stream and get_sdw_stream ops in
intel_pdm_dai_ops.
I will send v2 to fix it. Sorry about it.

> 
> /mnt/kernel/drivers/soundwire/intel.c:1142:3: error: 'const struct
> snd_soc_dai_ops' has no member named 'set_sdw_stream'; did you mean
> 'set_stream'?
>  1142 |  .set_sdw_stream = intel_pdm_set_sdw_stream,
>       |   ^~~~~~~~~~~~~~
>       |   set_stream
> /mnt/kernel/drivers/soundwire/intel.c:1142:20: error: initialization of 'int
> (*)(struct snd_pcm_substream *, struct snd_pcm_hw_params *, struct
> snd_soc_dai *)' from incompatible pointer type 'int (*)(struct snd_soc_dai *,
> void *, int)' [-Werror=incompatible-pointer-types]
>  1142 |  .set_sdw_stream = intel_pdm_set_sdw_stream,
>       |                    ^~~~~~~~~~~~~~~~~~~~~~~~
> /mnt/kernel/drivers/soundwire/intel.c:1142:20: note: (near initialization for
> 'intel_pdm_dai_ops.hw_params')
> /mnt/kernel/drivers/soundwire/intel.c:1143:3: error: 'const struct
> snd_soc_dai_ops' has no member named 'get_sdw_stream'; did you mean
> 'get_stream'?
>  1143 |  .get_sdw_stream = intel_get_sdw_stream,
>       |   ^~~~~~~~~~~~~~
>       |   get_stream
> /mnt/kernel/drivers/soundwire/intel.c:1143:20: error: initialization of 'int
> (*)(struct snd_pcm_substream *, struct snd_soc_dai *)' from incompatible
> pointer type 'void * (*)(struct snd_soc_dai *, int)' [-Werror=incompatible-
> pointer-types]
>  1143 |  .get_sdw_stream = intel_get_sdw_stream,
>       |                    ^~~~~~~~~~~~~~~~~~~~
> /mnt/kernel/drivers/soundwire/intel.c:1143:20: note: (near initialization for
> 'intel_pdm_dai_ops.hw_free')
> cc1: all warnings being treated as errors
> make[3]: *** [/mnt/kernel/scripts/Makefile.build:287:
> drivers/soundwire/intel.o] Error 1
> make[3]: *** Waiting for unfinished jobs....
> make[2]: *** [/mnt/kernel/scripts/Makefile.build:549: drivers/soundwire]
> Error 2
> make[2]: *** Waiting for unfinished jobs....
> make[1]: *** [/mnt/kernel/Makefile:1846: drivers] Error 2
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:219: __sub-make] Error 2
diff mbox series

Patch

diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index 4fcc3ba93004..558390af44b6 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -1178,9 +1178,6 @@  int sdw_cdns_pdi_init(struct sdw_cdns *cdns,
 	cdns->pcm.num_bd = config.pcm_bd;
 	cdns->pcm.num_in = config.pcm_in;
 	cdns->pcm.num_out = config.pcm_out;
-	cdns->pdm.num_bd = config.pdm_bd;
-	cdns->pdm.num_in = config.pdm_in;
-	cdns->pdm.num_out = config.pdm_out;
 
 	/* Allocate PDIs for PCMs */
 	stream = &cdns->pcm;
@@ -1211,32 +1208,6 @@  int sdw_cdns_pdi_init(struct sdw_cdns *cdns,
 	stream->num_pdi = stream->num_bd + stream->num_in + stream->num_out;
 	cdns->num_ports = stream->num_pdi;
 
-	/* Allocate PDIs for PDMs */
-	stream = &cdns->pdm;
-	ret = cdns_allocate_pdi(cdns, &stream->bd,
-				stream->num_bd, offset);
-	if (ret)
-		return ret;
-
-	offset += stream->num_bd;
-
-	ret = cdns_allocate_pdi(cdns, &stream->in,
-				stream->num_in, offset);
-	if (ret)
-		return ret;
-
-	offset += stream->num_in;
-
-	ret = cdns_allocate_pdi(cdns, &stream->out,
-				stream->num_out, offset);
-
-	if (ret)
-		return ret;
-
-	/* Update total number of PDM PDIs */
-	stream->num_pdi = stream->num_bd + stream->num_in + stream->num_out;
-	cdns->num_ports += stream->num_pdi;
-
 	return 0;
 }
 EXPORT_SYMBOL(sdw_cdns_pdi_init);
@@ -1681,7 +1652,7 @@  int sdw_cdns_probe(struct sdw_cdns *cdns)
 EXPORT_SYMBOL(sdw_cdns_probe);
 
 int cdns_set_sdw_stream(struct snd_soc_dai *dai,
-			void *stream, bool pcm, int direction)
+			void *stream, int direction)
 {
 	struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai);
 	struct sdw_cdns_dma_data *dma;
@@ -1705,10 +1676,7 @@  int cdns_set_sdw_stream(struct snd_soc_dai *dai,
 		if (!dma)
 			return -ENOMEM;
 
-		if (pcm)
-			dma->stream_type = SDW_STREAM_PCM;
-		else
-			dma->stream_type = SDW_STREAM_PDM;
+		dma->stream_type = SDW_STREAM_PCM;
 
 		dma->bus = &cdns->bus;
 		dma->link_id = cdns->instance;
diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h
index aa4b9b0eb2a8..595d72c15d97 100644
--- a/drivers/soundwire/cadence_master.h
+++ b/drivers/soundwire/cadence_master.h
@@ -17,7 +17,7 @@ 
  * @h_ch_num: high channel for PDI
  * @ch_count: total channel count for PDI
  * @dir: data direction
- * @type: stream type, PDM or PCM
+ * @type: stream type, (only PCM supported)
  */
 struct sdw_cdns_pdi {
 	int num;
@@ -62,17 +62,11 @@  struct sdw_cdns_streams {
  * @pcm_bd: number of bidirectional PCM streams supported
  * @pcm_in: number of input PCM streams supported
  * @pcm_out: number of output PCM streams supported
- * @pdm_bd: number of bidirectional PDM streams supported
- * @pdm_in: number of input PDM streams supported
- * @pdm_out: number of output PDM streams supported
  */
 struct sdw_cdns_stream_config {
 	unsigned int pcm_bd;
 	unsigned int pcm_in;
 	unsigned int pcm_out;
-	unsigned int pdm_bd;
-	unsigned int pdm_in;
-	unsigned int pdm_out;
 };
 
 /**
@@ -111,7 +105,6 @@  struct sdw_cdns_dma_data {
  * @ports: Data ports
  * @num_ports: Total number of data ports
  * @pcm: PCM streams
- * @pdm: PDM streams
  * @registers: Cadence registers
  * @link_up: Link status
  * @msg_count: Messages sent on bus
@@ -129,7 +122,6 @@  struct sdw_cdns {
 	int num_ports;
 
 	struct sdw_cdns_streams pcm;
-	struct sdw_cdns_streams pdm;
 
 	int pdi_loopback_source;
 	int pdi_loopback_target;
@@ -188,7 +180,7 @@  cdns_xfer_msg_defer(struct sdw_bus *bus,
 int cdns_bus_conf(struct sdw_bus *bus, struct sdw_bus_params *params);
 
 int cdns_set_sdw_stream(struct snd_soc_dai *dai,
-			void *stream, bool pcm, int direction);
+			void *stream, int direction);
 
 void sdw_cdns_check_self_clearing_bits(struct sdw_cdns *cdns, const char *string,
 				       bool initial_delay, int reset_iterations);
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 45ea55a7d0c8..79ba0e3f6dac 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -564,7 +564,7 @@  static void intel_pdi_init(struct sdw_intel *sdw,
 {
 	void __iomem *shim = sdw->link_res->shim;
 	unsigned int link_id = sdw->instance;
-	int pcm_cap, pdm_cap;
+	int pcm_cap;
 
 	/* PCM Stream Capability */
 	pcm_cap = intel_readw(shim, SDW_SHIM_PCMSCAP(link_id));
@@ -575,41 +575,25 @@  static void intel_pdi_init(struct sdw_intel *sdw,
 
 	dev_dbg(sdw->cdns.dev, "PCM cap bd:%d in:%d out:%d\n",
 		config->pcm_bd, config->pcm_in, config->pcm_out);
-
-	/* PDM Stream Capability */
-	pdm_cap = intel_readw(shim, SDW_SHIM_PDMSCAP(link_id));
-
-	config->pdm_bd = FIELD_GET(SDW_SHIM_PDMSCAP_BSS, pdm_cap);
-	config->pdm_in = FIELD_GET(SDW_SHIM_PDMSCAP_ISS, pdm_cap);
-	config->pdm_out = FIELD_GET(SDW_SHIM_PDMSCAP_OSS, pdm_cap);
-
-	dev_dbg(sdw->cdns.dev, "PDM cap bd:%d in:%d out:%d\n",
-		config->pdm_bd, config->pdm_in, config->pdm_out);
 }
 
 static int
-intel_pdi_get_ch_cap(struct sdw_intel *sdw, unsigned int pdi_num, bool pcm)
+intel_pdi_get_ch_cap(struct sdw_intel *sdw, unsigned int pdi_num)
 {
 	void __iomem *shim = sdw->link_res->shim;
 	unsigned int link_id = sdw->instance;
 	int count;
 
-	if (pcm) {
-		count = intel_readw(shim, SDW_SHIM_PCMSYCHC(link_id, pdi_num));
+	count = intel_readw(shim, SDW_SHIM_PCMSYCHC(link_id, pdi_num));
 
-		/*
-		 * WORKAROUND: on all existing Intel controllers, pdi
-		 * number 2 reports channel count as 1 even though it
-		 * supports 8 channels. Performing hardcoding for pdi
-		 * number 2.
-		 */
-		if (pdi_num == 2)
-			count = 7;
-
-	} else {
-		count = intel_readw(shim, SDW_SHIM_PDMSCAP(link_id));
-		count = FIELD_GET(SDW_SHIM_PDMSCAP_CPSS, count);
-	}
+	/*
+	 * WORKAROUND: on all existing Intel controllers, pdi
+	 * number 2 reports channel count as 1 even though it
+	 * supports 8 channels. Performing hardcoding for pdi
+	 * number 2.
+	 */
+	if (pdi_num == 2)
+		count = 7;
 
 	/* zero based values for channel count in register */
 	count++;
@@ -620,12 +604,12 @@  intel_pdi_get_ch_cap(struct sdw_intel *sdw, unsigned int pdi_num, bool pcm)
 static int intel_pdi_get_ch_update(struct sdw_intel *sdw,
 				   struct sdw_cdns_pdi *pdi,
 				   unsigned int num_pdi,
-				   unsigned int *num_ch, bool pcm)
+				   unsigned int *num_ch)
 {
 	int i, ch_count = 0;
 
 	for (i = 0; i < num_pdi; i++) {
-		pdi->ch_count = intel_pdi_get_ch_cap(sdw, pdi->num, pcm);
+		pdi->ch_count = intel_pdi_get_ch_cap(sdw, pdi->num);
 		ch_count += pdi->ch_count;
 		pdi++;
 	}
@@ -635,25 +619,23 @@  static int intel_pdi_get_ch_update(struct sdw_intel *sdw,
 }
 
 static int intel_pdi_stream_ch_update(struct sdw_intel *sdw,
-				      struct sdw_cdns_streams *stream, bool pcm)
+				      struct sdw_cdns_streams *stream)
 {
 	intel_pdi_get_ch_update(sdw, stream->bd, stream->num_bd,
-				&stream->num_ch_bd, pcm);
+				&stream->num_ch_bd);
 
 	intel_pdi_get_ch_update(sdw, stream->in, stream->num_in,
-				&stream->num_ch_in, pcm);
+				&stream->num_ch_in);
 
 	intel_pdi_get_ch_update(sdw, stream->out, stream->num_out,
-				&stream->num_ch_out, pcm);
+				&stream->num_ch_out);
 
 	return 0;
 }
 
 static int intel_pdi_ch_update(struct sdw_intel *sdw)
 {
-	/* First update PCM streams followed by PDM streams */
-	intel_pdi_stream_ch_update(sdw, &sdw->cdns.pcm, true);
-	intel_pdi_stream_ch_update(sdw, &sdw->cdns.pdm, false);
+	intel_pdi_stream_ch_update(sdw, &sdw->cdns.pcm);
 
 	return 0;
 }
@@ -840,7 +822,6 @@  static int intel_hw_params(struct snd_pcm_substream *substream,
 	struct sdw_port_config *pconfig;
 	int ch, dir;
 	int ret;
-	bool pcm = true;
 
 	dma = snd_soc_dai_get_dma_data(dai, substream);
 	if (!dma)
@@ -852,13 +833,7 @@  static int intel_hw_params(struct snd_pcm_substream *substream,
 	else
 		dir = SDW_DATA_DIR_TX;
 
-	if (dma->stream_type == SDW_STREAM_PDM)
-		pcm = false;
-
-	if (pcm)
-		pdi = sdw_cdns_alloc_pdi(cdns, &cdns->pcm, ch, dir, dai->id);
-	else
-		pdi = sdw_cdns_alloc_pdi(cdns, &cdns->pdm, ch, dir, dai->id);
+	pdi = sdw_cdns_alloc_pdi(cdns, &cdns->pcm, ch, dir, dai->id);
 
 	if (!pdi) {
 		ret = -EINVAL;
@@ -888,12 +863,7 @@  static int intel_hw_params(struct snd_pcm_substream *substream,
 	sconfig.frame_rate = params_rate(params);
 	sconfig.type = dma->stream_type;
 
-	if (dma->stream_type == SDW_STREAM_PDM) {
-		sconfig.frame_rate *= 50;
-		sconfig.bps = 1;
-	} else {
-		sconfig.bps = snd_pcm_format_width(params_format(params));
-	}
+	sconfig.bps = snd_pcm_format_width(params_format(params));
 
 	/* Port configuration */
 	pconfig = kzalloc(sizeof(*pconfig), GFP_KERNEL);
@@ -1012,13 +982,7 @@  static void intel_shutdown(struct snd_pcm_substream *substream,
 static int intel_pcm_set_sdw_stream(struct snd_soc_dai *dai,
 				    void *stream, int direction)
 {
-	return cdns_set_sdw_stream(dai, stream, true, direction);
-}
-
-static int intel_pdm_set_sdw_stream(struct snd_soc_dai *dai,
-				    void *stream, int direction)
-{
-	return cdns_set_sdw_stream(dai, stream, false, direction);
+	return cdns_set_sdw_stream(dai, stream, direction);
 }
 
 static void *intel_get_sdw_stream(struct snd_soc_dai *dai,
@@ -1133,16 +1097,6 @@  static const struct snd_soc_dai_ops intel_pcm_dai_ops = {
 	.get_stream = intel_get_sdw_stream,
 };
 
-static const struct snd_soc_dai_ops intel_pdm_dai_ops = {
-	.startup = intel_startup,
-	.hw_params = intel_hw_params,
-	.prepare = intel_prepare,
-	.hw_free = intel_hw_free,
-	.shutdown = intel_shutdown,
-	.set_sdw_stream = intel_pdm_set_sdw_stream,
-	.get_sdw_stream = intel_get_sdw_stream,
-};
-
 static const struct snd_soc_component_driver dai_component = {
 	.name           = "soundwire",
 	.suspend	= intel_component_dais_suspend
@@ -1151,7 +1105,7 @@  static const struct snd_soc_component_driver dai_component = {
 static int intel_create_dai(struct sdw_cdns *cdns,
 			    struct snd_soc_dai_driver *dais,
 			    enum intel_pdi_type type,
-			    u32 num, u32 off, u32 max_ch, bool pcm)
+			    u32 num, u32 off, u32 max_ch)
 {
 	int i;
 
@@ -1180,10 +1134,7 @@  static int intel_create_dai(struct sdw_cdns *cdns,
 			dais[i].capture.formats = SNDRV_PCM_FMTBIT_S16_LE;
 		}
 
-		if (pcm)
-			dais[i].ops = &intel_pcm_dai_ops;
-		else
-			dais[i].ops = &intel_pdm_dai_ops;
+		dais[i].ops = &intel_pcm_dai_ops;
 	}
 
 	return 0;
@@ -1197,7 +1148,7 @@  static int intel_register_dai(struct sdw_intel *sdw)
 	int num_dai, ret, off = 0;
 
 	/* DAIs are created based on total number of PDIs supported */
-	num_dai = cdns->pcm.num_pdi + cdns->pdm.num_pdi;
+	num_dai = cdns->pcm.num_pdi;
 
 	dais = devm_kcalloc(cdns->dev, num_dai, sizeof(*dais), GFP_KERNEL);
 	if (!dais)
@@ -1207,39 +1158,19 @@  static int intel_register_dai(struct sdw_intel *sdw)
 	stream = &cdns->pcm;
 
 	ret = intel_create_dai(cdns, dais, INTEL_PDI_IN, cdns->pcm.num_in,
-			       off, stream->num_ch_in, true);
+			       off, stream->num_ch_in);
 	if (ret)
 		return ret;
 
 	off += cdns->pcm.num_in;
 	ret = intel_create_dai(cdns, dais, INTEL_PDI_OUT, cdns->pcm.num_out,
-			       off, stream->num_ch_out, true);
+			       off, stream->num_ch_out);
 	if (ret)
 		return ret;
 
 	off += cdns->pcm.num_out;
 	ret = intel_create_dai(cdns, dais, INTEL_PDI_BD, cdns->pcm.num_bd,
-			       off, stream->num_ch_bd, true);
-	if (ret)
-		return ret;
-
-	/* Create PDM DAIs */
-	stream = &cdns->pdm;
-	off += cdns->pcm.num_bd;
-	ret = intel_create_dai(cdns, dais, INTEL_PDI_IN, cdns->pdm.num_in,
-			       off, stream->num_ch_in, false);
-	if (ret)
-		return ret;
-
-	off += cdns->pdm.num_in;
-	ret = intel_create_dai(cdns, dais, INTEL_PDI_OUT, cdns->pdm.num_out,
-			       off, stream->num_ch_out, false);
-	if (ret)
-		return ret;
-
-	off += cdns->pdm.num_out;
-	ret = intel_create_dai(cdns, dais, INTEL_PDI_BD, cdns->pdm.num_bd,
-			       off, stream->num_ch_bd, false);
+			       off, stream->num_ch_bd);
 	if (ret)
 		return ret;