clk: implement clock rate protection mechanism
Get series with b4
b4 am [email protected]Versions newest first · linked by author and title
- v3 Latest posting 2/23 of 10 patches 0 repliesNew
- v3 posting 1/29 of 10 patches 0 repliesSuperseded
3 of 10 patches are in this archive. The cover letter announces 10.
Patches in v3 posting 2/2 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 4/10 | clk: use round rate to bail out early in set_rate | +0 | 0 replies | |
| 5/10 | clk: add support for clock protection | +0 | 3 replies | |
| 5/10 | clk: add support for clock protection | +0 | 7 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v3 yet.
Cover letter [PATCH v3 00/10] clk: implement clock rate protection mechanism
Quoted 39 lines · Jerome Brunet
Quoted 38 lines · Stephen Boyd
Quoted 36 lines · Jerome Brunet
I managed to have a run on kci - based on v4.12-rc6:
https://kernelci.org/boot/all/job/khilman/branch/to-build/kernel/v4.12-rc6-1
0-ge
a373ddef830/There was no build regression but kci did find one boot regression on qcom platforms:
* qcom-apq8064-cm-qs600
* qcom-apq8064-ifc6410it seems the problem is coming from the clock used by the mmc driver (
drivers/mmc/host/mmci.c)the driver does following sequence:
*get_clk
*prepare_enable
*get_rate
*set_rate
* ...with clock
SDCx_clk(qcom_apq8064.dtsi:1037). This clock hasCLK_SET_RATE_PARENT
flag so it will transmit the request to its parent.
The parent of this clock isSDCx_srcwhich has theCLK_SET_RATE_GATEflag.This can't work with sequence above. The flag was clearly bypassed before, so I
think it would be best to removeCLK_SET_RATE_GATEfrom theSDCx_srcclocksStephen, would you agree ?
There was an issue with the chip board as well but this particular board has not
been very reliable in this lab lately. It has failed on several other job.
Quoted 8 lines
Quoted 6 lines
Yes the flag was never working before and we've been ignoring the subtle problems that it causes to change the rate while the clk is enabled. On these older SoCs, all the clks marked with
CLK_SET_RATE_GATEneed to implement something to forcibly disable all child clks across the rate change. That is the hardware recommended procedure.Seems that HW has been doing fine w/o recommended procedure so far ;)
Dubious at best. There's only light testing with upstream kernels on these SoCs, and probably "doing fine" is more like "nobody has reported a problem" in this case.
Quoted 20 lines
Quoted 13 lines
So something like
get_clk
prepare_enable
get_rate
set_rate
call ->set_rate() op
forcibly disable children of rcgs actually change the rate of rcg forcibly enable all children that were disabledQuoted 6 lines
and have that happen under one big spinlock local to the qcom driver I suppose so that enable/disable can't leak into the rate change. Fun! I'll make an attempt at implementing the right solution tomorrow/tonight. Of course, we can remove the flag from qcom drivers if it's blocking this series.
Quoted 6 lines
Yeah, it is probably blocking the series.
In the following mail, I've only replied where I wanted to discuss a bit more. For everything else, please consider the comments agreed and the modifications done as requested.
Ok. Good that we can move forward in some way so this is not a blocker. I've almost completed writing the qcom code to force clks on and off so it will be resolved one way or the other soon.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project