lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <MW5PR11MB58598BE9AE5947A2D385ABA8E5D7A@MW5PR11MB5859.namprd11.prod.outlook.com>
Date: Mon, 16 Oct 2023 06:56:19 +0000
From: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>
To: "Kitszel, Przemyslaw" <przemyslaw.kitszel@...el.com>,
	"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"Nguyen, Anthony L" <anthony.l.nguyen@...el.com>, "Jagielski, Jedrzej"
	<jedrzej.jagielski@...el.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH iwl-next v5] i40e: add restore default speed when changed
 PHY doesn't support it



> -----Original Message-----
> From: Kitszel, Przemyslaw <przemyslaw.kitszel@...el.com>
> Sent: Friday, October 13, 2023 2:35 PM
> To: Loktionov, Aleksandr <aleksandr.loktionov@...el.com>; intel-wired-
> lan@...ts.osuosl.org; Nguyen, Anthony L <anthony.l.nguyen@...el.com>;
> Jagielski, Jedrzej <jedrzej.jagielski@...el.com>
> Cc: netdev@...r.kernel.org
> 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 <radoslawx.tyl@...el.com>
> > Signed-off-by: Radoslaw Tyl <radoslawx.tyl@...el.com>
> > Reviewed-by: Jedrzej Jagielski <jedrzej.jagielski@...el.com>
> > Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
> > ---
> > v1->v2 fixed Reviewed-by tags
> > v2->v3 fixed commit messages and tags
> > v3->v4 fixed commit message typo
> > v4->v5 cc to netdev@...r.kernel.org
> 
> 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.

> author to Radoslaw.
> 
The patch was seriously modified, so we are co-authors.
And I'd better leave my e-mail, which is still alive, on top for better community support.

> > ---
> > ---
> >   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.");
> >   		}
> >   	}
> >   }

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ