On 3/15/24 11:58, Jerome Brunet wrote:
>
> On Fri 15 Mar 2024 at 02:21, Jan Dakinevich <jan.dakinevich@xxxxxxxxxxxxxxxxx> wrote:
>
>> Existing values were insufficient to produce accurate clock for audio
>> devices. New values are safe and most suitable to produce 48000Hz sample
>> rate.
>
> The hifi pll is not about 48k only. I see no reason to restrict the PLL
> to a single setting.
> > You've provided no justification why the PLL driver can't reach the same
> setting for 48k. The setting below is just the crude part. the fine
> tuning is done done with the frac parameter so I doubt this provides a
> more accurate rate.
>
You are right, it is not about 48k only. However, there are two issues.
First, indeed, I could just extend the range of multipliers to 1..255.
But I am unsure if hifi_pll is able to handle whole range of
mulptipliers. The value 128 is taken from Amlogic's branch, and I am
pretty sure that it works.
Second, unfortunately frac parameter currently doesn't work. When frac
is used enabling of hifi_pll fails in meson_clk_pll_wait_lock(). I see
it when try to use 44100Hz and multipliers' range is set to 1..255. So,
support of other rates than 48k requires extra effort.
>>
>> Signed-off-by: Jan Dakinevich <jan.dakinevich@xxxxxxxxxxxxxxxxx>
>> ---
>> drivers/clk/meson/a1-pll.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/clk/meson/a1-pll.c b/drivers/clk/meson/a1-pll.c
>> index 4325e8a6a3ef..00e06d03445b 100644
>> --- a/drivers/clk/meson/a1-pll.c
>> +++ b/drivers/clk/meson/a1-pll.c
>> @@ -74,9 +74,9 @@ static struct clk_regmap fixed_pll = {
>> },
>> };
>>
>> -static const struct pll_mult_range hifi_pll_mult_range = {
>> - .min = 32,
>> - .max = 64,
>> +static const struct pll_params_table hifi_pll_params_table[] = {
>> + PLL_PARAMS(128, 5),
>> + { },
>> };
>>
>> static const struct reg_sequence hifi_init_regs[] = {
>> @@ -124,7 +124,7 @@ static struct clk_regmap hifi_pll = {
>> .shift = 6,
>> .width = 1,
>> },
>> - .range = &hifi_pll_mult_range,
>> + .table = hifi_pll_params_table,
>> .init_regs = hifi_init_regs,
>> .init_count = ARRAY_SIZE(hifi_init_regs),
>> },
>
>
--
Best regards
Jan Dakinevich
