Re: [PATCH net,stable] phy: sfp: fix high power modules without diag mode

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Fri, Dec 03, 2021 at 02:55:17PM +0100, Bjørn Mork wrote:
> "Russell King (Oracle)" <linux@armlinux.org.uk> writes:
> 
> > Thinking a little more, how about this:
> >
> >  drivers/net/phy/sfp.c | 25 +++++++++++++++++++++----
> >  1 file changed, 21 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> > index 51a1da50c608..4c900d063b19 100644
> > --- a/drivers/net/phy/sfp.c
> > +++ b/drivers/net/phy/sfp.c
> > @@ -1752,17 +1752,20 @@ static int sfp_sm_probe_for_phy(struct sfp *sfp)
> >  static int sfp_module_parse_power(struct sfp *sfp)
> >  {
> >  	u32 power_mW = 1000;
> > +	bool supports_a2;
> >  
> >  	if (sfp->id.ext.options & cpu_to_be16(SFP_OPTIONS_POWER_DECL))
> >  		power_mW = 1500;
> >  	if (sfp->id.ext.options & cpu_to_be16(SFP_OPTIONS_HIGH_POWER_LEVEL))
> >  		power_mW = 2000;
> >  
> > +	supports_a2 = sfp->id.ext.sff8472_compliance !=
> > +				SFP_SFF8472_COMPLIANCE_NONE ||
> > +		      sfp->id.ext.diagmon & SFP_DIAGMON_DDM;
> > +
> >  	if (power_mW > sfp->max_power_mW) {
> >  		/* Module power specification exceeds the allowed maximum. */
> > -		if (sfp->id.ext.sff8472_compliance ==
> > -			SFP_SFF8472_COMPLIANCE_NONE &&
> > -		    !(sfp->id.ext.diagmon & SFP_DIAGMON_DDM)) {
> > +		if (!supports_a2) {
> >  			/* The module appears not to implement bus address
> >  			 * 0xa2, so assume that the module powers up in the
> >  			 * indicated mode.
> > @@ -1779,11 +1782,24 @@ static int sfp_module_parse_power(struct sfp *sfp)
> >  		}
> >  	}
> >  
> > +	if (power_mW <= 1000) {
> > +		/* Modules below 1W do not require a power change sequence */
> > +		return 0;
> > +	}
> > +
> > +	if (!supports_a2) {
> > +		/* The module power level is below the host maximum and the
> > +		 * module appears not to implement bus address 0xa2, so assume
> > +		 * that the module powers up in the indicated mode.
> > +		 */
> > +		return 0;
> > +	}
> > +
> >  	/* If the module requires a higher power mode, but also requires
> >  	 * an address change sequence, warn the user that the module may
> >  	 * not be functional.
> >  	 */
> > -	if (sfp->id.ext.diagmon & SFP_DIAGMON_ADDRMODE && power_mW > 1000) {
> > +	if (sfp->id.ext.diagmon & SFP_DIAGMON_ADDRMODE) {
> >  		dev_warn(sfp->dev,
> >  			 "Address Change Sequence not supported but module requires %u.%uW, module may not be functional\n",
> >  			 power_mW / 1000, (power_mW / 100) % 10);
> 
> Looks nice to me at least.  But I don't have the hardware to test it.

I don't have the hardware either, so I can't test it - but it does need
testing. I assume as you've reported it and sent a patch, you know
someone who has run into this issue? It would be great if you could ask
them to test it and let us know if it solves the problem.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!