linux-kernel - Re: [PATCH 03/19] soundwire: amd: register sdw controller dai ops

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: <67ed911d-0a01-7f71-9258-495e2e143664@linux.intel.com>
Date:   Wed, 11 Jan 2023 15:59:20 +0100
From:   Amadeusz Sławiński 
        <amadeuszx.slawinski@...ux.intel.com>
To:     Vijendar Mukunda <Vijendar.Mukunda@....com>, broonie@...nel.org,
        vkoul@...nel.org, alsa-devel@...a-project.org
Cc:     Mastan.Katragadda@....com, Sunil-kumar.Dommati@....com,
        Basavaraj.Hiregoudar@....com,
        Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        open list <linux-kernel@...r.kernel.org>,
        Mario.Limonciello@....com, arungopal.kondaveeti@....com,
        Sanyog Kale <sanyog.r.kale@...el.com>,
        Bard Liao <yung-chuan.liao@...ux.intel.com>
Subject: Re: [PATCH 03/19] soundwire: amd: register sdw controller dai ops

On 1/11/2023 10:02 AM, Vijendar Mukunda wrote:
> Register dai ops for two controller instances.
> 
> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@....com>
> ---

> +static int amd_sdwc_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
> +{
> +	struct amd_sdwc_ctrl *ctrl = snd_soc_dai_get_drvdata(dai);
> +	struct sdw_amd_dma_data *dma;
> +	int ret;
> +
> +	dma = snd_soc_dai_get_dma_data(dai, substream);
> +	if (!dma)
> +		return -EIO;
> +
> +	ret = sdw_stream_remove_master(&ctrl->bus, dma->stream);
> +	if (ret < 0) {
> +		dev_err(dai->dev, "remove master from stream %s failed: %d\n",
> +			dma->stream->name, ret);
> +		return ret;
> +	}
> +	dma->hw_params = NULL;
> +	return 0;
> +}
> +
> +static int amd_set_sdw_stream(struct snd_soc_dai *dai, void *stream, int direction)
> +{
> +	struct amd_sdwc_ctrl *ctrl = snd_soc_dai_get_drvdata(dai);
> +	struct sdw_amd_dma_data *dma;
> +
> +	if (stream) {
> +		if (direction == SNDRV_PCM_STREAM_PLAYBACK)
> +			dma = dai->playback_dma_data;
> +		else
> +			dma = dai->capture_dma_data;
> +

The patch itself looks ok, but I have generic ASoC API question. Could 
we perhaps change snd_soc_dai_get_dma_data() definition, so that instead 
of it being:
static inline void *snd_soc_dai_get_dma_data(const struct snd_soc_dai 
*dai, const struct snd_pcm_substream *ss)
it would be something like:
static inline void *snd_soc_dai_get_dma_data(const struct snd_soc_dai 
*dai, int direction)

it would require converting current calls from something like
dma = snd_soc_dai_get_dma_data(dai, substream);
to
dma = snd_soc_dai_get_dma_data(dai, substream->stream);
but would also allow for use in code like above?
It would become just:
dma = snd_soc_dai_get_dma_data(dai, direction);

The more I'm looking at the soc-dai.h header the more I like this idea, 
as other functions in the area seem to pass stream/direction explicitly 
instead of substream.

Mark, what do you think?