Messages in this thread | | | Date | Tue, 4 Aug 2020 01:13:34 -0700 | From | Nicolin Chen <> | Subject | Re: [PATCH] ASoC: fsl_sai: Clean code for synchronize mode |
| |
On Tue, Aug 04, 2020 at 03:53:51PM +0800, Shengjiu Wang wrote:
> > > /* Check if the opposite FRDE is also disabled */ > > > regmap_read(sai->regmap, FSL_SAI_xCSR(!tx, ofs), &xcsr); > > > + if (sai->synchronous[tx] && !sai->synchronous[!tx] && !(xcsr & FSL_SAI_CSR_FRDE)) > > > + fsl_sai_config_disable(sai, !tx); > > > > > + if (sai->synchronous[tx] || !sai->synchronous[!tx] || !(xcsr & FSL_SAI_CSR_FRDE)) > > > + fsl_sai_config_disable(sai, tx); > > > > The first "||" should probably be "&&". > > No. it is !(!sai->synchronous[tx] && sai->synchronous[!tx] && (xcsr & > FSL_SAI_CSR_FRDE)) > so then convert to > (sai->synchronous[tx] || !sai->synchronous[!tx] || !(xcsr & FSL_SAI_CSR_FRDE)) > > if change to &&, then it won't work for: > sai->synchronous[tx] = false, sai->synchronous[!tx]=false.
Ahh..probably should be if (!(sync[dir] && !sync[adir]) || !frde)
I have a (seemingly) correct version in my sample code.
And...please untangle the logic using the given example -- adding helper function(s) and comments. And, though the driver does have places using array[tx] and array[!tx], better not to use any more boolean type variable as an array index, as it's hard to read.
| |