[PATCH] firmware: arm_scmi: get only min/max clock rates

From: Etienne Carriere
Date: Mon Jul 29 2024 - 03:30:16 EST


Remove limitation of 16 clock rates max for discrete clock rates
description.

Driver clk-scmi.c in only interested in the min and max clock rates.
Get these by querying the first and last discrete rates with SCMI
clock protocol message ID CLK_DESCRIBE_RATES since the SMCI clock
protocol specification states that rates enumerated by this
command are to be enumerated in "numeric ascending order" [1].

Link: https://developer.arm.com/documentation/den0056 [1]
Signed-off-by: Etienne Carriere <etienne.carriere@xxxxxxxxxxx>
---
drivers/clk/clk-scmi.c | 4 +-
drivers/firmware/arm_scmi/clock.c | 184 +++++++++++-------------------
include/linux/scmi_protocol.h | 4 +-
3 files changed, 68 insertions(+), 124 deletions(-)

diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c
index d86a02563f6c..abe59da06324 100644
--- a/drivers/clk/clk-scmi.c
+++ b/drivers/clk/clk-scmi.c
@@ -234,8 +234,8 @@ static int scmi_clk_ops_init(struct device *dev, struct scmi_clk *sclk,
if (num_rates <= 0)
return -EINVAL;

- min_rate = sclk->info->list.rates[0];
- max_rate = sclk->info->list.rates[num_rates - 1];
+ min_rate = sclk->info->list.min_rate;
+ max_rate = sclk->info->list.max_rate;
} else {
min_rate = sclk->info->range.min_rate;
max_rate = sclk->info->range.max_rate;
diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
index 134019297d08..88fd240052c0 100644
--- a/drivers/firmware/arm_scmi/clock.c
+++ b/drivers/firmware/arm_scmi/clock.c
@@ -404,140 +404,84 @@ static int scmi_clock_attributes_get(const struct scmi_protocol_handle *ph,
return ret;
}

-static int rate_cmp_func(const void *_r1, const void *_r2)
+static int
+scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id,
+ struct scmi_clock_info *clk)
{
- const u64 *r1 = _r1, *r2 = _r2;
-
- if (*r1 < *r2)
- return -1;
- else if (*r1 == *r2)
- return 0;
- else
- return 1;
-}
+ struct scmi_msg_clock_describe_rates *msg;
+ const struct scmi_msg_resp_clock_describe_rates *resp;
+ unsigned int num_returned, num_remaining;
+ struct scmi_xfer *t;
+ int ret;

-static void iter_clk_describe_prepare_message(void *message,
- const unsigned int desc_index,
- const void *priv)
-{
- struct scmi_msg_clock_describe_rates *msg = message;
- const struct scmi_clk_ipriv *p = priv;
+ /* Get either the range triplet or the min rate & rates count */
+ ret = ph->xops->xfer_get_init(ph, CLOCK_DESCRIBE_RATES, sizeof(*msg), 0,
+ &t);
+ if (ret)
+ return ret;

- msg->id = cpu_to_le32(p->clk_id);
- /* Set the number of rates to be skipped/already read */
- msg->rate_index = cpu_to_le32(desc_index);
-}
+ msg = t->tx.buf;
+ msg->id = cpu_to_le32(clk_id);
+ msg->rate_index = 0;

-static int
-iter_clk_describe_update_state(struct scmi_iterator_state *st,
- const void *response, void *priv)
-{
- u32 flags;
- struct scmi_clk_ipriv *p = priv;
- const struct scmi_msg_resp_clock_describe_rates *r = response;
-
- 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);
-
- /* Warn about out of spec replies ... */
- if (!p->clk->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,
- st->rx_len);
-
- /*
- * A known quirk: a triplet is returned but num_returned != 3
- * Check for a safe payload size and fix.
- */
- if (st->num_returned != 3 && st->num_remaining == 0 &&
- st->rx_len == sizeof(*r) + sizeof(__le32) * 2 * 3) {
- st->num_returned = 3;
- st->num_remaining = 0;
- } else {
- dev_err(p->dev,
- "Cannot fix out-of-spec reply !\n");
- return -EPROTO;
- }
- }
+ resp = t->rx.buf;

- return 0;
-}
+ ret = ph->xops->do_xfer(ph, t);
+ if (ret)
+ goto out;

-static int
-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];
+ clk->rate_discrete = RATE_DISCRETE(resp->num_rates_flags);
+ num_returned = NUM_RETURNED(resp->num_rates_flags);
+ num_remaining = NUM_REMAINING(resp->num_rates_flags);

- *rate = RATE_TO_U64(r->rate[st->loop_idx]);
- p->clk->list.num_rates++;
- }
+ if (clk->rate_discrete) {
+ clk->list.num_rates = num_returned + num_remaining;
+ clk->list.min_rate = RATE_TO_U64(resp->rate[0]);

- return ret;
-}
+ if (num_remaining) {
+ msg->rate_index = clk->list.num_rates - 1;

-static int
-scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id,
- struct scmi_clock_info *clk)
-{
- int ret;
- void *iter;
- struct scmi_iterator_ops ops = {
- .prepare_message = iter_clk_describe_prepare_message,
- .update_state = iter_clk_describe_update_state,
- .process_response = iter_clk_describe_process_response,
- };
- struct scmi_clk_ipriv cpriv = {
- .clk_id = clk_id,
- .clk = clk,
- .dev = ph->dev,
- };
+ ret = ph->xops->do_xfer(ph, t);
+ if (ret)
+ goto out;

- iter = ph->hops->iter_response_init(ph, &ops, SCMI_MAX_NUM_RATES,
- CLOCK_DESCRIBE_RATES,
- sizeof(struct scmi_msg_clock_describe_rates),
- &cpriv);
- if (IS_ERR(iter))
- return PTR_ERR(iter);
+ clk->list.max_rate = RATE_TO_U64(resp->rate[0]);
+ } else {
+ u64 max = RATE_TO_U64(resp->rate[num_returned - 1]);

- ret = ph->hops->iter_response_run(iter);
- if (ret)
- return ret;
+ clk->list.max_rate = max;
+ }
+ } else {
+ /* Warn about out of spec replies ... */
+ if (num_returned != 3 || num_remaining != 0) {
+ dev_warn(ph->dev,
+ "Out-of-spec CLOCK_DESCRIBE_RATES reply for %s - returned:%d remaining:%d rx_len:%zd\n",
+ clk->name, num_returned, num_remaining,
+ t->rx.len);
+
+ /*
+ * A known quirk: a triplet is returned but
+ * num_returned != 3, check for a safe payload
+ * size to use returned info.
+ */
+ if (num_remaining != 0 ||
+ t->rx.len != sizeof(*resp) +
+ sizeof(__le32) * 2 * 3) {
+ dev_err(ph->dev,
+ "Cannot fix out-of-spec reply !\n");
+ ret = -EPROTO;
+ goto out;
+ }
+ }

- if (!clk->rate_discrete) {
- 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);
+ clk->range.min_rate = RATE_TO_U64(resp->rate[0]);
+ clk->range.max_rate = RATE_TO_U64(resp->rate[1]);
+ clk->range.step_size = RATE_TO_U64(resp->rate[2]);
}

+out:
+ ph->xops->xfer_put(ph, t);
+
return ret;
}

diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 3a9bb5b9a9e8..fe4b464419a0 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -15,7 +15,6 @@

#define SCMI_MAX_STR_SIZE 64
#define SCMI_SHORT_NAME_MAX_SIZE 16
-#define SCMI_MAX_NUM_RATES 16

/**
* struct scmi_revision_info - version information structure
@@ -54,7 +53,8 @@ struct scmi_clock_info {
union {
struct {
int num_rates;
- u64 rates[SCMI_MAX_NUM_RATES];
+ u64 min_rate;
+ u64 max_rate;
} list;
struct {
u64 min_rate;
--
2.25.1