Re: [PATCH 1/3] drm/i2c: tda998x: Improve tda998x_configure_audio() audio related pdata

[Russell King - ARM Linux <linux@armlinux.org.uk>]

On Tue, Aug 02, 2016 at 03:05:07PM +0300, Jyri Sarha wrote:
> @@ -787,19 +792,13 @@ tda998x_configure_audio(struct tda998x_priv *priv,
>  	reg_clear(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_RST_CTS);
>  
>  	/* Write the channel status */
> -	buf[0] = IEC958_AES0_CON_NOT_COPYRIGHT;
> -	buf[1] = 0x00;
> -	buf[2] = IEC958_AES3_CON_FS_NOTID;
> -	buf[3] = IEC958_AES4_CON_ORIGFS_NOTID |
> -			IEC958_AES4_CON_MAX_WORDLEN_24;
> -	reg_write_range(priv, REG_CH_STAT_B(0), buf, 4);
> +	reg_write_range(priv, REG_CH_STAT_B(0), params->status, 4);

Another couple of points here:

1. I think we should only write the channel status if I2S mode is
selected - channel status won't be used on SPDIF mode as SPDIF
supplies its own channel status.

2. I think we should continue writing all the channel status in a
single I2C transaction, and not breaking it into several different
transactions to cater for the odd register order.  I don't have
any direct evidence that one way is better than the other in terms
of atomicity of the update, but I think having it as a single
transaction stands a better chance of the chip atomically updating
the channel status.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.