Skip to content

Commit

Permalink
clk: Stop forwarding clk_rate_requests to the parent
Browse files Browse the repository at this point in the history
If the clock cannot modify its rate and has CLK_SET_RATE_PARENT,
clk_mux_determine_rate_flags(), clk_core_round_rate_nolock() and a
number of drivers will forward the clk_rate_request to the parent clock.

clk_core_round_rate_nolock() will pass the pointer directly, which means
that we pass a clk_rate_request to the parent that has the rate,
min_rate and max_rate of the child, and the best_parent_rate and
best_parent_hw fields will be relative to the child as well, so will
point to our current clock and its rate. The most common case for
CLK_SET_RATE_PARENT is that the child and parent clock rates will be
equal, so the rate field isn't a worry, but the other fields are.

Similarly, if the parent clock driver ever modifies the best_parent_rate
or best_parent_hw, this will be applied to the child once the call to
clk_core_round_rate_nolock() is done. best_parent_hw is probably not
going to be a valid parent, and best_parent_rate might lead to a parent
rate change different to the one that was initially computed.

clk_mux_determine_rate_flags() and the affected drivers will copy the
request before forwarding it to the parents, so they won't be affected
by the latter issue, but the former is still going to be there and will
lead to erroneous data and context being passed to the various clock
drivers in the same sub-tree.

Let's create two new functions, clk_core_forward_rate_req() and
clk_hw_forward_rate_request() for the framework and the clock providers
that will copy a request from a child clock and update the context to
match the parent's. We also update the relevant call sites in the
framework and drivers to use that new function.

Let's also add a test to make sure we avoid regressions there.

Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # imx8mp
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> # exynos4210, meson g12b
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
  • Loading branch information
mripard authored and intel-lab-lkp committed May 12, 2022
1 parent 4a557d1 commit 2034099
Show file tree
Hide file tree
Showing 7 changed files with 273 additions and 15 deletions.
4 changes: 3 additions & 1 deletion drivers/clk/at91/clk-generated.c
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ static int clk_generated_determine_rate(struct clk_hw *hw,
{
struct clk_generated *gck = to_clk_generated(hw);
struct clk_hw *parent = NULL;
struct clk_rate_request req_parent = *req;
long best_rate = -EINVAL;
unsigned long min_rate, parent_rate;
int best_diff = -1;
Expand Down Expand Up @@ -188,6 +187,9 @@ static int clk_generated_determine_rate(struct clk_hw *hw,
goto end;

for (div = 1; div < GENERATED_MAX_DIV + 2; div++) {
struct clk_rate_request req_parent;

clk_hw_forward_rate_request(hw, parent, req, &req_parent);
req_parent.rate = req->rate * div;
if (__clk_determine_rate(parent, &req_parent))
continue;
Expand Down
9 changes: 6 additions & 3 deletions drivers/clk/at91/clk-master.c
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,6 @@ static int clk_sama7g5_master_determine_rate(struct clk_hw *hw,
struct clk_rate_request *req)
{
struct clk_master *master = to_clk_master(hw);
struct clk_rate_request req_parent = *req;
struct clk_hw *parent;
long best_rate = LONG_MIN, best_diff = LONG_MIN;
unsigned long parent_rate;
Expand Down Expand Up @@ -618,11 +617,15 @@ static int clk_sama7g5_master_determine_rate(struct clk_hw *hw,
goto end;

for (div = 0; div < MASTER_PRES_MAX + 1; div++) {
struct clk_rate_request req_parent;
unsigned long req_rate;

if (div == MASTER_PRES_MAX)
req_parent.rate = req->rate * 3;
req_rate = req->rate * 3;
else
req_parent.rate = req->rate << div;
req_rate = req->rate << div;

clk_hw_forward_rate_request(hw, req, parent, &req_parent, req_rate);
if (__clk_determine_rate(parent, &req_parent))
continue;

Expand Down
4 changes: 2 additions & 2 deletions drivers/clk/at91/clk-peripheral.c
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,6 @@ static int clk_sam9x5_peripheral_determine_rate(struct clk_hw *hw,
{
struct clk_sam9x5_peripheral *periph = to_clk_sam9x5_peripheral(hw);
struct clk_hw *parent = clk_hw_get_parent(hw);
struct clk_rate_request req_parent = *req;
unsigned long parent_rate = clk_hw_get_rate(parent);
unsigned long tmp_rate;
long best_rate = LONG_MIN;
Expand Down Expand Up @@ -302,8 +301,9 @@ static int clk_sam9x5_peripheral_determine_rate(struct clk_hw *hw,
goto end;

for (shift = 0; shift <= PERIPHERAL_MAX_SHIFT; shift++) {
req_parent.rate = req->rate << shift;
struct clk_rate_request req_parent;

clk_hw_forward_rate_request(hw, req, parent, &req_parent, req->rate << shift);
if (__clk_determine_rate(parent, &req_parent))
continue;

Expand Down
6 changes: 4 additions & 2 deletions drivers/clk/clk-composite.c
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,11 @@ static int clk_composite_determine_rate(struct clk_hw *hw,
req->best_parent_hw = NULL;

if (clk_hw_get_flags(hw) & CLK_SET_RATE_NO_REPARENT) {
struct clk_rate_request tmp_req = *req;
struct clk_rate_request tmp_req;

parent = clk_hw_get_parent(mux_hw);

clk_hw_forward_rate_request(hw, req, parent, &tmp_req, req->rate);
ret = clk_composite_determine_rate_for_parent(rate_hw,
&tmp_req,
parent,
Expand All @@ -104,12 +105,13 @@ static int clk_composite_determine_rate(struct clk_hw *hw,
}

for (i = 0; i < clk_hw_get_num_parents(mux_hw); i++) {
struct clk_rate_request tmp_req = *req;
struct clk_rate_request tmp_req;

parent = clk_hw_get_parent_by_index(mux_hw, i);
if (!parent)
continue;

clk_hw_forward_rate_request(hw, req, parent, &tmp_req, req->rate);
ret = clk_composite_determine_rate_for_parent(rate_hw,
&tmp_req,
parent,
Expand Down
84 changes: 77 additions & 7 deletions drivers/clk/clk.c
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,10 @@ static bool mux_is_better_rate(unsigned long rate, unsigned long now,
return now <= rate && now > best;
}

static void clk_core_init_rate_req(struct clk_core * const core,
struct clk_rate_request *req,
unsigned long rate);

static int clk_core_round_rate_nolock(struct clk_core *core,
struct clk_rate_request *req);

Expand All @@ -559,24 +563,45 @@ static bool clk_core_has_parent(struct clk_core *core, struct clk_core *parent)
return false;
}

static void
clk_core_forward_rate_req(struct clk_core *core,
const struct clk_rate_request *old_req,
struct clk_core *parent,
struct clk_rate_request *req,
unsigned long parent_rate)
{
if (WARN_ON(!clk_core_has_parent(core, parent)))
return;

clk_core_init_rate_req(parent, req, parent_rate);

if (req->min_rate < old_req->min_rate)
req->min_rate = old_req->min_rate;

if (req->max_rate > old_req->max_rate)
req->max_rate = old_req->max_rate;
}

int clk_mux_determine_rate_flags(struct clk_hw *hw,
struct clk_rate_request *req,
unsigned long flags)
{
struct clk_core *core = hw->core, *parent, *best_parent = NULL;
int i, num_parents, ret;
unsigned long best = 0;
struct clk_rate_request parent_req = *req;

/* if NO_REPARENT flag set, pass through to current parent */
if (core->flags & CLK_SET_RATE_NO_REPARENT) {
parent = core->parent;
if (core->flags & CLK_SET_RATE_PARENT) {
struct clk_rate_request parent_req;

if (!parent) {
req->rate = 0;
return 0;
}

clk_core_forward_rate_req(core, req, parent, &parent_req, req->rate);
ret = clk_core_round_rate_nolock(parent, &parent_req);
if (ret)
return ret;
Expand All @@ -594,23 +619,29 @@ int clk_mux_determine_rate_flags(struct clk_hw *hw,
/* find the parent that can provide the fastest rate <= rate */
num_parents = core->num_parents;
for (i = 0; i < num_parents; i++) {
unsigned long parent_rate;

parent = clk_core_get_parent_by_index(core, i);
if (!parent)
continue;

if (core->flags & CLK_SET_RATE_PARENT) {
parent_req = *req;
struct clk_rate_request parent_req;

clk_core_forward_rate_req(core, req, parent, &parent_req, req->rate);
ret = clk_core_round_rate_nolock(parent, &parent_req);
if (ret)
continue;

parent_rate = parent_req.rate;
} else {
parent_req.rate = clk_core_get_rate_nolock(parent);
parent_rate = clk_core_get_rate_nolock(parent);
}

if (mux_is_better_rate(req->rate, parent_req.rate,
if (mux_is_better_rate(req->rate, parent_rate,
best, flags)) {
best_parent = parent;
best = parent_req.rate;
best = parent_rate;
}
}

Expand Down Expand Up @@ -1448,6 +1479,31 @@ void clk_hw_init_rate_request(const struct clk_hw *hw,
}
EXPORT_SYMBOL_GPL(clk_hw_init_rate_request);

/**
* clk_hw_forward_rate_request - Forwards a clk_rate_request to a clock's parent
* @hw: the original clock that got the rate request
* @old_req: the original clk_rate_request structure we want to forward
* @parent: the clk we want to forward @old_req to
* @req: the clk_rate_request structure we want to initialise
* @parent_rate: The rate which is to be requested to @parent
*
* Initializes a clk_rate_request structure to submit to a clock parent
* in __clk_determine_rate() or similar functions.
*/
void clk_hw_forward_rate_request(const struct clk_hw *hw,
const struct clk_rate_request *old_req,
const struct clk_hw *parent,
struct clk_rate_request *req,
unsigned long parent_rate)
{
if (WARN_ON(!hw || !old_req || !parent || !req))
return;

clk_core_forward_rate_req(hw->core, old_req,
parent->core, req,
parent_rate);
}

static bool clk_core_can_round(struct clk_core * const core)
{
return core->ops->determine_rate || core->ops->round_rate;
Expand All @@ -1456,6 +1512,8 @@ static bool clk_core_can_round(struct clk_core * const core)
static int clk_core_round_rate_nolock(struct clk_core *core,
struct clk_rate_request *req)
{
int ret;

lockdep_assert_held(&prepare_lock);

if (!core) {
Expand All @@ -1465,8 +1523,20 @@ static int clk_core_round_rate_nolock(struct clk_core *core,

if (clk_core_can_round(core))
return clk_core_determine_round_nolock(core, req);
else if (core->flags & CLK_SET_RATE_PARENT)
return clk_core_round_rate_nolock(core->parent, req);

if (core->flags & CLK_SET_RATE_PARENT) {
struct clk_rate_request parent_req;

clk_core_forward_rate_req(core, req, core->parent, &parent_req, req->rate);
ret = clk_core_round_rate_nolock(core->parent, &parent_req);
if (ret)
return ret;

req->best_parent_rate = parent_req.rate;
req->rate = parent_req.rate;

return 0;
}

req->rate = core->rate;
return 0;
Expand Down

0 comments on commit 2034099

Please sign in to comment.