On 08/04/16 17:07, Russell King - ARM Linux wrote: > On Tue, Aug 02, 2016 at 03:05:08PM +0300, Jyri Sarha wrote: >> + memcpy(audio.status, params->iec.status, >> + min(sizeof(audio.status), sizeof(params->iec.status))); > > As mentioned in the other patch, the audio status does not directly > correspond with the AES bytes, so this ends up causing the driver to > write the wrong bytes to the wrong registers. > As I said earlier, I'd rather have it as plain AES/IEC958 channel status bits buffer. >> + ret = tda998x_configure_audio(priv, >> + &audio, >> + priv->encoder.crtc->hwmode.clock); > > What happens if audio changes at the same time that the video mode > changes? What protection is there against two threads concurrently > executing tda998x_configure_audio() ? > Oh, yes. I definitely need some locking here. The same lock could protect the atomicity of tda998x_audio_params update and usage. >> + priv->audio_pdev = platform_device_register_data( >> + dev, HDMI_CODEC_DRV_NAME, PLATFORM_DEVID_AUTO, >> + &codec_data, sizeof(codec_data)); > > I'd much prefer that this was a child of the I2C device, which will > show the relationship between this virtual platform device and the > device which it's associated with. That can be done via > platform_device_register_full(). > That may be a problem. The ASoC card device tree binding current looks for the ASoC DAI's parent's of-node if it can not find the node for the DAI-device itself. Indicating ASoC DAI-link by referencing the parent i2c device simply won't work, because there may be other ASoC codecs on the same i2c bus. Adding a separate DT-node for a virtual audio device should be possible, but it needs some thinking and may have its own problems. I can not follow the reasoning behind this, is this really necessary? Best regards, Jyri -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
