| Message ID | 20220816112530.1837489-1-maxime@cerno.tech (mailing list archive) |
|---|---|
| Headers | show |
| Series | clk: More clock rate fixes and tests | expand |
Hello Maxime, Am Dienstag, 16. August 2022, 13:25:05 CEST schrieb Maxime Ripard: > Thanks to the feedback I got on the previous series, I found and fixed a > number of bugs in the clock framework and how it deals with rates, > especially when it comes to orphan clocks. > > In order to make sure this doesn't pop up again as a regression, I've > extended the number of tests. > > The first patch reintroduces the clk_set_rate_range call on clk_put, but > this time will only do so if there was a range set on that clock to > begin with. It should be less intrusive, and reduce the number of > potential side effects considerably. > > We then have a fix for the qcom rcg2 issue that has been reported > recently. > > All the other patches should be probably be flagged as fixes, but > they've never seem to have shown any real-world issues until now, and > they aren't all really trivial to backport either, so I'm not sure it's > worth it. > > There's also some documentation improvements for recalc_rate and > clk_get_rate to hopefully make the documentation less ambiguous and > acknowledge that recalc_rate() returning 0 on error is fine. > > Let me know what you think, > Maxime > > Changes from v8: > - Fixed a regression when probing a clock driver backed by a device > accessed through a bus that might sleep With this v9 series my TQMa8MPxL platform boots without issue, also with the i2c attached audio codec driver successfully probed. Thanks! Best regards, Alexander > Changes from v7: > - Dropped the RPi fixes > - Rebased on 6.0-rc1 > > Changes from v6: > - Fixed a kernel build bot warning > > Changes from v5: > - Rebased on current next (next-20220711) > - Dropped clk_get_rate_range, and used a custom function instead > - Switched all tests to use clk_hw_get_clk() instead of struct clk_hw->clk > - Removed some intermediate variables > - Added some comments > - Dropped clk_get_parent() changes > - Dropped test on clk_hw pointer non-NULL in clk_hw_get_name > - Made clk_has_parent more const > > Changes from v4: > - Fix build breakage on SAM9x60 > > Changes from v3: > - constness warning fix in clk_core_forward_rate_req > > Changes from v2: > - Rebased on top of current next > - Fixed locking issue in clk_get_rate_range > > Changes from v1: > - Rebased on top of next-20220428 > - Dropped the patch to prevent non-orphan clocks from registering if > their recalc_rate hook returns 0 > - Added some patches to clarify the clk_get_rate and recalc_rate > documentation > - Dropped the patch to skip the range setup on an orphan clock that > was introducing a regression on RaspberryPi3 when a monitor wasn't > connected at boot > - Added a patch to skip the rate clamping in clk_round_rate() when > min_rate == max_rate == 0 > - Added a new set of functions to query the clk boundaries and fix a > regression with the RaspberryPi4 > - Fixed all the drivers hand-crafting their clk_rate_request > - Reworded the test suite descriptions > - Reordered a few patches to ease the review > - Reworded some commit logs to better explain the issues they address > - Collected the Tested-by of Alexander and Marek > - More tests > > Maxime Ripard (25): > clk: test: Switch to clk_hw_get_clk > clk: Drop the rate range on clk_put() > clk: Skip clamping when rounding if there's no boundaries > clk: Mention that .recalc_rate can return 0 on error > clk: Clarify clk_get_rate() expectations > clk: tests: Add test suites description > clk: tests: Add reference to the orphan mux bug report > clk: tests: Add tests for uncached clock > clk: tests: Add tests for single parent mux > clk: tests: Add tests for mux with multiple parents > clk: tests: Add some tests for orphan with multiple parents > clk: Take into account uncached clocks in clk_set_rate_range() > clk: Set req_rate on reparenting > clk: Change clk_core_init_rate_req prototype > clk: Move clk_core_init_rate_req() from clk_core_round_rate_nolock() > to its caller > clk: Introduce clk_hw_init_rate_request() > clk: Add our request boundaries in clk_core_init_rate_req > clk: Switch from __clk_determine_rate to clk_core_round_rate_nolock > clk: Introduce clk_core_has_parent() > clk: Constify clk_has_parent() > clk: Stop forwarding clk_rate_requests to the parent > clk: Zero the clk_rate_request structure > clk: Introduce the clk_hw_get_rate_range function > clk: qcom: clk-rcg2: Take clock boundaries into consideration for > gfx3d > clk: tests: Add missing test case for ranges > > drivers/clk/at91/clk-generated.c | 5 +- > drivers/clk/at91/clk-master.c | 9 +- > drivers/clk/at91/clk-peripheral.c | 4 +- > drivers/clk/clk-composite.c | 6 +- > drivers/clk/clk-divider.c | 20 +- > drivers/clk/clk.c | 288 ++++-- > drivers/clk/clk_test.c | 1413 ++++++++++++++++++++++++++++- > drivers/clk/qcom/clk-rcg2.c | 9 + > include/linux/clk-provider.h | 18 +- > include/linux/clk.h | 2 +- > 10 files changed, 1665 insertions(+), 109 deletions(-)
On Tue, 16 Aug 2022 at 16:55, Maxime Ripard <maxime@cerno.tech> wrote: > > Hi, > > Thanks to the feedback I got on the previous series, I found and fixed a > number of bugs in the clock framework and how it deals with rates, > especially when it comes to orphan clocks. > > In order to make sure this doesn't pop up again as a regression, I've > extended the number of tests. > > The first patch reintroduces the clk_set_rate_range call on clk_put, but > this time will only do so if there was a range set on that clock to > begin with. It should be less intrusive, and reduce the number of > potential side effects considerably. > > We then have a fix for the qcom rcg2 issue that has been reported > recently. > > All the other patches should be probably be flagged as fixes, but > they've never seem to have shown any real-world issues until now, and > they aren't all really trivial to backport either, so I'm not sure it's > worth it. > > There's also some documentation improvements for recalc_rate and > clk_get_rate to hopefully make the documentation less ambiguous and > acknowledge that recalc_rate() returning 0 on error is fine. > > Let me know what you think, > Maxime > > Changes from v8: > - Fixed a regression when probing a clock driver backed by a device accessed > through a bus that might sleep > > Changes from v7: > - Dropped the RPi fixes > - Rebased on 6.0-rc1 > > Changes from v6: > - Fixed a kernel build bot warning > > Changes from v5: > - Rebased on current next (next-20220711) > - Dropped clk_get_rate_range, and used a custom function instead > - Switched all tests to use clk_hw_get_clk() instead of struct clk_hw->clk > - Removed some intermediate variables > - Added some comments > - Dropped clk_get_parent() changes > - Dropped test on clk_hw pointer non-NULL in clk_hw_get_name > - Made clk_has_parent more const > > Changes from v4: > - Fix build breakage on SAM9x60 > > Changes from v3: > - constness warning fix in clk_core_forward_rate_req > > Changes from v2: > - Rebased on top of current next > - Fixed locking issue in clk_get_rate_range > > Changes from v1: > - Rebased on top of next-20220428 > - Dropped the patch to prevent non-orphan clocks from registering if > their recalc_rate hook returns 0 > - Added some patches to clarify the clk_get_rate and recalc_rate > documentation > - Dropped the patch to skip the range setup on an orphan clock that > was introducing a regression on RaspberryPi3 when a monitor wasn't > connected at boot > - Added a patch to skip the rate clamping in clk_round_rate() when > min_rate == max_rate == 0 > - Added a new set of functions to query the clk boundaries and fix a > regression with the RaspberryPi4 > - Fixed all the drivers hand-crafting their clk_rate_request > - Reworded the test suite descriptions > - Reordered a few patches to ease the review > - Reworded some commit logs to better explain the issues they address > - Collected the Tested-by of Alexander and Marek > - More tests > > Maxime Ripard (25): > clk: test: Switch to clk_hw_get_clk > clk: Drop the rate range on clk_put() > clk: Skip clamping when rounding if there's no boundaries > clk: Mention that .recalc_rate can return 0 on error > clk: Clarify clk_get_rate() expectations > clk: tests: Add test suites description > clk: tests: Add reference to the orphan mux bug report > clk: tests: Add tests for uncached clock > clk: tests: Add tests for single parent mux > clk: tests: Add tests for mux with multiple parents > clk: tests: Add some tests for orphan with multiple parents > clk: Take into account uncached clocks in clk_set_rate_range() > clk: Set req_rate on reparenting > clk: Change clk_core_init_rate_req prototype > clk: Move clk_core_init_rate_req() from clk_core_round_rate_nolock() > to its caller > clk: Introduce clk_hw_init_rate_request() > clk: Add our request boundaries in clk_core_init_rate_req > clk: Switch from __clk_determine_rate to clk_core_round_rate_nolock > clk: Introduce clk_core_has_parent() > clk: Constify clk_has_parent() > clk: Stop forwarding clk_rate_requests to the parent > clk: Zero the clk_rate_request structure > clk: Introduce the clk_hw_get_rate_range function > clk: qcom: clk-rcg2: Take clock boundaries into consideration for > gfx3d > clk: tests: Add missing test case for ranges > > drivers/clk/at91/clk-generated.c | 5 +- > drivers/clk/at91/clk-master.c | 9 +- > drivers/clk/at91/clk-peripheral.c | 4 +- > drivers/clk/clk-composite.c | 6 +- > drivers/clk/clk-divider.c | 20 +- > drivers/clk/clk.c | 288 ++++-- > drivers/clk/clk_test.c | 1413 ++++++++++++++++++++++++++++- > drivers/clk/qcom/clk-rcg2.c | 9 + > include/linux/clk-provider.h | 18 +- > include/linux/clk.h | 2 +- > 10 files changed, 1665 insertions(+), 109 deletions(-) I have done build testing, boot testing and LTP syscalls testing on arm64 devices Rpi4 and db410c all test pass. Tested-by: Linux Kernel Functional Testing <lkft@linaro.org> Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org> Build link: https://builds.tuxbuild.com/2DUDJVgnEeotDGSNrcOuFswbTRm/ Test runs links: https://lkft.validation.linaro.org/scheduler/job/5402030 https://lkft.validation.linaro.org/scheduler/job/5402031 - Naresh
Stephen, Mike, On Tue, Aug 16, 2022 at 01:25:05PM +0200, Maxime Ripard wrote: > Thanks to the feedback I got on the previous series, I found and fixed a > number of bugs in the clock framework and how it deals with rates, > especially when it comes to orphan clocks. > > In order to make sure this doesn't pop up again as a regression, I've > extended the number of tests. > > The first patch reintroduces the clk_set_rate_range call on clk_put, but > this time will only do so if there was a range set on that clock to > begin with. It should be less intrusive, and reduce the number of > potential side effects considerably. > > We then have a fix for the qcom rcg2 issue that has been reported > recently. > > All the other patches should be probably be flagged as fixes, but > they've never seem to have shown any real-world issues until now, and > they aren't all really trivial to backport either, so I'm not sure it's > worth it. > > There's also some documentation improvements for recalc_rate and > clk_get_rate to hopefully make the documentation less ambiguous and > acknowledge that recalc_rate() returning 0 on error is fine. I'm not sure what to do at that point. Back in July, you felt uncomfortable merging that series so close to the merge window. I've sent a new series as soon as -rc1 was out, got a kernelci run to test it and on a number of other platforms. And now we're close to rc4, which means that you're going to bring the "we're too late now for 6.1, let's target 6.2". I believe this series fixes a number of real bugs in the CCF, in addition to extending quite a lot the unit test coverage of the framework. But if you just don't care, please just say so. I really don't want to waste any more time rebasing and sending that series, and pinging you on a regular basis if it's not going anywhere. Maxime
Quoting Maxime Ripard (2022-09-02 07:53:05) > > I believe this series fixes a number of real bugs in the CCF, in > addition to extending quite a lot the unit test coverage of the > framework. But if you just don't care, please just say so. I really > don't want to waste any more time rebasing and sending that series, and > pinging you on a regular basis if it's not going anywhere. > I've merged the patch series to clk-next. If things go sideways in the next week we can figure out what to do. Thanks for sticking with it!
Hi Stephen, On Sat, Sep 17, 2022 at 01:31:35AM -0700, Stephen Boyd wrote: > Quoting Maxime Ripard (2022-09-02 07:53:05) > > > > I believe this series fixes a number of real bugs in the CCF, in > > addition to extending quite a lot the unit test coverage of the > > framework. But if you just don't care, please just say so. I really > > don't want to waste any more time rebasing and sending that series, and > > pinging you on a regular basis if it's not going anywhere. > > > > I've merged the patch series to clk-next. If things go sideways in the > next week we can figure out what to do. Thanks for sticking with it! It looks like you picked it up in a separate branch, but didnt't merge/push the merge into clk-next? Maxime
Hi, Thanks to the feedback I got on the previous series, I found and fixed a number of bugs in the clock framework and how it deals with rates, especially when it comes to orphan clocks. In order to make sure this doesn't pop up again as a regression, I've extended the number of tests. The first patch reintroduces the clk_set_rate_range call on clk_put, but this time will only do so if there was a range set on that clock to begin with. It should be less intrusive, and reduce the number of potential side effects considerably. We then have a fix for the qcom rcg2 issue that has been reported recently. All the other patches should be probably be flagged as fixes, but they've never seem to have shown any real-world issues until now, and they aren't all really trivial to backport either, so I'm not sure it's worth it. There's also some documentation improvements for recalc_rate and clk_get_rate to hopefully make the documentation less ambiguous and acknowledge that recalc_rate() returning 0 on error is fine. Let me know what you think, Maxime Changes from v8: - Fixed a regression when probing a clock driver backed by a device accessed through a bus that might sleep Changes from v7: - Dropped the RPi fixes - Rebased on 6.0-rc1 Changes from v6: - Fixed a kernel build bot warning Changes from v5: - Rebased on current next (next-20220711) - Dropped clk_get_rate_range, and used a custom function instead - Switched all tests to use clk_hw_get_clk() instead of struct clk_hw->clk - Removed some intermediate variables - Added some comments - Dropped clk_get_parent() changes - Dropped test on clk_hw pointer non-NULL in clk_hw_get_name - Made clk_has_parent more const Changes from v4: - Fix build breakage on SAM9x60 Changes from v3: - constness warning fix in clk_core_forward_rate_req Changes from v2: - Rebased on top of current next - Fixed locking issue in clk_get_rate_range Changes from v1: - Rebased on top of next-20220428 - Dropped the patch to prevent non-orphan clocks from registering if their recalc_rate hook returns 0 - Added some patches to clarify the clk_get_rate and recalc_rate documentation - Dropped the patch to skip the range setup on an orphan clock that was introducing a regression on RaspberryPi3 when a monitor wasn't connected at boot - Added a patch to skip the rate clamping in clk_round_rate() when min_rate == max_rate == 0 - Added a new set of functions to query the clk boundaries and fix a regression with the RaspberryPi4 - Fixed all the drivers hand-crafting their clk_rate_request - Reworded the test suite descriptions - Reordered a few patches to ease the review - Reworded some commit logs to better explain the issues they address - Collected the Tested-by of Alexander and Marek - More tests Maxime Ripard (25): clk: test: Switch to clk_hw_get_clk clk: Drop the rate range on clk_put() clk: Skip clamping when rounding if there's no boundaries clk: Mention that .recalc_rate can return 0 on error clk: Clarify clk_get_rate() expectations clk: tests: Add test suites description clk: tests: Add reference to the orphan mux bug report clk: tests: Add tests for uncached clock clk: tests: Add tests for single parent mux clk: tests: Add tests for mux with multiple parents clk: tests: Add some tests for orphan with multiple parents clk: Take into account uncached clocks in clk_set_rate_range() clk: Set req_rate on reparenting clk: Change clk_core_init_rate_req prototype clk: Move clk_core_init_rate_req() from clk_core_round_rate_nolock() to its caller clk: Introduce clk_hw_init_rate_request() clk: Add our request boundaries in clk_core_init_rate_req clk: Switch from __clk_determine_rate to clk_core_round_rate_nolock clk: Introduce clk_core_has_parent() clk: Constify clk_has_parent() clk: Stop forwarding clk_rate_requests to the parent clk: Zero the clk_rate_request structure clk: Introduce the clk_hw_get_rate_range function clk: qcom: clk-rcg2: Take clock boundaries into consideration for gfx3d clk: tests: Add missing test case for ranges drivers/clk/at91/clk-generated.c | 5 +- drivers/clk/at91/clk-master.c | 9 +- drivers/clk/at91/clk-peripheral.c | 4 +- drivers/clk/clk-composite.c | 6 +- drivers/clk/clk-divider.c | 20 +- drivers/clk/clk.c | 288 ++++-- drivers/clk/clk_test.c | 1413 ++++++++++++++++++++++++++++- drivers/clk/qcom/clk-rcg2.c | 9 + include/linux/clk-provider.h | 18 +- include/linux/clk.h | 2 +- 10 files changed, 1665 insertions(+), 109 deletions(-)