wifi: ath10k: prefer stack-allocated __le32 variables

Commit Message

Dmitry Antipov Oct. 27, 2023, 8:23 a.m. UTC

There isn't too much sense to 'kzalloc()' buffer for the only
__le32 value which is going to be freed in the same function,
so switch to stack-allocated one in 'ath10k_sdio_writesb32()'
and 'ath10k_sdio_diag_read32()'. Compile tested only.

Signed-off-by: Dmitry Antipov <[e-mail]>
---
 drivers/net/wireless/ath/ath10k/sdio.c | 47 +++++++-------------------
 1 file changed, 13 insertions(+), 34 deletions(-)

Comments

Johannes Berg Oct. 27, 2023, 9:08 a.m. UTC | #1

On Fri, 2023-10-27 at 11:23 +0300, Dmitry Antipov wrote:
> There isn't too much sense to 'kzalloc()' buffer

Actually, there is. Consider what's done with it.

johannes

Dmitry Antipov Oct. 27, 2023, 9:50 a.m. UTC | #2

On 10/27/23 12:08, Johannes Berg wrote:

> Actually, there is. Consider what's done with it.

It seems I should say sorry for being overconfident with this :-(.
Locals can't be used for scatterlists/DMA/etc., right?

Dmitry

Johannes Berg Oct. 27, 2023, 9:53 a.m. UTC | #3

On Fri, 2023-10-27 at 12:50 +0300, Dmitry Antipov wrote:


> Locals can't be used for scatterlists/DMA/etc., right?

Well, a scatterlist by itself doesn't imply anything necessarily, it's
just a data structure.

But yes, you cannot do DMA from the stack, which seems to happen in this
case, or at least can happen.

johannes

Patch

diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index 56fbcfb80bf8..5ce1bad417cd 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -238,36 +238,25 @@  static int ath10k_sdio_write32(struct ath10k *ar, u32 addr, u32 val)
 	return ret;
 }
 
-static int ath10k_sdio_writesb32(struct ath10k *ar, u32 addr, u32 val)
+static int ath10k_sdio_writesb32(struct ath10k *ar, u32 addr, u32 value)
 {
 	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
 	struct sdio_func *func = ar_sdio->func;
-	__le32 *buf;
+	__le32 val = __cpu_to_le32(value);
 	int ret;
 
-	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
-
-	*buf = cpu_to_le32(val);
-
 	sdio_claim_host(func);
 
-	ret = sdio_writesb(func, addr, buf, sizeof(*buf));
-	if (ret) {
+	ret = sdio_writesb(func, addr, &val, sizeof(val));
+	if (ret)
 		ath10k_warn(ar, "failed to write value 0x%x to fixed sb address 0x%x: %d\n",
-			    val, addr, ret);
-		goto out;
-	}
-
-	ath10k_dbg(ar, ATH10K_DBG_SDIO, "sdio writesb32 addr 0x%x val 0x%x\n",
-		   addr, val);
+			    value, addr, ret);
+	else
+		ath10k_dbg(ar, ATH10K_DBG_SDIO, "sdio writesb32 addr 0x%x val 0x%x\n",
+			   addr, value);
 
-out:
 	sdio_release_host(func);
 
-	kfree(buf);
-
 	return ret;
 }
 
@@ -1758,24 +1747,14 @@  static int ath10k_sdio_hif_diag_read(struct ath10k *ar, u32 address, void *buf,
 	return ret;
 }
 
-static int ath10k_sdio_diag_read32(struct ath10k *ar, u32 address,
-				   u32 *value)
+static int ath10k_sdio_diag_read32(struct ath10k *ar, u32 address, u32 *value)
 {
-	__le32 *val;
+	__le32 val;
 	int ret;
 
-	val = kzalloc(sizeof(*val), GFP_KERNEL);
-	if (!val)
-		return -ENOMEM;
-
-	ret = ath10k_sdio_hif_diag_read(ar, address, val, sizeof(*val));
-	if (ret)
-		goto out;
-
-	*value = __le32_to_cpu(*val);
-
-out:
-	kfree(val);
+	ret = ath10k_sdio_hif_diag_read(ar, address, &val, sizeof(val));
+	if (!ret)
+		*value = __le32_to_cpu(val);
 
 	return ret;
 }