* Re: [Intel-wired-lan] [PATCH net-next] ixgbe: Remove redundant self-assignments in ACI command execution
@ 2025-01-13 15:23 Kwapulinski, Piotr
2025-01-14 0:22 ` Dheeraj Reddy Jonnalagadda
0 siblings, 1 reply; 6+ messages in thread
From: Kwapulinski, Piotr @ 2025-01-13 15:23 UTC (permalink / raw)
To: dheeraj.linuxdev@gmail.com
Cc: andrew+netdev@lunn.ch, Nguyen, Anthony L, davem@davemloft.net,
edumazet@google.com, intel-wired-lan@lists.osuosl.org,
kuba@kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, pabeni@redhat.com, Kitszel, Przemyslaw,
Swiatkowski, Michal
[-- Attachment #1: Type: text/plain, Size: 3918 bytes --]
>[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.linuxdev@gmail.com<mailto:dheeraj.linuxdev@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]);
}
}
[-- Attachment #2: Type: text/html, Size: 12010 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Intel-wired-lan] [PATCH net-next] ixgbe: Remove redundant self-assignments in ACI command execution
2025-01-13 15:23 [Intel-wired-lan] [PATCH net-next] ixgbe: Remove redundant self-assignments in ACI command execution Kwapulinski, Piotr
@ 2025-01-14 0:22 ` Dheeraj Reddy Jonnalagadda
2025-01-14 12:32 ` Kwapulinski, Piotr
0 siblings, 1 reply; 6+ messages in thread
From: Dheeraj Reddy Jonnalagadda @ 2025-01-14 0:22 UTC (permalink / raw)
To: Kwapulinski, Piotr
Cc: andrew+netdev@lunn.ch, Nguyen, Anthony L, davem@davemloft.net,
edumazet@google.com, intel-wired-lan@lists.osuosl.org,
kuba@kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, pabeni@redhat.com, Kitszel, Przemyslaw,
Swiatkowski, Michal
On Mon, Jan 13, 2025 at 03:23:31PM +0000, Kwapulinski, Piotr wrote:
> >[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.linuxdev@gmail.com<mailto:dheeraj.linuxdev@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]);
> }
> }
>
Hello Piotr,
Thank you for suggesting the fix. I will prepare the new patch and send
it over.
-Dheeraj
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Intel-wired-lan] [PATCH net-next] ixgbe: Remove redundant self-assignments in ACI command execution
2025-01-14 0:22 ` Dheeraj Reddy Jonnalagadda
@ 2025-01-14 12:32 ` Kwapulinski, Piotr
2025-01-15 3:37 ` Dheeraj Reddy Jonnalagadda
0 siblings, 1 reply; 6+ messages in thread
From: Kwapulinski, Piotr @ 2025-01-14 12:32 UTC (permalink / raw)
To: Dheeraj Reddy Jonnalagadda
Cc: andrew+netdev@lunn.ch, Nguyen, Anthony L, davem@davemloft.net,
edumazet@google.com, intel-wired-lan@lists.osuosl.org,
kuba@kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, pabeni@redhat.com, Kitszel, Przemyslaw,
Swiatkowski, Michal
>-----Original Message-----
>From: Dheeraj Reddy Jonnalagadda <dheeraj.linuxdev@gmail.com>
>Sent: Tuesday, January 14, 2025 1:23 AM
>To: Kwapulinski, Piotr <piotr.kwapulinski@intel.com>
>Cc: andrew+netdev@lunn.ch; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; davem@davemloft.net; edumazet@google.com; intel-wired-lan@lists.osuosl.org; kuba@kernel.org; linux-kernel@vger.kernel.org; netdev@vger.kernel.org; pabeni@redhat.com; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Swiatkowski, Michal <michal.swiatkowski@intel.com>
>Subject: Re: [Intel-wired-lan] [PATCH net-next] ixgbe: Remove redundant self-assignments in ACI command execution
>
>On Mon, Jan 13, 2025 at 03:23:31PM +0000, Kwapulinski, Piotr wrote:
>> >[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?selectedIs
>> >sue=1602757
>> >Signed-off-by: Dheeraj Reddy Jonnalagadda
>> >dheeraj.linuxdev@gmail.com<mailto:dheeraj.linuxdev@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]);
>> }
>> }
>>
>
>Hello Piotr,
>
>Thank you for suggesting the fix. I will prepare the new patch and send it over.
>
>-Dheeraj
Hello,
As a result of internal review from Przemek, it may be improved as follows:
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
index e0f773c..0ec944c 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),
+ cpu_to_le32(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] = le32_to_cpu(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] = le32_to_cpu(raw_desc[i]);
}
}
Thank you,
Piotr
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Intel-wired-lan] [PATCH net-next] ixgbe: Remove redundant self-assignments in ACI command execution
2025-01-14 12:32 ` Kwapulinski, Piotr
@ 2025-01-15 3:37 ` Dheeraj Reddy Jonnalagadda
0 siblings, 0 replies; 6+ messages in thread
From: Dheeraj Reddy Jonnalagadda @ 2025-01-15 3:37 UTC (permalink / raw)
To: Kwapulinski, Piotr
Cc: andrew+netdev@lunn.ch, Nguyen, Anthony L, davem@davemloft.net,
edumazet@google.com, intel-wired-lan@lists.osuosl.org,
kuba@kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, pabeni@redhat.com, Kitszel, Przemyslaw,
Swiatkowski, Michal
On Tue, Jan 14, 2025 at 12:32:47PM +0000, Kwapulinski, Piotr wrote:
> >-----Original Message-----
> >From: Dheeraj Reddy Jonnalagadda <dheeraj.linuxdev@gmail.com>
> >Sent: Tuesday, January 14, 2025 1:23 AM
> >To: Kwapulinski, Piotr <piotr.kwapulinski@intel.com>
> >Cc: andrew+netdev@lunn.ch; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; davem@davemloft.net; edumazet@google.com; intel-wired-lan@lists.osuosl.org; kuba@kernel.org; linux-kernel@vger.kernel.org; netdev@vger.kernel.org; pabeni@redhat.com; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Swiatkowski, Michal <michal.swiatkowski@intel.com>
> >Subject: Re: [Intel-wired-lan] [PATCH net-next] ixgbe: Remove redundant self-assignments in ACI command execution
> >
> >On Mon, Jan 13, 2025 at 03:23:31PM +0000, Kwapulinski, Piotr wrote:
> >> >[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?selectedIs
> >> >sue=1602757
> >> >Signed-off-by: Dheeraj Reddy Jonnalagadda
> >> >dheeraj.linuxdev@gmail.com<mailto:dheeraj.linuxdev@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]);
> >> }
> >> }
> >>
> >
> >Hello Piotr,
> >
> >Thank you for suggesting the fix. I will prepare the new patch and send it over.
> >
> >-Dheeraj
>
> Hello,
> As a result of internal review from Przemek, it may be improved as follows:
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
> index e0f773c..0ec944c 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),
> + cpu_to_le32(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] = le32_to_cpu(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] = le32_to_cpu(raw_desc[i]);
> }
> }
>
> Thank you,
> Piotr
Hello Piotr,
Thank you. I will update the patch accordingly and send it over.
-Dheeraj
^ permalink raw reply [flat|nested] 6+ messages in thread
* [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; 6+ 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.linuxdev@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];
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [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, 0 replies; 6+ messages in thread
From: Michal Swiatkowski @ 2025-01-08 6:29 UTC (permalink / raw)
To: Dheeraj Reddy Jonnalagadda
Cc: anthony.l.nguyen, przemyslaw.kitszel, andrew+netdev, davem,
edumazet, kuba, pabeni, intel-wired-lan, netdev
On Wed, Jan 08, 2025 at 11:06:14AM +0530, Dheeraj Reddy Jonnalagadda wrote:
> 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.linuxdev@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];
raw_desc should be always in le32, probably there was a conversion which
was removed between patchset versions. Adding orginal author in CC to
confirm that no conversion here is fine.
Thanks,
Michal
> }
> }
>
> @@ -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];
> }
> }
>
> --
> 2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-01-15 3:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-13 15:23 [Intel-wired-lan] [PATCH net-next] ixgbe: Remove redundant self-assignments in ACI command execution Kwapulinski, Piotr
2025-01-14 0:22 ` Dheeraj Reddy Jonnalagadda
2025-01-14 12:32 ` Kwapulinski, Piotr
2025-01-15 3:37 ` Dheeraj Reddy Jonnalagadda
-- strict thread matches above, loose matches on Subject: below --
2025-01-08 5:36 Dheeraj Reddy Jonnalagadda
2025-01-08 6:29 ` Michal Swiatkowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox