On 10/16/23 08:56, Loktionov, Aleksandr wrote:


-----Original Message-----
From: Kitszel, Przemyslaw <[email protected]>
Sent: Friday, October 13, 2023 2:35 PM
To: Loktionov, Aleksandr <[email protected]>; intel-wired-
[email protected]; Nguyen, Anthony L <[email protected]>;
Jagielski, Jedrzej <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH iwl-next v5] i40e: add restore default speed when changed
PHY doesn't support it

On 10/13/23 13:52, Aleksandr Loktionov wrote:
Currently, there was no link after plugging a different type of PHY
module if user forced previous PHY specific link type/speed before.

Add reset link speed settings to the default values for PHY module, if
different PHY module is inserted and currently defined user-specified
speed is not compatible with this module.

Co-developed-by: Radoslaw Tyl <[email protected]>
Signed-off-by: Radoslaw Tyl <[email protected]>
Reviewed-by: Jedrzej Jagielski <[email protected]>
Signed-off-by: Aleksandr Loktionov <[email protected]>
---
v1->v2 fixed Reviewed-by tags
v2->v3 fixed commit messages and tags
v3->v4 fixed commit message typo
v4->v5 cc to [email protected]

good move!,
now you have to focus on the rules more, like those:
do not post next version before 24h of prev one;

Ok

more metadata:
I would remove the word 'add' from the Subject line; You still need to change
I don't agree, because it's not a fix it's a new feature implementation => 
'add' feature.
If you have a better idea please suggest a full commit title which fits 72 
chars. What I have now it my best.


i40e: support restoring default speed when changed PHY does not


author to Radoslaw.

The patch was seriously modified, so we are co-authors.

sure, my rule of thumb then is to put bigger contributor as main author,
and the other/s as co-authors,
perhaps some people believe that chronologically first should be the main?

I always try to split patch into more
(each of them still being of value when standing alone ofc).

anyway, it's just a comment, not change-request :)

And I'd better leave my e-mail, which is still alive, on top for better 
community support.

doesn't matter, your address is there as a Submitter anyway :)


---
---
   drivers/net/ethernet/intel/i40e/i40e_main.c | 65 +++++++++++++++++++--
   1 file changed, 61 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index d0d0218..6829720 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -10076,6 +10076,55 @@ static void i40e_reset_subtask(struct i40e_pf
*pf)
        rtnl_unlock();
   }

+/**
+ * i40e_restore_supported_phy_link_speed - Restore default PHY speed
+ * @pf: board private structure
+ *
+ * Set PHY module speeds according to values got from
+ * initial link speed abilites.
+ **/
+static int i40e_restore_supported_phy_link_speed(struct i40e_pf *pf)
+{
+       struct i40e_aq_get_phy_abilities_resp abilities;
+       struct i40e_aq_set_phy_config config = {0};

just `= {};`

+       struct i40e_hw *hw = &pf->hw;
+       int err;
+
+       err = i40e_aq_get_phy_capabilities(hw, false, false, &abilities, NULL);
+       if (err) {
+               dev_dbg(&pf->pdev->dev, "failed to get phy cap., ret =  %i
last_status =  %s\n",
+                       err, i40e_aq_str(&pf->hw, pf->hw.aq.asq_last_status));
+               return err;
+       }
+       config.eee_capability = abilities.eee_capability;
+       config.phy_type_ext = abilities.phy_type_ext;
+       config.low_power_ctrl = abilities.d3_lpan;
+       config.abilities = abilities.abilities;
+       config.abilities |= I40E_AQ_PHY_ENABLE_AN;
+       config.phy_type = abilities.phy_type;
+       config.eeer = abilities.eeer_val;
+       config.fec_config = abilities.fec_cfg_curr_mod_ext_info &
+                           I40E_AQ_PHY_FEC_CONFIG_MASK;
+       err = i40e_aq_get_phy_capabilities(hw, false, true, &abilities, NULL);
+       if (err) {
+               dev_dbg(&pf->pdev->dev, "get supported phy types ret =  %i
+last_status =  %s\n",

s/  / /g

(in English: replace double spaces by single ones)

+                       err, i40e_aq_str(&pf->hw, pf->hw.aq.asq_last_status));
+               return err;
+       }
+       config.link_speed = abilities.link_speed;
+
+       err = i40e_aq_set_phy_config(hw, &config, NULL);
+       if (err)
+               return err;
+       err = i40e_aq_set_link_restart_an(hw, true, NULL);
+       if (err)
+               return err;
+
+       pf->hw.phy.link_info.requested_speeds = config.link_speed;
+
+       return err;
+}
+
   /**
    * i40e_handle_link_event - Handle link event
    * @pf: board private structure
@@ -10086,6 +10135,7 @@ static void i40e_handle_link_event(struct
i40e_pf *pf,
   {
        struct i40e_aqc_get_link_status *status =
                (struct i40e_aqc_get_link_status *)&e->desc.params.raw;
+       int err;

        /* Do a new status request to re-enable LSE reporting
         * and load new status information into the hw struct @@ -10109,10
+10159,17 @@ static void i40e_handle_link_event(struct i40e_pf *pf,
                    (!(status->an_info & I40E_AQ_QUALIFIED_MODULE)) &&
                    (!(status->link_info & I40E_AQ_LINK_UP)) &&
                    (!(pf->flags &
I40E_FLAG_LINK_DOWN_ON_CLOSE_ENABLED))) {
-                       dev_err(&pf->pdev->dev,
-                               "Rx/Tx is disabled on this device because an
unsupported SFP module type was detected.\n");
-                       dev_err(&pf->pdev->dev,
-                               "Refer to the Intel(R) Ethernet Adapters and
Devices User Guide for a list of supported modules.\n");
+                       err = i40e_restore_supported_phy_link_speed(pf);
+                       if (err) {
+                               dev_err(&pf->pdev->dev,
+                                       "Rx/Tx is disabled on this device
because an unsupported SFP module type was detected.\n");
+                               dev_err(&pf->pdev->dev,
+                                       "Refer to the Intel(R) Ethernet
Adapters and Devices User Guide
+for a list of supported modules.\n");
+
+                               return;
+                       }
+
+                       dev_info(&pf->pdev->dev, "The selected speed is
incompatible with
+the connected media type. Resetting to the default speed setting for
+the media type.");
                }
        }
   }


_______________________________________________
Intel-wired-lan mailing list
[email protected]
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Reply via email to