Hi Stephen, Thanks for your review On Fri, Feb 18, 2022 at 06:20:46PM -0800, Stephen Boyd wrote: > It would also be good to add a test that tries to set the clk rate with > clk_set_rate() after a range has been set that is outside the acceptable > range and verify that it fails, and one that tries to set it within the > range and make sure it succeeds (and changes it to be exactly what was > set). Do we expect it to fail though? If we do: clk_set_range_range(clk, 1000, 2000); clk_set_rate(3000); The current behaviour is that the rate is going to be rounded to 2000, but it doesn't fail. Or is it what you meant by fail? ie, that the return code is 0, but the rate isn't what we asked for? > Similarly, a call to set two disjoint ranges and verify that the call > that tries to set the second disjoint range fails. Ok > We want to test the failure paths as well, to make sure we don't start > causing them to pass, unless it's expected. Do you have any other failure condition you want to test? I already tried to come up with those I could think of, but I clearly missed some if you said that :) > This patch could also contain the failure scenario you're experiencing > and mark it as expecting to fail. Then the patch that fixes it in the > core could mark the test as expecting to pass, which may help us > understand more easily what exactly changed instead of having to > figure that out after the fact by reading the entire test.> Ok Maxime
Attachment:
signature.asc
Description: PGP signature
