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

From: Etienne Carriere
Date: Tue Dec 03 2024 - 12:40:33 EST


Remove limitation of 16 clock rates max for discrete clock rates
description when the SCMI firmware supports SCMI Clock protocol v2.0
or later.

Driver clk-scmi.c is 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 CLOCK_DESCRIBE_RATES since the SCMI
specification v2.0 and later states that rates enumerated by this
command are to be enumerated in "numeric ascending order" [1].

Preserve the implementation that queries all discrete rates (16 rates
max) to support SCMI firmware built on SCMI specification v1.0 [2]
where SCMI Clock protocol v1.0 does not explicitly require rates
described with CLOCK_DESCRIBE_RATES to be in ascending order.

Link: https://developer.arm.com/documentation/den0056 [1]
Link: https://developer.arm.com/documentation/den0056/a [2]
Signed-off-by: Etienne Carriere <etienne.carriere@xxxxxxxxxxx>
---
Changes since patch series v1:
- Preserve the original implementation and keep using it for SCMI
Clock protocol v1.0.

---
drivers/clk/clk-scmi.c | 4 +-
drivers/firmware/arm_scmi/clock.c | 112 ++++++++++++++++++++++++++++--
include/linux/scmi_protocol.h | 4 +-
3 files changed, 110 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c
index 15510c2ff21c..09ccd6cea7f2 100644
--- a/drivers/clk/clk-scmi.c
+++ b/drivers/clk/clk-scmi.c
@@ -244,8 +244,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 2ed2279388f0..34fde0b88098 100644
--- a/drivers/firmware/arm_scmi/clock.c
+++ b/drivers/firmware/arm_scmi/clock.c
@@ -223,10 +223,21 @@ scmi_clock_protocol_attributes_get(const struct scmi_protocol_handle *ph,
return ret;
}

+/*
+ * Support SCMI_CLOCK protocol v1.0 as described in SCMI specification v1.0
+ * that do not explicitly require clock rates described with command
+ * CLOCK_DESCRIBE_RATES to be in ascending order. The Linux legacy
+ * implementation for these clock supports a max of 16 rates.
+ * In SCMI specification v2.0 and later, rates must be in ascending order
+ * to we query only to min and max rates values.
+ */
+#define SCMI_MAX_NUM_RATES_V1 16
+
struct scmi_clk_ipriv {
struct device *dev;
u32 clk_id;
struct scmi_clock_info *clk;
+ u64 rates[SCMI_MAX_NUM_RATES_V1];
};

static void iter_clk_possible_parents_prepare_message(void *message, unsigned int desc_index,
@@ -493,7 +504,7 @@ iter_clk_describe_process_response(const struct scmi_protocol_handle *ph,
break;
}
} else {
- u64 *rate = &p->clk->list.rates[st->desc_index + st->loop_idx];
+ u64 *rate = &p->rates[st->desc_index + st->loop_idx];

*rate = RATE_TO_U64(r->rate[st->loop_idx]);
p->clk->list.num_rates++;
@@ -519,7 +530,7 @@ scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id,
.dev = ph->dev,
};

- iter = ph->hops->iter_response_init(ph, &ops, SCMI_MAX_NUM_RATES,
+ iter = ph->hops->iter_response_init(ph, &ops, ARRAY_SIZE(cpriv.rates),
CLOCK_DESCRIBE_RATES,
sizeof(struct scmi_msg_clock_describe_rates),
&cpriv);
@@ -535,10 +546,95 @@ scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id,
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);
+ sort(cpriv.rates, clk->list.num_rates,
+ sizeof(cpriv.rates[0]), rate_cmp_func, NULL);
+ clk->list.min_rate = cpriv.rates[0];
+ clk->list.max_rate = cpriv.rates[clk->list.num_rates - 1];
+ }
+
+ return ret;
+}
+
+static int scmi_clock_get_rates_bound(const struct scmi_protocol_handle *ph,
+ u32 clk_id, struct scmi_clock_info *clk)
+{
+ 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;
+
+ /* 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 = t->tx.buf;
+ msg->id = cpu_to_le32(clk_id);
+ msg->rate_index = 0;
+
+ resp = t->rx.buf;
+
+ ret = ph->xops->do_xfer(ph, t);
+ if (ret)
+ goto out;
+
+ 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);
+
+ if (clk->rate_discrete) {
+ clk->list.num_rates = num_returned + num_remaining;
+ clk->list.min_rate = RATE_TO_U64(resp->rate[0]);
+
+ if (num_remaining) {
+ ph->xops->reset_rx_to_maxsz(ph, t);
+ msg->id = cpu_to_le32(clk_id);
+ msg->rate_index = cpu_to_le32(clk->list.num_rates - 1);
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret)
+ clk->list.max_rate = RATE_TO_U64(resp->rate[0]);
+ } else {
+ u64 max = RATE_TO_U64(resp->rate[num_returned - 1]);
+
+ clk->list.max_rate = max;
+ }
+ } else {
+ /* We expect a triplet, 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;
+ }
+ }
+
+ 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]);
+
+ dev_dbg(ph->dev, "Min %llu Max %llu Step %llu Hz\n",
+ clk->range.min_rate, clk->range.max_rate,
+ clk->range.step_size);
}

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

@@ -1089,8 +1185,12 @@ static int scmi_clock_protocol_init(const struct scmi_protocol_handle *ph)
struct scmi_clock_info *clk = cinfo->clk + clkid;

ret = scmi_clock_attributes_get(ph, clkid, cinfo, version);
- if (!ret)
- scmi_clock_describe_rates_get(ph, clkid, clk);
+ if (!ret) {
+ if (PROTOCOL_REV_MAJOR(version) >= 0x2)
+ scmi_clock_get_rates_bound(ph, clkid, clk);
+ else
+ scmi_clock_describe_rates_get(ph, clkid, clk);
+ }
}

if (PROTOCOL_REV_MAJOR(version) >= 0x3) {
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 688466a0e816..240478bb8476 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