diff mbox series

[net-next,7/7] net: sfp: get rid of DM7052 hack when enabling high power

Message ID E1ol98G-00EDT1-Q6@rmk-PC.armlinux.org.uk (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: sfp: improve high power module implementation | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 37 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Russell King (Oracle) Oct. 19, 2022, 1:29 p.m. UTC
Since we no longer mis-detect high-power mode with the DM7052 module,
we no longer need the hack in sfp_module_enable_high_power(), and can
now switch this to use sfp_modify_u8().

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/sfp.c | 29 ++++++-----------------------
 1 file changed, 6 insertions(+), 23 deletions(-)

Comments

Andrew Lunn Oct. 21, 2022, 3:52 p.m. UTC | #1
On Wed, Oct 19, 2022 at 02:29:16PM +0100, Russell King (Oracle) wrote:
> Since we no longer mis-detect high-power mode with the DM7052 module,
> we no longer need the hack in sfp_module_enable_high_power(), and can
> now switch this to use sfp_modify_u8().
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Jakub Kicinski Oct. 21, 2022, 4:16 p.m. UTC | #2
On Fri, 21 Oct 2022 17:52:52 +0200 Andrew Lunn wrote:
> On Wed, Oct 19, 2022 at 02:29:16PM +0100, Russell King (Oracle) wrote:
> > Since we no longer mis-detect high-power mode with the DM7052 module,
> > we no longer need the hack in sfp_module_enable_high_power(), and can
> > now switch this to use sfp_modify_u8().
> > 
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>  
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

FWIW there is a v2 of this, somewhat mis-subjected (pw-bot's
auto-Superseding logic missed it for example):
https://lore.kernel.org/all/E1oltef-00Fwwz-3t@rmk-PC.armlinux.org.uk/
Russell King (Oracle) Oct. 21, 2022, 7:14 p.m. UTC | #3
On Fri, Oct 21, 2022 at 09:16:18AM -0700, Jakub Kicinski wrote:
> On Fri, 21 Oct 2022 17:52:52 +0200 Andrew Lunn wrote:
> > On Wed, Oct 19, 2022 at 02:29:16PM +0100, Russell King (Oracle) wrote:
> > > Since we no longer mis-detect high-power mode with the DM7052 module,
> > > we no longer need the hack in sfp_module_enable_high_power(), and can
> > > now switch this to use sfp_modify_u8().
> > > 
> > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>  
> > 
> > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
> FWIW there is a v2 of this, somewhat mis-subjected (pw-bot's
> auto-Superseding logic missed it for example):
> https://lore.kernel.org/all/E1oltef-00Fwwz-3t@rmk-PC.armlinux.org.uk/

Yes, sadly I forgot to update the subject line prefix. Getting
everything correct every time is quite difficult.
diff mbox series

Patch

diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 921bbedd9b22..39fd1811375c 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -1837,31 +1837,14 @@  static int sfp_module_parse_power(struct sfp *sfp)
 
 static int sfp_sm_mod_hpower(struct sfp *sfp, bool enable)
 {
-	u8 val;
 	int err;
 
-	err = sfp_read(sfp, true, SFP_EXT_STATUS, &val, sizeof(val));
-	if (err != sizeof(val)) {
-		dev_err(sfp->dev, "Failed to read EEPROM: %pe\n", ERR_PTR(err));
-		return -EAGAIN;
-	}
-
-	/* DM7052 reports as a high power module, responds to reads (with
-	 * all bytes 0xff) at 0x51 but does not accept writes.  In any case,
-	 * if the bit is already set, we're already in high power mode.
-	 */
-	if (!!(val & SFP_EXT_STATUS_PWRLVL_SELECT) == enable)
-		return 0;
-
-	if (enable)
-		val |= SFP_EXT_STATUS_PWRLVL_SELECT;
-	else
-		val &= ~SFP_EXT_STATUS_PWRLVL_SELECT;
-
-	err = sfp_write(sfp, true, SFP_EXT_STATUS, &val, sizeof(val));
-	if (err != sizeof(val)) {
-		dev_err(sfp->dev, "Failed to write EEPROM: %pe\n",
-			ERR_PTR(err));
+	err = sfp_modify_u8(sfp, true, SFP_EXT_STATUS,
+			    SFP_EXT_STATUS_PWRLVL_SELECT,
+			    enable ? SFP_EXT_STATUS_PWRLVL_SELECT : 0);
+	if (err != sizeof(u8)) {
+		dev_err(sfp->dev, "failed to %sable high power: %pe\n",
+			enable ? "en" : "dis", ERR_PTR(err));
 		return -EAGAIN;
 	}