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

[Takashi Iwai]

On Sat, 15 Aug 2015 17:25:18 +0200,
Lin, Mengdong wrote:
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai at suse.de]
> > Sent: Saturday, August 15, 2015 10:46 PM
>  
> > On Sat, 15 Aug 2015 15:49:42 +0200,
> > Lin, Mengdong wrote:
> > >
> > > > -----Original Message-----
> > > > From: Takashi Iwai [mailto:tiwai at suse.de]
> > > > Sent: Saturday, August 15, 2015 3:38 PM
> > >
> > > > On Fri, 14 Aug 2015 22:34:28 +0200,
> > > > Mark Brown wrote:
> > > > >
> > > > > On Mon, Aug 10, 2015 at 10:48:37PM +0800, mengdong.lin at 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.
> > > >
> > > > Thanks for checking this.  Mengdong, if there is a change in uapi/*,
> > > > this means touching ABI.  So, this change isn't acceptable once when
> > > > the kernel is released.  At least, the backward compatibility *must*
> > > > be kept no matter which version is.
> > >
> > > Thanks for your review, Mark and Takashi.
> > > This series including the ABI change, is not for v4.2, but is an RFC to adding
> > DAI/DAI links support in topology.
> > > I'm sorry that I should have explicitly said this in my comments.
> > >
> > > Is there some topic branch which can accept topology patches for next
> > kernel release?
> > > Can we change topology ABI in v4.3?
> > 
> > In general "yes, but only if it's backward-compatible".
> > 
> > > Now only Intel SKL driver is using topology code, so the affect should be
> > limited.
> > > And I feel using bitwise flag is simpler in both user space and in kernel
> > coding.
> > 
> > Yes, using FMTBIT_* is better, but this must not justify to break ABI.  Either
> > we disable this API/ABI for 4.2, or make the change backward compatible.
> 
> Many thanks for your clarification!
> We'd better keep current ABI unchanged to keep backward compatibility.
> 
> In addition, does "disable this API/ABI for 4.2" mean disabling all topology features, or only some structures for v4.2?

I suppose the whole topology feature.

> But alsa-lib already integrated this ABI version.

Yes, but it's no big problem, as we haven't released alsa-lib changes
yet.

> I'm worried that If we have to add new fields to ABI structures, how to keep the backward compatibility.

There are reserved areas for the future extension, and usually take a
part of them for the additional fields.  Even if doing that, it's
still very sensitive to change the structure in general, though.


Takashi