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

[Lin, Mengdong]

> -----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?
But alsa-lib already integrated this ABI version.

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

Thanks
Mengdong

> 
> > We can increase the topology ABI number when there is ABI change, and
> the topology driver always checks this version.
> 
> It's a big NO, sorry, this isn't acceptable.  Again, the kernel must give the
> backward compatibility.  That is, the kernel must accept the old dialect the
> user space talks if user space wants.  You cannot force user space to
> communicate in a new ABI out of sudden.
> 
> So, yes, ABI version bump is needed if you change ABI.  But it means that the
> kernel accepts all ABIs up to this version, not meaning
> *only* this ABI version.
> 
> Hopefully this clarifies the situation.
> 
> 
> thanks,
> 
> Takashi