linux-kernel - Re: [PATCH v2 2/8] ASoC: qdsp6: q6afe: add support to pcm ports

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: <20200210133143.GG7685@sirena.org.uk>
Date:   Mon, 10 Feb 2020 13:31:43 +0000
From:   Mark Brown <broonie@...nel.org>
To:     Adam Serbinski <adam@...binski.com>
Cc:     Srini Kandagatla <srinivas.kandagatla@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Andy Gross <agross@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Patrick Lai <plai@...eaurora.org>,
        Banajit Goswami <bgoswami@...eaurora.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, alsa-devel@...a-project.org,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/8] ASoC: qdsp6: q6afe: add support to pcm ports

On Sun, Feb 09, 2020 at 10:47:42AM -0500, Adam Serbinski wrote:

>  
> +#define AFE_API_VERSION_PCM_CONFIG	0x1
> +/* Enumeration for the auxiliary PCM synchronization signal
> + * provided by an external source.
> + */
> +
> +#define AFE_PORT_PCM_SYNC_SRC_EXTERNAL 0x0
> +/*	Enumeration for the auxiliary PCM synchronization signal
> + * provided by an internal source.
> + */

This is a *weird* commenting style for these #defines and it's not
consistent within the block, I'm seeing at least 3 different styles.

> +/*  Payload of the #AFE_PARAM_ID_PCM_CONFIG command's
> + * (PCM configuration parameter).
> + */
> +
> +struct afe_param_id_pcm_cfg {

Similar weird commenting here, please follow coding-style.rst.

> +	switch (cfg->fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> +	case SND_SOC_DAIFMT_CBS_CFS:
> +		pcfg->pcm_cfg.sync_src = AFE_PORT_PCM_SYNC_SRC_INTERNAL;
> +		break;
> +	case SND_SOC_DAIFMT_CBM_CFM:
> +		/* CPU is slave */
> +		pcfg->pcm_cfg.sync_src = AFE_PORT_PCM_SYNC_SRC_EXTERNAL;
> +		break;
> +	default:
> +		break;
> +	}

Why is this not returning an error on unsupported values?

> +
> +	switch (cfg->sample_rate) {
> +	case 8000:
> +		pcfg->pcm_cfg.frame_setting = AFE_PORT_PCM_BITS_PER_FRAME_128;
> +		break;
> +	case 16000:
> +		pcfg->pcm_cfg.frame_setting = AFE_PORT_PCM_BITS_PER_FRAME_64;
> +		break;
> +	}

Same here.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)