* [PATCH net-next v2] drivers/net/ethernet: check return value of e1e_rphy() @ 2022-08-26 6:56 ` Li Zhong 0 siblings, 0 replies; 7+ messages in thread From: Li Zhong @ 2022-08-26 6:56 UTC (permalink / raw) To: netdev, linux-kernel, intel-wired-lan Cc: jesse.brandeburg, anthony.l.nguyen, davem, edumazet, kuba, pabeni, Li Zhong e1e_rphy() could return error value, which needs to be checked and reported for debugging and diagnose. Signed-off-by: Li Zhong <floridsleeves@gmail.com> --- drivers/net/ethernet/intel/e1000e/phy.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/intel/e1000e/phy.c b/drivers/net/ethernet/intel/e1000e/phy.c index fd07c3679bb1..060b263348ce 100644 --- a/drivers/net/ethernet/intel/e1000e/phy.c +++ b/drivers/net/ethernet/intel/e1000e/phy.c @@ -2697,9 +2697,14 @@ static s32 e1000_access_phy_wakeup_reg_bm(struct e1000_hw *hw, u32 offset, void e1000_power_up_phy_copper(struct e1000_hw *hw) { u16 mii_reg = 0; + int ret; /* The PHY will retain its settings across a power down/up cycle */ - e1e_rphy(hw, MII_BMCR, &mii_reg); + ret = e1e_rphy(hw, MII_BMCR, &mii_reg); + if (ret) { + e_dbg("Error reading PHY register\n"); + return; + } mii_reg &= ~BMCR_PDOWN; e1e_wphy(hw, MII_BMCR, mii_reg); } @@ -2715,9 +2720,14 @@ void e1000_power_up_phy_copper(struct e1000_hw *hw) void e1000_power_down_phy_copper(struct e1000_hw *hw) { u16 mii_reg = 0; + int ret; /* The PHY will retain its settings across a power down/up cycle */ - e1e_rphy(hw, MII_BMCR, &mii_reg); + ret = e1e_rphy(hw, MII_BMCR, &mii_reg); + if (ret) { + e_dbg("Error reading PHY register\n"); + return; + } mii_reg |= BMCR_PDOWN; e1e_wphy(hw, MII_BMCR, mii_reg); usleep_range(1000, 2000); @@ -3037,7 +3047,11 @@ s32 e1000_link_stall_workaround_hv(struct e1000_hw *hw) return 0; /* Do not apply workaround if in PHY loopback bit 14 set */ - e1e_rphy(hw, MII_BMCR, &data); + ret_val = e1e_rphy(hw, MII_BMCR, &data); + if (ret_val) { + e_dbg("Error reading PHY register\n"); + return ret_val; + } if (data & BMCR_LOOPBACK) return 0; -- 2.25.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Intel-wired-lan] [PATCH net-next v2] drivers/net/ethernet: check return value of e1e_rphy() @ 2022-08-26 6:56 ` Li Zhong 0 siblings, 0 replies; 7+ messages in thread From: Li Zhong @ 2022-08-26 6:56 UTC (permalink / raw) To: netdev, linux-kernel, intel-wired-lan Cc: edumazet, Li Zhong, kuba, pabeni, davem e1e_rphy() could return error value, which needs to be checked and reported for debugging and diagnose. Signed-off-by: Li Zhong <floridsleeves@gmail.com> --- drivers/net/ethernet/intel/e1000e/phy.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/intel/e1000e/phy.c b/drivers/net/ethernet/intel/e1000e/phy.c index fd07c3679bb1..060b263348ce 100644 --- a/drivers/net/ethernet/intel/e1000e/phy.c +++ b/drivers/net/ethernet/intel/e1000e/phy.c @@ -2697,9 +2697,14 @@ static s32 e1000_access_phy_wakeup_reg_bm(struct e1000_hw *hw, u32 offset, void e1000_power_up_phy_copper(struct e1000_hw *hw) { u16 mii_reg = 0; + int ret; /* The PHY will retain its settings across a power down/up cycle */ - e1e_rphy(hw, MII_BMCR, &mii_reg); + ret = e1e_rphy(hw, MII_BMCR, &mii_reg); + if (ret) { + e_dbg("Error reading PHY register\n"); + return; + } mii_reg &= ~BMCR_PDOWN; e1e_wphy(hw, MII_BMCR, mii_reg); } @@ -2715,9 +2720,14 @@ void e1000_power_up_phy_copper(struct e1000_hw *hw) void e1000_power_down_phy_copper(struct e1000_hw *hw) { u16 mii_reg = 0; + int ret; /* The PHY will retain its settings across a power down/up cycle */ - e1e_rphy(hw, MII_BMCR, &mii_reg); + ret = e1e_rphy(hw, MII_BMCR, &mii_reg); + if (ret) { + e_dbg("Error reading PHY register\n"); + return; + } mii_reg |= BMCR_PDOWN; e1e_wphy(hw, MII_BMCR, mii_reg); usleep_range(1000, 2000); @@ -3037,7 +3047,11 @@ s32 e1000_link_stall_workaround_hv(struct e1000_hw *hw) return 0; /* Do not apply workaround if in PHY loopback bit 14 set */ - e1e_rphy(hw, MII_BMCR, &data); + ret_val = e1e_rphy(hw, MII_BMCR, &data); + if (ret_val) { + e_dbg("Error reading PHY register\n"); + return ret_val; + } if (data & BMCR_LOOPBACK) return 0; -- 2.25.1 _______________________________________________ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v2] drivers/net/ethernet: check return value of e1e_rphy() 2022-08-26 6:56 ` [Intel-wired-lan] " Li Zhong (?) @ 2022-08-29 18:03 ` Neftin, Sasha 2022-08-30 7:05 ` Li Zhong -1 siblings, 1 reply; 7+ messages in thread From: Neftin, Sasha @ 2022-08-29 18:03 UTC (permalink / raw) To: Li Zhong, netdev, linux-kernel, intel-wired-lan Cc: Fuxbrumer, Devora, edumazet, Ruinskiy, Dima, kuba, pabeni, davem On 8/26/2022 09:56, Li Zhong wrote: > e1e_rphy() could return error value, which needs to be checked and > reported for debugging and diagnose. > > Signed-off-by: Li Zhong <floridsleeves@gmail.com> > --- > drivers/net/ethernet/intel/e1000e/phy.c | 20 +++++++++++++++++--- > 1 file changed, 17 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/intel/e1000e/phy.c b/drivers/net/ethernet/intel/e1000e/phy.c > index fd07c3679bb1..060b263348ce 100644 > --- a/drivers/net/ethernet/intel/e1000e/phy.c > +++ b/drivers/net/ethernet/intel/e1000e/phy.c > @@ -2697,9 +2697,14 @@ static s32 e1000_access_phy_wakeup_reg_bm(struct e1000_hw *hw, u32 offset, > void e1000_power_up_phy_copper(struct e1000_hw *hw) > { > u16 mii_reg = 0; > + int ret; > > /* The PHY will retain its settings across a power down/up cycle */ > - e1e_rphy(hw, MII_BMCR, &mii_reg); > + ret = e1e_rphy(hw, MII_BMCR, &mii_reg); > + if (ret) { > + e_dbg("Error reading PHY register\n"); > + return; > + } > mii_reg &= ~BMCR_PDOWN; > e1e_wphy(hw, MII_BMCR, mii_reg); > } > @@ -2715,9 +2720,14 @@ void e1000_power_up_phy_copper(struct e1000_hw *hw) > void e1000_power_down_phy_copper(struct e1000_hw *hw) > { > u16 mii_reg = 0; > + int ret; > > /* The PHY will retain its settings across a power down/up cycle */ > - e1e_rphy(hw, MII_BMCR, &mii_reg); > + ret = e1e_rphy(hw, MII_BMCR, &mii_reg); > + if (ret) { > + e_dbg("Error reading PHY register\n"); > + return; > + } > mii_reg |= BMCR_PDOWN; > e1e_wphy(hw, MII_BMCR, mii_reg); > usleep_range(1000, 2000); > @@ -3037,7 +3047,11 @@ s32 e1000_link_stall_workaround_hv(struct e1000_hw *hw) > return 0; > > /* Do not apply workaround if in PHY loopback bit 14 set */ > - e1e_rphy(hw, MII_BMCR, &data); > + ret_val = e1e_rphy(hw, MII_BMCR, &data); > + if (ret_val) { > + e_dbg("Error reading PHY register\n"); > + return ret_val; > + } > if (data & BMCR_LOOPBACK) > return 0; > Generally, I am good with this patch. One question - it is old HW, any idea how to check it? (82577/82578 GbE LOM - from 2008) Li, how did you check it manually? _______________________________________________ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v2] drivers/net/ethernet: check return value of e1e_rphy() 2022-08-29 18:03 ` Neftin, Sasha @ 2022-08-30 7:05 ` Li Zhong 0 siblings, 0 replies; 7+ messages in thread From: Li Zhong @ 2022-08-30 7:05 UTC (permalink / raw) To: Neftin, Sasha Cc: netdev, linux-kernel, intel-wired-lan, edumazet, kuba, pabeni, davem, Nguyen, Anthony L, Ruinskiy, Dima, Fuxbrumer, Devora, naamax.meir, Brandeburg, Jesse On Mon, Aug 29, 2022 at 11:03 AM Neftin, Sasha <sasha.neftin@intel.com> wrote: > > On 8/26/2022 09:56, Li Zhong wrote: > > e1e_rphy() could return error value, which needs to be checked and > > reported for debugging and diagnose. > > > > Signed-off-by: Li Zhong <floridsleeves@gmail.com> > > --- > > drivers/net/ethernet/intel/e1000e/phy.c | 20 +++++++++++++++++--- > > 1 file changed, 17 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/e1000e/phy.c b/drivers/net/ethernet/intel/e1000e/phy.c > > index fd07c3679bb1..060b263348ce 100644 > > --- a/drivers/net/ethernet/intel/e1000e/phy.c > > +++ b/drivers/net/ethernet/intel/e1000e/phy.c > > @@ -2697,9 +2697,14 @@ static s32 e1000_access_phy_wakeup_reg_bm(struct e1000_hw *hw, u32 offset, > > void e1000_power_up_phy_copper(struct e1000_hw *hw) > > { > > u16 mii_reg = 0; > > + int ret; > > > > /* The PHY will retain its settings across a power down/up cycle */ > > - e1e_rphy(hw, MII_BMCR, &mii_reg); > > + ret = e1e_rphy(hw, MII_BMCR, &mii_reg); > > + if (ret) { > > + e_dbg("Error reading PHY register\n"); > > + return; > > + } > > mii_reg &= ~BMCR_PDOWN; > > e1e_wphy(hw, MII_BMCR, mii_reg); > > } > > @@ -2715,9 +2720,14 @@ void e1000_power_up_phy_copper(struct e1000_hw *hw) > > void e1000_power_down_phy_copper(struct e1000_hw *hw) > > { > > u16 mii_reg = 0; > > + int ret; > > > > /* The PHY will retain its settings across a power down/up cycle */ > > - e1e_rphy(hw, MII_BMCR, &mii_reg); > > + ret = e1e_rphy(hw, MII_BMCR, &mii_reg); > > + if (ret) { > > + e_dbg("Error reading PHY register\n"); > > + return; > > + } > > mii_reg |= BMCR_PDOWN; > > e1e_wphy(hw, MII_BMCR, mii_reg); > > usleep_range(1000, 2000); > > @@ -3037,7 +3047,11 @@ s32 e1000_link_stall_workaround_hv(struct e1000_hw *hw) > > return 0; > > > > /* Do not apply workaround if in PHY loopback bit 14 set */ > > - e1e_rphy(hw, MII_BMCR, &data); > > + ret_val = e1e_rphy(hw, MII_BMCR, &data); > > + if (ret_val) { > > + e_dbg("Error reading PHY register\n"); > > + return ret_val; > > + } > > if (data & BMCR_LOOPBACK) > > return 0; > > > Generally, I am good with this patch. One question - it is old HW, any > idea how to check it? (82577/82578 GbE LOM - from 2008) > Li, how did you check it manually? These bugs are detected by static analysis. Therefore it's not run-time checked. However, since currently there is no error handling after e1e_rphy(), so I think it's necessary to at least check it and expect there is a chance to fail. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v2] drivers/net/ethernet: check return value of e1e_rphy() @ 2022-08-30 7:05 ` Li Zhong 0 siblings, 0 replies; 7+ messages in thread From: Li Zhong @ 2022-08-30 7:05 UTC (permalink / raw) To: Neftin, Sasha Cc: Fuxbrumer, Devora, Ruinskiy, Dima, netdev, linux-kernel, edumazet, intel-wired-lan, kuba, pabeni, davem On Mon, Aug 29, 2022 at 11:03 AM Neftin, Sasha <sasha.neftin@intel.com> wrote: > > On 8/26/2022 09:56, Li Zhong wrote: > > e1e_rphy() could return error value, which needs to be checked and > > reported for debugging and diagnose. > > > > Signed-off-by: Li Zhong <floridsleeves@gmail.com> > > --- > > drivers/net/ethernet/intel/e1000e/phy.c | 20 +++++++++++++++++--- > > 1 file changed, 17 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/e1000e/phy.c b/drivers/net/ethernet/intel/e1000e/phy.c > > index fd07c3679bb1..060b263348ce 100644 > > --- a/drivers/net/ethernet/intel/e1000e/phy.c > > +++ b/drivers/net/ethernet/intel/e1000e/phy.c > > @@ -2697,9 +2697,14 @@ static s32 e1000_access_phy_wakeup_reg_bm(struct e1000_hw *hw, u32 offset, > > void e1000_power_up_phy_copper(struct e1000_hw *hw) > > { > > u16 mii_reg = 0; > > + int ret; > > > > /* The PHY will retain its settings across a power down/up cycle */ > > - e1e_rphy(hw, MII_BMCR, &mii_reg); > > + ret = e1e_rphy(hw, MII_BMCR, &mii_reg); > > + if (ret) { > > + e_dbg("Error reading PHY register\n"); > > + return; > > + } > > mii_reg &= ~BMCR_PDOWN; > > e1e_wphy(hw, MII_BMCR, mii_reg); > > } > > @@ -2715,9 +2720,14 @@ void e1000_power_up_phy_copper(struct e1000_hw *hw) > > void e1000_power_down_phy_copper(struct e1000_hw *hw) > > { > > u16 mii_reg = 0; > > + int ret; > > > > /* The PHY will retain its settings across a power down/up cycle */ > > - e1e_rphy(hw, MII_BMCR, &mii_reg); > > + ret = e1e_rphy(hw, MII_BMCR, &mii_reg); > > + if (ret) { > > + e_dbg("Error reading PHY register\n"); > > + return; > > + } > > mii_reg |= BMCR_PDOWN; > > e1e_wphy(hw, MII_BMCR, mii_reg); > > usleep_range(1000, 2000); > > @@ -3037,7 +3047,11 @@ s32 e1000_link_stall_workaround_hv(struct e1000_hw *hw) > > return 0; > > > > /* Do not apply workaround if in PHY loopback bit 14 set */ > > - e1e_rphy(hw, MII_BMCR, &data); > > + ret_val = e1e_rphy(hw, MII_BMCR, &data); > > + if (ret_val) { > > + e_dbg("Error reading PHY register\n"); > > + return ret_val; > > + } > > if (data & BMCR_LOOPBACK) > > return 0; > > > Generally, I am good with this patch. One question - it is old HW, any > idea how to check it? (82577/82578 GbE LOM - from 2008) > Li, how did you check it manually? These bugs are detected by static analysis. Therefore it's not run-time checked. However, since currently there is no error handling after e1e_rphy(), so I think it's necessary to at least check it and expect there is a chance to fail. _______________________________________________ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2] drivers/net/ethernet: check return value of e1e_rphy() 2022-08-26 6:56 ` [Intel-wired-lan] " Li Zhong @ 2022-08-29 22:07 ` Tony Nguyen -1 siblings, 0 replies; 7+ messages in thread From: Tony Nguyen @ 2022-08-29 22:07 UTC (permalink / raw) To: Li Zhong, netdev, linux-kernel, intel-wired-lan Cc: jesse.brandeburg, davem, edumazet, kuba, pabeni On 8/25/2022 11:56 PM, Li Zhong wrote: nit: Could you mention the driver name in the commit title e.g. e1000e: check return value of e1e_rphy() Thanks, Tony > e1e_rphy() could return error value, which needs to be checked and > reported for debugging and diagnose. > > Signed-off-by: Li Zhong <floridsleeves@gmail.com> > --- > drivers/net/ethernet/intel/e1000e/phy.c | 20 +++++++++++++++++--- > 1 file changed, 17 insertions(+), 3 deletions(-) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v2] drivers/net/ethernet: check return value of e1e_rphy() @ 2022-08-29 22:07 ` Tony Nguyen 0 siblings, 0 replies; 7+ messages in thread From: Tony Nguyen @ 2022-08-29 22:07 UTC (permalink / raw) To: Li Zhong, netdev, linux-kernel, intel-wired-lan Cc: kuba, edumazet, pabeni, davem On 8/25/2022 11:56 PM, Li Zhong wrote: nit: Could you mention the driver name in the commit title e.g. e1000e: check return value of e1e_rphy() Thanks, Tony > e1e_rphy() could return error value, which needs to be checked and > reported for debugging and diagnose. > > Signed-off-by: Li Zhong <floridsleeves@gmail.com> > --- > drivers/net/ethernet/intel/e1000e/phy.c | 20 +++++++++++++++++--- > 1 file changed, 17 insertions(+), 3 deletions(-) _______________________________________________ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-08-30 12:54 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-26 6:56 [PATCH net-next v2] drivers/net/ethernet: check return value of e1e_rphy() Li Zhong 2022-08-26 6:56 ` [Intel-wired-lan] " Li Zhong 2022-08-29 18:03 ` Neftin, Sasha 2022-08-30 7:05 ` Li Zhong 2022-08-30 7:05 ` Li Zhong 2022-08-29 22:07 ` Tony Nguyen 2022-08-29 22:07 ` [Intel-wired-lan] " Tony Nguyen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.