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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240904161216.5zlwzf52za33wxoy@thinkpad>
Date: Wed, 4 Sep 2024 21:42:16 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Frank Li <Frank.li@....com>
Cc: Richard Zhu <hongxing.zhu@....com>,
	Lucas Stach <l.stach@...gutronix.de>,
	Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczyński <kw@...ux.com>,
	Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
	Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>,
	Jingoo Han <jingoohan1@...il.com>,
	Chuanhua Lei <lchuanhua@...linear.com>,
	Marek Vasut <marek.vasut+renesas@...il.com>,
	Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
	linux-pci@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	imx@...ts.linux.dev, linux-kernel@...r.kernel.org,
	linux-renesas-soc@...r.kernel.org, linux-arm-msm@...r.kernel.org,
	abel.vesa@...aro.org, johan+linaro@...nel.org,
	Shashank Babu Chinta Venkata <quic_schintav@...cinc.com>
Subject: Re: [PATCH v6 1/4] PCI: dwc: Rename 'dw_pcie::link_gen' to
 'dw_pcie::max_link_speed'

On Wed, Sep 04, 2024 at 11:58:58AM -0400, Frank Li wrote:
> On Wed, Sep 04, 2024 at 12:41:57PM +0530, Manivannan Sadhasivam wrote:
> > 'link_gen' field is now holding the maximum supported link speed set either
> > by the controller driver or by DT through 'max-link-speed' property.
> >
> > But the name 'link_gen' sounds like the negotiated link speed of the PCIe
> > link. So let's rename it to 'max_link_speed' to make it clear that it holds
> > the maximum supported link speed of the controller.
> >
> > NOTE: For the sake of clarity, I've used 'max_link_speed' instead of
> > 'max_link_gen'. Also the link speed and link generation values map 1:1.
> 
> Maybe a little confuse is about unit of max_link_speed. the word 'gen'
> (1, 2, 3...), we know it PCIe[1,2,3...].  But word "speed" look like should
> be some mHz.
> 

Ideally, the DT property should've used the definitions in pci.h, but it ended
up accepting the PCIe Gen version. Still, it is named as 'max-link-speed', so I
wanted to keept the same name for the variable.

- Mani

> Reviewed-by: Frank Li <Frank.Li@....com>
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c        |  8 ++++----
> >  drivers/pci/controller/dwc/pcie-designware.c | 12 ++++++------
> >  drivers/pci/controller/dwc/pcie-designware.h |  2 +-
> >  drivers/pci/controller/dwc/pcie-intel-gw.c   |  4 ++--
> >  drivers/pci/controller/dwc/pcie-rcar-gen4.c  |  6 +++---
> >  5 files changed, 16 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > index 964d67756eb2..ef12a4f31740 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -847,12 +847,12 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
> >  	if (ret)
> >  		goto err_reset_phy;
> >
> > -	if (pci->link_gen > 1) {
> > +	if (pci->max_link_speed > 1) {
> >  		/* Allow faster modes after the link is up */
> >  		dw_pcie_dbi_ro_wr_en(pci);
> >  		tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
> >  		tmp &= ~PCI_EXP_LNKCAP_SLS;
> > -		tmp |= pci->link_gen;
> > +		tmp |= pci->max_link_speed;
> >  		dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, tmp);
> >
> >  		/*
> > @@ -1386,8 +1386,8 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> >  		imx6_pcie->tx_swing_low = 127;
> >
> >  	/* Limit link speed */
> > -	pci->link_gen = 1;
> > -	of_property_read_u32(node, "fsl,max-link-speed", &pci->link_gen);
> > +	pci->max_link_speed = 1;
> > +	of_property_read_u32(node, "fsl,max-link-speed", &pci->max_link_speed);
> >
> >  	imx6_pcie->vpcie = devm_regulator_get_optional(&pdev->dev, "vpcie");
> >  	if (IS_ERR(imx6_pcie->vpcie)) {
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 1b5aba1f0c92..86c49ba097c6 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -166,8 +166,8 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
> >  			return ret;
> >  	}
> >
> > -	if (pci->link_gen < 1)
> > -		pci->link_gen = of_pci_get_max_link_speed(np);
> > +	if (pci->max_link_speed < 1)
> > +		pci->max_link_speed = of_pci_get_max_link_speed(np);
> >
> >  	of_property_read_u32(np, "num-lanes", &pci->num_lanes);
> >
> > @@ -687,7 +687,7 @@ void dw_pcie_upconfig_setup(struct dw_pcie *pci)
> >  }
> >  EXPORT_SYMBOL_GPL(dw_pcie_upconfig_setup);
> >
> > -static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen)
> > +static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 max_link_speed)
> >  {
> >  	u32 cap, ctrl2, link_speed;
> >  	u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> > @@ -696,7 +696,7 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen)
> >  	ctrl2 = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCTL2);
> >  	ctrl2 &= ~PCI_EXP_LNKCTL2_TLS;
> >
> > -	switch (pcie_link_speed[link_gen]) {
> > +	switch (pcie_link_speed[max_link_speed]) {
> >  	case PCIE_SPEED_2_5GT:
> >  		link_speed = PCI_EXP_LNKCTL2_TLS_2_5GT;
> >  		break;
> > @@ -1058,8 +1058,8 @@ void dw_pcie_setup(struct dw_pcie *pci)
> >  {
> >  	u32 val;
> >
> > -	if (pci->link_gen > 0)
> > -		dw_pcie_link_set_max_speed(pci, pci->link_gen);
> > +	if (pci->max_link_speed > 0)
> > +		dw_pcie_link_set_max_speed(pci, pci->max_link_speed);
> >
> >  	/* Configure Gen1 N_FTS */
> >  	if (pci->n_fts[0]) {
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index 53c4c8f399c8..22765564f301 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -421,7 +421,7 @@ struct dw_pcie {
> >  	u32			type;
> >  	unsigned long		caps;
> >  	int			num_lanes;
> > -	int			link_gen;
> > +	int			max_link_speed;
> >  	u8			n_fts[2];
> >  	struct dw_edma_chip	edma;
> >  	struct clk_bulk_data	app_clks[DW_PCIE_NUM_APP_CLKS];
> > diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
> > index acbe4f6d3291..676d2aba4fbd 100644
> > --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
> > +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> > @@ -132,7 +132,7 @@ static void intel_pcie_link_setup(struct intel_pcie *pcie)
> >
> >  static void intel_pcie_init_n_fts(struct dw_pcie *pci)
> >  {
> > -	switch (pci->link_gen) {
> > +	switch (pci->max_link_speed) {
> >  	case 3:
> >  		pci->n_fts[1] = PORT_AFR_N_FTS_GEN3;
> >  		break;
> > @@ -252,7 +252,7 @@ static int intel_pcie_wait_l2(struct intel_pcie *pcie)
> >  	int ret;
> >  	struct dw_pcie *pci = &pcie->pci;
> >
> > -	if (pci->link_gen < 3)
> > +	if (pci->max_link_speed < 3)
> >  		return 0;
> >
> >  	/* Send PME_TURN_OFF message */
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > index f0f3ebd1a033..00ad4832f2cf 100644
> > --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > @@ -141,10 +141,10 @@ static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
> >  	}
> >
> >  	/*
> > -	 * Require direct speed change with retrying here if the link_gen is
> > -	 * PCIe Gen2 or higher.
> > +	 * Require direct speed change with retrying here if the max_link_speed
> > +	 * is PCIe Gen2 or higher.
> >  	 */
> > -	changes = min_not_zero(dw->link_gen, RCAR_MAX_LINK_SPEED) - 1;
> > +	changes = min_not_zero(dw->max_link_speed, RCAR_MAX_LINK_SPEED) - 1;
> >
> >  	/*
> >  	 * Since dw_pcie_setup_rc() sets it once, PCIe Gen2 will be trained.
> >
> > --
> > 2.25.1
> >

-- 
மணிவண்ணன் சதாசிவம்

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ