[PATCH] firmware: arm_scmi: extract domain ID validation helpers

[PATCH] firmware: arm_scmi: extract domain ID validation helpers

[Artem Shimko]

Refactor the voltage protocol implementation by moving repeated domain
ID boundary checks into dedicated inline functions. This reduces code
duplication and improves maintainability.

The new helpers are scmi_voltage_domain_validate_id() which validates
a domain ID and returns an error code, and scmi_voltage_domain_lookup()
which returns a pointer to the domain structure or an error pointer.
These are used consistently across the voltage operations.
No functional changes are introduced - the validation logic remains
identical to the original inline checks.

Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
Hello maintainers and reviewers,

Continuing the refactoring work from the previous one [1], this patch
extracts repeated domain ID boundary checks in the voltage protocol into
dedicated helper functions. This improves code maintainability by reducing
duplication and centralizing the validation logic.

The patch introduces two inline helpers:
* scmi_voltage_domain_validate_id(): validates domain ID and returns an
error code
* scmi_voltage_domain_lookup(): returns pointer to domain struct or error

Thank you!

[1] https://lore.kernel.org/all/20251103161044.2269377-1-a.shimko.dev@gmail.com/T/#u
--
Regards,
artem

 drivers/firmware/arm_scmi/voltage.c | 59 ++++++++++++++++++++---------
 1 file changed, 41 insertions(+), 18 deletions(-)

diff --git a/drivers/firmware/arm_scmi/voltage.c b/drivers/firmware/arm_scmi/voltage.c
index 17127880e10a..62010df24053 100644
--- a/drivers/firmware/arm_scmi/voltage.c
+++ b/drivers/firmware/arm_scmi/voltage.c
@@ -262,15 +262,37 @@ static int scmi_voltage_descriptors_get(const struct scmi_protocol_handle *ph,
 	return ret;
 }
 
+static inline int
+scmi_voltage_domain_validate_id(const struct scmi_protocol_handle *ph, u32 domain_id)
+{
+	struct voltage_info *vinfo = ph->get_priv(ph);
+
+	if (domain_id >= vinfo->num_domains)
+		return -EINVAL;
+
+	return 0;
+}
+
+static inline struct scmi_voltage_info *
+scmi_voltage_domain_lookup(const struct scmi_protocol_handle *ph, u32 domain_id)
+{
+	struct voltage_info *vinfo = ph->get_priv(ph);
+
+	if (domain_id >= vinfo->num_domains)
+		return ERR_PTR(-EINVAL);
+
+	return vinfo->domains + domain_id;
+}
+
 static int __scmi_voltage_get_u32(const struct scmi_protocol_handle *ph,
 				  u8 cmd_id, u32 domain_id, u32 *value)
 {
 	int ret;
 	struct scmi_xfer *t;
-	struct voltage_info *vinfo = ph->get_priv(ph);
 
-	if (domain_id >= vinfo->num_domains)
-		return -EINVAL;
+	ret = scmi_voltage_domain_validate_id(ph, domain_id);
+	if (ret)
+		return ret;
 
 	ret = ph->xops->xfer_get_init(ph, cmd_id, sizeof(__le32), 0, &t);
 	if (ret)
@@ -290,11 +312,11 @@ static int scmi_voltage_config_set(const struct scmi_protocol_handle *ph,
 {
 	int ret;
 	struct scmi_xfer *t;
-	struct voltage_info *vinfo = ph->get_priv(ph);
 	struct scmi_msg_cmd_config_set *cmd;
 
-	if (domain_id >= vinfo->num_domains)
-		return -EINVAL;
+	ret = scmi_voltage_domain_validate_id(ph, domain_id);
+	if (ret)
+		return ret;
 
 	ret = ph->xops->xfer_get_init(ph, VOLTAGE_CONFIG_SET,
 				     sizeof(*cmd), 0, &t);
@@ -325,25 +347,23 @@ static int scmi_voltage_level_set(const struct scmi_protocol_handle *ph,
 {
 	int ret;
 	struct scmi_xfer *t;
-	struct voltage_info *vinfo = ph->get_priv(ph);
 	struct scmi_msg_cmd_level_set *cmd;
-	struct scmi_voltage_info *v;
+	struct scmi_voltage_info *vi;
 
-	if (domain_id >= vinfo->num_domains)
-		return -EINVAL;
+	vi = scmi_voltage_domain_lookup(ph, domain_id);
+	if (IS_ERR(vi))
+		return PTR_ERR(vi);
 
 	ret = ph->xops->xfer_get_init(ph, VOLTAGE_LEVEL_SET,
 				      sizeof(*cmd), 0, &t);
 	if (ret)
 		return ret;
 
-	v = vinfo->domains + domain_id;
-
 	cmd = t->tx.buf;
 	cmd->domain_id = cpu_to_le32(domain_id);
 	cmd->voltage_level = cpu_to_le32(volt_uV);
 
-	if (!v->async_level_set || mode != SCMI_VOLTAGE_LEVEL_SET_AUTO) {
+	if (!vi->async_level_set || mode != SCMI_VOLTAGE_LEVEL_SET_AUTO) {
 		cmd->flags = cpu_to_le32(0x0);
 		ret = ph->xops->do_xfer(ph, t);
 	} else {
@@ -356,7 +376,7 @@ static int scmi_voltage_level_set(const struct scmi_protocol_handle *ph,
 			if (le32_to_cpu(resp->domain_id) == domain_id)
 				dev_dbg(ph->dev,
 					"Voltage domain %d set async to %d\n",
-					v->id,
+					vi->id,
 					le32_to_cpu(resp->voltage_level));
 			else
 				ret = -EPROTO;
@@ -377,13 +397,16 @@ static int scmi_voltage_level_get(const struct scmi_protocol_handle *ph,
 static const struct scmi_voltage_info * __must_check
 scmi_voltage_info_get(const struct scmi_protocol_handle *ph, u32 domain_id)
 {
-	struct voltage_info *vinfo = ph->get_priv(ph);
+	struct scmi_voltage_info *vi;
 
-	if (domain_id >= vinfo->num_domains ||
-	    !vinfo->domains[domain_id].num_levels)
+	vi = scmi_voltage_domain_lookup(ph, domain_id);
+	if (IS_ERR(vi))
 		return NULL;
 
-	return vinfo->domains + domain_id;
+	if (!vi->num_levels)
+		return NULL;
+
+	return vi;
 }
 
 static int scmi_voltage_domains_num_get(const struct scmi_protocol_handle *ph)
-- 
2.43.0

Re: [PATCH] firmware: arm_scmi: extract domain ID validation helpers

[Artem Shimko]

On Thu, Jan 22, 2026 at 7:17 PM Artem Shimko <a.shimko.dev@gmail.com> wrote:
>
> Refactor the voltage protocol implementation by moving repeated domain
> ID boundary checks into dedicated inline functions. This reduces code
> duplication and improves maintainability.
>
> The new helpers are scmi_voltage_domain_validate_id() which validates
> a domain ID and returns an error code, and scmi_voltage_domain_lookup()
> which returns a pointer to the domain structure or an error pointer.
> These are used consistently across the voltage operations.
> No functional changes are introduced - the validation logic remains
> identical to the original inline checks.
>

Hello maintainers,

Just a gentle ping.

Do you have any thoughts or feedback on this approach, or are these
changes even irrelevant?

I'm happy to respond to any comments or prepare a new version if needed.

Thank you for your time and for reviewing this patch.
--
Regards,
Artem

Re: [PATCH] firmware: arm_scmi: extract domain ID validation helpers

[Sudeep Holla]

On Tue, Mar 17, 2026 at 03:05:59PM +0300, Artem Shimko wrote:
> On Thu, Jan 22, 2026 at 7:17 PM Artem Shimko <a.shimko.dev@gmail.com> wrote:
> >
> > Refactor the voltage protocol implementation by moving repeated domain
> > ID boundary checks into dedicated inline functions. This reduces code
> > duplication and improves maintainability.
> >
> > The new helpers are scmi_voltage_domain_validate_id() which validates
> > a domain ID and returns an error code, and scmi_voltage_domain_lookup()
> > which returns a pointer to the domain structure or an error pointer.
> > These are used consistently across the voltage operations.
> > No functional changes are introduced - the validation logic remains
> > identical to the original inline checks.
> >
> 
> Hello maintainers,
> 
> Just a gentle ping.
> 
> Do you have any thoughts or feedback on this approach, or are these
> changes even irrelevant?
> 

I applied and then realised it is not necessary to churn so much to just
unify the domain ID validation. So I dropped it.

-- 
Regards,
Sudeep

Re: [PATCH] firmware: arm_scmi: extract domain ID validation helpers

[Artem Shimko]

Hi Sudeep,

On Tue, Mar 17, 2026 at 4:37 PM Sudeep Holla <sudeep.holla@kernel.org> wrote:

> I applied and then realised it is not necessary to churn so much to just
> unify the domain ID validation. So I dropped it.

Got it.

Thank you.
--
Regards,
Artem