linux-kernel - Re: [PATCH RESEND v2 4/5] clk: mxl: Add validation for register reads/writes

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 for Android: free password hash cracker in your pocket

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

Message-ID: <MN2PR19MB369322D272DF711D2185BBF0B15D9@MN2PR19MB3693.namprd19.prod.outlook.com>
Date:   Wed, 5 Oct 2022 10:52:41 +0000
From:   Rahul Tanwar <rtanwar@...linear.com>
To:     Stephen Boyd <sboyd@...nel.org>,
        "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
        "mturquette@...libre.com" <mturquette@...libre.com>
CC:     linux-lgm-soc <linux-lgm-soc@...linear.com>,
        Yi xin Zhu <yzhu@...linear.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RESEND v2 4/5] clk: mxl: Add validation for register
 reads/writes

[Resend due to mail delivery failure in earlier reply - one email id got 
corrupted somehow in earlier reply]

Hi Stephen,


On 30/9/2022 9:02 am, Stephen Boyd wrote:
> This email was sent from outside of MaxLinear.
> 
> 
> Quoting Rahul Tanwar (2022-09-28 23:10:10)
>> On 29/9/2022 8:20 am, Stephen Boyd wrote:
>>>> +       u32 mask;
>>>>
>>>> +       /*
>>>> +        * Some clocks support parent clock dividers but they do not
>>>> +        * support clock gating (clk enable/disable). Such types of
>>>> +        * clocks might call this function with width as 0 during
>>>> +        * clk_prepare_enable() call. Handle such cases by not doing
>>>> +        * anything during clk_prepare_enable() but handle clk_set_rate()
>>>> +        * correctly
>>>> +        */
>>>> +       if (!width)
>>>> +               return;
>>>
>>> Why are the clk_ops assigned in a way that makes the code get here? Why
>>> can't we have different clk_ops, or not register the clks at all, when
>>> the hardware can't be written?
>>
>>
>> The hardware can actually be written for such clks but only for
>> clk_set_rate() op for setting the clk rate. Just that hardware does not
>> provide any way to enable/disable such clks.
>>
>> Alternative way to handle such clks could be that the clk consumer does
>> not invoke clk_prepare_enable() before invoking clk_set_rate(). But we
>> want to avoid making changes in the clk consumer code to keep it
>> standard. And handle it here by just validating the width parameter.
> 
> Why not have different clk_ops then that doesn't do anything for
> enable/disable and only does it for set_rate?
> 


There is only one clk entry which falls in this category. Adding a
different clk_ops for just one clk would need many more lines of code
addition which appears to be a overkill.

I have removed this change in v3 and used the driver internal flag to
handle this particular clk. That requires minimal change and looks
logical addition.

Thanks,
Rahul


>