| Message ID | 20230825093219.2685912-8-yoshihiro.shimoda.uh@renesas.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: rcar-gen4: Add R-Car Gen4 PCIe support | expand |
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
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
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
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 --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)
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(-)