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 _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
