| Message ID | 20170620212234.GA10957@embeddedgus |
|---|---|
| State | Accepted |
| Delegated to: | Jeff Kirsher |
| Headers | show |
Gustavo, The return value of ret_val seems used to check if the access to PHY/NVM got its semaphore, generally speaking, it is needed for every PHY access of this driver. Reviewed-by: Ethan Zhao <ethan.zhao@oracle.com> On Wed, Jun 21, 2017 at 5:22 AM, Gustavo A. R. Silva <garsilva@embeddedor.com> wrote: > Check return value from call to e1e_wphy(). This value is being > checked during previous calls to function e1e_wphy() and it seems > a check was missing here. > > Addresses-Coverity-ID: 1226905 > Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com> > --- > drivers/net/ethernet/intel/e1000e/ich8lan.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c > index 68ea8b4..d6d4ed7 100644 > --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c > +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c > @@ -2437,6 +2437,8 @@ static s32 e1000_hv_phy_workarounds_ich8lan(struct e1000_hw *hw) > if (hw->phy.revision < 2) { > e1000e_phy_sw_reset(hw); > ret_val = e1e_wphy(hw, MII_BMCR, 0x3140); > + if (ret_val) > + return ret_val; > } > } > > -- > 2.5.0 >
Hi Ethan, Quoting Ethan Zhao <ethan.kernel@gmail.com>: > Gustavo, > > The return value of ret_val seems used to check if the access to PHY/NVM > got its semaphore, generally speaking, it is needed for every PHY > access of this driver. > > Reviewed-by: Ethan Zhao <ethan.zhao@oracle.com> > Thank you very much for the clarification. > On Wed, Jun 21, 2017 at 5:22 AM, Gustavo A. R. Silva > <garsilva@embeddedor.com> wrote: >> Check return value from call to e1e_wphy(). This value is being >> checked during previous calls to function e1e_wphy() and it seems >> a check was missing here. >> >> Addresses-Coverity-ID: 1226905 >> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com> >> --- >> drivers/net/ethernet/intel/e1000e/ich8lan.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c >> b/drivers/net/ethernet/intel/e1000e/ich8lan.c >> index 68ea8b4..d6d4ed7 100644 >> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c >> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c >> @@ -2437,6 +2437,8 @@ static s32 >> e1000_hv_phy_workarounds_ich8lan(struct e1000_hw *hw) >> if (hw->phy.revision < 2) { >> e1000e_phy_sw_reset(hw); >> ret_val = e1e_wphy(hw, MII_BMCR, 0x3140); >> + if (ret_val) >> + return ret_val; >> } >> } >> >> -- >> 2.5.0 >> -- Gustavo A. R. Silva
On 21/06/2017 22:52, Gustavo A. R. Silva wrote: > Hi Ethan, > > Quoting Ethan Zhao <ethan.kernel@gmail.com>: > >> Gustavo, >> >> The return value of ret_val seems used to check if the access to >> PHY/NVM >> got its semaphore, generally speaking, it is needed for every PHY >> access of this driver. >> >> Reviewed-by: Ethan Zhao <ethan.zhao@oracle.com> >> > > Thank you very much for the clarification. > >> On Wed, Jun 21, 2017 at 5:22 AM, Gustavo A. R. Silva >> <garsilva@embeddedor.com> wrote: >>> Check return value from call to e1e_wphy(). This value is being >>> checked during previous calls to function e1e_wphy() and it seems >>> a check was missing here. >>> >>> Addresses-Coverity-ID: 1226905 >>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com> >>> --- >>> drivers/net/ethernet/intel/e1000e/ich8lan.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c >>> b/drivers/net/ethernet/intel/e1000e/ich8lan.c >>> index 68ea8b4..d6d4ed7 100644 >>> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c >>> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c >>> @@ -2437,6 +2437,8 @@ static s32 >>> e1000_hv_phy_workarounds_ich8lan(struct e1000_hw *hw) >>> if (hw->phy.revision < 2) { >>> e1000e_phy_sw_reset(hw); >>> ret_val = e1e_wphy(hw, MII_BMCR, 0x3140); >>> + if (ret_val) >>> + return ret_val; >>> } >>> } >>> >>> -- >>> 2.5.0 >>> > > -- > Gustavo A. R. Silva > > > > > > _______________________________________________ > Intel-wired-lan mailing list > Intel-wired-lan@osuosl.org > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan We will accept this patch.
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf > Of Gustavo A. R. Silva > Sent: Tuesday, June 20, 2017 2:23 PM > To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com> > Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; linux- > kernel@vger.kernel.org; Gustavo A. R. Silva <garsilva@embeddedor.com> > Subject: [Intel-wired-lan] [PATCH] net: intel: e1000e: add check on > e1e_wphy() return value > > Check return value from call to e1e_wphy(). This value is being > checked during previous calls to function e1e_wphy() and it seems > a check was missing here. > > Addresses-Coverity-ID: 1226905 > Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com> > --- > drivers/net/ethernet/intel/e1000e/ich8lan.c | 2 ++ > 1 file changed, 2 insertions(+) Tested-by: Aaron Brown <aaron.f.brown@intel.com>
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c index 68ea8b4..d6d4ed7 100644 --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c @@ -2437,6 +2437,8 @@ static s32 e1000_hv_phy_workarounds_ich8lan(struct e1000_hw *hw) if (hw->phy.revision < 2) { e1000e_phy_sw_reset(hw); ret_val = e1e_wphy(hw, MII_BMCR, 0x3140); + if (ret_val) + return ret_val; } }
Check return value from call to e1e_wphy(). This value is being checked during previous calls to function e1e_wphy() and it seems a check was missing here. Addresses-Coverity-ID: 1226905 Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com> --- drivers/net/ethernet/intel/e1000e/ich8lan.c | 2 ++ 1 file changed, 2 insertions(+)