linux-kernel - Re: [PATCH 01/25] clk: meson: a1: restrict an amount of 'hifi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cbfd9c66-cca5-49f5-9468-43710c48518e@salutedevices.com>
Date: Sun, 17 Mar 2024 17:17:12 +0300
From: Jan Dakinevich <jan.dakinevich@...utedevices.com>
To: Jerome Brunet <jbrunet@...libre.com>
CC: Neil Armstrong <neil.armstrong@...aro.org>, Michael Turquette
	<mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>, Rob Herring
	<robh@...nel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>, Philipp Zabel <p.zabel@...gutronix.de>,
	Kevin Hilman <khilman@...libre.com>, Martin Blumenstingl
	<martin.blumenstingl@...glemail.com>, Liam Girdwood <lgirdwood@...il.com>,
	Mark Brown <broonie@...nel.org>, Linus Walleij <linus.walleij@...aro.org>,
	Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
	<linux-amlogic@...ts.infradead.org>, <linux-clk@...r.kernel.org>,
	<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>, <alsa-devel@...a-project.org>,
	<linux-sound@...r.kernel.org>, <linux-gpio@...r.kernel.org>,
	<kernel@...utedevices.com>
Subject: Re: [PATCH 01/25] clk: meson: a1: restrict an amount of 'hifi_pll'
 params



On 3/15/24 11:58, Jerome Brunet wrote:
> 
> On Fri 15 Mar 2024 at 02:21, Jan Dakinevich <jan.dakinevich@...utedevices.com> 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@...utedevices.com>
>> ---
>>  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