Re: [Intel-wired-lan] [PATCH net-next] ixgbe: Remove redundant self-assignments in ACI command execution

[Kwapulinski, Piotr]

>[Intel-wired-lan] [PATCH net-next] ixgbe: Remove redundant self-assignments in 
>ACI command execution
>@ 2025-01-08  5:36 Dheeraj Reddy Jonnalagadda
>  2025-01-08  6:29 ` Michal Swiatkowski
>  0 siblings, 1 reply; 2+ messages in thread
>From: Dheeraj Reddy Jonnalagadda @ 2025-01-08  5:36 UTC (permalink / raw)
>  To: anthony.l.nguyen, przemyslaw.kitszel
>  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, intel-wired-lan,
>             netdev, linux-kernel, Dheeraj Reddy Jonnalagadda
>
>Remove redundant statements in ixgbe_aci_send_cmd_execute() where
>raw_desc[i] is assigned to itself. These self-assignments have no
>effect and can be safely removed.
>
>Fixes: 46761fd52a88 ("ixgbe: Add support for E610 FW Admin Command Interface")
>Closes: 
>https://scan7.scan.coverity.com/#/project-view/52337/11354?selectedIssue=1602757
>Signed-off-by: Dheeraj Reddy Jonnalagadda 
>dheeraj.linux...@gmail.com<mailto:dheeraj.linux...@gmail.com>
>---
> drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c 
>b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
>index 683c668672d6..408c0874cdc2 100644
>--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
>+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
>@@ -145,7 +145,6 @@ static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw,
>             if ((hicr & IXGBE_PF_HICR_SV)) {
>                            for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; 
> i++) {
>                                           raw_desc[i] = IXGBE_READ_REG(hw, 
> IXGBE_PF_HIDA(i));
>-                                         raw_desc[i] = raw_desc[i];
>                            }
>             }
>
>@@ -153,7 +152,6 @@ static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw,
>             if ((hicr & IXGBE_PF_HICR_EV) && !(hicr & IXGBE_PF_HICR_C)) {
>                            for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; 
> i++) {
>                                           raw_desc[i] = IXGBE_READ_REG(hw, 
> IXGBE_PF_HIDA_2(i));
>-                                         raw_desc[i] = raw_desc[i];
>                            }
>             }
>

Hello,
Possible solution may be as follows. I may also prepare the fix myself. Please 
let me know.
Thanks,
Piotr

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
index e0f773c..af51e5a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
@@ -113,7 +113,8 @@ static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw,

        /* Descriptor is written to specific registers */
        for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++)
-               IXGBE_WRITE_REG(hw, IXGBE_PF_HIDA(i), raw_desc[i]);
+               IXGBE_WRITE_REG(hw, IXGBE_PF_HIDA(i),
+                               le32_to_cpu(raw_desc[i]));

        /* SW has to set PF_HICR.C bit and clear PF_HICR.SV and
         * PF_HICR_EV
@@ -145,7 +146,7 @@ static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw,
        if ((hicr & IXGBE_PF_HICR_SV)) {
                for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++) {
                        raw_desc[i] = IXGBE_READ_REG(hw, IXGBE_PF_HIDA(i));
-                       raw_desc[i] = raw_desc[i];
+                       raw_desc[i] = cpu_to_le32(raw_desc[i]);
                }
        }

@@ -153,7 +154,7 @@ static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw,
        if ((hicr & IXGBE_PF_HICR_EV) && !(hicr & IXGBE_PF_HICR_C)) {
                for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++) {
                        raw_desc[i] = IXGBE_READ_REG(hw, IXGBE_PF_HIDA_2(i));
-                       raw_desc[i] = raw_desc[i];
+                       raw_desc[i] = cpu_to_le32(raw_desc[i]);
                }
        }