[PATCH v2 0/2] qcom: remove memcpy()ing from cmd-db driver

[PATCH v2 0/2] qcom: remove memcpy()ing from cmd-db driver

[Stephen Boyd]

Some changes to the cmd-db code to not do any more copying
of memory. Instead, we'll just hand out pointers to things
inside cmd-db. 

Changes from v1:
 * Fixed patch#2 for GPU wreckage

Cc: Mahesh Sivasubramanian <msivasub@codeaurora.org>
Cc: Lina Iyer <ilina@codeaurora.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Evan Green <evgreen@chromium.org>
Cc: Jordan Crouse <jcrouse@codeaurora.org>
Cc: Rob Clark <robdclark@gmail.com>

Stephen Boyd (2):
  soc: qcom: cmd-db: Remove memcpy()ing from cmd_db_get_header()
  soc: qcom: cmd-db: Stop memcpy()ing in cmd_db_read_aux_data()

 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 34 +++++-----
 drivers/soc/qcom/cmd-db.c             | 93 +++++++++------------------
 include/soc/qcom/cmd-db.h             | 12 +---
 3 files changed, 49 insertions(+), 90 deletions(-)

-- 
Sent by a computer through tubes

[PATCH v2 1/2] soc: qcom: cmd-db: Remove memcpy()ing from cmd_db_get_header()

[Stephen Boyd]

The cmd_db_get_header() function is a static local function that doesn't
need to copy anything from one place to another. Instead, it can just
point into the region by returning pointers to what we're looking for.
If we do that, we should mark what we're returning as const so that code
can't modify cmd-db without an obvious cast.

Cc: Mahesh Sivasubramanian <msivasub@codeaurora.org>
Cc: Lina Iyer <ilina@codeaurora.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Evan Green <evgreen@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/soc/qcom/cmd-db.c | 60 +++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 34 deletions(-)

diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
index a6f646295f06..5c9cc6824891 100644
--- a/drivers/soc/qcom/cmd-db.c
+++ b/drivers/soc/qcom/cmd-db.c
@@ -101,8 +101,7 @@ static bool cmd_db_magic_matches(const struct cmd_db_header *header)
 
 static struct cmd_db_header *cmd_db_header;
 
-
-static inline void *rsc_to_entry_header(struct rsc_hdr *hdr)
+static inline const void *rsc_to_entry_header(const struct rsc_hdr *hdr)
 {
 	u16 offset = le16_to_cpu(hdr->header_offset);
 
@@ -110,7 +109,7 @@ static inline void *rsc_to_entry_header(struct rsc_hdr *hdr)
 }
 
 static inline void *
-rsc_offset(struct rsc_hdr *hdr, struct entry_header *ent)
+rsc_offset(const struct rsc_hdr *hdr, const struct entry_header *ent)
 {
 	u16 offset = le16_to_cpu(hdr->data_offset);
 	u16 loffset = le16_to_cpu(ent->offset);
@@ -134,11 +133,11 @@ int cmd_db_ready(void)
 }
 EXPORT_SYMBOL(cmd_db_ready);
 
-static int cmd_db_get_header(const char *id, struct entry_header *eh,
-			     struct rsc_hdr *rh)
+static int cmd_db_get_header(const char *id, const struct entry_header **eh,
+			     const struct rsc_hdr **rh)
 {
-	struct rsc_hdr *rsc_hdr;
-	struct entry_header *ent;
+	const struct rsc_hdr *rsc_hdr;
+	const struct entry_header *ent;
 	int ret, i, j;
 	u8 query[8];
 
@@ -146,9 +145,6 @@ static int cmd_db_get_header(const char *id, struct entry_header *eh,
 	if (ret)
 		return ret;
 
-	if (!eh || !rh)
-		return -EINVAL;
-
 	/* Pad out query string to same length as in DB */
 	strncpy(query, id, sizeof(query));
 
@@ -159,14 +155,13 @@ static int cmd_db_get_header(const char *id, struct entry_header *eh,
 
 		ent = rsc_to_entry_header(rsc_hdr);
 		for (j = 0; j < le16_to_cpu(rsc_hdr->cnt); j++, ent++) {
-			if (memcmp(ent->id, query, sizeof(ent->id)) == 0)
-				break;
-		}
-
-		if (j < le16_to_cpu(rsc_hdr->cnt)) {
-			memcpy(eh, ent, sizeof(*ent));
-			memcpy(rh, rsc_hdr, sizeof(*rh));
-			return 0;
+			if (memcmp(ent->id, query, sizeof(ent->id)) == 0) {
+				if (eh)
+					*eh = ent;
+				if (rh)
+					*rh = rsc_hdr;
+				return 0;
+			}
 		}
 	}
 
@@ -186,12 +181,11 @@ static int cmd_db_get_header(const char *id, struct entry_header *eh,
 u32 cmd_db_read_addr(const char *id)
 {
 	int ret;
-	struct entry_header ent;
-	struct rsc_hdr rsc_hdr;
+	const struct entry_header *ent;
 
-	ret = cmd_db_get_header(id, &ent, &rsc_hdr);
+	ret = cmd_db_get_header(id, &ent, NULL);
 
-	return ret < 0 ? 0 : le32_to_cpu(ent.addr);
+	return ret < 0 ? 0 : le32_to_cpu(ent->addr);
 }
 EXPORT_SYMBOL(cmd_db_read_addr);
 
@@ -207,8 +201,8 @@ EXPORT_SYMBOL(cmd_db_read_addr);
 int cmd_db_read_aux_data(const char *id, u8 *data, size_t len)
 {
 	int ret;
-	struct entry_header ent;
-	struct rsc_hdr rsc_hdr;
+	const struct entry_header *ent;
+	const struct rsc_hdr *rsc_hdr;
 	u16 ent_len;
 
 	if (!data)
@@ -218,12 +212,12 @@ int cmd_db_read_aux_data(const char *id, u8 *data, size_t len)
 	if (ret)
 		return ret;
 
-	ent_len = le16_to_cpu(ent.len);
+	ent_len = le16_to_cpu(ent->len);
 	if (len < ent_len)
 		return -EINVAL;
 
 	len = min_t(u16, ent_len, len);
-	memcpy(data, rsc_offset(&rsc_hdr, &ent), len);
+	memcpy(data, rsc_offset(rsc_hdr, ent), len);
 
 	return len;
 }
@@ -239,12 +233,11 @@ EXPORT_SYMBOL(cmd_db_read_aux_data);
 size_t cmd_db_read_aux_data_len(const char *id)
 {
 	int ret;
-	struct entry_header ent;
-	struct rsc_hdr rsc_hdr;
+	const struct entry_header *ent;
 
-	ret = cmd_db_get_header(id, &ent, &rsc_hdr);
+	ret = cmd_db_get_header(id, &ent, NULL);
 
-	return ret < 0 ? 0 : le16_to_cpu(ent.len);
+	return ret < 0 ? 0 : le16_to_cpu(ent->len);
 }
 EXPORT_SYMBOL(cmd_db_read_aux_data_len);
 
@@ -258,15 +251,14 @@ EXPORT_SYMBOL(cmd_db_read_aux_data_len);
 enum cmd_db_hw_type cmd_db_read_slave_id(const char *id)
 {
 	int ret;
-	struct entry_header ent;
-	struct rsc_hdr rsc_hdr;
+	const struct entry_header *ent;
 	u32 addr;
 
-	ret = cmd_db_get_header(id, &ent, &rsc_hdr);
+	ret = cmd_db_get_header(id, &ent, NULL);
 	if (ret < 0)
 		return CMD_DB_HW_INVALID;
 
-	addr = le32_to_cpu(ent.addr);
+	addr = le32_to_cpu(ent->addr);
 	return (addr >> SLAVE_ID_SHIFT) & SLAVE_ID_MASK;
 }
 EXPORT_SYMBOL(cmd_db_read_slave_id);
-- 
Sent by a computer through tubes

[PATCH v2 2/2] soc: qcom: cmd-db: Stop memcpy()ing in cmd_db_read_aux_data()

[Stephen Boyd]

Let's change the function signature to return the pointer to memory or
an error pointer on failure, and take an argument that lets us return
the size of the aux data read. This way we can remove the
cmd_db_read_aux_data_len() API entirely and also get rid of the memcpy
operation from cmd_db to the caller.

Cc: Mahesh Sivasubramanian <msivasub@codeaurora.org>
Cc: Lina Iyer <ilina@codeaurora.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Evan Green <evgreen@chromium.org>
Cc: Jordan Crouse <jcrouse@codeaurora.org>
Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 34 ++++++++++-----------
 drivers/soc/qcom/cmd-db.c             | 43 +++++----------------------
 include/soc/qcom/cmd-db.h             | 12 ++------
 3 files changed, 28 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index bbb8126ec5c5..e4387a3fa745 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -860,23 +860,23 @@ static int a6xx_gmu_memory_probe(struct a6xx_gmu *gmu)
 }
 
 /* Get the list of RPMh voltage levels from cmd-db */
-static int a6xx_gmu_rpmh_arc_cmds(const char *id, void *vals, int size)
+static const u16 *a6xx_gmu_rpmh_arc_cmds(const char *id, int *size)
 {
-	u32 len = cmd_db_read_aux_data_len(id);
-
-	if (!len)
-		return 0;
-
-	if (WARN_ON(len > size))
-		return -EINVAL;
-
-	cmd_db_read_aux_data(id, vals, len);
+	size_t len;
+	const u16 *vals;
 
+	vals = cmd_db_read_aux_data(id, &len);
 	/*
 	 * The data comes back as an array of unsigned shorts so adjust the
 	 * count accordingly
 	 */
-	return len >> 1;
+	len >>= 1;
+
+	if (!len || WARN_ON(len > 16))
+		len = 0;
+
+	*size = len;
+	return vals;
 }
 
 /* Return the 'arc-level' for the given frequency */
@@ -907,8 +907,8 @@ static u32 a6xx_gmu_get_arc_level(struct device *dev, unsigned long freq)
 
 static int a6xx_gmu_rpmh_arc_votes_init(struct device *dev, u32 *votes,
 		unsigned long *freqs, int freqs_count,
-		u16 *pri, int pri_count,
-		u16 *sec, int sec_count)
+		const u16 *pri, int pri_count,
+		const u16 *sec, int sec_count)
 {
 	int i, j;
 
@@ -970,14 +970,14 @@ static int a6xx_gmu_rpmh_votes_init(struct a6xx_gmu *gmu)
 	struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
 	struct msm_gpu *gpu = &adreno_gpu->base;
 
-	u16 gx[16], cx[16], mx[16];
+	const u16 *gx, *cx, *mx;
 	u32 gxcount, cxcount, mxcount;
 	int ret;
 
 	/* Get the list of available voltage levels for each component */
-	gxcount = a6xx_gmu_rpmh_arc_cmds("gfx.lvl", gx, sizeof(gx));
-	cxcount = a6xx_gmu_rpmh_arc_cmds("cx.lvl", cx, sizeof(cx));
-	mxcount = a6xx_gmu_rpmh_arc_cmds("mx.lvl", mx, sizeof(mx));
+	gx = a6xx_gmu_rpmh_arc_cmds("gfx.lvl", &gxcount);
+	cx = a6xx_gmu_rpmh_arc_cmds("cx.lvl", &cxcount);
+	mx = a6xx_gmu_rpmh_arc_cmds("mx.lvl", &mxcount);
 
 	/* Build the GX votes */
 	ret = a6xx_gmu_rpmh_arc_votes_init(&gpu->pdev->dev, gmu->gx_arc_votes,
diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
index 5c9cc6824891..c701b3b010f1 100644
--- a/drivers/soc/qcom/cmd-db.c
+++ b/drivers/soc/qcom/cmd-db.c
@@ -192,55 +192,28 @@ EXPORT_SYMBOL(cmd_db_read_addr);
 /**
  * cmd_db_read_aux_data() - Query command db for aux data.
  *
- *  @id: Resource to retrieve AUX Data on.
- *  @data: Data buffer to copy returned aux data to. Returns size on NULL
- *  @len: Caller provides size of data buffer passed in.
+ *  @id: Resource to retrieve AUX Data on
+ *  @len: size of data buffer returned
  *
- *  Return: size of data on success, errno otherwise
+ *  Return: pointer to data on success, error pointer otherwise
  */
-int cmd_db_read_aux_data(const char *id, u8 *data, size_t len)
+const void *cmd_db_read_aux_data(const char *id, size_t *len)
 {
 	int ret;
 	const struct entry_header *ent;
 	const struct rsc_hdr *rsc_hdr;
-	u16 ent_len;
-
-	if (!data)
-		return -EINVAL;
 
 	ret = cmd_db_get_header(id, &ent, &rsc_hdr);
 	if (ret)
-		return ret;
-
-	ent_len = le16_to_cpu(ent->len);
-	if (len < ent_len)
-		return -EINVAL;
+		return ERR_PTR(ret);
 
-	len = min_t(u16, ent_len, len);
-	memcpy(data, rsc_offset(rsc_hdr, ent), len);
+	if (len)
+		*len = le16_to_cpu(ent->len);
 
-	return len;
+	return rsc_offset(rsc_hdr, ent);
 }
 EXPORT_SYMBOL(cmd_db_read_aux_data);
 
-/**
- * cmd_db_read_aux_data_len - Get the length of the auxiliary data stored in DB.
- *
- * @id: Resource to retrieve AUX Data.
- *
- * Return: size on success, 0 on error
- */
-size_t cmd_db_read_aux_data_len(const char *id)
-{
-	int ret;
-	const struct entry_header *ent;
-
-	ret = cmd_db_get_header(id, &ent, NULL);
-
-	return ret < 0 ? 0 : le16_to_cpu(ent->len);
-}
-EXPORT_SYMBOL(cmd_db_read_aux_data_len);
-
 /**
  * cmd_db_read_slave_id - Get the slave ID for a given resource address
  *
diff --git a/include/soc/qcom/cmd-db.h b/include/soc/qcom/cmd-db.h
index 578180cbc134..af9722223925 100644
--- a/include/soc/qcom/cmd-db.h
+++ b/include/soc/qcom/cmd-db.h
@@ -18,9 +18,7 @@ enum cmd_db_hw_type {
 #if IS_ENABLED(CONFIG_QCOM_COMMAND_DB)
 u32 cmd_db_read_addr(const char *resource_id);
 
-int cmd_db_read_aux_data(const char *resource_id, u8 *data, size_t len);
-
-size_t cmd_db_read_aux_data_len(const char *resource_id);
+const void *cmd_db_read_aux_data(const char *resource_id, size_t *len);
 
 enum cmd_db_hw_type cmd_db_read_slave_id(const char *resource_id);
 
@@ -29,12 +27,8 @@ int cmd_db_ready(void);
 static inline u32 cmd_db_read_addr(const char *resource_id)
 { return 0; }
 
-static inline int cmd_db_read_aux_data(const char *resource_id, u8 *data,
-				       size_t len)
-{ return -ENODEV; }
-
-static inline size_t cmd_db_read_aux_data_len(const char *resource_id)
-{ return -ENODEV; }
+static inline const void *cmd_db_read_aux_data(const char *resource_id, size_t *len)
+{ return ERR_PTR(-ENODEV); }
 
 static inline enum cmd_db_hw_type cmd_db_read_slave_id(const char *resource_id)
 { return -ENODEV; }
-- 
Sent by a computer through tubes

Re: [PATCH v2 2/2] soc: qcom: cmd-db: Stop memcpy()ing in cmd_db_read_aux_data()

[Jordan Crouse]

On Mon, Sep 17, 2018 at 01:40:07PM -0700, Stephen Boyd wrote:
> Let's change the function signature to return the pointer to memory or
> an error pointer on failure, and take an argument that lets us return
> the size of the aux data read. This way we can remove the
> cmd_db_read_aux_data_len() API entirely and also get rid of the memcpy
> operation from cmd_db to the caller.
> 
> Cc: Mahesh Sivasubramanian <msivasub@codeaurora.org>
> Cc: Lina Iyer <ilina@codeaurora.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Evan Green <evgreen@chromium.org>
> Cc: Jordan Crouse <jcrouse@codeaurora.org>
> Cc: Rob Clark <robdclark@gmail.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 34 ++++++++++-----------
>  drivers/soc/qcom/cmd-db.c             | 43 +++++----------------------
>  include/soc/qcom/cmd-db.h             | 12 ++------
>  3 files changed, 28 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index bbb8126ec5c5..e4387a3fa745 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -860,23 +860,23 @@ static int a6xx_gmu_memory_probe(struct a6xx_gmu *gmu)
>  }
>  
>  /* Get the list of RPMh voltage levels from cmd-db */
> -static int a6xx_gmu_rpmh_arc_cmds(const char *id, void *vals, int size)
> +static const u16 *a6xx_gmu_rpmh_arc_cmds(const char *id, int *size)
>  {
> -	u32 len = cmd_db_read_aux_data_len(id);
> -
> -	if (!len)
> -		return 0;
> -
> -	if (WARN_ON(len > size))
> -		return -EINVAL;
> -
> -	cmd_db_read_aux_data(id, vals, len);
> +	size_t len;
> +	const u16 *vals;
>  
> +	vals = cmd_db_read_aux_data(id, &len);
>  	/*
>  	 * The data comes back as an array of unsigned shorts so adjust the
>  	 * count accordingly
>  	 */
> -	return len >> 1;
> +	len >>= 1;
> +
> +	if (!len || WARN_ON(len > 16))
> +		len = 0;

I don't think we care any more about the size of the array since there isn't any
data being copied.

> +
> +	*size = len;
> +	return vals;
>  }

I'm not sure this function has any value any more. We could just call
cmd_db_read_aux_data() directly for each id and adjust the size on the fly
when we call a6xx_gmu_rpmh_arc_votes_init.

>  /* Return the 'arc-level' for the given frequency */
> @@ -907,8 +907,8 @@ static u32 a6xx_gmu_get_arc_level(struct device *dev, unsigned long freq)
>  
>  static int a6xx_gmu_rpmh_arc_votes_init(struct device *dev, u32 *votes,
>  		unsigned long *freqs, int freqs_count,
> -		u16 *pri, int pri_count,
> -		u16 *sec, int sec_count)
> +		const u16 *pri, int pri_count,
> +		const u16 *sec, int sec_count)
>  {
>  	int i, j;
>  
> @@ -970,14 +970,14 @@ static int a6xx_gmu_rpmh_votes_init(struct a6xx_gmu *gmu)
>  	struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
>  	struct msm_gpu *gpu = &adreno_gpu->base;
>  
> -	u16 gx[16], cx[16], mx[16];
> +	const u16 *gx, *cx, *mx;
>  	u32 gxcount, cxcount, mxcount;
>  	int ret;
>  
>  	/* Get the list of available voltage levels for each component */
> -	gxcount = a6xx_gmu_rpmh_arc_cmds("gfx.lvl", gx, sizeof(gx));
> -	cxcount = a6xx_gmu_rpmh_arc_cmds("cx.lvl", cx, sizeof(cx));
> -	mxcount = a6xx_gmu_rpmh_arc_cmds("mx.lvl", mx, sizeof(mx));
> +	gx = a6xx_gmu_rpmh_arc_cmds("gfx.lvl", &gxcount);
> +	cx = a6xx_gmu_rpmh_arc_cmds("cx.lvl", &cxcount);
> +	mx = a6xx_gmu_rpmh_arc_cmds("mx.lvl", &mxcount);
>  
>  	/* Build the GX votes */
>  	ret = a6xx_gmu_rpmh_arc_votes_init(&gpu->pdev->dev, gmu->gx_arc_votes,
> diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
> index 5c9cc6824891..c701b3b010f1 100644
> --- a/drivers/soc/qcom/cmd-db.c
> +++ b/drivers/soc/qcom/cmd-db.c
> @@ -192,55 +192,28 @@ EXPORT_SYMBOL(cmd_db_read_addr);
>  /**
>   * cmd_db_read_aux_data() - Query command db for aux data.
>   *
> - *  @id: Resource to retrieve AUX Data on.
> - *  @data: Data buffer to copy returned aux data to. Returns size on NULL
> - *  @len: Caller provides size of data buffer passed in.
> + *  @id: Resource to retrieve AUX Data on
> + *  @len: size of data buffer returned

Is the GPU the only consumer of this function?  Would it make more sense to
return the number of entries rather than the size? I would suppose that would
imply that the caller knows the size of the entries but we have to know that
anyway if we're going to walk the list.

>   *
> - *  Return: size of data on success, errno otherwise
> + *  Return: pointer to data on success, error pointer otherwise
>   */
> -int cmd_db_read_aux_data(const char *id, u8 *data, size_t len)
> +const void *cmd_db_read_aux_data(const char *id, size_t *len)
>  {
>  	int ret;
>  	const struct entry_header *ent;
>  	const struct rsc_hdr *rsc_hdr;
> -	u16 ent_len;
> -
> -	if (!data)
> -		return -EINVAL;
>  
>  	ret = cmd_db_get_header(id, &ent, &rsc_hdr);
>  	if (ret)
> -		return ret;
> -
> -	ent_len = le16_to_cpu(ent->len);
> -	if (len < ent_len)
> -		return -EINVAL;
> +		return ERR_PTR(ret);
>  
> -	len = min_t(u16, ent_len, len);
> -	memcpy(data, rsc_offset(rsc_hdr, ent), len);
> +	if (len)
> +		*len = le16_to_cpu(ent->len);
>  
> -	return len;
> +	return rsc_offset(rsc_hdr, ent);
>  }
>  EXPORT_SYMBOL(cmd_db_read_aux_data);

<snip>

Jordan
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 2/2] soc: qcom: cmd-db: Stop memcpy()ing in cmd_db_read_aux_data()

[Stephen Boyd]

Quoting Jordan Crouse (2018-09-17 14:14:14)
> On Mon, Sep 17, 2018 at 01:40:07PM -0700, Stephen Boyd wrote:
> > +     vals = cmd_db_read_aux_data(id, &len);
> >       /*
> >        * The data comes back as an array of unsigned shorts so adjust the
> >        * count accordingly
> >        */
> > -     return len >> 1;
> > +     len >>= 1;
> > +
> > +     if (!len || WARN_ON(len > 16))
> > +             len = 0;
> 
> I don't think we care any more about the size of the array since there isn't any
> data being copied.
> 

Ok, sounds good.

> > +
> > +     *size = len;
> > +     return vals;
> >  }
> 
> I'm not sure this function has any value any more. We could just call
> cmd_db_read_aux_data() directly for each id and adjust the size on the fly
> when we call a6xx_gmu_rpmh_arc_votes_init.

Ok. Let me attempt to do more code reduction!

> 
> >  /* Return the 'arc-level' for the given frequency */
> > @@ -907,8 +907,8 @@ static u32 a6xx_gmu_get_arc_level(struct device *dev, unsigned long freq)
> >  
> >  static int a6xx_gmu_rpmh_arc_votes_init(struct device *dev, u32 *votes,
> >               unsigned long *freqs, int freqs_count,
> > -             u16 *pri, int pri_count,
> > -             u16 *sec, int sec_count)
> > +             const u16 *pri, int pri_count,
> > +             const u16 *sec, int sec_count)
> >  {
> >       int i, j;
> >  
> > @@ -970,14 +970,14 @@ static int a6xx_gmu_rpmh_votes_init(struct a6xx_gmu *gmu)
> >       struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
> >       struct msm_gpu *gpu = &adreno_gpu->base;
> >  
> > -     u16 gx[16], cx[16], mx[16];
> > +     const u16 *gx, *cx, *mx;
> >       u32 gxcount, cxcount, mxcount;
> >       int ret;
> >  
> >       /* Get the list of available voltage levels for each component */
> > -     gxcount = a6xx_gmu_rpmh_arc_cmds("gfx.lvl", gx, sizeof(gx));
> > -     cxcount = a6xx_gmu_rpmh_arc_cmds("cx.lvl", cx, sizeof(cx));
> > -     mxcount = a6xx_gmu_rpmh_arc_cmds("mx.lvl", mx, sizeof(mx));
> > +     gx = a6xx_gmu_rpmh_arc_cmds("gfx.lvl", &gxcount);
> > +     cx = a6xx_gmu_rpmh_arc_cmds("cx.lvl", &cxcount);
> > +     mx = a6xx_gmu_rpmh_arc_cmds("mx.lvl", &mxcount);
> >  
> >       /* Build the GX votes */
> >       ret = a6xx_gmu_rpmh_arc_votes_init(&gpu->pdev->dev, gmu->gx_arc_votes,
> > diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
> > index 5c9cc6824891..c701b3b010f1 100644
> > --- a/drivers/soc/qcom/cmd-db.c
> > +++ b/drivers/soc/qcom/cmd-db.c
> > @@ -192,55 +192,28 @@ EXPORT_SYMBOL(cmd_db_read_addr);
> >  /**
> >   * cmd_db_read_aux_data() - Query command db for aux data.
> >   *
> > - *  @id: Resource to retrieve AUX Data on.
> > - *  @data: Data buffer to copy returned aux data to. Returns size on NULL
> > - *  @len: Caller provides size of data buffer passed in.
> > + *  @id: Resource to retrieve AUX Data on
> > + *  @len: size of data buffer returned
> 
> Is the GPU the only consumer of this function?  Would it make more sense to
> return the number of entries rather than the size? I would suppose that would
> imply that the caller knows the size of the entries but we have to know that
> anyway if we're going to walk the list.

I think the bus scaling driver also calls this function. I don't see
much advantage to doing that here when we can just consider the aux_data
as a big blob of data that is interpreted however the consumer cares.