Re: [PATCH v4 2/2] clk: scmi: add set/get_parent support
From: Cristian Marussi <cristian.marussi@arm.com>
Date: 2023-10-03 12:13:41
Also in:
linux-clk, lkml
On Tue, Oct 03, 2023 at 07:48:49PM +0800, Peng Fan (OSS) wrote:
From: Peng Fan <peng.fan@nxp.com> SCMI v3.2 adds set/get parent clock commands, so update the clk driver to support them.
Hi, in general LGTM, BUT I have just spotted one more *bad* thing that have to be fixe down below which I have missed previously, my bad.
quoted hunk ↗ jump to hunk
Signed-off-by: Peng Fan <peng.fan@nxp.com> --- drivers/clk/clk-scmi.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-)diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c index 2e1337b511eb..e7a27fda561b 100644 --- a/drivers/clk/clk-scmi.c +++ b/drivers/clk/clk-scmi.c@@ -24,6 +24,7 @@ struct scmi_clk { struct clk_hw hw; const struct scmi_clock_info *info; const struct scmi_protocol_handle *ph; + struct clk_parent_data *parent_data; }; #define to_scmi_clk(clk) container_of(clk, struct scmi_clk, hw)@@ -78,6 +79,43 @@ static int scmi_clk_set_rate(struct clk_hw *hw, unsigned long rate, return scmi_proto_clk_ops->rate_set(clk->ph, clk->id, rate); } +static int scmi_clk_set_parent(struct clk_hw *hw, u8 parent_index) +{ + struct scmi_clk *clk = to_scmi_clk(hw); + + return scmi_proto_clk_ops->parent_set(clk->ph, clk->id, parent_index); +} + +static u8 scmi_clk_get_parent(struct clk_hw *hw) +{ + struct scmi_clk *clk = to_scmi_clk(hw); + u32 parent_id, p_idx; + int ret; + + ret = scmi_proto_clk_ops->parent_get(clk->ph, clk->id, &parent_id); + if (ret) + return 0; + + for (p_idx = 0; p_idx < clk->info->num_parents; p_idx++) { + if (clk->parent_data[p_idx].index == parent_id) + break; + } + + if (p_idx == clk->info->num_parents) + return 0; + + return p_idx; +} + +static int scmi_clk_determine_rate(struct clk_hw *hw, struct clk_rate_request *req) +{ + /* + * Suppose all the requested rates are supported, and let firmware + * to handle the left work. + */ + return 0; +} + static int scmi_clk_enable(struct clk_hw *hw) { struct scmi_clk *clk = to_scmi_clk(hw);@@ -139,6 +177,9 @@ static const struct clk_ops scmi_clk_ops = { .set_rate = scmi_clk_set_rate, .prepare = scmi_clk_enable, .unprepare = scmi_clk_disable, + .set_parent = scmi_clk_set_parent, + .get_parent = scmi_clk_get_parent, + .determine_rate = scmi_clk_determine_rate, }; static const struct clk_ops scmi_atomic_clk_ops = {@@ -148,6 +189,9 @@ static const struct clk_ops scmi_atomic_clk_ops = { .enable = scmi_clk_atomic_enable, .disable = scmi_clk_atomic_disable, .is_enabled = scmi_clk_atomic_is_enabled, + .set_parent = scmi_clk_set_parent, + .get_parent = scmi_clk_get_parent, + .determine_rate = scmi_clk_determine_rate, }; static int scmi_clk_ops_init(struct device *dev, struct scmi_clk *sclk,@@ -158,9 +202,10 @@ static int scmi_clk_ops_init(struct device *dev, struct scmi_clk *sclk, struct clk_init_data init = { .flags = CLK_GET_RATE_NOCACHE, - .num_parents = 0, + .num_parents = sclk->info->num_parents, .ops = scmi_ops, .name = sclk->info->name, + .parent_data = sclk->parent_data, }; sclk->hw.init = &init;@@ -250,6 +295,17 @@ static int scmi_clocks_probe(struct scmi_device *sdev) else scmi_ops = &scmi_clk_ops; + /* Initialize clock parent data. */ + if (sclk->info->num_parents > 0) { + sclk->parent_data = devm_kcalloc(dev, sclk->info->num_parents, + sizeof(*sclk->parent_data), GFP_KERNEL); +
Here, you missed to check the return value of devm_kcalloc() before carrying on.
+ for (int i = 0; i < sclk->info->num_parents; i++) {
+ sclk->parent_data[i].index = sclk->info->parents[i];
+ sclk->parent_data[i].hw = hws[sclk->info->parents[i]];
+ }
+ }
+Other than this, FWIW: Reviewed-by: Cristian Marussi <cristian.marussi@arm.com> Thanks, Cristian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel