From: "Winiarska, Iwona" <iwona.winiarska@intel.com>
To: "andrew@codeconstruct.com.au" <andrew@codeconstruct.com.au>,
"mripard@kernel.org" <mripard@kernel.org>,
"sboyd@kernel.org" <sboyd@kernel.org>,
"bmasney@redhat.com" <bmasney@redhat.com>,
"joel@jms.id.au" <joel@jms.id.au>
Cc: "linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>,
"openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>,
"linux-aspeed@lists.ozlabs.org" <linux-aspeed@lists.ozlabs.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] peci: controller: peci-aspeed: convert from round_rate() to determine_rate()
Date: Tue, 21 Oct 2025 21:07:47 +0000 [thread overview]
Message-ID: <ec059291186911553b38ddaab4375b51e89bd0af.camel@intel.com> (raw)
In-Reply-To: <20250810-peci-round-rate-v1-1-ec96d216a455@redhat.com>
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,
prev parent reply other threads:[~2025-10-21 21:07 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-10 22:21 [PATCH] peci: controller: peci-aspeed: convert from round_rate() to determine_rate() Brian Masney
2025-09-14 11:56 ` Brian Masney
2025-09-15 5:06 ` Andrew Jeffery
2025-10-16 16:11 ` Brian Masney
2025-10-17 6:22 ` Andrew Jeffery
2025-10-17 6:41 ` Greg Kroah-Hartman
2025-10-17 7:27 ` Andrew Jeffery
2025-10-17 7:43 ` Greg Kroah-Hartman
2025-10-19 23:00 ` Andrew Jeffery
2025-10-21 21:01 ` Winiarska, Iwona
2025-10-21 22:35 ` Andrew Jeffery
2025-11-06 23:11 ` Brian Masney
2025-10-21 21:07 ` Winiarska, Iwona [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ec059291186911553b38ddaab4375b51e89bd0af.camel@intel.com \
--to=iwona.winiarska@intel.com \
--cc=andrew@codeconstruct.com.au \
--cc=bmasney@redhat.com \
--cc=joel@jms.id.au \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-aspeed@lists.ozlabs.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mripard@kernel.org \
--cc=openbmc@lists.ozlabs.org \
--cc=sboyd@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).