Hello, Mork!

Sorry for the late reply.
Thanks to you and Russell for your great contribution.

I will test Russell's patch in a few days.
Please wait a while.

2021年12月6日(月) 19:35 Bjørn Mork [off-list ref]:

"Russell King (Oracle)" [off-list ref] writes:

quoted

On Fri, Dec 03, 2021 at 02:55:17PM +0100, Bjørn Mork wrote:

quoted

"Russell King (Oracle)" [off-list ref] writes:

quoted

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.

Hello Teruyama!

Any chance you can test this proposed fix from Russel?  I believe it
should fix the issue with your NTT OCU SFP as well.


Bjørn

--
株式会社スプリングボード
照山　周一郎
teruyama@springboard-inc.jp
http://www.springboard-inc.jp/
〒110-0005
東京都台東区上野3丁目2番2号
アイオス秋葉原505号室