RE: [PATCH] ASoC: brcm: Add DSL/PON SoC audio driver

[Kevin Li <kevin-ke.li@broadcom.com>]

Hi Mark,

The SoC I2S block we currently have shares one clock and frame sync signal
for both playback and capture stream, plus playback and capture can only
have one master at a time. If we set playback and capture master at same
time, it will have jitter on clock and FS.

Based on above, for playback and capture, whichever starts first, it will
be master, another stream will be slave if it starts before the first
stream shutting  down. So working as master or slave for a stream is
depending on another stream's status.

Same thing for shutting down a stream. A master stream shutting down will
have to set another stream to master if another stream is on working
status as a slave. A glitch/Jitter will happen at this moment. But we
minimize it.

That is why this master/slave mode handles in startup/shutdown functions.
Not sure how other company handles this kind of I2S block.

Let me know if it is still not clear.

Yes, set_fmt() is not needed. Will be addressed along with rest questions
you mentioned on the next patch, with a patch version.


Thanks!
Kevin


> +	struct bcm_i2s_priv *i2s_priv = snd_soc_dai_get_drvdata(dai);
> +	struct regmap *regmap_i2s = i2s_priv->regmap_i2s;
> +
> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +		regmap_update_bits(regmap_i2s, I2S_TX_CFG,
> +				   I2S_TX_OUT_R | I2S_TX_DATA_ALIGNMENT |
> +				   I2S_TX_DATA_ENABLE |
I2S_TX_CLOCK_ENABLE,
> +				   I2S_TX_OUT_R | I2S_TX_DATA_ALIGNMENT |
> +				   I2S_TX_DATA_ENABLE |
I2S_TX_CLOCK_ENABLE);
> +		regmap_write(regmap_i2s, I2S_TX_IRQ_CTL, 0);
> +		regmap_write(regmap_i2s, I2S_TX_IRQ_IFF_THLD, 0);
> +		regmap_write(regmap_i2s, I2S_TX_IRQ_OFF_THLD, 1);
> +
> +		regmap_read(regmap_i2s, I2S_RX_CFG_2, &slaveMode);
> +		if (slaveMode & I2S_RX_SLAVE_MODE_MASK)
> +			regmap_update_bits(regmap_i2s, I2S_TX_CFG_2,
> +					   I2S_TX_SLAVE_MODE_MASK,
> +					   I2S_TX_MASTER_MODE);
> +		else
> +			regmap_update_bits(regmap_i2s, I2S_TX_CFG_2,
> +					   I2S_TX_SLAVE_MODE_MASK,
> +					   I2S_TX_SLAVE_MODE);

Setting master or slave mode should be done with a set_fmt() operation but
your set_fmt() operation was empty.  How would this be configured?