target/linux/bcm27xx/patches-5.15/950-0855-clk-Stop-forwarding-clk_rate_requests-to-the-parent.patch

   1 From 6d969d963a86c88ca1d6dcbc25447e2937e833b1 Mon Sep 17 00:00:00 2001
   2 From: Maxime Ripard <maxime@cerno.tech>
   3 Date: Sat, 2 Apr 2022 14:36:59 +0200
   4 Subject: [PATCH] clk: Stop forwarding clk_rate_requests to the parent
   5
   6 If the clock cannot modify its rate and has CLK_SET_RATE_PARENT,
   7 clk_mux_determine_rate_flags() and clk_core_round_rate_nolock() will
   8 call clk_core_round_rate_nolock() with its parent clock but use the
   9 request of the child node either directly (clk_core_round_rate_nolock())
  10 or by copying it (clk_mux_determine_rate_flags()).
  11
  12 Both cases are problematic since the parent will now have a request with
  13 the best parent fields of the child (so pointing to itself) and the
  14 boundaries of the child as well.
  15
  16 clk_core_round_rate_nolock() is even worse since we would directly
  17 modify the caller structure if the parent was ever to modify its own
  18 parent or its parent rate, then returning to the caller a best parent
  19 that isn't a parent of the clock we just called clk_determine_rate()
  20 onto.
  21
  22 Let's create a new function that will create a new request to forward to
  23 the parent, clk_core_forward_rate_req() and update the relevant call
  24 sites to that new function.
  25
  26 Let's also add a test to make sure we avoid regressions there.
  27
  28 Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # imx8mp
  29 Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> # exynos4210, meson g12b
  30 Signed-off-by: Maxime Ripard <maxime@cerno.tech>
  31 ---
  32  drivers/clk/clk.c      |  58 ++++++++++++--
  33  drivers/clk/clk_test.c | 176 +++++++++++++++++++++++++++++++++++++++++
  34  2 files changed, 227 insertions(+), 7 deletions(-)
  35
  36 --- a/drivers/clk/clk.c
  37 +++ b/drivers/clk/clk.c
  38 @@ -542,6 +542,10 @@ static bool mux_is_better_rate(unsigned
  39         return now <= rate && now > best;
  40  }
  41
  42 +static void clk_core_init_rate_req(struct clk_core * const core,
  43 +                                  struct clk_rate_request *req,
  44 +                                  unsigned long rate);
  45 +
  46  static int clk_core_round_rate_nolock(struct clk_core *core,
  47                                       struct clk_rate_request *req);
  48
  49 @@ -565,6 +569,24 @@ static bool clk_core_has_parent(struct c
  50         return false;
  51  }
  52
  53 +static void
  54 +clk_core_forward_rate_req(struct clk_core * const core,
  55 +                         struct clk_core * const parent,
  56 +                         struct clk_rate_request * const old_req,
  57 +                         struct clk_rate_request *req)
  58 +{
  59 +       if (WARN_ON(!clk_core_has_parent(core, parent)))
  60 +               return;
  61 +
  62 +       clk_core_init_rate_req(parent, req, old_req->rate);
  63 +
  64 +       if (req->min_rate < old_req->min_rate)
  65 +               req->min_rate = old_req->min_rate;
  66 +
  67 +       if (req->max_rate > old_req->max_rate)
  68 +               req->max_rate = old_req->max_rate;
  69 +}
  70 +
  71  int clk_mux_determine_rate_flags(struct clk_hw *hw,
  72                                  struct clk_rate_request *req,
  73                                  unsigned long flags)
  74 @@ -572,17 +594,19 @@ int clk_mux_determine_rate_flags(struct
  75         struct clk_core *core = hw->core, *parent, *best_parent = NULL;
  76         int i, num_parents, ret;
  77         unsigned long best = 0;
  78 -       struct clk_rate_request parent_req = *req;
  79
  80         /* if NO_REPARENT flag set, pass through to current parent */
  81         if (core->flags & CLK_SET_RATE_NO_REPARENT) {
  82                 parent = core->parent;
  83                 if (core->flags & CLK_SET_RATE_PARENT) {
  84 +                       struct clk_rate_request parent_req;
  85 +
  86                         if (!parent) {
  87                                 req->rate = 0;
  88                                 return 0;
  89                         }
  90
  91 +                       clk_core_forward_rate_req(core, parent, req, &parent_req);
  92                         ret = clk_core_round_rate_nolock(parent, &parent_req);
  93                         if (ret)
  94                                 return ret;
  95 @@ -600,23 +624,29 @@ int clk_mux_determine_rate_flags(struct
  96         /* find the parent that can provide the fastest rate <= rate */
  97         num_parents = core->num_parents;
  98         for (i = 0; i < num_parents; i++) {
  99 +               unsigned long parent_rate;
 100 +
 101                 parent = clk_core_get_parent_by_index(core, i);
 102                 if (!parent)
 103                         continue;
 104
 105                 if (core->flags & CLK_SET_RATE_PARENT) {
 106 -                       parent_req = *req;
 107 +                       struct clk_rate_request parent_req;
 108 +
 109 +                       clk_core_forward_rate_req(core, parent, req, &parent_req);
 110                         ret = clk_core_round_rate_nolock(parent, &parent_req);
 111                         if (ret)
 112                                 continue;
 113 +
 114 +                       parent_rate = parent_req.rate;
 115                 } else {
 116 -                       parent_req.rate = clk_core_get_rate_nolock(parent);
 117 +                       parent_rate = clk_core_get_rate_nolock(parent);
 118                 }
 119
 120 -               if (mux_is_better_rate(req->rate, parent_req.rate,
 121 +               if (mux_is_better_rate(req->rate, parent_rate,
 122                                        best, flags)) {
 123                         best_parent = parent;
 124 -                       best = parent_req.rate;
 125 +                       best = parent_rate;
 126                 }
 127         }
 128
 129 @@ -1450,6 +1480,8 @@ static bool clk_core_can_round(struct cl
 130  static int clk_core_round_rate_nolock(struct clk_core *core,
 131                                       struct clk_rate_request *req)
 132  {
 133 +       int ret;
 134 +
 135         lockdep_assert_held(&prepare_lock);
 136
 137         if (!core) {
 138 @@ -1459,8 +1491,20 @@ static int clk_core_round_rate_nolock(st
 139
 140         if (clk_core_can_round(core))
 141                 return clk_core_determine_round_nolock(core, req);
 142 -       else if (core->flags & CLK_SET_RATE_PARENT)
 143 -               return clk_core_round_rate_nolock(core->parent, req);
 144 +
 145 +       if (core->flags & CLK_SET_RATE_PARENT) {
 146 +               struct clk_rate_request parent_req;
 147 +
 148 +               clk_core_forward_rate_req(core, core->parent, req, &parent_req);
 149 +               ret = clk_core_round_rate_nolock(core->parent, &parent_req);
 150 +               if (ret)
 151 +                       return ret;
 152 +
 153 +               req->best_parent_rate = parent_req.rate;
 154 +               req->rate = parent_req.rate;
 155 +
 156 +               return 0;
 157 +       }
 158
 159         req->rate = core->rate;
 160         return 0;
 161 --- a/drivers/clk/clk_test.c
 162 +++ b/drivers/clk/clk_test.c
 163 @@ -999,6 +999,34 @@ clk_test_single_parent_mux_set_range_dis
 164
 165  /*
 166   * Test that for a clock that can't modify its rate and with a single
 167 + * parent, if we set a range on the parent and then call
 168 + * clk_round_rate(), the boundaries of the parent are taken into
 169 + * account.
 170 + */
 171 +static void
 172 +clk_test_single_parent_mux_set_range_round_rate_parent_only(struct kunit *test)
 173 +{
 174 +       struct clk_single_parent_ctx *ctx = test->priv;
 175 +       struct clk_hw *hw = &ctx->hw;
 176 +       struct clk *clk = hw->clk;
 177 +       struct clk *parent;
 178 +       unsigned long rate;
 179 +       int ret;
 180 +
 181 +       parent = clk_get_parent(clk);
 182 +       KUNIT_ASSERT_PTR_NE(test, parent, NULL);
 183 +
 184 +       ret = clk_set_rate_range(parent, DUMMY_CLOCK_RATE_1, DUMMY_CLOCK_RATE_2);
 185 +       KUNIT_ASSERT_EQ(test, ret, 0);
 186 +
 187 +       rate = clk_round_rate(clk, DUMMY_CLOCK_RATE_1 - 1000);
 188 +       KUNIT_ASSERT_GT(test, rate, 0);
 189 +       KUNIT_EXPECT_GE(test, rate, DUMMY_CLOCK_RATE_1);
 190 +       KUNIT_EXPECT_LE(test, rate, DUMMY_CLOCK_RATE_2);
 191 +}
 192 +
 193 +/*
 194 + * Test that for a clock that can't modify its rate and with a single
 195   * parent, if we set a range on the parent and a more restrictive one on
 196   * the child, and then call clk_round_rate(), the boundaries of the
 197   * two clocks are taken into account.
 198 @@ -1033,12 +1061,50 @@ clk_test_single_parent_mux_set_range_rou
 199         KUNIT_EXPECT_LE(test, rate, DUMMY_CLOCK_RATE_2 - 1000);
 200  }
 201
 202 +/*
 203 + * Test that for a clock that can't modify its rate and with a single
 204 + * parent, if we set a range on the child and a more restrictive one on
 205 + * the parent, and then call clk_round_rate(), the boundaries of the
 206 + * two clocks are taken into account.
 207 + */
 208 +static void
 209 +clk_test_single_parent_mux_set_range_round_rate_parent_smaller(struct kunit *test)
 210 +{
 211 +       struct clk_single_parent_ctx *ctx = test->priv;
 212 +       struct clk_hw *hw = &ctx->hw;
 213 +       struct clk *clk = hw->clk;
 214 +       struct clk *parent;
 215 +       unsigned long rate;
 216 +       int ret;
 217 +
 218 +       parent = clk_get_parent(clk);
 219 +       KUNIT_ASSERT_PTR_NE(test, parent, NULL);
 220 +
 221 +       ret = clk_set_rate_range(parent, DUMMY_CLOCK_RATE_1 + 1000, DUMMY_CLOCK_RATE_2 - 1000);
 222 +       KUNIT_ASSERT_EQ(test, ret, 0);
 223 +
 224 +       ret = clk_set_rate_range(clk, DUMMY_CLOCK_RATE_1, DUMMY_CLOCK_RATE_2);
 225 +       KUNIT_ASSERT_EQ(test, ret, 0);
 226 +
 227 +       rate = clk_round_rate(clk, DUMMY_CLOCK_RATE_1 - 1000);
 228 +       KUNIT_ASSERT_GT(test, rate, 0);
 229 +       KUNIT_EXPECT_GE(test, rate, DUMMY_CLOCK_RATE_1 + 1000);
 230 +       KUNIT_EXPECT_LE(test, rate, DUMMY_CLOCK_RATE_2 - 1000);
 231 +
 232 +       rate = clk_round_rate(clk, DUMMY_CLOCK_RATE_2 + 1000);
 233 +       KUNIT_ASSERT_GT(test, rate, 0);
 234 +       KUNIT_EXPECT_GE(test, rate, DUMMY_CLOCK_RATE_1 + 1000);
 235 +       KUNIT_EXPECT_LE(test, rate, DUMMY_CLOCK_RATE_2 - 1000);
 236 +}
 237 +
 238  static struct kunit_case clk_single_parent_mux_test_cases[] = {
 239         KUNIT_CASE(clk_test_single_parent_mux_get_parent),
 240         KUNIT_CASE(clk_test_single_parent_mux_has_parent),
 241         KUNIT_CASE(clk_test_single_parent_mux_set_range_disjoint_child_last),
 242         KUNIT_CASE(clk_test_single_parent_mux_set_range_disjoint_parent_last),
 243         KUNIT_CASE(clk_test_single_parent_mux_set_range_round_rate_child_smaller),
 244 +       KUNIT_CASE(clk_test_single_parent_mux_set_range_round_rate_parent_only),
 245 +       KUNIT_CASE(clk_test_single_parent_mux_set_range_round_rate_parent_smaller),
 246         {}
 247  };
 248
 249 @@ -1945,7 +2011,117 @@ static struct kunit_suite clk_range_mini
 250         .test_cases = clk_range_minimize_test_cases,
 251  };
 252
 253 +struct clk_leaf_mux_ctx {
 254 +       struct clk_multiple_parent_ctx mux_ctx;
 255 +       struct clk_hw hw;
 256 +};
 257 +
 258 +static int
 259 +clk_leaf_mux_set_rate_parent_test_init(struct kunit *test)
 260 +{
 261 +       struct clk_leaf_mux_ctx *ctx;
 262 +       const char *top_parents[2] = { "parent-0", "parent-1" };
 263 +       int ret;
 264 +
 265 +       ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
 266 +       if (!ctx)
 267 +               return -ENOMEM;
 268 +       test->priv = ctx;
 269 +
 270 +       ctx->mux_ctx.parents_ctx[0].hw.init = CLK_HW_INIT_NO_PARENT("parent-0",
 271 +                                                                   &clk_dummy_rate_ops,
 272 +                                                                   0);
 273 +       ctx->mux_ctx.parents_ctx[0].rate = DUMMY_CLOCK_RATE_1;
 274 +       ret = clk_hw_register(NULL, &ctx->mux_ctx.parents_ctx[0].hw);
 275 +       if (ret)
 276 +               return ret;
 277 +
 278 +       ctx->mux_ctx.parents_ctx[1].hw.init = CLK_HW_INIT_NO_PARENT("parent-1",
 279 +                                                                   &clk_dummy_rate_ops,
 280 +                                                                   0);
 281 +       ctx->mux_ctx.parents_ctx[1].rate = DUMMY_CLOCK_RATE_2;
 282 +       ret = clk_hw_register(NULL, &ctx->mux_ctx.parents_ctx[1].hw);
 283 +       if (ret)
 284 +               return ret;
 285 +
 286 +       ctx->mux_ctx.current_parent = 0;
 287 +       ctx->mux_ctx.hw.init = CLK_HW_INIT_PARENTS("test-mux", top_parents,
 288 +                                                  &clk_multiple_parents_mux_ops,
 289 +                                                  0);
 290 +       ret = clk_hw_register(NULL, &ctx->mux_ctx.hw);
 291 +       if (ret)
 292 +               return ret;
 293 +
 294 +       ctx->hw.init = CLK_HW_INIT_HW("test-clock", &ctx->mux_ctx.hw,
 295 +                                     &clk_dummy_single_parent_ops,
 296 +                                     CLK_SET_RATE_PARENT);
 297 +       ret = clk_hw_register(NULL, &ctx->hw);
 298 +       if (ret)
 299 +               return ret;
 300 +
 301 +       return 0;
 302 +}
 303 +
 304 +static void clk_leaf_mux_set_rate_parent_test_exit(struct kunit *test)
 305 +{
 306 +       struct clk_leaf_mux_ctx *ctx = test->priv;
 307 +
 308 +       clk_hw_unregister(&ctx->hw);
 309 +       clk_hw_unregister(&ctx->mux_ctx.hw);
 310 +       clk_hw_unregister(&ctx->mux_ctx.parents_ctx[0].hw);
 311 +       clk_hw_unregister(&ctx->mux_ctx.parents_ctx[1].hw);
 312 +}
 313 +
 314 +/*
 315 + * Test that, for a clock that will forward any rate request to its
 316 + * parent, the rate request structure returned by __clk_determine_rate
 317 + * is sane and will be what we expect.
 318 + */
 319 +static void clk_leaf_mux_set_rate_parent_determine_rate(struct kunit *test)
 320 +{
 321 +       struct clk_leaf_mux_ctx *ctx = test->priv;
 322 +       struct clk_hw *hw = &ctx->hw;
 323 +       struct clk *clk = hw->clk;
 324 +       struct clk_rate_request req;
 325 +       unsigned long rate;
 326 +       int ret;
 327 +
 328 +       rate = clk_get_rate(clk);
 329 +       KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
 330 +
 331 +       clk_hw_init_rate_request(hw, &req, DUMMY_CLOCK_RATE_2);
 332 +
 333 +       ret = __clk_determine_rate(hw, &req);
 334 +       KUNIT_ASSERT_EQ(test, ret, 0);
 335 +
 336 +       KUNIT_EXPECT_EQ(test, req.rate, DUMMY_CLOCK_RATE_2);
 337 +       KUNIT_EXPECT_EQ(test, req.best_parent_rate, DUMMY_CLOCK_RATE_2);
 338 +       KUNIT_EXPECT_PTR_EQ(test, req.best_parent_hw, &ctx->mux_ctx.hw);
 339 +}
 340 +
 341 +static struct kunit_case clk_leaf_mux_set_rate_parent_test_cases[] = {
 342 +       KUNIT_CASE(clk_leaf_mux_set_rate_parent_determine_rate),
 343 +       {}
 344 +};
 345 +
 346 +/*
 347 + * Test suite for a clock whose parent is a mux with multiple parents.
 348 + * The leaf clock has CLK_SET_RATE_PARENT, and will forward rate
 349 + * requests to the mux, which will then select which parent is the best
 350 + * fit for a given rate.
 351 + *
 352 + * These tests are supposed to exercise the behaviour of muxes, and the
 353 + * proper selection of parents.
 354 +  */
 355 +static struct kunit_suite clk_leaf_mux_set_rate_parent_test_suite = {
 356 +       .name = "clk-leaf-mux-set-rate-parent",
 357 +       .init = clk_leaf_mux_set_rate_parent_test_init,
 358 +       .exit = clk_leaf_mux_set_rate_parent_test_exit,
 359 +       .test_cases = clk_leaf_mux_set_rate_parent_test_cases,
 360 +};
 361 +
 362  kunit_test_suites(
 363 +       &clk_leaf_mux_set_rate_parent_test_suite,
 364         &clk_test_suite,
 365         &clk_multiple_parents_mux_test_suite,
 366         &clk_orphan_transparent_multiple_parent_mux_test_suite,