diff mbox series

[v20,07/19] PCI: dwc: Add missing PCI_EXP_LNKCAP_MLW handling

Message ID 20230825093219.2685912-8-yoshihiro.shimoda.uh@renesas.com (mailing list archive)
State Mainlined
Commit 89db0793c9f2da265ecb6c1681f899d9af157f37
Delegated to: Geert Uytterhoeven
Headers show
Series PCI: rcar-gen4: Add R-Car Gen4 PCIe support | expand

Commit Message

Yoshihiro Shimoda Aug. 25, 2023, 9:32 a.m. UTC
Update dw_pcie_link_set_max_link_width() to set PCI_EXP_LNKCAP_MLW.
In accordance with the DW PCIe RC/EP HW manuals [1,2,3,...] aside with
the PORT_LINK_CTRL_OFF.LINK_CAPABLE and GEN2_CTRL_OFF.NUM_OF_LANES[8:0]
field there is another one which needs to be updated. It's
LINK_CAPABILITIES_REG.PCIE_CAP_MAX_LINK_WIDTH. If it isn't done at
the very least the maximum link-width capability CSR won't expose
the actual maximum capability.

[1] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
    Version 4.60a, March 2015, p.1032
[2] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
    Version 4.70a, March 2016, p.1065
[3] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
    Version 4.90a, March 2016, p.1057
...
[X] DesignWare Cores PCI Express Controller Databook - DWC PCIe Endpoint,
      Version 5.40a, March 2019, p.1396
[X+1] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
      Version 5.40a, March 2019, p.1266

Suggested-by: Serge Semin <fancer.lancer@gmail.com>
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
---
 drivers/pci/controller/dwc/pcie-designware.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Bjorn Helgaas Sept. 14, 2023, 4 p.m. UTC | #1
On Fri, Aug 25, 2023 at 06:32:07PM +0900, Yoshihiro Shimoda wrote:
> Update dw_pcie_link_set_max_link_width() to set PCI_EXP_LNKCAP_MLW.
> In accordance with the DW PCIe RC/EP HW manuals [1,2,3,...] aside with
> the PORT_LINK_CTRL_OFF.LINK_CAPABLE and GEN2_CTRL_OFF.NUM_OF_LANES[8:0]
> field there is another one which needs to be updated. It's
> LINK_CAPABILITIES_REG.PCIE_CAP_MAX_LINK_WIDTH. If it isn't done at
> the very least the maximum link-width capability CSR won't expose
> the actual maximum capability.
> 
> [1] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
>     Version 4.60a, March 2015, p.1032
> [2] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
>     Version 4.70a, March 2016, p.1065
> [3] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
>     Version 4.90a, March 2016, p.1057
> ...
> [X] DesignWare Cores PCI Express Controller Databook - DWC PCIe Endpoint,
>       Version 5.40a, March 2019, p.1396
> [X+1] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
>       Version 5.40a, March 2019, p.1266

Is there value in keeping all four of these Root Port citations?  I
assume that if you have the most recent one (X+1), it completely
obsoletes the older ones, so you should never have to look at the
older ones?

Bjorn
Serge Semin Sept. 14, 2023, 8:48 p.m. UTC | #2
On Thu, Sep 14, 2023 at 11:00:58AM -0500, Bjorn Helgaas wrote:
> On Fri, Aug 25, 2023 at 06:32:07PM +0900, Yoshihiro Shimoda wrote:
> > Update dw_pcie_link_set_max_link_width() to set PCI_EXP_LNKCAP_MLW.
> > In accordance with the DW PCIe RC/EP HW manuals [1,2,3,...] aside with
> > the PORT_LINK_CTRL_OFF.LINK_CAPABLE and GEN2_CTRL_OFF.NUM_OF_LANES[8:0]
> > field there is another one which needs to be updated. It's
> > LINK_CAPABILITIES_REG.PCIE_CAP_MAX_LINK_WIDTH. If it isn't done at
> > the very least the maximum link-width capability CSR won't expose
> > the actual maximum capability.
> > 
> > [1] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
> >     Version 4.60a, March 2015, p.1032
> > [2] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
> >     Version 4.70a, March 2016, p.1065
> > [3] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
> >     Version 4.90a, March 2016, p.1057
> > ...
> > [X] DesignWare Cores PCI Express Controller Databook - DWC PCIe Endpoint,
> >       Version 5.40a, March 2019, p.1396
> > [X+1] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
> >       Version 5.40a, March 2019, p.1266
> 

> Is there value in keeping all four of these Root Port citations?  I
> assume that if you have the most recent one (X+1), it completely
> obsoletes the older ones, so you should never have to look at the
> older ones?

In general the procedure may differ from one device version to
another. Though it doesn't concern DW PCIe IP-cores. So by citing all
these manuals I implied that all DW PCIe controllers expect the same
link-width initialization procedure. Keeping that in mind I guess the
text could be indeed simplified by keeping only two citations (note
[X] and [X+1] refer to the Root Port and End-point HW databooks of the
same IP-core version) and noting in the text that the procedure is
common for the older DW PCIe controllers too.

In anyway I wouldn't say that new IP-core databooks obsolete the
old one since the driver supports all old and new controllers. So
before introducing a generic procedure we need to make sure that it
will work for all the known to be supported devices. From that
perspective citing all the available databooks gets to make sense.

-Serge(y)

> 
> Bjorn
Bjorn Helgaas Sept. 14, 2023, 8:59 p.m. UTC | #3
On Thu, Sep 14, 2023 at 11:48:39PM +0300, Serge Semin wrote:
> On Thu, Sep 14, 2023 at 11:00:58AM -0500, Bjorn Helgaas wrote:
> > On Fri, Aug 25, 2023 at 06:32:07PM +0900, Yoshihiro Shimoda wrote:
> > > Update dw_pcie_link_set_max_link_width() to set PCI_EXP_LNKCAP_MLW.
> > > In accordance with the DW PCIe RC/EP HW manuals [1,2,3,...] aside with
> > > the PORT_LINK_CTRL_OFF.LINK_CAPABLE and GEN2_CTRL_OFF.NUM_OF_LANES[8:0]
> > > field there is another one which needs to be updated. It's
> > > LINK_CAPABILITIES_REG.PCIE_CAP_MAX_LINK_WIDTH. If it isn't done at
> > > the very least the maximum link-width capability CSR won't expose
> > > the actual maximum capability.
> > > 
> > > [1] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
> > >     Version 4.60a, March 2015, p.1032
> > > [2] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
> > >     Version 4.70a, March 2016, p.1065
> > > [3] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
> > >     Version 4.90a, March 2016, p.1057
> > > ...
> > > [X] DesignWare Cores PCI Express Controller Databook - DWC PCIe Endpoint,
> > >       Version 5.40a, March 2019, p.1396
> > > [X+1] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
> > >       Version 5.40a, March 2019, p.1266
> 
> > Is there value in keeping all four of these Root Port citations?  I
> > assume that if you have the most recent one (X+1), it completely
> > obsoletes the older ones, so you should never have to look at the
> > older ones?
> 
> In general the procedure may differ from one device version to
> another. Though it doesn't concern DW PCIe IP-cores. So by citing all
> these manuals I implied that all DW PCIe controllers expect the same
> link-width initialization procedure. Keeping that in mind I guess the
> text could be indeed simplified by keeping only two citations (note
> [X] and [X+1] refer to the Root Port and End-point HW databooks of the
> same IP-core version) and noting in the text that the procedure is
> common for the older DW PCIe controllers too.
> 
> In anyway I wouldn't say that new IP-core databooks obsolete the
> old one since the driver supports all old and new controllers. So
> before introducing a generic procedure we need to make sure that it
> will work for all the known to be supported devices. From that
> perspective citing all the available databooks gets to make sense.

You mean that instead of merely *adding* new details about new
devices, v5.40a might OMIT details specific to older devices covered
by v4.60a?  That sounds like ... kind of an unhelpful way to manage
the spec, but if so, I see your point.

Bjorn
Serge Semin Sept. 14, 2023, 9:25 p.m. UTC | #4
On Thu, Sep 14, 2023 at 03:59:06PM -0500, Bjorn Helgaas wrote:
> On Thu, Sep 14, 2023 at 11:48:39PM +0300, Serge Semin wrote:
> > On Thu, Sep 14, 2023 at 11:00:58AM -0500, Bjorn Helgaas wrote:
> > > On Fri, Aug 25, 2023 at 06:32:07PM +0900, Yoshihiro Shimoda wrote:
> > > > Update dw_pcie_link_set_max_link_width() to set PCI_EXP_LNKCAP_MLW.
> > > > In accordance with the DW PCIe RC/EP HW manuals [1,2,3,...] aside with
> > > > the PORT_LINK_CTRL_OFF.LINK_CAPABLE and GEN2_CTRL_OFF.NUM_OF_LANES[8:0]
> > > > field there is another one which needs to be updated. It's
> > > > LINK_CAPABILITIES_REG.PCIE_CAP_MAX_LINK_WIDTH. If it isn't done at
> > > > the very least the maximum link-width capability CSR won't expose
> > > > the actual maximum capability.
> > > > 
> > > > [1] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
> > > >     Version 4.60a, March 2015, p.1032
> > > > [2] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
> > > >     Version 4.70a, March 2016, p.1065
> > > > [3] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
> > > >     Version 4.90a, March 2016, p.1057
> > > > ...
> > > > [X] DesignWare Cores PCI Express Controller Databook - DWC PCIe Endpoint,
> > > >       Version 5.40a, March 2019, p.1396
> > > > [X+1] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
> > > >       Version 5.40a, March 2019, p.1266
> > 
> > > Is there value in keeping all four of these Root Port citations?  I
> > > assume that if you have the most recent one (X+1), it completely
> > > obsoletes the older ones, so you should never have to look at the
> > > older ones?
> > 
> > In general the procedure may differ from one device version to
> > another. Though it doesn't concern DW PCIe IP-cores. So by citing all
> > these manuals I implied that all DW PCIe controllers expect the same
> > link-width initialization procedure. Keeping that in mind I guess the
> > text could be indeed simplified by keeping only two citations (note
> > [X] and [X+1] refer to the Root Port and End-point HW databooks of the
> > same IP-core version) and noting in the text that the procedure is
> > common for the older DW PCIe controllers too.
> > 
> > In anyway I wouldn't say that new IP-core databooks obsolete the
> > old one since the driver supports all old and new controllers. So
> > before introducing a generic procedure we need to make sure that it
> > will work for all the known to be supported devices. From that
> > perspective citing all the available databooks gets to make sense.
> 

> You mean that instead of merely *adding* new details about new
> devices, v5.40a might OMIT details specific to older devices covered
> by v4.60a?  That sounds like ... kind of an unhelpful way to manage
> the spec, but if so, I see your point.

Right. I can't say for all the Synopsys IP-core documents but for
instance DW PCIe, DW uMCTL2 DDRC, DW *MAC docs tend to omit a lot of
changes history info. The most extensive list of changes is available
in the release notes provided as a separate document.

-Serge(y)

> 
> Bjorn
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 5cca34140d2a..c4998194fe74 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -730,7 +730,8 @@  static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen)
 
 static void dw_pcie_link_set_max_link_width(struct dw_pcie *pci, u32 num_lanes)
 {
-	u32 lwsc, plc;
+	u32 lnkcap, lwsc, plc;
+	u8 cap;
 
 	if (!num_lanes)
 		return;
@@ -766,6 +767,12 @@  static void dw_pcie_link_set_max_link_width(struct dw_pcie *pci, u32 num_lanes)
 	}
 	dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, plc);
 	dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, lwsc);
+
+	cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	lnkcap = dw_pcie_readl_dbi(pci, cap + PCI_EXP_LNKCAP);
+	lnkcap &= ~PCI_EXP_LNKCAP_MLW;
+	lnkcap |= FIELD_PREP(PCI_EXP_LNKCAP_MLW, num_lanes);
+	dw_pcie_writel_dbi(pci, cap + PCI_EXP_LNKCAP, lnkcap);
 }
 
 void dw_pcie_iatu_detect(struct dw_pcie *pci)