linux-kernel - Re: [PATCH 2/5] soundwire: stream: add helper to startup/shutdown streams

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200630160349.GR2599@vkoul-mobl>
Date:   Tue, 30 Jun 2020 21:33:49 +0530
From:   Vinod Koul <vkoul@...nel.org>
To:     Bard Liao <yung-chuan.liao@...ux.intel.com>
Cc:     alsa-devel@...a-project.org, linux-kernel@...r.kernel.org,
        tiwai@...e.de, broonie@...nel.org, gregkh@...uxfoundation.org,
        jank@...ence.com, srinivas.kandagatla@...aro.org,
        rander.wang@...ux.intel.com, ranjani.sridharan@...ux.intel.com,
        hui.wang@...onical.com, pierre-louis.bossart@...ux.intel.com,
        sanyog.r.kale@...el.com, slawomir.blauciak@...el.com,
        mengdong.lin@...el.com, bard.liao@...el.com
Subject: Re: [PATCH 2/5] soundwire: stream: add helper to startup/shutdown
 streams

On 23-06-20, 05:23, Bard Liao wrote:
> From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
> 
> To handle streams at the dailink level, expose two helpers that will
> be called from machine drivers.
> 
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>
> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@...ux.intel.com>
> Reviewed-by: Kai Vehmanen <kai.vehmanen@...ux.intel.com>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
> Signed-off-by: Bard Liao <yung-chuan.liao@...ux.intel.com>
> ---
>  drivers/soundwire/stream.c    | 90 +++++++++++++++++++++++++++++++++++
>  include/linux/soundwire/sdw.h |  2 +
>  2 files changed, 92 insertions(+)
> 
> diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> index a9a72574b34a..48daf4b48f51 100644
> --- a/drivers/soundwire/stream.c
> +++ b/drivers/soundwire/stream.c
> @@ -13,6 +13,9 @@
>  #include <linux/slab.h>
>  #include <linux/soundwire/sdw_registers.h>
>  #include <linux/soundwire/sdw.h>
> +#include <sound/core.h>

Do we really need core header?

> +#include <sound/pcm.h>
> +#include <sound/soc.h>
>  #include "bus.h"
>  
>  /*
> @@ -1826,3 +1829,90 @@ int sdw_deprepare_stream(struct sdw_stream_runtime *stream)
>  	return ret;
>  }
>  EXPORT_SYMBOL(sdw_deprepare_stream);
> +
> +static int set_stream(struct snd_pcm_substream *substream,
> +		      struct sdw_stream_runtime *sdw_stream)

sdw_set_stream() please

> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct snd_soc_dai *dai;
> +	int ret = 0;
> +	int i;
> +
> +	/* Set stream pointer on all DAIs */
> +	for_each_rtd_dais(rtd, i, dai) {
> +		ret = snd_soc_dai_set_sdw_stream(dai, sdw_stream,
> +						 substream->stream);
> +		if (ret < 0) {
> +			dev_err(rtd->dev,
> +				"failed to set stream pointer on dai %s",
> +				dai->name);

lets use one line and shiny new 100 char limit, would make code read
better!

> +			break;

So on error should unset of stream pointer be done?

> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +int sdw_startup_stream(void *sdw_substream)

Can we have kernel doc style Documentation for exported APIs?

> +{
> +	struct snd_pcm_substream *substream = sdw_substream;
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct sdw_stream_runtime *sdw_stream;
> +	char *name;
> +	int ret;
> +
> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> +		name = kasprintf(GFP_KERNEL, "%s-Playback", substream->name);
> +	else
> +		name = kasprintf(GFP_KERNEL, "%s-Capture", substream->name);
> +
> +	if (!name)
> +		return -ENOMEM;
> +
> +	sdw_stream = sdw_alloc_stream(name);
> +	if (!sdw_stream) {
> +		dev_err(rtd->dev, "alloc stream failed for substream DAI %s",
> +			substream->name);
> +		ret = -ENOMEM;
> +		goto error;
> +	}
> +
> +	ret = set_stream(substream, sdw_stream);
> +	if (ret < 0)
> +		goto release_stream;
> +	return 0;
> +
> +release_stream:
> +	sdw_release_stream(sdw_stream);
> +	set_stream(substream, NULL);
> +error:
> +	kfree(name);
> +	return ret;
> +}
> +EXPORT_SYMBOL(sdw_startup_stream);
-- 
~Vinod