[alsa-devel] [PATCH 1/4] ALSA: control: return payload length for TLV operation

[Takashi Iwai]

On Mon, 12 Sep 2016 18:03:16 +0200,
Charles Keepax wrote:
> 
> On Mon, Sep 12, 2016 at 05:28:58PM +0200, Takashi Iwai wrote:
> > On Mon, 12 Sep 2016 17:25:31 +0200,
> > Vinod Koul wrote:
> > > 
> > > On Mon, Sep 12, 2016 at 01:37:37PM +0100, Charles Keepax wrote:
> > > > On Tue, Sep 06, 2016 at 12:30:35PM +0900, Takashi Sakamoto wrote:
> > > > > On Sep 5 2016 05:45, Takashi Iwai wrote:
> > > 
> > > Sorry havent been able to follow this yet :(
> > > 
> > > 
> > > But yes current Skylake Chromebooks ship with this code so we cant break it.
> > > 
> > > I am not sure what is the issue with API though. (sorry haven't read the
> > > thread yet). The tlv was designed to allow people send bytes larger than 512
> > > down to kernel. The Type cna be anything (implementation specific, though we
> > > haven't used it yet), length the blob length and then the bytes blob.
> > 
> > Yes, and this part is missing in wm_adsp driver.  It passes the blob
> > without TLV encoding, i.e. starting from the offset zero without type
> > and length encoding.
> > 
> > > We provide a tunnel and pass these to DSP. They maybe module coefficients,
> > > hotwording blobs etc.
> > 
> > So, does Intel driver pass the blob in TLV format?  Then we have two
> > different implementations.
> > 
> 
> So looking at the Skylake code it does look like certainly the
> read opertaion returns a TLV header, its a bit less clear with
> the write operation it looks like it does use one but sometimes
> it will be passed straight through to the firmware.
> 
> So looks more like we really should take the pain of the ABI
> change and update wm_adsp to be consistent, two implementations
> is not good. Although I do feel we should add/strip the header in
> ASoC rather than expecting the driver callbacks to do so, seems
> odd to push that requirement into end drivers, it has certainly
> taken me by surpise. Also I guess with the current tinyalsa
> implementation the end user has to add/strip the headers whereas
> really I would have thought tinyalsa should be doing that.

If we can align the behavior, it'd be great.
And, if we can fix the wm_adsp side, we can have the common handler of
TLV inside the ASoC caller side, as one of Sakamoto-san's patches
showed.

> > Also, still another point is to be decided: is passing an arbitrary
> > size via info callback for an element without read/write access bits
> > (but with TLV bit) a right behavior?
> > 
> 
> So I guess the question would be if you couldn't read the
> controls size from info how would you find out the control size?

Well, I'm not against the idea to expose the size in callback.  The
behavior without read/write bits is just undefined, and we need a
clear definition to avoid further confusion.  I guess the introduction
of a new flag would be the start.


thanks,

Takashi