linux-kernel - Re: [PATCH 1/3] clk: imx: pll14xx: avoid modify dram pll

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] [day] [month] [year] [list]

Date:   Tue, 14 Jan 2020 00:11:48 +0000
From:   Leonard Crestez <leonard.crestez@....com>
To:     Shawn Guo <shawnguo@...nel.org>, Peng Fan <peng.fan@....com>
CC:     "sboyd@...nel.org" <sboyd@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        Abel Vesa <abel.vesa@....com>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        dl-linux-imx <linux-imx@....com>,
        Aisheng Dong <aisheng.dong@....com>,
        "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Anson Huang <anson.huang@....com>, Jacky Bai <ping.bai@....com>
Subject: Re: [PATCH 1/3] clk: imx: pll14xx: avoid modify dram pll

On 12.01.2020 04:33, Shawn Guo wrote:
> On Mon, Dec 30, 2019 at 09:13:00AM +0000, Peng Fan wrote:
>> From: Peng Fan <peng.fan@....com>
>>
>> The dram pll is only expected to be modified in firmware,
>> so we should only support read clk frequency in Linux Kernel.
>>
>> Signed-off-by: Peng Fan <peng.fan@....com>
> 
> @Leonard, do you agree?

I'm not sure this improves anything.

As far as I understand the only way this could be a problem is if a 
driver deliberately does clk_set_rate on the dram pll and this shouldn't 
happen.

There is an infinite number of ways for clock consumers to break the 
system by manipulating clocks and providers don't need to guard against 
every scenario.

>> ---
>>   drivers/clk/imx/clk-pll14xx.c | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
>> index 5b0519a81a7a..9288b21d4d59 100644
>> --- a/drivers/clk/imx/clk-pll14xx.c
>> +++ b/drivers/clk/imx/clk-pll14xx.c
>> @@ -69,8 +69,6 @@ struct imx_pll14xx_clk imx_1443x_pll = {
>>   
>>   struct imx_pll14xx_clk imx_1443x_dram_pll = {
>>   	.type = PLL_1443X,
>> -	.rate_table = imx_pll1443x_tbl,
>> -	.rate_count = ARRAY_SIZE(imx_pll1443x_tbl),
>>   	.flags = CLK_GET_RATE_NOCACHE,
>>   };
>>   
>> @@ -376,6 +374,10 @@ static const struct clk_ops clk_pll1443x_ops = {
>>   	.set_rate	= clk_pll1443x_set_rate,
>>   };
>>   
>> +static const struct clk_ops clk_pll1443x_min_ops = {
>> +	.recalc_rate	= clk_pll1443x_recalc_rate,
>> +};
>> +
>>   struct clk_hw *imx_clk_hw_pll14xx(const char *name, const char *parent_name,
>>   				  void __iomem *base,
>>   				  const struct imx_pll14xx_clk *pll_clk)
>> @@ -403,7 +405,10 @@ struct clk_hw *imx_clk_hw_pll14xx(const char *name, const char *parent_name,
>>   			init.ops = &clk_pll1416x_ops;
>>   		break;
>>   	case PLL_1443X:
>> -		init.ops = &clk_pll1443x_ops;
>> +		if (!pll_clk->rate_table)
>> +			init.ops = &clk_pll1443x_min_ops;
>> +		else
>> +			init.ops = &clk_pll1443x_ops;
>>   		break;
>>   	default:
>>   		pr_err("%s: Unknown pll type for pll clk %s\n",