On Mon, Aug 03, 2020 at 04:04:23PM +0800, Shengjiu Wang wrote:
clock generation. The TCSR.TE is no need to enabled when only RX is enabled.
You are correct if there's only RX running without TX joining. However, that's something we can't guarantee. Then we'd enable TE after RE is enabled, which is against what RM recommends:
# From 54.3.3.1 Synchronous mode in IMX6SXRM # If the receiver bit clock and frame sync are to be used by # both the transmitter and receiver, it is recommended that # the receiver is the last enabled and the first disabled.
I remember I did this "ugly" design by strictly following what RM says. If hardware team has updated the RM or removed this limitation, please quote in the commit logs.
There is no change in RM and same recommandation.
My change does not violate the RM. The direction which generates the clock is still last enabled.
Using Tx syncing with Rx clock for example, T1: arecord (non-stop) => set RE T2: aplay => set TE then RE (but RE is already set at T1)
Anything that I am missing?
if (!sai->synchronous[TX] && sai->synchronous[RX] && !tx) {regmap_update_bits(sai->regmap, FSL_SAI_xCSR((!tx), ofs),FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);} else if (!sai->synchronous[RX] && sai->synchronous[TX] && tx) {regmap_update_bits(sai->regmap, FSL_SAI_xCSR((!tx), ofs),FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);Two identical regmap_update_bits calls -- both on !tx (RX?)
The content for regmap_update_bits is the same, but the precondition is different. The first one is for tx=false and enable TCSR.TE. (TX generate clock) The second one is for tx=true and enable RSCR.RE (RX generate clock)
Why not merge them?
+ if ((!sai->synchronous[TX] && sai->synchronous[RX] && !tx) || + ((!sai->synchronous[RX] && sai->synchronous[TX] && tx) {