On 12/03/2015 01:03 PM, Takashi Iwai wrote:
On Thu, 03 Dec 2015 12:13:04 +0100, Russell King - ARM Linux wrote:
On Thu, Dec 03, 2015 at 11:58:14AM +0100, Moise Gergaud wrote:
On 12/02/2015 08:00 PM, Russell King - ARM Linux wrote:
Again, I'm not happy with this change. The intention here is that we validate all arguments before we change anything. This breaks that principle.
We propose this change for compressed mode (IEC61937/non linear PCM). In case of compressed mode, iec958 channel status byte 0 bit1 = 1, then we cannot use snd_pcm_create_iec958_consumer.
Why can you not use snd_pcm_create_iec958_consumer() for this case? You are allowed to modify the values you get afterwards in order to handle this case if you so wish to toggle this bit.
However, if you are using data mode, you really should be using the AES bytes passed by the application through alsalib and into the kernel via the IEC958 controls. There are three controls specified for this:
SNDRV_CTL_NAME_IEC958("", PLAYBACK, CON_MASK) - the bits which can be changed for consumer mode. SNDRV_CTL_NAME_IEC958("", PLAYBACK, PRO_MASK) - the bits which can be changed for professional mode. SNDRV_CTL_NAME_IEC958("", PLAYBACK, DEFAULT) - the value of the changable bits.
When using this, it is userspace's responsibility to set the sample rate appropriately for the stream.
The only case to use snd_pcm_create_iec958_consumer() is to generate the status bytes for PCM audio, where the userspace API doesn't facilitiate generating and/or passing the AES status bytes.
That's true. OTOH, in most drivers, we try to be kind not to be too strict for this requirement and adjust the status bits accordingly. This comes from the lessons we learned how user-space doesn't care the things.
So, if we want to keep that good uncle attitude, this change doesn't look so bad to me.
shall I deliver a new version of this patch with header correction or shall I abandon this patch?
thanks,
Takashi