Re: [PATCH] peci: controller: peci-aspeed: convert from round_rate() to determine_rate()

Winiarska, Iwona
2025-10-21T21:07:50Z

On Sun, 2025-08-10 at 18:21 -0400, Brian Masney wrote:
> The round_rate() clk ops is deprecated, so migrate this driver from
> round_rate() to determine_rate() using the Coccinelle semantic patch
> appended to the "under-the-cut" portion of the patch.
>
> Signed-off-by: Brian Masney <bmasney@redhat.com>

Reviewed-by: Iwona Winiarska <iwona.winiarska@intel.com>

Thanks
-Iwona

> ---
> Coccinelle semantic patch is below. It's large and I don't want to
> pollute the kernel changelog with the same code hundreds of times,
> so that's why it's included under the cut. For subsystems with more
> than one patch, I've included it on the cover letter.
>
>     virtual patch
>
>     // Look up the current name of the round_rate function
>     @ has_round_rate @
>     identifier round_rate_name =~ ".*_round_rate";
>     identifier hw_param, rate_param, parent_rate_param;
>     @@
>
>     long round_rate_name(struct clk_hw *hw_param, unsigned long rate_param,
>                   unsigned long *parent_rate_param)
>     {
>      ...
>     }
>
>     // Rename the route_rate function name to determine_rate()
>     @ script:python generate_name depends on has_round_rate @
>     round_rate_name << has_round_rate.round_rate_name;
>     new_name;
>     @@
>
>     coccinelle.new_name = round_rate_name.replace("_round_rate",
> "_determine_rate")
>
>     // Change rate to req->rate; also change occurrences of 'return XXX'.
>     @ chg_rate depends on generate_name @
>     identifier has_round_rate.round_rate_name;
>     identifier has_round_rate.hw_param;
>     identifier has_round_rate.rate_param;
>     identifier has_round_rate.parent_rate_param;
>     identifier ERR =~ "E.*";
>     expression E;
>     @@
>
>     long round_rate_name(struct clk_hw *hw_param, unsigned long rate_param,
>                   unsigned long *parent_rate_param)
>     {
>     <...
>     (
>     -return -ERR;
>     +return -ERR;
>     |
>     - return rate_param;
>     + return 0;
>     |
>     - return E;
>     + req->rate = E;
>     +
>     + return 0;
>     |
>     - rate_param
>     + req->rate
>     )
>     ...>
>     }
>
>     // Coccinelle only transforms the first occurrence of the rate parameter
>     // Run a second time. FIXME: Is there a better way to do this?
>     @ chg_rate2 depends on generate_name @
>     identifier has_round_rate.round_rate_name;
>     identifier has_round_rate.hw_param;
>     identifier has_round_rate.rate_param;
>     identifier has_round_rate.parent_rate_param;
>     @@
>
>     long round_rate_name(struct clk_hw *hw_param, unsigned long rate_param,
>                   unsigned long *parent_rate_param)
>     {
>     <...
>     - rate_param
>     + req->rate
>     ...>
>     }
>
>     // Change parent_rate to req->best_parent_rate
>     @ chg_parent_rate depends on generate_name @
>     identifier has_round_rate.round_rate_name;
>     identifier has_round_rate.hw_param;
>     identifier has_round_rate.rate_param;
>     identifier has_round_rate.parent_rate_param;
>     @@
>
>     long round_rate_name(struct clk_hw *hw_param, unsigned long rate_param,
>                   unsigned long *parent_rate_param)
>     {
>     <...
>     (
>     - *parent_rate_param
>     + req->best_parent_rate
>     |
>     - parent_rate_param
>     + &req->best_parent_rate
>     )
>     ...>
>     }
>
>     // Convert the function definition from round_rate() to determine_rate()
>     @ func_definition depends on chg_rate @
>     identifier has_round_rate.round_rate_name;
>     identifier has_round_rate.hw_param;
>     identifier has_round_rate.rate_param;
>     identifier has_round_rate.parent_rate_param;
>     identifier generate_name.new_name;
>     @@
>
>     - long round_rate_name(struct clk_hw *hw_param, unsigned long rate_param,
>     -               unsigned long *parent_rate_param)
>     + int new_name(struct clk_hw *hw, struct clk_rate_request *req)
>     {
>         ...
>     }
>
>     // Update the ops from round_rate() to determine_rate()
>     @ ops depends on func_definition @
>     identifier has_round_rate.round_rate_name;
>     identifier generate_name.new_name;
>     @@
>
>     {
>         ...,
>     -   .round_rate = round_rate_name,
>     +   .determine_rate = new_name,
>         ...,
>     }
>
> Note that I used coccinelle 1.2 instead of 1.3 since the newer version
> adds unnecessary braces as described in this post.
> https://lore.kernel.org/cocci/67642477-5f3e-4b2a-914d-579a54f48cbd@intel.com/
> ---
>  drivers/peci/controller/peci-aspeed.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/peci/controller/peci-aspeed.c
> b/drivers/peci/controller/peci-aspeed.c
> index
> ad3a7d71ed4c61e6123dcbbad264812cb83cbb07..a0c99ecf7f3805a1cdac55a8d5db9c61ad3c
> f37c 100644
> --- a/drivers/peci/controller/peci-aspeed.c
> +++ b/drivers/peci/controller/peci-aspeed.c
> @@ -362,12 +362,14 @@ static int clk_aspeed_peci_set_rate(struct clk_hw *hw,
> unsigned long rate,
>   return 0;
>  }
>
> -static long clk_aspeed_peci_round_rate(struct clk_hw *hw, unsigned long rate,
> -        unsigned long *prate)
> +static int clk_aspeed_peci_determine_rate(struct clk_hw *hw,
> +   struct clk_rate_request *req)
>  {
> - int div = clk_aspeed_peci_get_div(rate, prate);
> + int div = clk_aspeed_peci_get_div(req->rate, &req->best_parent_rate);
>
> - return DIV_ROUND_UP_ULL(*prate, div);
> + req->rate = DIV_ROUND_UP_ULL(req->best_parent_rate, div);
> +
> + return 0;
>  }
>
>  static unsigned long clk_aspeed_peci_recalc_rate(struct clk_hw *hw, unsigned
> long prate)
> @@ -394,7 +396,7 @@ static unsigned long clk_aspeed_peci_recalc_rate(struct
> clk_hw *hw, unsigned lon
>
>  static const struct clk_ops clk_aspeed_peci_ops = {
>   .set_rate = clk_aspeed_peci_set_rate,
> - .round_rate = clk_aspeed_peci_round_rate,
> + .determine_rate = clk_aspeed_peci_determine_rate,
>   .recalc_rate = clk_aspeed_peci_recalc_rate,
>  };
>
>
> ---
> base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
> change-id: 20250729-peci-round-rate-543639297ec6
>
> Best regards,