>-----Original Message----- >From: Shannon Nelson [mailto:shannon.nel...@oracle.com] >Sent: Thursday, February 01, 2018 4:44 PM >To: Tantilov, Emil S <emil.s.tanti...@intel.com> >Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org >Subject: Re: [Intel-wired-lan] Possible read-modify-write bug in ixgbe >x550 phy setup > >On 2/1/2018 4:34 PM, Tantilov, Emil S wrote: >>> -----Original Message----- >>> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On >>> Behalf Of Shannon Nelson >>> Sent: Thursday, February 01, 2018 3:46 PM >>> To: Tantilov, Emil S <emil.s.tanti...@intel.com> >>> Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org >>> Subject: [Intel-wired-lan] Possible read-modify-write bug in ixgbe >x550 >>> phy setup >>> >>> Hi Emil, >>> >>> I was looking through a set of ixgbe patches and came across this >commit >>> >>> commit 410a494902777c11f95031d9ed757d7f8f09c5c6 >>> ixgbe: add write flush when configuring CS4223/7 >>> >>> and am wondering about the setting of reg_phy_ext in the middle of >>> ixgbe_setup_mac_link_sfp_x550a(). It looks like it is read from the >>> PHY, modified to remove the CX1 and SR mode bits, but then those bits >>> are overwritten in the "if (setup_linear)" block immediately >following, >>> and that is what gets written back out. >> >> Hi Shannon, >> >> This is pretty standard clear before set, so you're right that it >would >> make more sense to have |= rather than =. >> >> Are you seeing an issue, or did you catch this via code inspection? > >Purely a code inspection, I saw this while looking at something else. > >sln
Thanks for letting us know. I will do some more testing with the actual HW before changing the code. Emil
