[PATCH v4 04/15] firmware: arm_scmi: Simplify clock rates exposed interface

Cristian Marussi <[email protected]>
Newsgroups org.kernel.vger.arm-scmi,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-clk,org.kernel.vger.linux-kernel,org.kernel.vger.linux-renesas-soc
Message-ID <[email protected]>
Introduce a new internal struct scmi_clock_desc so as to be able to hide,
in the future, some of the needlessly public fields currently kept inside
scmi_clock_info, while keeping exposed only the two new min_rate and
max_rate fields for each clock.

No functional change.

Reviewed-by: Peng Fan <[email protected]>
Signed-off-by: Cristian Marussi <[email protected]>
---
v1 --> v2
 - removed useless parenthesis
 - reworded comit message
---
 drivers/firmware/arm_scmi/clock.c | 145 +++++++++++++++---------------
 include/linux/scmi_protocol.h     |   2 +
 2 files changed, 74 insertions(+), 73 deletions(-)

diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
index 54b55517b759..467b13a3a18f 100644
--- a/drivers/firmware/arm_scmi/clock.c
+++ b/drivers/firmware/arm_scmi/clock.c
@@ -157,13 +157,27 @@ struct scmi_clock_rate_notify_payld {
 	__le32 rate_high;
 };
 
+struct scmi_clock_desc {
+	u32 id;
+	bool rate_discrete;
+	unsigned int num_rates;
+	u64 rates[SCMI_MAX_NUM_RATES];
+#define	RATE_MIN	0
+#define	RATE_MAX	1
+#define	RATE_STEP	2
+	struct scmi_clock_info info;
+};
+
+#define to_desc(p)	(container_of(p, struct scmi_clock_desc, info))
+
 struct clock_info {
 	int num_clocks;
 	int max_async_req;
 	bool notify_rate_changed_cmd;
 	bool notify_rate_change_requested_cmd;
 	atomic_t cur_async_req;
-	struct scmi_clock_info *clk;
+	struct scmi_clock_desc *clkds;
+#define CLOCK_INFO(c, i)	(&(((c)->clkds + (i))->info))
 	int (*clock_config_set)(const struct scmi_protocol_handle *ph,
 				u32 clk_id, enum clk_state state,
 				enum scmi_clock_oem_config oem_type,
@@ -185,7 +199,7 @@ scmi_clock_domain_lookup(struct clock_info *ci, u32 clk_id)
 	if (clk_id >= ci->num_clocks)
 		return ERR_PTR(-EINVAL);
 
-	return ci->clk + clk_id;
+	return CLOCK_INFO(ci, clk_id);
 }
 
 static int
@@ -226,8 +240,7 @@ scmi_clock_protocol_attributes_get(const struct scmi_protocol_handle *ph,
 
 struct scmi_clk_ipriv {
 	struct device *dev;
-	u32 clk_id;
-	struct scmi_clock_info *clk;
+	struct scmi_clock_desc *clkd;
 };
 
 static void iter_clk_possible_parents_prepare_message(void *message, unsigned int desc_index,
@@ -236,7 +249,7 @@ static void iter_clk_possible_parents_prepare_message(void *message, unsigned in
 	struct scmi_msg_clock_possible_parents *msg = message;
 	const struct scmi_clk_ipriv *p = priv;
 
-	msg->id = cpu_to_le32(p->clk_id);
+	msg->id = cpu_to_le32(p->clkd->id);
 	/* Set the number of OPPs to be skipped/already read */
 	msg->skip_parents = cpu_to_le32(desc_index);
 }
@@ -246,7 +259,6 @@ static int iter_clk_possible_parents_update_state(struct scmi_iterator_state *st
 {
 	const struct scmi_msg_resp_clock_possible_parents *r = response;
 	struct scmi_clk_ipriv *p = priv;
-	struct device *dev = ((struct scmi_clk_ipriv *)p)->dev;
 	u32 flags;
 
 	flags = le32_to_cpu(r->num_parent_flags);
@@ -258,12 +270,13 @@ static int iter_clk_possible_parents_update_state(struct scmi_iterator_state *st
 	 * assume it's returned+remaining on first call.
 	 */
 	if (!st->max_resources) {
-		p->clk->num_parents = st->num_returned + st->num_remaining;
-		p->clk->parents = devm_kcalloc(dev, p->clk->num_parents,
-					       sizeof(*p->clk->parents),
-					       GFP_KERNEL);
-		if (!p->clk->parents) {
-			p->clk->num_parents = 0;
+		p->clkd->info.num_parents = st->num_returned + st->num_remaining;
+		p->clkd->info.parents = devm_kcalloc(p->dev,
+						     p->clkd->info.num_parents,
+						     sizeof(*p->clkd->info.parents),
+						     GFP_KERNEL);
+		if (!p->clkd->info.parents) {
+			p->clkd->info.num_parents = 0;
 			return -ENOMEM;
 		}
 		st->max_resources = st->num_returned + st->num_remaining;
@@ -280,29 +293,27 @@ static int iter_clk_possible_parents_process_response(const struct scmi_protocol
 	const struct scmi_msg_resp_clock_possible_parents *r = response;
 	struct scmi_clk_ipriv *p = priv;
 
-	u32 *parent = &p->clk->parents[st->desc_index + st->loop_idx];
+	u32 *parent = &p->clkd->info.parents[st->desc_index + st->loop_idx];
 
 	*parent = le32_to_cpu(r->possible_parents[st->loop_idx]);
 
 	return 0;
 }
 
-static int scmi_clock_possible_parents(const struct scmi_protocol_handle *ph, u32 clk_id,
-				       struct scmi_clock_info *clk)
+static int scmi_clock_possible_parents(const struct scmi_protocol_handle *ph,
+				       u32 clk_id, struct clock_info *cinfo)
 {
 	struct scmi_iterator_ops ops = {
 		.prepare_message = iter_clk_possible_parents_prepare_message,
 		.update_state = iter_clk_possible_parents_update_state,
 		.process_response = iter_clk_possible_parents_process_response,
 	};
-
+	struct scmi_clock_desc *clkd = &cinfo->clkds[clk_id];
 	struct scmi_clk_ipriv ppriv = {
-		.clk_id = clk_id,
-		.clk = clk,
+		.clkd = clkd,
 		.dev = ph->dev,
 	};
 	void *iter;
-	int ret;
 
 	iter = ph->hops->iter_response_init(ph, &ops, 0,
 					    CLOCK_POSSIBLE_PARENTS_GET,
@@ -311,9 +322,7 @@ static int scmi_clock_possible_parents(const struct scmi_protocol_handle *ph, u3
 	if (IS_ERR(iter))
 		return PTR_ERR(iter);
 
-	ret = ph->hops->iter_response_run(iter);
-
-	return ret;
+	return ph->hops->iter_response_run(iter);
 }
 
 static int
@@ -352,7 +361,7 @@ static int scmi_clock_attributes_get(const struct scmi_protocol_handle *ph,
 	u32 attributes;
 	struct scmi_xfer *t;
 	struct scmi_msg_resp_clock_attributes *attr;
-	struct scmi_clock_info *clk = cinfo->clk + clk_id;
+	struct scmi_clock_info *clk = CLOCK_INFO(cinfo, clk_id);
 
 	ret = ph->xops->xfer_get_init(ph, CLOCK_ATTRIBUTES,
 				      sizeof(clk_id), sizeof(*attr), &t);
@@ -394,7 +403,7 @@ static int scmi_clock_attributes_get(const struct scmi_protocol_handle *ph,
 			clk->rate_change_requested_notifications = true;
 		if (PROTOCOL_REV_MAJOR(ph->version) >= 0x3) {
 			if (SUPPORTS_PARENT_CLOCK(attributes))
-				scmi_clock_possible_parents(ph, clk_id, clk);
+				scmi_clock_possible_parents(ph, clk_id, cinfo);
 			if (SUPPORTS_GET_PERMISSIONS(attributes))
 				scmi_clock_get_permissions(ph, clk_id, clk);
 			if (SUPPORTS_EXTENDED_CONFIG(attributes))
@@ -424,7 +433,7 @@ static void iter_clk_describe_prepare_message(void *message,
 	struct scmi_msg_clock_describe_rates *msg = message;
 	const struct scmi_clk_ipriv *p = priv;
 
-	msg->id = cpu_to_le32(p->clk_id);
+	msg->id = cpu_to_le32(p->clkd->id);
 	/* Set the number of rates to be skipped/already read */
 	msg->rate_index = cpu_to_le32(desc_index);
 }
@@ -457,14 +466,14 @@ iter_clk_describe_update_state(struct scmi_iterator_state *st,
 	flags = le32_to_cpu(r->num_rates_flags);
 	st->num_remaining = NUM_REMAINING(flags);
 	st->num_returned = NUM_RETURNED(flags);
-	p->clk->rate_discrete = RATE_DISCRETE(flags);
+	p->clkd->rate_discrete = RATE_DISCRETE(flags);
 
 	/* Warn about out of spec replies ... */
-	if (!p->clk->rate_discrete &&
+	if (!p->clkd->rate_discrete &&
 	    (st->num_returned != 3 || st->num_remaining != 0)) {
 		dev_warn(p->dev,
 			 "Out-of-spec CLOCK_DESCRIBE_RATES reply for %s - returned:%d remaining:%d rx_len:%zd\n",
-			 p->clk->name, st->num_returned, st->num_remaining,
+			 p->clkd->info.name, st->num_returned, st->num_remaining,
 			 st->rx_len);
 
 		SCMI_QUIRK(clock_rates_triplet_out_of_spec,
@@ -479,38 +488,19 @@ iter_clk_describe_process_response(const struct scmi_protocol_handle *ph,
 				   const void *response,
 				   struct scmi_iterator_state *st, void *priv)
 {
-	int ret = 0;
 	struct scmi_clk_ipriv *p = priv;
 	const struct scmi_msg_resp_clock_describe_rates *r = response;
 
-	if (!p->clk->rate_discrete) {
-		switch (st->desc_index + st->loop_idx) {
-		case 0:
-			p->clk->range.min_rate = RATE_TO_U64(r->rate[0]);
-			break;
-		case 1:
-			p->clk->range.max_rate = RATE_TO_U64(r->rate[1]);
-			break;
-		case 2:
-			p->clk->range.step_size = RATE_TO_U64(r->rate[2]);
-			break;
-		default:
-			ret = -EINVAL;
-			break;
-		}
-	} else {
-		u64 *rate = &p->clk->list.rates[st->desc_index + st->loop_idx];
+	p->clkd->rates[st->desc_index + st->loop_idx] =
+		RATE_TO_U64(r->rate[st->loop_idx]);
+	p->clkd->num_rates++;
 
-		*rate = RATE_TO_U64(r->rate[st->loop_idx]);
-		p->clk->list.num_rates++;
-	}
-
-	return ret;
+	return 0;
 }
 
 static int
 scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id,
-			      struct scmi_clock_info *clk)
+			      struct clock_info *cinfo)
 {
 	int ret;
 	void *iter;
@@ -519,9 +509,9 @@ scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id,
 		.update_state = iter_clk_describe_update_state,
 		.process_response = iter_clk_describe_process_response,
 	};
+	struct scmi_clock_desc *clkd = &cinfo->clkds[clk_id];
 	struct scmi_clk_ipriv cpriv = {
-		.clk_id = clk_id,
-		.clk = clk,
+		.clkd = clkd,
 		.dev = ph->dev,
 	};
 
@@ -536,16 +526,23 @@ scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id,
 	if (ret)
 		return ret;
 
-	if (!clk->rate_discrete) {
+	/* empty set ? */
+	if (!clkd->num_rates)
+		return 0;
+
+	if (!clkd->rate_discrete) {
+		clkd->info.max_rate = clkd->rates[RATE_MAX];
 		dev_dbg(ph->dev, "Min %llu Max %llu Step %llu Hz\n",
-			clk->range.min_rate, clk->range.max_rate,
-			clk->range.step_size);
-	} else if (clk->list.num_rates) {
-		sort(clk->list.rates, clk->list.num_rates,
-		     sizeof(clk->list.rates[0]), rate_cmp_func, NULL);
+			clkd->rates[RATE_MIN], clkd->rates[RATE_MAX],
+			clkd->rates[RATE_STEP]);
+	} else {
+		sort(clkd->rates, clkd->num_rates,
+		     sizeof(clkd->rates[0]), rate_cmp_func, NULL);
+		clkd->info.max_rate = clkd->rates[clkd->num_rates - 1];
 	}
+	clkd->info.min_rate = clkd->rates[RATE_MIN];
 
-	return ret;
+	return 0;
 }
 
 static int
@@ -630,6 +627,7 @@ static int scmi_clock_determine_rate(const struct scmi_protocol_handle *ph,
 {
 	u64 fmin, fmax, ftmp;
 	struct scmi_clock_info *clk;
+	struct scmi_clock_desc *clkd;
 	struct clock_info *ci = ph->get_priv(ph);
 
 	if (!rate)
@@ -639,15 +637,17 @@ static int scmi_clock_determine_rate(const struct scmi_protocol_handle *ph,
 	if (IS_ERR(clk))
 		return PTR_ERR(clk);
 
+	clkd = to_desc(clk);
+
 	/*
 	 * If we can't figure out what rate it will be, so just return the
 	 * rate back to the caller.
 	 */
-	if (clk->rate_discrete)
+	if (clkd->rate_discrete)
 		return 0;
 
-	fmin = clk->range.min_rate;
-	fmax = clk->range.max_rate;
+	fmin = clk->min_rate;
+	fmax = clk->max_rate;
 	if (*rate <= fmin) {
 		*rate = fmin;
 		return 0;
@@ -657,10 +657,10 @@ static int scmi_clock_determine_rate(const struct scmi_protocol_handle *ph,
 	}
 
 	ftmp = *rate - fmin;
-	ftmp += clk->range.step_size - 1; /* to round up */
-	ftmp = div64_ul(ftmp, clk->range.step_size);
+	ftmp += clkd->rates[RATE_STEP] - 1; /* to round up */
+	ftmp = div64_ul(ftmp, clkd->rates[RATE_STEP]);
 
-	*rate = ftmp * clk->range.step_size + fmin;
+	*rate = ftmp * clkd->rates[RATE_STEP] + fmin;
 
 	return 0;
 }
@@ -1122,17 +1122,16 @@ static int scmi_clock_protocol_init(const struct scmi_protocol_handle *ph)
 	if (ret)
 		return ret;
 
-	cinfo->clk = devm_kcalloc(ph->dev, cinfo->num_clocks,
-				  sizeof(*cinfo->clk), GFP_KERNEL);
-	if (!cinfo->clk)
+	cinfo->clkds = devm_kcalloc(ph->dev, cinfo->num_clocks,
+				    sizeof(*cinfo->clkds), GFP_KERNEL);
+	if (!cinfo->clkds)
 		return -ENOMEM;
 
 	for (clkid = 0; clkid < cinfo->num_clocks; clkid++) {
-		struct scmi_clock_info *clk = cinfo->clk + clkid;
-
+		cinfo->clkds[clkid].id = clkid;
 		ret = scmi_clock_attributes_get(ph, clkid, cinfo);
 		if (!ret)
-			scmi_clock_describe_rates_get(ph, clkid, clk);
+			scmi_clock_describe_rates_get(ph, clkid, cinfo);
 	}
 
 	if (PROTOCOL_REV_MAJOR(ph->version) >= 0x3) {
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 28579c145045..7283302b0c85 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -51,6 +51,8 @@ struct scmi_clock_info {
 	bool rate_ctrl_forbidden;
 	bool parent_ctrl_forbidden;
 	bool extended_config;
+	u64 min_rate;
+	u64 max_rate;
 	union {
 		struct {
 			int num_rates;
-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.