linux-kernel - Re: [PATCH v2 1/2] ALSA: pcm: add SNDRV_PCM_FORMAT

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: <c9310a8f-d16d-ee8f-fe86-8316e71cf7fc@sakamocchi.jp>
Date:   Sun, 26 Nov 2017 18:27:51 +0900
From:   Takashi Sakamoto <o-takashi@...amocchi.jp>
To:     "Maciej S. Szmigiero" <mail@...iej.szmigiero.name>,
        Timur Tabi <timur@...i.org>,
        Nicolin Chen <nicoleotsuka@...il.com>,
        Xiubo Li <Xiubo.Lee@...il.com>,
        Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>
Cc:     alsa-devel@...a-project.org, Liam Girdwood <lgirdwood@...il.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Mark Brown <broonie@...nel.org>,
        Fabio Estevam <fabio.estevam@....com>,
        linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v2 1/2] ALSA: pcm: add SNDRV_PCM_FORMAT_{S, U}20

Hi,

On Nov 24 2017 08:31, Maciej S. Szmigiero wrote:
> This format is similar to existing SNDRV_PCM_FORMAT_{S,U}20_3 that keep
> 20-bit PCM samples in 3 bytes, however i.MX6 platform SSI FIFO does not
> allow 3-byte accesses (including DMA) so a 4-byte (more conventional)
> format is needed for it.
> 
> Signed-off-by: Maciej S. Szmigiero <mail@...iej.szmigiero.name>
> ---
> Changes from v1: Drop "_4" suffix from these formats since they aren't
> non-standard ones, use empty format slots starting from format number 25
> for them, add information that they are LSB justified formats.
> 
> Corresponding alsa-lib changes will be posted as soon as this patch is
> merged on the kernel side, to keep alsa-lib and kernel synchronized.
> 
>   include/sound/pcm.h         |  8 ++++++++
>   include/sound/soc-dai.h     |  2 ++
>   include/uapi/sound/asound.h |  9 +++++++++
>   sound/core/pcm_misc.c       | 16 ++++++++++++++++
>   4 files changed, 35 insertions(+)
> ...
> diff --git a/sound/core/pcm_misc.c b/sound/core/pcm_misc.c
> index 9be81025372f..c62bfe27106f 100644
> --- a/sound/core/pcm_misc.c
> +++ b/sound/core/pcm_misc.c
> @@ -170,6 +170,22 @@ static struct pcm_format_data pcm_formats[(INT)SNDRV_PCM_FORMAT_LAST+1] = {
>   	[SNDRV_PCM_FORMAT_GSM] = {
>   		.le = -1, .signd = -1,
>   	},
> +	[SNDRV_PCM_FORMAT_S20_LE] = {
> +		.width = 20, .phys = 32, .le = 1, .signd = 1,
> +		.silence = {},
> +	},
> +	[SNDRV_PCM_FORMAT_S20_BE] = {
> +		.width = 20, .phys = 32, .le = 0, .signd = 1,
> +		.silence = {},
> +	},
> +	[SNDRV_PCM_FORMAT_U20_LE] = {
> +		.width = 20, .phys = 32, .le = 1, .signd = 0,
> +		.silence = { 0x00, 0x00, 0x08, 0x00 },
> +	},
> +	[SNDRV_PCM_FORMAT_U20_BE] = {
> +		.width = 20, .phys = 32, .le = 0, .signd = 0,
> +		.silence = { 0x00, 0x08, 0x00, 0x00 },
> +	},
>   	[SNDRV_PCM_FORMAT_SPECIAL] = {
>   		.le = -1, .signd = -1,
>   	},

Before applying this patch:
166         /* FIXME: the following three formats are not defined 
properly yet */
167         [SNDRV_PCM_FORMAT_MPEG] = {
168                 .le = -1, .signd = -1,
169         },
170         [SNDRV_PCM_FORMAT_GSM] = {
171                 .le = -1, .signd = -1,
172         },
173         [SNDRV_PCM_FORMAT_SPECIAL] = {
174                 .le = -1, .signd = -1,
175         },

After applying this patch:

166         /* FIXME: the following three formats are not defined 
properly yet */
167         [SNDRV_PCM_FORMAT_MPEG] = {
168                 .le = -1, .signd = -1,
169         },
170         [SNDRV_PCM_FORMAT_GSM] = {
171                 .le = -1, .signd = -1,
172         },
173         [SNDRV_PCM_FORMAT_S20_LE] = {
174                 .width = 20, .phys = 32, .le = 1, .signd = 1,
175                 .silence = {},
176         },
177         [SNDRV_PCM_FORMAT_S20_BE] = {
178                 .width = 20, .phys = 32, .le = 0, .signd = 1,
179                 .silence = {},
180         },
181         [SNDRV_PCM_FORMAT_U20_LE] = {
182                 .width = 20, .phys = 32, .le = 1, .signd = 0,
183                 .silence = { 0x00, 0x00, 0x08, 0x00 },
184         },
185         [SNDRV_PCM_FORMAT_U20_BE] = {
186                 .width = 20, .phys = 32, .le = 0, .signd = 0,
187                 .silence = { 0x00, 0x08, 0x00, 0x00 },
188         },
189         [SNDRV_PCM_FORMAT_SPECIAL] = {
190                 .le = -1, .signd = -1,
191         },

I think it good to add an alternative comment for each of entry which is 
not defined yet, like:

-> 166         /* FIXME: this format is not defined properly yet */
167         [SNDRV_PCM_FORMAT_MPEG] = {
168                 .le = -1, .signd = -1,
169         },
-> 170         /* FIXME: this format is not defined properly yet */
171         [SNDRV_PCM_FORMAT_GSM] = {
172                 .le = -1, .signd = -1,
173         },
174         [SNDRV_PCM_FORMAT_S20_LE] = {
175                 .width = 20, .phys = 32, .le = 1, .signd = 1,
176                 .silence = {},
177         },
178         [SNDRV_PCM_FORMAT_S20_BE] = {
179                 .width = 20, .phys = 32, .le = 0, .signd = 1,
180                 .silence = {},
181         },
182         [SNDRV_PCM_FORMAT_U20_LE] = {
183                 .width = 20, .phys = 32, .le = 1, .signd = 0,
184                 .silence = { 0x00, 0x00, 0x08, 0x00 },
185         },
186         [SNDRV_PCM_FORMAT_U20_BE] = {
187                 .width = 20, .phys = 32, .le = 0, .signd = 0,
188                 .silence = { 0x00, 0x08, 0x00, 0x00 },
189         },
-> 190         /* FIXME: this format is not defined properly yet */
191         [SNDRV_PCM_FORMAT_SPECIAL] = {
192                 .le = -1, .signd = -1,
193         },


Regards

Takashi Sakamoto