[PATCH net-next v2 0/5] eea: Improvements in the driver

[PATCH net-next v2 0/5] eea: Improvements in the driver

[Andy Shevchenko]

My attention was caught by the explicit call of string_escape_str() which is
not required in this case and saves a lot of memory on the stack (see patch 1).
But then I noticed more to improve, hence this mini-series.

In v2:
- addressed Sashiko conserns
- added patches 2-5

v1: 20260929082722.3368741-1-andriy.shevchenko@linux.intel.com

Andy Shevchenko (5):
  eea: Drop temporary buffer by using %*pEhp directly
  eea: Remove unneeded assignment
  eea: Refactor error handling in eea_adminq_config_host_info()
  eea: unify return code variable name
  eea: Use 2-argument strscpy()

 drivers/net/ethernet/alibaba/eea/eea_adminq.c | 65 +++++++++----------
 .../net/ethernet/alibaba/eea/eea_ethtool.c    |  4 +-
 2 files changed, 32 insertions(+), 37 deletions(-)

-- 
2.50.1

[PATCH net-next v2 1/5] eea: Drop temporary buffer by using %*pEhp directly

[Andy Shevchenko]

Instead of calling string_escape_str() followed by %s specifier,
use %*pEhp specifier directly. With this being done, drop temporary
buffer.

Note, as Sashiko pointed out in [1] the %*pEp, which is the direct
substitution of the original call to string_escape_str() with ESCAPE_NP
flag set, makes the whole thing like "no escape". To add some sanity
to this the used specifier also includes ESCAPE_HEX.

Link: https://lore.kernel.org/r/179089728102.434549.12368098081790653201@kernel.org [1]
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/alibaba/eea/eea_adminq.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/alibaba/eea/eea_adminq.c b/drivers/net/ethernet/alibaba/eea/eea_adminq.c
index 73d0caf25700..f2284fc885c3 100644
--- a/drivers/net/ethernet/alibaba/eea/eea_adminq.c
+++ b/drivers/net/ethernet/alibaba/eea/eea_adminq.c
@@ -522,16 +522,9 @@ void eea_adminq_config_host_info(struct eea_net *enet)
 			dev_warn(dev, "The hardware-driven state validation may be abnormal.\n");
 
 		if (rep->has_reply) {
-			char buf[EEA_HINFO_MAX_REP_LEN] = {0};
-
 			rep->reply_str[EEA_HINFO_MAX_REP_LEN - 1] = '\0';
-
-			string_escape_str(rep->reply_str, buf, sizeof(buf),
-					  ESCAPE_NP, NULL);
-
-			buf[EEA_HINFO_MAX_REP_LEN - 1] = '\0';
-
-			dev_warn(dev, "Device replied: %s\n", buf);
+			dev_warn(dev, "Device replied: %*pEhp\n",
+				 (int)strlen(rep->reply_str), rep->reply_str);
 		}
 	}
 
-- 
2.50.1

[PATCH net-next v2 2/5] eea: Remove unneeded assignment

[Andy Shevchenko]

The initial err is not used anywhere, drop the unneeded assignment.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/alibaba/eea/eea_adminq.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/alibaba/eea/eea_adminq.c b/drivers/net/ethernet/alibaba/eea/eea_adminq.c
index f2284fc885c3..4876f8a1e686 100644
--- a/drivers/net/ethernet/alibaba/eea/eea_adminq.c
+++ b/drivers/net/ethernet/alibaba/eea/eea_adminq.c
@@ -266,7 +266,7 @@ int eea_create_adminq(struct eea_net *enet, u32 qid)
 	u32 db_size, q_size, num;
 	struct eea_ring *ering;
 	struct eea_aq *aq;
-	int err = -ENOMEM;
+	int err;
 
 	num = enet->edev->rx_num + enet->edev->tx_num;
 	aq = &enet->adminq;
@@ -347,11 +347,12 @@ static void qcfg_fill(struct eea_aq_create *qcfg, struct eea_ring *ering,
 
 int eea_adminq_create_q(struct eea_net *enet, u32 num, u32 flags)
 {
-	int i, db_size, q_size, err = -ENOMEM;
 	struct eea_net_cfg *cfg;
 	struct eea_ring *ering;
+	int i, db_size, q_size;
 	struct eea_aq *aq;
 	u32 reply_len;
+	int err;
 
 	cfg = &enet->cfg;
 	aq = &enet->adminq;
@@ -483,7 +484,7 @@ void eea_adminq_config_host_info(struct eea_net *enet)
 	struct device *dev = enet->edev->dma_dev;
 	struct eea_aq_host_info_cfg *cfg;
 	struct eea_aq_host_info_rep *rep;
-	int rc = -ENOMEM;
+	int rc;
 
 	cfg = kzalloc_obj(*cfg);
 	if (!cfg)
-- 
2.50.1

[PATCH net-next v2 3/5] eea: Refactor error handling in eea_adminq_config_host_info()

[Andy Shevchenko]

Use traditional pattern to check for error first in
eea_adminq_config_host_info().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/alibaba/eea/eea_adminq.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/alibaba/eea/eea_adminq.c b/drivers/net/ethernet/alibaba/eea/eea_adminq.c
index 4876f8a1e686..92eeff32b00c 100644
--- a/drivers/net/ethernet/alibaba/eea/eea_adminq.c
+++ b/drivers/net/ethernet/alibaba/eea/eea_adminq.c
@@ -517,18 +517,19 @@ void eea_adminq_config_host_info(struct eea_net *enet)
 
 	rc = eea_adminq_exec(enet, EEA_AQ_CMD_HOST_INFO,
 			     cfg, sizeof(*cfg), rep, sizeof(*rep), NULL);
+	if (rc)
+		goto err_free_rep;
 
-	if (!rc) {
-		if (rep->op_code == EEA_HINFO_REP_BAD)
-			dev_warn(dev, "The hardware-driven state validation may be abnormal.\n");
+	if (rep->op_code == EEA_HINFO_REP_BAD)
+		dev_warn(dev, "The hardware-driven state validation may be abnormal.\n");
 
-		if (rep->has_reply) {
-			rep->reply_str[EEA_HINFO_MAX_REP_LEN - 1] = '\0';
-			dev_warn(dev, "Device replied: %*pEhp\n",
-				 (int)strlen(rep->reply_str), rep->reply_str);
-		}
+	if (rep->has_reply) {
+		rep->reply_str[EEA_HINFO_MAX_REP_LEN - 1] = '\0';
+		dev_warn(dev, "Device replied: %*pEhp\n",
+			 (int)strlen(rep->reply_str), rep->reply_str);
 	}
 
+err_free_rep:
 	kfree(rep);
 err_free_cfg:
 	kfree(cfg);
-- 
2.50.1

[PATCH net-next v2 4/5] eea: unify return code variable name

[Andy Shevchenko]

In one case 'rc' is used in the other 'ret', the most use 'err'.
Make the latter use the former, id est 'err'.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/alibaba/eea/eea_adminq.c | 34 +++++++++----------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/alibaba/eea/eea_adminq.c b/drivers/net/ethernet/alibaba/eea/eea_adminq.c
index 92eeff32b00c..f493afbcc41b 100644
--- a/drivers/net/ethernet/alibaba/eea/eea_adminq.c
+++ b/drivers/net/ethernet/alibaba/eea/eea_adminq.c
@@ -133,7 +133,7 @@ static int eea_adminq_submit(struct eea_net *enet, u16 cmd,
 {
 	struct eea_aq_cdesc *cdesc;
 	struct eea_aq_desc *desc;
-	int ret;
+	int err;
 
 	if (enet->adminq.broken)
 		return -EIO;
@@ -163,9 +163,9 @@ static int eea_adminq_submit(struct eea_net *enet, u16 cmd,
 	if ((enet->adminq.num % enet->adminq.ring->num) == 0)
 		enet->adminq.phase ^= EEA_RING_DESC_F_AQ_PHASE;
 
-	ret = read_poll_timeout(eea_ering_cq_get_desc, cdesc, cdesc, 10,
+	err = read_poll_timeout(eea_ering_cq_get_desc, cdesc, cdesc, 10,
 				EEA_AQ_TIMEOUT_US, false, enet->adminq.ring);
-	if (ret) {
+	if (err) {
 		netdev_err(enet->netdev,
 			   "adminq exec timeout. cmd: %d reset device.\n",
 			   cmd);
@@ -173,21 +173,21 @@ static int eea_adminq_submit(struct eea_net *enet, u16 cmd,
 		 * potential DMA writes after the memory is freed.
 		 */
 		eea_device_broken(enet);
-		return ret;
+		return err;
 	}
 
 	/* Returns 0 on success, or a negative error code on failure. */
-	ret = le32_to_cpu(cdesc->status);
+	err = le32_to_cpu(cdesc->status);
 
 	eea_ering_cq_ack_desc(enet->adminq.ring, 1);
 
-	if (ret)
+	if (err)
 		netdev_err(enet->netdev,
-			   "adminq exec failed. cmd: %d ret %d\n", cmd, ret);
+			   "adminq exec failed. cmd: %d ret %d\n", cmd, err);
 	else
 		*reply_len = le32_to_cpu(cdesc->reply_len);
 
-	return ret;
+	return err;
 }
 
 static int eea_adminq_exec(struct eea_net *enet, u16 cmd,
@@ -198,7 +198,7 @@ static int eea_adminq_exec(struct eea_net *enet, u16 cmd,
 	dma_addr_t req_addr = 0, res_addr = 0;
 	struct device *dma;
 	u32 reply_len = 0;
-	int ret;
+	int err;
 
 	if (reply)
 		*reply = 0;
@@ -214,19 +214,19 @@ static int eea_adminq_exec(struct eea_net *enet, u16 cmd,
 	if (res) {
 		res_addr = dma_map_single(dma, res, res_size, DMA_FROM_DEVICE);
 		if (unlikely(dma_mapping_error(dma, res_addr))) {
-			ret = -ENOMEM;
+			err = -ENOMEM;
 			goto err_unmap_req;
 		}
 	}
 
 	mutex_lock(&enet->adminq.lock);
-	ret = eea_adminq_submit(enet, cmd, req_addr, res_addr,
+	err = eea_adminq_submit(enet, cmd, req_addr, res_addr,
 				req_size, res_size, &reply_len);
 	mutex_unlock(&enet->adminq.lock);
 	if (res) {
 		dma_unmap_single(dma, res_addr, res_size, DMA_FROM_DEVICE);
 
-		if (ret)
+		if (err)
 			memset(res, 0, res_size);
 		else if (res_size > reply_len)
 			memset(res + reply_len, 0, res_size - reply_len);
@@ -239,7 +239,7 @@ static int eea_adminq_exec(struct eea_net *enet, u16 cmd,
 	if (req)
 		dma_unmap_single(dma, req_addr, req_size, DMA_TO_DEVICE);
 
-	return ret;
+	return err;
 }
 
 void eea_destroy_adminq(struct eea_net *enet)
@@ -484,7 +484,7 @@ void eea_adminq_config_host_info(struct eea_net *enet)
 	struct device *dev = enet->edev->dma_dev;
 	struct eea_aq_host_info_cfg *cfg;
 	struct eea_aq_host_info_rep *rep;
-	int rc;
+	int err;
 
 	cfg = kzalloc_obj(*cfg);
 	if (!cfg)
@@ -515,9 +515,9 @@ void eea_adminq_config_host_info(struct eea_net *enet)
 	strscpy(cfg->os_ver_str, utsname()->release, sizeof(cfg->os_ver_str));
 	strscpy(cfg->isa_str, utsname()->machine, sizeof(cfg->isa_str));
 
-	rc = eea_adminq_exec(enet, EEA_AQ_CMD_HOST_INFO,
-			     cfg, sizeof(*cfg), rep, sizeof(*rep), NULL);
-	if (rc)
+	err = eea_adminq_exec(enet, EEA_AQ_CMD_HOST_INFO,
+			      cfg, sizeof(*cfg), rep, sizeof(*rep), NULL);
+	if (err)
 		goto err_free_rep;
 
 	if (rep->op_code == EEA_HINFO_REP_BAD)
-- 
2.50.1

[PATCH net-next v2 5/5] eea: Use 2-argument strscpy()

[Andy Shevchenko]

From: Andy Shevchenko <andy.shevchenko@gmail.com>

Use 2-argument strscpy(), which is not only shorter but also provides
an additional check that destination buffer is an array.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/alibaba/eea/eea_adminq.c  | 4 ++--
 drivers/net/ethernet/alibaba/eea/eea_ethtool.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/alibaba/eea/eea_adminq.c b/drivers/net/ethernet/alibaba/eea/eea_adminq.c
index f493afbcc41b..942f1f1911af 100644
--- a/drivers/net/ethernet/alibaba/eea/eea_adminq.c
+++ b/drivers/net/ethernet/alibaba/eea/eea_adminq.c
@@ -512,8 +512,8 @@ void eea_adminq_config_host_info(struct eea_net *enet)
 	cfg->pci_bdf            = cpu_to_le16(eea_pci_bdf(enet->edev));
 	cfg->pci_domain         = cpu_to_le32(eea_pci_domain_nr(enet->edev));
 
-	strscpy(cfg->os_ver_str, utsname()->release, sizeof(cfg->os_ver_str));
-	strscpy(cfg->isa_str, utsname()->machine, sizeof(cfg->isa_str));
+	strscpy(cfg->os_ver_str, utsname()->release);
+	strscpy(cfg->isa_str, utsname()->machine);
 
 	err = eea_adminq_exec(enet, EEA_AQ_CMD_HOST_INFO,
 			      cfg, sizeof(*cfg), rep, sizeof(*rep), NULL);
diff --git a/drivers/net/ethernet/alibaba/eea/eea_ethtool.c b/drivers/net/ethernet/alibaba/eea/eea_ethtool.c
index 479779aa2dd8..15f4c7b2d898 100644
--- a/drivers/net/ethernet/alibaba/eea/eea_ethtool.c
+++ b/drivers/net/ethernet/alibaba/eea/eea_ethtool.c
@@ -40,8 +40,8 @@ static void eea_get_drvinfo(struct net_device *netdev,
 	struct eea_net *enet = netdev_priv(netdev);
 	struct eea_device *edev = enet->edev;
 
-	strscpy(info->driver,   KBUILD_MODNAME,     sizeof(info->driver));
-	strscpy(info->bus_info, eea_pci_name(edev), sizeof(info->bus_info));
+	strscpy(info->driver, KBUILD_MODNAME);
+	strscpy(info->bus_info, eea_pci_name(edev));
 }
 
 static void eea_get_ringparam(struct net_device *netdev,
-- 
2.50.1

Re: [PATCH net-next v2 2/5] eea: Remove unneeded assignment

[Joe Damato]

On Fri, Oct 02, 2026 at 11:59:20AM +0200, Andy Shevchenko wrote:
> The initial err is not used anywhere, drop the unneeded assignment.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/net/ethernet/alibaba/eea/eea_adminq.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Joe Damato <joe@dama.to>

Re: [PATCH net-next v2 3/5] eea: Refactor error handling in eea_adminq_config_host_info()

[Joe Damato]

On Fri, Oct 02, 2026 at 11:59:21AM +0200, Andy Shevchenko wrote:
> Use traditional pattern to check for error first in
> eea_adminq_config_host_info().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/net/ethernet/alibaba/eea/eea_adminq.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)

Reviewed-by: Joe Damato <joe@dama.to>

Re: [PATCH net-next v2 4/5] eea: unify return code variable name

[Joe Damato]

On Fri, Oct 02, 2026 at 11:59:22AM +0200, Andy Shevchenko wrote:
> In one case 'rc' is used in the other 'ret', the most use 'err'.
> Make the latter use the former, id est 'err'.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/net/ethernet/alibaba/eea/eea_adminq.c | 34 +++++++++----------
>  1 file changed, 17 insertions(+), 17 deletions(-)

Reviewed-by: Joe Damato <joe@dama.to>

Re: [PATCH net-next v2 5/5] eea: Use 2-argument strscpy()

[Joe Damato]

On Fri, Oct 02, 2026 at 11:59:23AM +0200, Andy Shevchenko wrote:
> From: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> Use 2-argument strscpy(), which is not only shorter but also provides
> an additional check that destination buffer is an array.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/net/ethernet/alibaba/eea/eea_adminq.c  | 4 ++--
>  drivers/net/ethernet/alibaba/eea/eea_ethtool.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>

Reviewed-by: Joe Damato <joe@dama.to>

Re: [PATCH net-next v2 1/5] eea: Drop temporary buffer by using %*pEhp directly

[Joe Damato]

On Fri, Oct 02, 2026 at 11:59:19AM +0200, Andy Shevchenko wrote:
> Instead of calling string_escape_str() followed by %s specifier,
> use %*pEhp specifier directly. With this being done, drop temporary
> buffer.
> 
> Note, as Sashiko pointed out in [1] the %*pEp, which is the direct
> substitution of the original call to string_escape_str() with ESCAPE_NP
> flag set, makes the whole thing like "no escape". To add some sanity
> to this the used specifier also includes ESCAPE_HEX.
> 
> Link: https://lore.kernel.org/r/179089728102.434549.12368098081790653201@kernel.org [1]
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/net/ethernet/alibaba/eea/eea_adminq.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/alibaba/eea/eea_adminq.c b/drivers/net/ethernet/alibaba/eea/eea_adminq.c
> index 73d0caf25700..f2284fc885c3 100644
> --- a/drivers/net/ethernet/alibaba/eea/eea_adminq.c
> +++ b/drivers/net/ethernet/alibaba/eea/eea_adminq.c
> @@ -522,16 +522,9 @@ void eea_adminq_config_host_info(struct eea_net *enet)
>  			dev_warn(dev, "The hardware-driven state validation may be abnormal.\n");
>  
>  		if (rep->has_reply) {
> -			char buf[EEA_HINFO_MAX_REP_LEN] = {0};
> -
>  			rep->reply_str[EEA_HINFO_MAX_REP_LEN - 1] = '\0';
> -
> -			string_escape_str(rep->reply_str, buf, sizeof(buf),
> -					  ESCAPE_NP, NULL);
> -
> -			buf[EEA_HINFO_MAX_REP_LEN - 1] = '\0';
> -
> -			dev_warn(dev, "Device replied: %s\n", buf);
> +			dev_warn(dev, "Device replied: %*pEhp\n",
> +				 (int)strlen(rep->reply_str), rep->reply_str);

i have no idea what this otuput could be but i suppose if you wanted to remove
the terminator write above, you could do:

   strnlen(rep->reply_str, ....)

instead, but that feels like a fairly useless nit so:

Reviewed-by: Joe Damato <joe@dama.to>