[PATCH 0/2] rt2x00,net/phy,neterion: Remove dead values

[PATCH 0/2] rt2x00,net/phy,neterion: Remove dead values

[Ariel Otilibili]

Hello,

This series clears out the Coverity-IDs 1525307, 1269173, & 1575053.

Thank you,

Ariel Otilibili (2):
  rt2x00: Remove unusued value
  net/phy,neterion: Remove dead values

 drivers/net/ethernet/neterion/s2io.c           | 2 --
 drivers/net/phy/nxp-c45-tja11xx-macsec.c       | 1 -
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 6 ------
 3 files changed, 9 deletions(-)

-- 
2.47.1

[PATCH 1/2] rt2x00: Remove unusued value

[Ariel Otilibili]

Coverity-ID: 1525307
Signed-off-by: Ariel Otilibili <ariel.otilibili-anieli@eurecom.fr>
---
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 60c2a12e9d5e..e5f553a1ea24 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -8882,13 +8882,10 @@ static void rt2800_rxiq_calibration(struct rt2x00_dev *rt2x00dev)
 
 	for (ch_idx = 0; ch_idx < 2; ch_idx = ch_idx + 1) {
 		if (ch_idx == 0) {
-			rfval = rfb0r1 & (~0x3);
 			rfval = rfb0r1 | 0x1;
 			rt2800_rfcsr_write_bank(rt2x00dev, 0, 1, rfval);
-			rfval = rfb0r2 & (~0x33);
 			rfval = rfb0r2 | 0x11;
 			rt2800_rfcsr_write_bank(rt2x00dev, 0, 2, rfval);
-			rfval = rfb0r42 & (~0x50);
 			rfval = rfb0r42 | 0x10;
 			rt2800_rfcsr_write_bank(rt2x00dev, 0, 42, rfval);
 
@@ -8901,13 +8898,10 @@ static void rt2800_rxiq_calibration(struct rt2x00_dev *rt2x00dev)
 
 			rt2800_bbp_dcoc_write(rt2x00dev, 1, 0x00);
 		} else {
-			rfval = rfb0r1 & (~0x3);
 			rfval = rfb0r1 | 0x2;
 			rt2800_rfcsr_write_bank(rt2x00dev, 0, 1, rfval);
-			rfval = rfb0r2 & (~0x33);
 			rfval = rfb0r2 | 0x22;
 			rt2800_rfcsr_write_bank(rt2x00dev, 0, 2, rfval);
-			rfval = rfb0r42 & (~0x50);
 			rfval = rfb0r42 | 0x40;
 			rt2800_rfcsr_write_bank(rt2x00dev, 0, 42, rfval);
 
-- 
2.47.1

Re: [PATCH 1/2] rt2x00: Remove unusued value

[Stanislaw Gruszka]

On Sat, Dec 21, 2024 at 01:39:32PM +0100, Ariel Otilibili wrote:
> Coverity-ID: 1525307
> Signed-off-by: Ariel Otilibili <ariel.otilibili-anieli@eurecom.fr>
> ---
>  drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> index 60c2a12e9d5e..e5f553a1ea24 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> @@ -8882,13 +8882,10 @@ static void rt2800_rxiq_calibration(struct rt2x00_dev *rt2x00dev)
>  
>  	for (ch_idx = 0; ch_idx < 2; ch_idx = ch_idx + 1) {
>  		if (ch_idx == 0) {
> -			rfval = rfb0r1 & (~0x3);
>  			rfval = rfb0r1 | 0x1;

I wonder if intention here was different, for example:

 			rfval = rfb0r1 & (~0x3);
  			rfval = rfval | 0x1;

For me the patch looks ok - it does not change existing behaviour,
since rfval is overwritten by second line anyway.

Acked-by: Stanislaw Gruszka <stf_xl@wp.pl>

But Tomislav and Daniel, please check if this code is correct.

>  			rt2800_rfcsr_write_bank(rt2x00dev, 0, 1, rfval);
> -			rfval = rfb0r2 & (~0x33);
>  			rfval = rfb0r2 | 0x11;
>  			rt2800_rfcsr_write_bank(rt2x00dev, 0, 2, rfval);
> -			rfval = rfb0r42 & (~0x50);
>  			rfval = rfb0r42 | 0x10;
>  			rt2800_rfcsr_write_bank(rt2x00dev, 0, 42, rfval);
>  
> @@ -8901,13 +8898,10 @@ static void rt2800_rxiq_calibration(struct rt2x00_dev *rt2x00dev)
>  
>  			rt2800_bbp_dcoc_write(rt2x00dev, 1, 0x00);
>  		} else {
> -			rfval = rfb0r1 & (~0x3);
>  			rfval = rfb0r1 | 0x2;
>  			rt2800_rfcsr_write_bank(rt2x00dev, 0, 1, rfval);
> -			rfval = rfb0r2 & (~0x33);
>  			rfval = rfb0r2 | 0x22;
>  			rt2800_rfcsr_write_bank(rt2x00dev, 0, 2, rfval);
> -			rfval = rfb0r42 & (~0x50);
>  			rfval = rfb0r42 | 0x40;
>  			rt2800_rfcsr_write_bank(rt2x00dev, 0, 42, rfval);
>  
> -- 
> 2.47.1
> 

Re: [PATCH 1/2] rt2x00: Remove unusued value

[Daniel Golle]

On Fri, Jan 03, 2025 at 09:55:40AM +0100, Stanislaw Gruszka wrote:
> On Sat, Dec 21, 2024 at 01:39:32PM +0100, Ariel Otilibili wrote:
> > Coverity-ID: 1525307
> > Signed-off-by: Ariel Otilibili <ariel.otilibili-anieli@eurecom.fr>
> > ---
> >  drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 6 ------
> >  1 file changed, 6 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> > index 60c2a12e9d5e..e5f553a1ea24 100644
> > --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> > +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> > @@ -8882,13 +8882,10 @@ static void rt2800_rxiq_calibration(struct rt2x00_dev *rt2x00dev)
> >  
> >  	for (ch_idx = 0; ch_idx < 2; ch_idx = ch_idx + 1) {
> >  		if (ch_idx == 0) {
> > -			rfval = rfb0r1 & (~0x3);
> >  			rfval = rfb0r1 | 0x1;
> 
> I wonder if intention here was different, for example:
> 
>  			rfval = rfb0r1 & (~0x3);
>   			rfval = rfval | 0x1;
> 
> For me the patch looks ok - it does not change existing behaviour,
> since rfval is overwritten by second line anyway.

I agree with the likely intention here, however, the vendor driver
also comes with the dead code, see
https://github.com/lixuande/rt2860v2/blob/master/files/rt2860v2/common/cmm_rf_cal.c#L2690

So this is certainly a bug in the vendor driver as well which got ported
bug-by-bug to rt2x00... Not sure what is the best thing to do in this
case.

> 
> Acked-by: Stanislaw Gruszka <stf_xl@wp.pl>
> 
> But Tomislav and Daniel, please check if this code is correct.
> 
> >  			rt2800_rfcsr_write_bank(rt2x00dev, 0, 1, rfval);
> > -			rfval = rfb0r2 & (~0x33);
> >  			rfval = rfb0r2 | 0x11;
> >  			rt2800_rfcsr_write_bank(rt2x00dev, 0, 2, rfval);
> > -			rfval = rfb0r42 & (~0x50);
> >  			rfval = rfb0r42 | 0x10;
> >  			rt2800_rfcsr_write_bank(rt2x00dev, 0, 42, rfval);
> >  
> > @@ -8901,13 +8898,10 @@ static void rt2800_rxiq_calibration(struct rt2x00_dev *rt2x00dev)
> >  
> >  			rt2800_bbp_dcoc_write(rt2x00dev, 1, 0x00);
> >  		} else {
> > -			rfval = rfb0r1 & (~0x3);
> >  			rfval = rfb0r1 | 0x2;
> >  			rt2800_rfcsr_write_bank(rt2x00dev, 0, 1, rfval);
> > -			rfval = rfb0r2 & (~0x33);
> >  			rfval = rfb0r2 | 0x22;
> >  			rt2800_rfcsr_write_bank(rt2x00dev, 0, 2, rfval);
> > -			rfval = rfb0r42 & (~0x50);
> >  			rfval = rfb0r42 | 0x40;
> >  			rt2800_rfcsr_write_bank(rt2x00dev, 0, 42, rfval);
> >  
> > -- 
> > 2.47.1
> > 
> 

Re: [PATCH 1/2] rt2x00: Remove unusued value

[Stanislaw Gruszka]

On Fri, Jan 03, 2025 at 11:40:52AM +0000, Daniel Golle wrote:
> On Fri, Jan 03, 2025 at 09:55:40AM +0100, Stanislaw Gruszka wrote:
> > On Sat, Dec 21, 2024 at 01:39:32PM +0100, Ariel Otilibili wrote:
> > > Coverity-ID: 1525307
> > > Signed-off-by: Ariel Otilibili <ariel.otilibili-anieli@eurecom.fr>
> > > ---
> > >  drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 6 ------
> > >  1 file changed, 6 deletions(-)
> > > 
> > > diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> > > index 60c2a12e9d5e..e5f553a1ea24 100644
> > > --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> > > +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> > > @@ -8882,13 +8882,10 @@ static void rt2800_rxiq_calibration(struct rt2x00_dev *rt2x00dev)
> > >  
> > >  	for (ch_idx = 0; ch_idx < 2; ch_idx = ch_idx + 1) {
> > >  		if (ch_idx == 0) {
> > > -			rfval = rfb0r1 & (~0x3);
> > >  			rfval = rfb0r1 | 0x1;
> > 
> > I wonder if intention here was different, for example:
> > 
> >  			rfval = rfb0r1 & (~0x3);
> >   			rfval = rfval | 0x1;
> > 
> > For me the patch looks ok - it does not change existing behaviour,
> > since rfval is overwritten by second line anyway.
> 
> I agree with the likely intention here, however, the vendor driver
> also comes with the dead code, see
> https://github.com/lixuande/rt2860v2/blob/master/files/rt2860v2/common/cmm_rf_cal.c#L2690
> 
> So this is certainly a bug in the vendor driver as well which got ported
> bug-by-bug to rt2x00... Not sure what is the best thing to do in this
> case.

As this was already tested and match vendor driver I would prefer
not to change behavior even if it looks suspicious.

Regards
Stanislaw

> > Acked-by: Stanislaw Gruszka <stf_xl@wp.pl>
> > 
> > But Tomislav and Daniel, please check if this code is correct.
> > 
> > >  			rt2800_rfcsr_write_bank(rt2x00dev, 0, 1, rfval);
> > > -			rfval = rfb0r2 & (~0x33);
> > >  			rfval = rfb0r2 | 0x11;
> > >  			rt2800_rfcsr_write_bank(rt2x00dev, 0, 2, rfval);
> > > -			rfval = rfb0r42 & (~0x50);
> > >  			rfval = rfb0r42 | 0x10;
> > >  			rt2800_rfcsr_write_bank(rt2x00dev, 0, 42, rfval);
> > >  
> > > @@ -8901,13 +8898,10 @@ static void rt2800_rxiq_calibration(struct rt2x00_dev *rt2x00dev)
> > >  
> > >  			rt2800_bbp_dcoc_write(rt2x00dev, 1, 0x00);
> > >  		} else {
> > > -			rfval = rfb0r1 & (~0x3);
> > >  			rfval = rfb0r1 | 0x2;
> > >  			rt2800_rfcsr_write_bank(rt2x00dev, 0, 1, rfval);
> > > -			rfval = rfb0r2 & (~0x33);
> > >  			rfval = rfb0r2 | 0x22;
> > >  			rt2800_rfcsr_write_bank(rt2x00dev, 0, 2, rfval);
> > > -			rfval = rfb0r42 & (~0x50);
> > >  			rfval = rfb0r42 | 0x40;
> > >  			rt2800_rfcsr_write_bank(rt2x00dev, 0, 42, rfval);
> > >  
> > > -- 
> > > 2.47.1
> > > 
> > 

Re: [PATCH 1/2] rt2x00: Remove unusued value

[Ariel Otilibili-Anieli]

Hello Stanislaw, hello Daniel; happy new year,

On Friday, January 03, 2025 14:10 CET, Stanislaw Gruszka <stf_xl@wp.pl> wrote:

> On Fri, Jan 03, 2025 at 11:40:52AM +0000, Daniel Golle wrote:
> > On Fri, Jan 03, 2025 at 09:55:40AM +0100, Stanislaw Gruszka wrote:
> > 
> > I agree with the likely intention here, however, the vendor driver
> > also comes with the dead code, see
> > https://github.com/lixuande/rt2860v2/blob/master/files/rt2860v2/common/cmm_rf_cal.c#L2690
> > 
> > So this is certainly a bug in the vendor driver as well which got ported
> > bug-by-bug to rt2x00... Not sure what is the best thing to do in this
> > case.
> 
> As this was already tested and match vendor driver I would prefer
> not to change behavior even if it looks suspicious.

Thanks for having looked into this; I much appreciate your feedback.

From what you two said, I understand that the patch should remove the duplicate code, and not change the logic behind.

Is this right?

If so; then, I have nothing else to do.
> 
> Regards
> Stanislaw
>

Re: [PATCH 1/2] rt2x00: Remove unusued value

[Stanislaw Gruszka]

Hi

On Fri, Jan 03, 2025 at 02:39:21PM +0100, Ariel Otilibili-Anieli wrote:
> On Friday, January 03, 2025 14:10 CET, Stanislaw Gruszka <stf_xl@wp.pl> wrote:
> 
> > On Fri, Jan 03, 2025 at 11:40:52AM +0000, Daniel Golle wrote:
> > > On Fri, Jan 03, 2025 at 09:55:40AM +0100, Stanislaw Gruszka wrote:
> > > 
> > > I agree with the likely intention here, however, the vendor driver
> > > also comes with the dead code, see
> > > https://github.com/lixuande/rt2860v2/blob/master/files/rt2860v2/common/cmm_rf_cal.c#L2690
> > > 
> > > So this is certainly a bug in the vendor driver as well which got ported
> > > bug-by-bug to rt2x00... Not sure what is the best thing to do in this
> > > case.
> > 
> > As this was already tested and match vendor driver I would prefer
> > not to change behavior even if it looks suspicious.
> 
> Thanks for having looked into this; I much appreciate your feedback.
> 
> From what you two said, I understand that the patch should remove the duplicate code, and not change the logic behind.
> 
> Is this right?

Yes. 

Regards
Stanislaw
> 
> If so; then, I have nothing else to do.
> > 
> > Regards
> > Stanislaw
> >
> 

Re: [PATCH 1/2] rt2x00: Remove unusued value

[Ariel Otilibili-Anieli]

Hi Stanislaw,

On Saturday, January 04, 2025 11:37 CET, Stanislaw Gruszka <stf_xl@wp.pl> wrote:

> Hi
> 
> On Fri, Jan 03, 2025 at 02:39:21PM +0100, Ariel Otilibili-Anieli wrote:
> > On Friday, January 03, 2025 14:10 CET, Stanislaw Gruszka <stf_xl@wp.pl> wrote:
> > 
> > 
> > Thanks for having looked into this; I much appreciate your feedback.
> > 
> > From what you two said, I understand that the patch should remove the duplicate code, and not change the logic behind.
> > 
> > Is this right?
> 
> Yes. 

Great, then; thanks for having acked the patch as such.
> 
> Regards
> Stanislaw
> > 
> > If so; then, I have nothing else to do.
> >

Re: [PATCH 1/2] rt2x00: Remove unusued value

[Daniel Golle]

H again,


On Sat, Jan 04, 2025 at 01:51:25PM +0100, Ariel Otilibili-Anieli wrote:
> Great, then; thanks for having acked the patch as such.

I just noticed that Shiji Yang had posted a series of patches for
OpenWrt which also addresses the same issue, however, instead of
removing the augmented assignment, it fixes it to the supposedly
originally intended way.

See
https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=package/kernel/mac80211/patches/rt2x00/621-04-rt2x00-fix-register-operation-on-RXIQ-calibration.patch;h=aa6f9c437c6447831490588b2cead6919accda58;hb=5d583901657bdfbbf9fad77d9247872427aa5c99

I suppose this was tested together with the other changes of the same
series, so we may want to pick that instead.


Cheers


Daniel

Re: [PATCH 1/2] rt2x00: Remove unusued value

[Ariel Otilibili-Anieli]

Hi Daniel, hi Shiji, hi Stanislaw,

On Sunday, January 05, 2025 22:21 CET, Daniel Golle <daniel@makrotopia.org> wrote:

> H again,
> 
> 
> On Sat, Jan 04, 2025 at 01:51:25PM +0100, Ariel Otilibili-Anieli wrote:
> > Great, then; thanks for having acked the patch as such.
> 
> I just noticed that Shiji Yang had posted a series of patches for
> OpenWrt which also addresses the same issue, however, instead of
> removing the augmented assignment, it fixes it to the supposedly
> originally intended way.
> 
> See
> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=package/kernel/mac80211/patches/rt2x00/621-04-rt2x00-fix-register-operation-on-RXIQ-calibration.patch;h=aa6f9c437c6447831490588b2cead6919accda58;hb=5d583901657bdfbbf9fad77d9247872427aa5c99
> 
> I suppose this was tested together with the other changes of the same
> series, so we may want to pick that instead.

Thanks for having put some time into the research, Daniel; I looked into the openwrt archives for 2024, none of Shiji’s messages mentions that patch.

Though, if you three agree, I will push a new series, modelled on that patch, and you as Suggested-by.

Have a good week,
Ariel
> 
> 
> Cheers
> 
> 
> Daniel
>

Re: [PATCH 1/2] rt2x00: Remove unusued value

[Stanislaw Gruszka]

Hi Ariel,

On Mon, Jan 06, 2025 at 08:23:34AM +0100, Ariel Otilibili-Anieli wrote:
> Hi Daniel, hi Shiji, hi Stanislaw,
> 
> On Sunday, January 05, 2025 22:21 CET, Daniel Golle <daniel@makrotopia.org> wrote:
> 
> > H again,
> > 
> > 
> > On Sat, Jan 04, 2025 at 01:51:25PM +0100, Ariel Otilibili-Anieli wrote:
> > > Great, then; thanks for having acked the patch as such.
> > 
> > I just noticed that Shiji Yang had posted a series of patches for
> > OpenWrt which also addresses the same issue, however, instead of
> > removing the augmented assignment, it fixes it to the supposedly
> > originally intended way.
> > 
> > See
> > https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=package/kernel/mac80211/patches/rt2x00/621-04-rt2x00-fix-register-operation-on-RXIQ-calibration.patch;h=aa6f9c437c6447831490588b2cead6919accda58;hb=5d583901657bdfbbf9fad77d9247872427aa5c99
> > 
> > I suppose this was tested together with the other changes of the same
> > series, so we may want to pick that instead.
> 
> Thanks for having put some time into the research, Daniel; I looked into the openwrt archives for 2024, none of Shiji’s messages mentions that patch.
> 
> Though, if you three agree, I will push a new series, modelled on that patch, and you as Suggested-by.

Please post that change. But to not mix it with
patches against other drivers in the same series.
(multiple rt2x00 patches in one patchset are ok).

And please use "wifi: rt2x00:" as subject prefix.

Thanks
Stanislaw

Re: [PATCH 1/2] rt2x00: Remove unusued value

[Jonas Gorski]

Hi,

On Mon, Jan 6, 2025 at 8:23 AM Ariel Otilibili-Anieli
<Ariel.Otilibili-Anieli@eurecom.fr> wrote:
>
> Hi Daniel, hi Shiji, hi Stanislaw,
>
> On Sunday, January 05, 2025 22:21 CET, Daniel Golle <daniel@makrotopia.org> wrote:
>
> > H again,
> >
> >
> > On Sat, Jan 04, 2025 at 01:51:25PM +0100, Ariel Otilibili-Anieli wrote:
> > > Great, then; thanks for having acked the patch as such.
> >
> > I just noticed that Shiji Yang had posted a series of patches for
> > OpenWrt which also addresses the same issue, however, instead of
> > removing the augmented assignment, it fixes it to the supposedly
> > originally intended way.
> >
> > See
> > https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=package/kernel/mac80211/patches/rt2x00/621-04-rt2x00-fix-register-operation-on-RXIQ-calibration.patch;h=aa6f9c437c6447831490588b2cead6919accda58;hb=5d583901657bdfbbf9fad77d9247872427aa5c99
> >
> > I suppose this was tested together with the other changes of the same
> > series, so we may want to pick that instead.
>
> Thanks for having put some time into the research, Daniel; I looked into the openwrt archives for 2024, none of Shiji’s messages mentions that patch.

You didn't find anything because these changes came in via a PR on
github: https://github.com/openwrt/openwrt/pull/16845 :) OpenWrt
accepts contributions both via email and PR on github.

Best Regards,
Jonas

Re: [1/2] wifi: rt2x00: Remove unused rfval values

[Kalle Valo]

Ariel Otilibili <ariel.otilibili-anieli@eurecom.fr> wrote:

> The intention here is not clear but as this was already tested and matches
> vendor driver it's better not to change behavior even if it looks suspicious.
> So just remove the unused values.
> 
> Coverity-ID: 1525307
> 
> Signed-off-by: Ariel Otilibili <ariel.otilibili-anieli@eurecom.fr>
> Acked-by: Stanislaw Gruszka <stf_xl@wp.pl>
> [kvalo@kernel.org: write commit message]

Patch applied to wireless-next.git, thanks.

280c8b39050b wifi: rt2x00: Remove unused rfval values

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20241221124445.1094460-2-ariel.otilibili-anieli@eurecom.fr/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH 2/2] net/phy,neterion: Remove dead values

[Ariel Otilibili]

Coverity-ID: 1269173, 1575053
Signed-off-by: Ariel Otilibili <ariel.otilibili-anieli@eurecom.fr>
---
 drivers/net/ethernet/neterion/s2io.c     | 2 --
 drivers/net/phy/nxp-c45-tja11xx-macsec.c | 1 -
 2 files changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/neterion/s2io.c b/drivers/net/ethernet/neterion/s2io.c
index f8016dc25e0a..4f89f9fd8043 100644
--- a/drivers/net/ethernet/neterion/s2io.c
+++ b/drivers/net/ethernet/neterion/s2io.c
@@ -1969,8 +1969,6 @@ static void en_dis_err_alarms(struct s2io_nic *nic, u16 mask, int flag)
 				   MC_ERR_REG_ECC_ALL_DBL | PLL_LOCK_N, flag,
 				   &bar0->mc_err_mask);
 	}
-	nic->general_int_mask = gen_int_mask;
-
 	/* Remove this line when alarm interrupts are enabled */
 	nic->general_int_mask = 0;
 }
diff --git a/drivers/net/phy/nxp-c45-tja11xx-macsec.c b/drivers/net/phy/nxp-c45-tja11xx-macsec.c
index 550ef08970f4..e15ab9ba2f50 100644
--- a/drivers/net/phy/nxp-c45-tja11xx-macsec.c
+++ b/drivers/net/phy/nxp-c45-tja11xx-macsec.c
@@ -818,7 +818,6 @@ static void nxp_c45_rx_sc_update(struct phy_device *phydev,
 	u32 cfg = 0;
 
 	nxp_c45_macsec_read(phydev, MACSEC_RXSC_CFG, &cfg);
-	cfg &= ~MACSEC_RXSC_CFG_VF_MASK;
 	cfg = phy_secy->secy->validate_frames << MACSEC_RXSC_CFG_VF_OFF;
 
 	phydev_dbg(phydev, "validate frames %u\n",
-- 
2.47.1

Re: [PATCH 2/2] net/phy,neterion: Remove dead values

[Andrew Lunn]

On Sat, Dec 21, 2024 at 01:39:33PM +0100, Ariel Otilibili wrote:
> Coverity-ID: 1269173, 1575053
> Signed-off-by: Ariel Otilibili <ariel.otilibili-anieli@eurecom.fr>
> ---
>  drivers/net/ethernet/neterion/s2io.c     | 2 --
>  drivers/net/phy/nxp-c45-tja11xx-macsec.c | 1 -

Please split this into a patch per driver.

>  2 files changed, 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/neterion/s2io.c b/drivers/net/ethernet/neterion/s2io.c
> index f8016dc25e0a..4f89f9fd8043 100644
> --- a/drivers/net/ethernet/neterion/s2io.c
> +++ b/drivers/net/ethernet/neterion/s2io.c
> @@ -1969,8 +1969,6 @@ static void en_dis_err_alarms(struct s2io_nic *nic, u16 mask, int flag)
>  				   MC_ERR_REG_ECC_ALL_DBL | PLL_LOCK_N, flag,
>  				   &bar0->mc_err_mask);
>  	}
> -	nic->general_int_mask = gen_int_mask;
> -
>  	/* Remove this line when alarm interrupts are enabled */
>  	nic->general_int_mask = 0;

So this change looks reasonable, but you need to update the comment,
because it is now wrong.

>  }
> diff --git a/drivers/net/phy/nxp-c45-tja11xx-macsec.c b/drivers/net/phy/nxp-c45-tja11xx-macsec.c
> index 550ef08970f4..e15ab9ba2f50 100644
> --- a/drivers/net/phy/nxp-c45-tja11xx-macsec.c
> +++ b/drivers/net/phy/nxp-c45-tja11xx-macsec.c
> @@ -818,7 +818,6 @@ static void nxp_c45_rx_sc_update(struct phy_device *phydev,
>  	u32 cfg = 0;
>  
>  	nxp_c45_macsec_read(phydev, MACSEC_RXSC_CFG, &cfg);
> -	cfg &= ~MACSEC_RXSC_CFG_VF_MASK;
>  	cfg = phy_secy->secy->validate_frames << MACSEC_RXSC_CFG_VF_OFF;

I can see why a static analysis tool would point this out, but you
need to include some justification why your change is correct, and it
is not a bug, maybe cfg should be written back after being masked? Or
the assignment to cfg should actually be an |= not = ?

	Andrew