On Fri, Jul 17, 2020 at 5:58 PM Arnaud Ferraris <arnaud.ferraris@xxxxxxxxxxxxx> wrote: > > > > Le 17/07/2020 à 01:37, Nicolin Chen a écrit : > >> @@ -507,8 +507,7 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair, bool use_ideal_rate) > >> > >> /* Enable Ideal Ratio mode */ > > > > The code is against the comments now -- need to update this line. > > It isn't, the following code still enables "Ideal Ratio" mode (see below) > > >> regmap_update_bits(asrc->regmap, REG_ASRCTR, > >> - ASRCTR_IDRi_MASK(index) | ASRCTR_USRi_MASK(index), > >> - ASRCTR_IDR(index) | ASRCTR_USR(index)); > >> + ASRCTR_IDRi_MASK(index), ASRCTR_IDR(index); > > > > The driver falls back to ideal ratio mode if there is no matched > > clock source. Your change seems to apply internal ratio mode any > > way? Probably would break the fallback routine. > > Strictly speaking, internal ratio is only enabled when we have matched > clock sources, and is used in addition to the calculated dividers > (allows the ASRC to better adjust to drifting/inaccurate physical > clocks). "Ideal Ratio" mode is different, and still enabled as a > fallback when no clock source is matched. > > Ideal ratio requires both USRi and IDRi bits to be set, and that would > still be the case if there is no matched clock source. > > The only difference my patch introduces is that USRi is always set (was > previously cleared for "normal" mode), and therefore only IDRi needs to > be set in order to enable ideal ratio mode. > In my experience, the USRi = 0, no matter the value of IDRi, it is internal ratio mode. USRi=1, IDRi=0, it is also internal ratio mode. So original code should be ok for internal ratio mode, no need to add this change. could you please double check it? best regards wang shengjiu
