>From 469c29a4d14a83155a280df4679a43ad4af2e1b4 Mon Sep 17 00:00:00 2001 From: Brian Masney Date: Fri, 14 Nov 2025 18:45:06 -0500 Subject: [PATCH] HACK: clk: move negotiate_rates member from struct clk_ops to struct clk_rate_request Content-type: text/plain Demonstrate one way to move the negotiate_rates member from struct clk_ops to struct clk_rate_request. Personally I think it's much cleaner to have this on struct clk_ops. Signed-off-by: Brian Masney --- Documentation/driver-api/clk.rst | 8 +++++--- drivers/clk/clk.c | 11 ++++++++--- drivers/clk/clk_test.c | 32 +++++++++++++++++++++++++------- include/linux/clk-provider.h | 19 ++++++++++++------- 4 files changed, 50 insertions(+), 20 deletions(-) diff --git a/Documentation/driver-api/clk.rst b/Documentation/driver-api/clk.rst index c46ee62ba5bd..47ed511432db 100644 --- a/Documentation/driver-api/clk.rst +++ b/Documentation/driver-api/clk.rst @@ -75,9 +75,6 @@ the operations defined in clk-provider.h:: void (*disable)(struct clk_hw *hw); int (*is_enabled)(struct clk_hw *hw); void (*disable_unused)(struct clk_hw *hw); - bool (*negotiate_rates)(struct clk_hw *hw, - struct clk_rate_request *req, - bool (*check_rate)(struct clk_core *, unsigned long)); unsigned long (*recalc_rate)(struct clk_hw *hw, unsigned long parent_rate); long (*round_rate)(struct clk_hw *hw, @@ -103,6 +100,11 @@ the operations defined in clk-provider.h:: struct dentry *dentry); }; +Note: The negotiate_rates callback is not part of struct clk_ops. Instead, it +is specified in struct clk_init_data during clock registration and is copied +to struct clk_rate_request for use during rate negotiations. It is invoked +through the rate request structure rather than through ops. + Hardware clk implementations ============================ diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 926b7d4b9ab8..8014eb719266 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -130,6 +130,9 @@ struct clk_parent_map { struct clk_core { const char *name; const struct clk_ops *ops; + bool (*negotiate_rates)(struct clk_hw *hw, + struct clk_rate_request *req, + bool (*check_rate)(struct clk_core *, unsigned long)); struct clk_hw *hw; struct module *owner; struct device *dev; @@ -427,8 +430,8 @@ static bool clk_core_is_enabled(struct clk_core *core) static int clk_core_use_v2_rate_negotiation(struct clk_core *core) { - bool has_v2_ops = core->ops->negotiate_rates || - (core->parent && core->parent->ops->negotiate_rates); + bool has_v2_ops = core->negotiate_rates || + (core->parent && core->parent->negotiate_rates); return has_v2_ops && modparam_clk_v2_rate_negotiation; } @@ -1713,6 +1716,7 @@ static void clk_core_init_rate_req(struct clk_core * const core, req->core = core; req->rate = rate; + req->negotiate_rates = core->negotiate_rates; clk_core_get_boundaries(core, &req->min_rate, &req->max_rate); parent = core->parent; @@ -2504,7 +2508,7 @@ static struct clk_core *clk_calc_new_rates(struct clk_core *core, struct clk_rate_request req; clk_core_init_rate_req(top, &req, top->new_rate); - if (!top->ops->negotiate_rates(top->hw, &req, clk_check_rate)) + if (!req.negotiate_rates || !req.negotiate_rates(top->hw, &req, clk_check_rate)) return NULL; clk_accept_rate_negotiations(top); @@ -4511,6 +4515,7 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw) goto fail_ops; } core->ops = init->ops; + core->negotiate_rates = init->negotiate_rates; core->dev = dev; clk_pm_runtime_init(core); diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c index ff53b8bdf872..be2a49d2e446 100644 --- a/drivers/clk/clk_test.c +++ b/drivers/clk/clk_test.c @@ -122,7 +122,6 @@ static const struct clk_ops clk_dummy_rate_ops = { .recalc_rate = clk_dummy_recalc_rate, .determine_rate = clk_dummy_determine_rate, .set_rate = clk_dummy_set_rate, - .negotiate_rates = clk_dummy_negotiate_rates, }; static const struct clk_ops clk_dummy_maximize_rate_ops = { @@ -218,7 +217,6 @@ static const struct clk_ops clk_dummy_div_ops = { .recalc_rate = clk_dummy_div_recalc_rate, .determine_rate = clk_dummy_div_determine_rate, .set_rate = clk_dummy_div_set_rate, - .negotiate_rates = clk_dummy_div_negotiate_rates, }; struct clk_dummy_gate { @@ -755,6 +753,26 @@ struct clk_rate_change_sibling_div_div_context { static int clk_rate_change_sibling_div_div_test_init(struct kunit *test) { struct clk_rate_change_sibling_div_div_context *ctx; + struct clk_init_data parent_init = { + .name = "parent", + .ops = &clk_dummy_rate_ops, + .negotiate_rates = clk_dummy_negotiate_rates, + .flags = 0, + }; + struct clk_init_data child1_init = { + .name = "child1", + .ops = &clk_dummy_div_ops, + .negotiate_rates = clk_dummy_div_negotiate_rates, + .flags = CLK_SET_RATE_PARENT, + .num_parents = 1, + }; + struct clk_init_data child2_init = { + .name = "child2", + .ops = &clk_dummy_div_ops, + .negotiate_rates = clk_dummy_div_negotiate_rates, + .flags = CLK_SET_RATE_PARENT, + .num_parents = 1, + }; int ret; ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL); @@ -762,22 +780,22 @@ static int clk_rate_change_sibling_div_div_test_init(struct kunit *test) return -ENOMEM; test->priv = ctx; - ctx->parent.hw.init = CLK_HW_INIT_NO_PARENT("parent", &clk_dummy_rate_ops, 0); + ctx->parent.hw.init = &parent_init; ctx->parent.negotiate_step_size = 1 * HZ_PER_MHZ; ctx->parent.rate = 24 * HZ_PER_MHZ; ret = clk_hw_register_kunit(test, NULL, &ctx->parent.hw); if (ret) return ret; - ctx->child1.hw.init = CLK_HW_INIT_HW("child1", &ctx->parent.hw, &clk_dummy_div_ops, - CLK_SET_RATE_PARENT); + child1_init.parent_hws = (const struct clk_hw*[]) { &ctx->parent.hw }; + ctx->child1.hw.init = &child1_init; ctx->child1.div = 1; ret = clk_hw_register_kunit(test, NULL, &ctx->child1.hw); if (ret) return ret; - ctx->child2.hw.init = CLK_HW_INIT_HW("child2", &ctx->parent.hw, &clk_dummy_div_ops, - CLK_SET_RATE_PARENT); + child2_init.parent_hws = (const struct clk_hw*[]) { &ctx->parent.hw }; + ctx->child2.hw.init = &child2_init; ctx->child2.div = 1; ret = clk_hw_register_kunit(test, NULL, &ctx->child2.hw); if (ret) diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 9041b17ba99e..bc162afc8cd8 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -53,6 +53,9 @@ struct dentry; * requested constraints. * @best_parent_hw: The most appropriate parent clock that fulfills the * requested constraints. + * @negotiate_rates: When a child clk requests a new rate that requires a rate + * change from the parent, this negotiates a new parent rate that's + * acceptable to all of the children. * */ struct clk_rate_request { @@ -62,6 +65,9 @@ struct clk_rate_request { unsigned long max_rate; unsigned long best_parent_rate; struct clk_hw *best_parent_hw; + bool (*negotiate_rates)(struct clk_hw *hw, + struct clk_rate_request *req, + bool (*check_rate)(struct clk_core *, unsigned long)); }; void clk_hw_init_rate_request(const struct clk_hw *hw, @@ -129,10 +135,6 @@ struct clk_duty { * @restore_context: Restore the context of the clock after a restoration * of power. * - * @negotiate_rates: When a child clk requests a new rate that requires a rate - * change from the parent, this negotiates a new parent rate that's - * acceptable to all of the children. - * * @recalc_rate: Recalculate the rate of this clock, by querying hardware. The * parent rate is an input parameter. It is up to the caller to * ensure that the prepare_mutex is held across this call. If the @@ -246,9 +248,6 @@ struct clk_ops { void (*disable_unused)(struct clk_hw *hw); int (*save_context)(struct clk_hw *hw); void (*restore_context)(struct clk_hw *hw); - bool (*negotiate_rates)(struct clk_hw *hw, - struct clk_rate_request *req, - bool (*check_rate)(struct clk_core *, unsigned long)); unsigned long (*recalc_rate)(struct clk_hw *hw, unsigned long parent_rate); long (*round_rate)(struct clk_hw *hw, unsigned long rate, @@ -295,6 +294,9 @@ struct clk_parent_data { * * @name: clock name * @ops: operations this clock supports + * @negotiate_rates: When a child clk requests a new rate that requires a rate + * change from the parent, this negotiates a new parent rate that's + * acceptable to all of the children. * @parent_names: array of string names for all possible parents * @parent_data: array of parent data for all possible parents (when some * parents are external to the clk controller) @@ -306,6 +308,9 @@ struct clk_parent_data { struct clk_init_data { const char *name; const struct clk_ops *ops; + bool (*negotiate_rates)(struct clk_hw *hw, + struct clk_rate_request *req, + bool (*check_rate)(struct clk_core *, unsigned long)); /* Only one of the following three should be assigned */ const char * const *parent_names; const struct clk_parent_data *parent_data; -- 2.51.1