| Message ID | 20220701085420.870306-7-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | treewide: PCI: renesas: Add R-Car Gen4 PCIe support | expand |
On Fri, Jul 01, 2022 at 05:54:13PM +0900, Yoshihiro Shimoda wrote: > Vendor-specific initialization needs this information so that > read it before host_init(). And then, we can assume that > dw_pcie_{ep,host}_init() gets the property so that dw_pcie_setup() > doesn't need to get it again. Patches 5 and 6 should be combined. We already have 2 drivers (keystone and tegra) that need this earlier still which this patch doesn't address. I think we need some sort of function to initialize the dw_pcie struct with defaults to be called after alloc or as part of alloc. To do the latter, there needs to be more unification on whether dw_pcie is a pointer or struct in the platform specific struct. That's a lot of churn though... > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- > drivers/pci/controller/dwc/pcie-designware-host.c | 2 ++ > drivers/pci/controller/dwc/pcie-designware.c | 1 - > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index 1e3972c487b5..cf875bdcfabb 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -336,6 +336,8 @@ int dw_pcie_host_init(struct pcie_port *pp) > if (pci->link_gen < 1) > pci->link_gen = of_pci_get_max_link_speed(np); > > + of_property_read_u32(np, "num-lanes", &pci->num_lanes); > + > /* Set default bus ops */ > bridge->ops = &dw_pcie_ops; > bridge->child_ops = &dw_child_pcie_ops; > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > index d92c8a25094f..101e892e22e8 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.c > +++ b/drivers/pci/controller/dwc/pcie-designware.c > @@ -726,7 +726,6 @@ void dw_pcie_setup(struct dw_pcie *pci) > val |= PORT_LINK_DLL_LINK_EN; > dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val); > > - of_property_read_u32(np, "num-lanes", &pci->num_lanes); > if (!pci->num_lanes) { > dev_dbg(pci->dev, "Using h/w default number of lanes\n"); > return; > -- > 2.25.1 > >
Hi Rob, > From: Rob Herring, Sent: Tuesday, July 12, 2022 3:34 AM > > On Fri, Jul 01, 2022 at 05:54:13PM +0900, Yoshihiro Shimoda wrote: > > Vendor-specific initialization needs this information so that > > read it before host_init(). And then, we can assume that > > dw_pcie_{ep,host}_init() gets the property so that dw_pcie_setup() > > doesn't need to get it again. > > Patches 5 and 6 should be combined. > > We already have 2 drivers (keystone and tegra) that need this earlier > still which this patch doesn't address. I think we need some sort of > function to initialize the dw_pcie struct with defaults to be called > after alloc or as part of alloc. To do the latter, there needs to be > more unification on whether dw_pcie is a pointer or struct in the > platform specific struct. That's a lot of churn though... Thank you for the review and comments. As I sent an email yesterday on the cover letter [1], I realized that Serge sent a lot of patches for dwc PCI common code. And, this patch could be completely replaced with the following patch because you already sent your Reviewed-by :) https://patchwork.kernel.org/project/linux-pci/patch/20220610085706.15741-15-Sergey.Semin@baikalelectronics.ru/ So, I'll drop patches 5 and 6 from this patch series. [1] https://lore.kernel.org/all/TYBPR01MB53417EFE0A6DC5AF3D54A0F3D8829@TYBPR01MB5341.jpnprd01.prod.outlook.com/ Best regards, Yoshihiro Shimoda > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > --- > > drivers/pci/controller/dwc/pcie-designware-host.c | 2 ++ > > drivers/pci/controller/dwc/pcie-designware.c | 1 - > > 2 files changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > > index 1e3972c487b5..cf875bdcfabb 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > > @@ -336,6 +336,8 @@ int dw_pcie_host_init(struct pcie_port *pp) > > if (pci->link_gen < 1) > > pci->link_gen = of_pci_get_max_link_speed(np); > > > > + of_property_read_u32(np, "num-lanes", &pci->num_lanes); > > + > > /* Set default bus ops */ > > bridge->ops = &dw_pcie_ops; > > bridge->child_ops = &dw_child_pcie_ops; > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > > index d92c8a25094f..101e892e22e8 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware.c > > +++ b/drivers/pci/controller/dwc/pcie-designware.c > > @@ -726,7 +726,6 @@ void dw_pcie_setup(struct dw_pcie *pci) > > val |= PORT_LINK_DLL_LINK_EN; > > dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val); > > > > - of_property_read_u32(np, "num-lanes", &pci->num_lanes); > > if (!pci->num_lanes) { > > dev_dbg(pci->dev, "Using h/w default number of lanes\n"); > > return; > > -- > > 2.25.1 > > > >
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index 1e3972c487b5..cf875bdcfabb 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -336,6 +336,8 @@ int dw_pcie_host_init(struct pcie_port *pp) if (pci->link_gen < 1) pci->link_gen = of_pci_get_max_link_speed(np); + of_property_read_u32(np, "num-lanes", &pci->num_lanes); + /* Set default bus ops */ bridge->ops = &dw_pcie_ops; bridge->child_ops = &dw_child_pcie_ops; diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c index d92c8a25094f..101e892e22e8 100644 --- a/drivers/pci/controller/dwc/pcie-designware.c +++ b/drivers/pci/controller/dwc/pcie-designware.c @@ -726,7 +726,6 @@ void dw_pcie_setup(struct dw_pcie *pci) val |= PORT_LINK_DLL_LINK_EN; dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val); - of_property_read_u32(np, "num-lanes", &pci->num_lanes); if (!pci->num_lanes) { dev_dbg(pci->dev, "Using h/w default number of lanes\n"); return;
Vendor-specific initialization needs this information so that read it before host_init(). And then, we can assume that dw_pcie_{ep,host}_init() gets the property so that dw_pcie_setup() doesn't need to get it again. Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- drivers/pci/controller/dwc/pcie-designware-host.c | 2 ++ drivers/pci/controller/dwc/pcie-designware.c | 1 - 2 files changed, 2 insertions(+), 1 deletion(-)