On Sep 5 2016 05:45, Takashi Iwai wrote:
Yeah, there are obviously some issues in the current implementation of wm_adsp and ASoC ext ctl. Although I'll unlikely take Sakamoto-san's patchset as is from a few reasons, these issues still should be addressed.
OK. I welcome to abandon this patchset ;)
First off, passing the binary blob directly via TLV callback is incorrect from the ABI perspective. When Vinod proposed the idea via TLV access originally, we thought they the data is encoded in TLV format. Alas, the resulted code didn't do that and it slipped into the upstream without consideration.
+1
Besides that, the second problem is the count value returned via snd_ctl_elem_info, as mentioned in the above. It's beyond the original control API design, and a kind of illegal usage.
+1
(Well, it's a philosophical argument: what one would expect for an element that has neither read nor write access...?)
It's an element with no sense for applications. A waste of codes in kernel land.
So, at this point, the main question is whether we keep this access pattern as is, as a sort of official one, and put some exceptional rule. Charles, how is the situation? Has it been already deployed to real systems?
If we may still change the wm_adsp behavior, we may "fix" the first issue by passing the blob properly encoded in TLV format, at least. OTOH, if we need to keep the current ABI abuse as is, one idea is to add a special flag in SNDRV_CTL_ELEM_ACCESS_* indicating this kind of control, and we define more strictly how the code should be implemented. Currently we can judge this element as a one that has no read/write access but with tlv r/w. But it's way too unclear.
The 'abuse' is a part of my understanding of ALSA SoC part. I need a bit time to switch my mind for this issue.
Regards
Takashi Sakamoto