Messages in this thread | | | From | Rahul Tanwar <> | Subject | Re: [PATCH RESEND v2 4/5] clk: mxl: Add validation for register reads/writes | Date | Wed, 5 Oct 2022 10:52:41 +0000 |
| |
[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
>
| |