Re: [PATCH v2 09/10] ASoC: topology: Change stream formats to bitwise flag

From: Mark Brown <broonie@kernel.org>
To: mengdong.lin@intel.com
Cc: tiwai@suse.de, alsa-devel@alsa-project.org, liam.r.girdwood@intel.com
Subject: Re: [PATCH v2 09/10] ASoC: topology: Change stream formats to bitwise flag
Date: Fri, 14 Aug 2015 21:34:28 +0100	[thread overview]
Message-ID: <20150814203428.GR10748@sirena.org.uk> (raw)
In-Reply-To: <b9a7ab97e4b4f96cced9faa2e2bce2b533d25a10.1439217448.git.mengdong.lin@intel.com>

[-- Attachment #1.1: Type: text/plain, Size: 1770 bytes --]

On Mon, Aug 10, 2015 at 10:48:37PM +0800, mengdong.lin@intel.com wrote:

>  struct snd_soc_tplg_stream_caps {
>  	__le32 size;		/* in bytes of this structure */
>  	char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
> -	__le64 formats[SND_SOC_TPLG_MAX_FORMATS];	/* supported formats SNDRV_PCM_FMTBIT_* */
> +	__le64 formats;	/* supported formats SNDRV_PCM_FORMAT_* */

Argh, this is *another* ABI change which you've buried in the middle of
a series you're sending right at the end of the release cycle (this was
sent after -rc6, we may not even get a -rc7...).  Given how close we are
to the release and the invasiveness of the changes in this series it's
very lucky I even saw it before release, I'm reading this on my flight
to Plumbers which means my bandwidth next week will be limited.

This should have at the very least been sent at the start of the series,
and really should have been sent separately.  It is also worrying that
there is nothing in the subject or changelog talking about the fact that
this is an ABI change or explaining why it is required.  The changelog
was simply a statement of the content of the diff, not a rationale:

| The toplogy user space tool will generate this bitwise flag by using
| SNDRV_PCM_FORMAT_* exposed by asound.h, and the topology core will copy
| this flag when generating DAI streams.

Once the release goes out this sort of incompatible change will be
unacceptable.  Given the number of changes that are going into the ABI
(we also had some others went in last week) I'm now giving some serious
thought to the idea that we should ask Linus to revert the topology code
for v4.2 and waiting for v4.3 so we've got more chance to make sure the
ABI is one we actually want to stick with.  Liam, Takashi I don't know
what you think?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]