| Message ID | 20221113191301.5526-18-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: dwc: Add generic resources and Baikal-T1 support | expand |
On Sun, Nov 13, 2022 at 10:12:58PM +0300, Serge Semin wrote: > Currently the DW PCIe Root Port and Endpoint CSR spaces are retrieved in > the separate parts of the DW PCIe core driver. It doesn't really make > sense since the both controller types have identical set of the core CSR > regions: DBI, DBI CS2 and iATU/eDMA. Thus we can simplify the DW PCIe Host > and EP initialization methods by moving the platform-specific registers > space getting and mapping into a common method. It gets to be even more > justified seeing the CSRs base address pointers are preserved in the > common DW PCIe descriptor. Note all the OF-based common DW PCIe settings > initialization will be moved to the new method too in order to have a > single function for all the generic platform properties handling in single > place. > > A nice side-effect of this change is that the pcie-designware-host.c and > pcie-designware-ep.c drivers are cleaned up from all the direct dw_pcie > storage modification, which makes the DW PCIe core, Root Port and Endpoint > modules more coherent. > You have clubbed both generic resource API and introducing CDM_CHECK flag. Please split them into separate patches. Thanks, Mani > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > Reviewed-by: Rob Herring <robh@kernel.org> > > --- > > Changelog v3: > - This is a new patch created on v3 lap of the series. > > Changelog v4: > - Convert the method name from dw_pcie_get_res() to > dw_pcie_get_resources(). (@Bjorn) > > Changelog v7: > - Get back device.of_node pointer to the dw_pcie_ep_init() method. > (@Yoshihiro) > --- > .../pci/controller/dwc/pcie-designware-ep.c | 25 +------ > .../pci/controller/dwc/pcie-designware-host.c | 15 +--- > drivers/pci/controller/dwc/pcie-designware.c | 75 ++++++++++++++----- > drivers/pci/controller/dwc/pcie-designware.h | 3 + > 4 files changed, 65 insertions(+), 53 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 237bb01d7852..f68d1ab83bb3 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -13,8 +13,6 @@ > #include <linux/pci-epc.h> > #include <linux/pci-epf.h> > > -#include "../../pci.h" > - > void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) > { > struct pci_epc *epc = ep->epc; > @@ -694,23 +692,9 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > > INIT_LIST_HEAD(&ep->func_list); > > - if (!pci->dbi_base) { > - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi"); > - pci->dbi_base = devm_pci_remap_cfg_resource(dev, res); > - if (IS_ERR(pci->dbi_base)) > - return PTR_ERR(pci->dbi_base); > - } > - > - if (!pci->dbi_base2) { > - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi2"); > - if (!res) { > - pci->dbi_base2 = pci->dbi_base + SZ_4K; > - } else { > - pci->dbi_base2 = devm_pci_remap_cfg_resource(dev, res); > - if (IS_ERR(pci->dbi_base2)) > - return PTR_ERR(pci->dbi_base2); > - } > - } > + ret = dw_pcie_get_resources(pci); > + if (ret) > + return ret; > > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space"); > if (!res) > @@ -739,9 +723,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > return -ENOMEM; > ep->outbound_addr = addr; > > - if (pci->link_gen < 1) > - pci->link_gen = of_pci_get_max_link_speed(np); > - > epc = devm_pci_epc_create(dev, &epc_ops); > if (IS_ERR(epc)) { > dev_err(dev, "Failed to create epc device\n"); > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index ea923c25e12d..3ab6ae3712c4 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -16,7 +16,6 @@ > #include <linux/pci_regs.h> > #include <linux/platform_device.h> > > -#include "../../pci.h" > #include "pcie-designware.h" > > static struct pci_ops dw_pcie_ops; > @@ -395,6 +394,10 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > > raw_spin_lock_init(&pp->lock); > > + ret = dw_pcie_get_resources(pci); > + if (ret) > + return ret; > + > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config"); > if (res) { > pp->cfg0_size = resource_size(res); > @@ -408,13 +411,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > return -ENODEV; > } > > - if (!pci->dbi_base) { > - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi"); > - pci->dbi_base = devm_pci_remap_cfg_resource(dev, res); > - if (IS_ERR(pci->dbi_base)) > - return PTR_ERR(pci->dbi_base); > - } > - > bridge = devm_pci_alloc_host_bridge(dev, 0); > if (!bridge) > return -ENOMEM; > @@ -429,9 +425,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > pp->io_base = pci_pio_to_address(win->res->start); > } > > - if (pci->link_gen < 1) > - pci->link_gen = of_pci_get_max_link_speed(np); > - > /* 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 9d78e7ca61e1..a8436027434d 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.c > +++ b/drivers/pci/controller/dwc/pcie-designware.c > @@ -11,6 +11,7 @@ > #include <linux/align.h> > #include <linux/bitops.h> > #include <linux/delay.h> > +#include <linux/ioport.h> > #include <linux/of.h> > #include <linux/of_platform.h> > #include <linux/sizes.h> > @@ -19,6 +20,59 @@ > #include "../../pci.h" > #include "pcie-designware.h" > > +int dw_pcie_get_resources(struct dw_pcie *pci) > +{ > + struct platform_device *pdev = to_platform_device(pci->dev); > + struct device_node *np = dev_of_node(pci->dev); > + struct resource *res; > + > + if (!pci->dbi_base) { > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi"); > + pci->dbi_base = devm_pci_remap_cfg_resource(pci->dev, res); > + if (IS_ERR(pci->dbi_base)) > + return PTR_ERR(pci->dbi_base); > + } > + > + /* DBI2 is mainly useful for the endpoint controller */ > + if (!pci->dbi_base2) { > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi2"); > + if (res) { > + pci->dbi_base2 = devm_pci_remap_cfg_resource(pci->dev, res); > + if (IS_ERR(pci->dbi_base2)) > + return PTR_ERR(pci->dbi_base2); > + } else { > + pci->dbi_base2 = pci->dbi_base + SZ_4K; > + } > + } > + > + /* For non-unrolled iATU/eDMA platforms this range will be ignored */ > + if (!pci->atu_base) { > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "atu"); > + if (res) { > + pci->atu_size = resource_size(res); > + pci->atu_base = devm_ioremap_resource(pci->dev, res); > + if (IS_ERR(pci->atu_base)) > + return PTR_ERR(pci->atu_base); > + } else { > + pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET; > + } > + } > + > + /* Set a default value suitable for at most 8 in and 8 out windows */ > + if (!pci->atu_size) > + pci->atu_size = SZ_4K; > + > + 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); > + > + if (of_property_read_bool(np, "snps,enable-cdm-check")) > + dw_pcie_cap_set(pci, CDM_CHECK); > + > + return 0; > +} > + > void dw_pcie_version_detect(struct dw_pcie *pci) > { > u32 ver; > @@ -639,25 +693,8 @@ static void dw_pcie_iatu_detect_regions(struct dw_pcie *pci) > > void dw_pcie_iatu_detect(struct dw_pcie *pci) > { > - struct platform_device *pdev = to_platform_device(pci->dev); > - > if (dw_pcie_iatu_unroll_enabled(pci)) { > dw_pcie_cap_set(pci, IATU_UNROLL); > - > - if (!pci->atu_base) { > - struct resource *res = > - platform_get_resource_byname(pdev, IORESOURCE_MEM, "atu"); > - if (res) { > - pci->atu_size = resource_size(res); > - pci->atu_base = devm_ioremap_resource(pci->dev, res); > - } > - if (!pci->atu_base || IS_ERR(pci->atu_base)) > - pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET; > - } > - > - if (!pci->atu_size) > - /* Pick a minimal default, enough for 8 in and 8 out windows */ > - pci->atu_size = SZ_4K; > } else { > pci->atu_base = pci->dbi_base + PCIE_ATU_VIEWPORT_BASE; > pci->atu_size = PCIE_ATU_VIEWPORT_SIZE; > @@ -675,7 +712,6 @@ void dw_pcie_iatu_detect(struct dw_pcie *pci) > > void dw_pcie_setup(struct dw_pcie *pci) > { > - struct device_node *np = pci->dev->of_node; > u32 val; > > if (pci->link_gen > 0) > @@ -703,14 +739,13 @@ void dw_pcie_setup(struct dw_pcie *pci) > val |= PORT_LINK_DLL_LINK_EN; > dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val); > > - if (of_property_read_bool(np, "snps,enable-cdm-check")) { > + if (dw_pcie_cap_is(pci, CDM_CHECK)) { > val = dw_pcie_readl_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS); > val |= PCIE_PL_CHK_REG_CHK_REG_CONTINUOUS | > PCIE_PL_CHK_REG_CHK_REG_START; > dw_pcie_writel_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS, 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; > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index c6dddacee3b1..081f169e6021 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -46,6 +46,7 @@ > > /* DWC PCIe controller capabilities */ > #define DW_PCIE_CAP_IATU_UNROLL 1 > +#define DW_PCIE_CAP_CDM_CHECK 2 > > #define dw_pcie_cap_is(_pci, _cap) \ > test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps) > @@ -338,6 +339,8 @@ struct dw_pcie { > #define to_dw_pcie_from_ep(endpoint) \ > container_of((endpoint), struct dw_pcie, ep) > > +int dw_pcie_get_resources(struct dw_pcie *pci); > + > void dw_pcie_version_detect(struct dw_pcie *pci); > > u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap); > -- > 2.38.1 > >
On Mon, Nov 14, 2022 at 12:16:54PM +0530, Manivannan Sadhasivam wrote: > On Sun, Nov 13, 2022 at 10:12:58PM +0300, Serge Semin wrote: > > Currently the DW PCIe Root Port and Endpoint CSR spaces are retrieved in > > the separate parts of the DW PCIe core driver. It doesn't really make > > sense since the both controller types have identical set of the core CSR > > regions: DBI, DBI CS2 and iATU/eDMA. Thus we can simplify the DW PCIe Host > > and EP initialization methods by moving the platform-specific registers > > space getting and mapping into a common method. It gets to be even more > > justified seeing the CSRs base address pointers are preserved in the > > common DW PCIe descriptor. Note all the OF-based common DW PCIe settings > > initialization will be moved to the new method too in order to have a > > single function for all the generic platform properties handling in single > > place. > > > > A nice side-effect of this change is that the pcie-designware-host.c and > > pcie-designware-ep.c drivers are cleaned up from all the direct dw_pcie > > storage modification, which makes the DW PCIe core, Root Port and Endpoint > > modules more coherent. > > > > You have clubbed both generic resource API and introducing CDM_CHECK flag. > Please split them into separate patches. This modification is a part of the new method dw_pcie_get_resources(). Without that method there is no point in adding the new flag. So no. It's better to have all of it in a single patch as a part of creating a coherent resources getter method. -Sergey > > Thanks, > Mani > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > --- > > > > Changelog v3: > > - This is a new patch created on v3 lap of the series. > > > > Changelog v4: > > - Convert the method name from dw_pcie_get_res() to > > dw_pcie_get_resources(). (@Bjorn) > > > > Changelog v7: > > - Get back device.of_node pointer to the dw_pcie_ep_init() method. > > (@Yoshihiro) > > --- > > .../pci/controller/dwc/pcie-designware-ep.c | 25 +------ > > .../pci/controller/dwc/pcie-designware-host.c | 15 +--- > > drivers/pci/controller/dwc/pcie-designware.c | 75 ++++++++++++++----- > > drivers/pci/controller/dwc/pcie-designware.h | 3 + > > 4 files changed, 65 insertions(+), 53 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > > index 237bb01d7852..f68d1ab83bb3 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > > @@ -13,8 +13,6 @@ > > #include <linux/pci-epc.h> > > #include <linux/pci-epf.h> > > > > -#include "../../pci.h" > > - > > void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) > > { > > struct pci_epc *epc = ep->epc; > > @@ -694,23 +692,9 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > > > > INIT_LIST_HEAD(&ep->func_list); > > > > - if (!pci->dbi_base) { > > - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi"); > > - pci->dbi_base = devm_pci_remap_cfg_resource(dev, res); > > - if (IS_ERR(pci->dbi_base)) > > - return PTR_ERR(pci->dbi_base); > > - } > > - > > - if (!pci->dbi_base2) { > > - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi2"); > > - if (!res) { > > - pci->dbi_base2 = pci->dbi_base + SZ_4K; > > - } else { > > - pci->dbi_base2 = devm_pci_remap_cfg_resource(dev, res); > > - if (IS_ERR(pci->dbi_base2)) > > - return PTR_ERR(pci->dbi_base2); > > - } > > - } > > + ret = dw_pcie_get_resources(pci); > > + if (ret) > > + return ret; > > > > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space"); > > if (!res) > > @@ -739,9 +723,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > > return -ENOMEM; > > ep->outbound_addr = addr; > > > > - if (pci->link_gen < 1) > > - pci->link_gen = of_pci_get_max_link_speed(np); > > - > > epc = devm_pci_epc_create(dev, &epc_ops); > > if (IS_ERR(epc)) { > > dev_err(dev, "Failed to create epc device\n"); > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > > index ea923c25e12d..3ab6ae3712c4 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > > @@ -16,7 +16,6 @@ > > #include <linux/pci_regs.h> > > #include <linux/platform_device.h> > > > > -#include "../../pci.h" > > #include "pcie-designware.h" > > > > static struct pci_ops dw_pcie_ops; > > @@ -395,6 +394,10 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > > > > raw_spin_lock_init(&pp->lock); > > > > + ret = dw_pcie_get_resources(pci); > > + if (ret) > > + return ret; > > + > > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config"); > > if (res) { > > pp->cfg0_size = resource_size(res); > > @@ -408,13 +411,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > > return -ENODEV; > > } > > > > - if (!pci->dbi_base) { > > - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi"); > > - pci->dbi_base = devm_pci_remap_cfg_resource(dev, res); > > - if (IS_ERR(pci->dbi_base)) > > - return PTR_ERR(pci->dbi_base); > > - } > > - > > bridge = devm_pci_alloc_host_bridge(dev, 0); > > if (!bridge) > > return -ENOMEM; > > @@ -429,9 +425,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > > pp->io_base = pci_pio_to_address(win->res->start); > > } > > > > - if (pci->link_gen < 1) > > - pci->link_gen = of_pci_get_max_link_speed(np); > > - > > /* 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 9d78e7ca61e1..a8436027434d 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware.c > > +++ b/drivers/pci/controller/dwc/pcie-designware.c > > @@ -11,6 +11,7 @@ > > #include <linux/align.h> > > #include <linux/bitops.h> > > #include <linux/delay.h> > > +#include <linux/ioport.h> > > #include <linux/of.h> > > #include <linux/of_platform.h> > > #include <linux/sizes.h> > > @@ -19,6 +20,59 @@ > > #include "../../pci.h" > > #include "pcie-designware.h" > > > > +int dw_pcie_get_resources(struct dw_pcie *pci) > > +{ > > + struct platform_device *pdev = to_platform_device(pci->dev); > > + struct device_node *np = dev_of_node(pci->dev); > > + struct resource *res; > > + > > + if (!pci->dbi_base) { > > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi"); > > + pci->dbi_base = devm_pci_remap_cfg_resource(pci->dev, res); > > + if (IS_ERR(pci->dbi_base)) > > + return PTR_ERR(pci->dbi_base); > > + } > > + > > + /* DBI2 is mainly useful for the endpoint controller */ > > + if (!pci->dbi_base2) { > > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi2"); > > + if (res) { > > + pci->dbi_base2 = devm_pci_remap_cfg_resource(pci->dev, res); > > + if (IS_ERR(pci->dbi_base2)) > > + return PTR_ERR(pci->dbi_base2); > > + } else { > > + pci->dbi_base2 = pci->dbi_base + SZ_4K; > > + } > > + } > > + > > + /* For non-unrolled iATU/eDMA platforms this range will be ignored */ > > + if (!pci->atu_base) { > > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "atu"); > > + if (res) { > > + pci->atu_size = resource_size(res); > > + pci->atu_base = devm_ioremap_resource(pci->dev, res); > > + if (IS_ERR(pci->atu_base)) > > + return PTR_ERR(pci->atu_base); > > + } else { > > + pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET; > > + } > > + } > > + > > + /* Set a default value suitable for at most 8 in and 8 out windows */ > > + if (!pci->atu_size) > > + pci->atu_size = SZ_4K; > > + > > + 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); > > + > > + if (of_property_read_bool(np, "snps,enable-cdm-check")) > > + dw_pcie_cap_set(pci, CDM_CHECK); > > + > > + return 0; > > +} > > + > > void dw_pcie_version_detect(struct dw_pcie *pci) > > { > > u32 ver; > > @@ -639,25 +693,8 @@ static void dw_pcie_iatu_detect_regions(struct dw_pcie *pci) > > > > void dw_pcie_iatu_detect(struct dw_pcie *pci) > > { > > - struct platform_device *pdev = to_platform_device(pci->dev); > > - > > if (dw_pcie_iatu_unroll_enabled(pci)) { > > dw_pcie_cap_set(pci, IATU_UNROLL); > > - > > - if (!pci->atu_base) { > > - struct resource *res = > > - platform_get_resource_byname(pdev, IORESOURCE_MEM, "atu"); > > - if (res) { > > - pci->atu_size = resource_size(res); > > - pci->atu_base = devm_ioremap_resource(pci->dev, res); > > - } > > - if (!pci->atu_base || IS_ERR(pci->atu_base)) > > - pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET; > > - } > > - > > - if (!pci->atu_size) > > - /* Pick a minimal default, enough for 8 in and 8 out windows */ > > - pci->atu_size = SZ_4K; > > } else { > > pci->atu_base = pci->dbi_base + PCIE_ATU_VIEWPORT_BASE; > > pci->atu_size = PCIE_ATU_VIEWPORT_SIZE; > > @@ -675,7 +712,6 @@ void dw_pcie_iatu_detect(struct dw_pcie *pci) > > > > void dw_pcie_setup(struct dw_pcie *pci) > > { > > - struct device_node *np = pci->dev->of_node; > > u32 val; > > > > if (pci->link_gen > 0) > > @@ -703,14 +739,13 @@ void dw_pcie_setup(struct dw_pcie *pci) > > val |= PORT_LINK_DLL_LINK_EN; > > dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val); > > > > - if (of_property_read_bool(np, "snps,enable-cdm-check")) { > > + if (dw_pcie_cap_is(pci, CDM_CHECK)) { > > val = dw_pcie_readl_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS); > > val |= PCIE_PL_CHK_REG_CHK_REG_CONTINUOUS | > > PCIE_PL_CHK_REG_CHK_REG_START; > > dw_pcie_writel_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS, 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; > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > > index c6dddacee3b1..081f169e6021 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware.h > > +++ b/drivers/pci/controller/dwc/pcie-designware.h > > @@ -46,6 +46,7 @@ > > > > /* DWC PCIe controller capabilities */ > > #define DW_PCIE_CAP_IATU_UNROLL 1 > > +#define DW_PCIE_CAP_CDM_CHECK 2 > > > > #define dw_pcie_cap_is(_pci, _cap) \ > > test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps) > > @@ -338,6 +339,8 @@ struct dw_pcie { > > #define to_dw_pcie_from_ep(endpoint) \ > > container_of((endpoint), struct dw_pcie, ep) > > > > +int dw_pcie_get_resources(struct dw_pcie *pci); > > + > > void dw_pcie_version_detect(struct dw_pcie *pci); > > > > u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap); > > -- > > 2.38.1 > > > > > > -- > மணிவண்ணன் சதாசிவம்
On Mon, Nov 14, 2022 at 11:39:03AM +0300, Serge Semin wrote: > On Mon, Nov 14, 2022 at 12:16:54PM +0530, Manivannan Sadhasivam wrote: > > On Sun, Nov 13, 2022 at 10:12:58PM +0300, Serge Semin wrote: > > > Currently the DW PCIe Root Port and Endpoint CSR spaces are retrieved in > > > the separate parts of the DW PCIe core driver. It doesn't really make > > > sense since the both controller types have identical set of the core CSR > > > regions: DBI, DBI CS2 and iATU/eDMA. Thus we can simplify the DW PCIe Host > > > and EP initialization methods by moving the platform-specific registers > > > space getting and mapping into a common method. It gets to be even more > > > justified seeing the CSRs base address pointers are preserved in the > > > common DW PCIe descriptor. Note all the OF-based common DW PCIe settings > > > initialization will be moved to the new method too in order to have a > > > single function for all the generic platform properties handling in single > > > place. > > > > > > A nice side-effect of this change is that the pcie-designware-host.c and > > > pcie-designware-ep.c drivers are cleaned up from all the direct dw_pcie > > > storage modification, which makes the DW PCIe core, Root Port and Endpoint > > > modules more coherent. > > > > > > > > You have clubbed both generic resource API and introducing CDM_CHECK flag. > > Please split them into separate patches. > > This modification is a part of the new method dw_pcie_get_resources(). > Without that method there is no point in adding the new flag. So no. > It's better to have all of it in a single patch as a part of creating > a coherent resources getter method. > Same comment as previous patch. I'll defer it to you. Thanks, Mani > -Sergey > > > > > Thanks, > > Mani > > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > > > --- > > > > > > Changelog v3: > > > - This is a new patch created on v3 lap of the series. > > > > > > Changelog v4: > > > - Convert the method name from dw_pcie_get_res() to > > > dw_pcie_get_resources(). (@Bjorn) > > > > > > Changelog v7: > > > - Get back device.of_node pointer to the dw_pcie_ep_init() method. > > > (@Yoshihiro) > > > --- > > > .../pci/controller/dwc/pcie-designware-ep.c | 25 +------ > > > .../pci/controller/dwc/pcie-designware-host.c | 15 +--- > > > drivers/pci/controller/dwc/pcie-designware.c | 75 ++++++++++++++----- > > > drivers/pci/controller/dwc/pcie-designware.h | 3 + > > > 4 files changed, 65 insertions(+), 53 deletions(-) > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > > > index 237bb01d7852..f68d1ab83bb3 100644 > > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > > > @@ -13,8 +13,6 @@ > > > #include <linux/pci-epc.h> > > > #include <linux/pci-epf.h> > > > > > > -#include "../../pci.h" > > > - > > > void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) > > > { > > > struct pci_epc *epc = ep->epc; > > > @@ -694,23 +692,9 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > > > > > > INIT_LIST_HEAD(&ep->func_list); > > > > > > - if (!pci->dbi_base) { > > > - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi"); > > > - pci->dbi_base = devm_pci_remap_cfg_resource(dev, res); > > > - if (IS_ERR(pci->dbi_base)) > > > - return PTR_ERR(pci->dbi_base); > > > - } > > > - > > > - if (!pci->dbi_base2) { > > > - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi2"); > > > - if (!res) { > > > - pci->dbi_base2 = pci->dbi_base + SZ_4K; > > > - } else { > > > - pci->dbi_base2 = devm_pci_remap_cfg_resource(dev, res); > > > - if (IS_ERR(pci->dbi_base2)) > > > - return PTR_ERR(pci->dbi_base2); > > > - } > > > - } > > > + ret = dw_pcie_get_resources(pci); > > > + if (ret) > > > + return ret; > > > > > > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space"); > > > if (!res) > > > @@ -739,9 +723,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > > > return -ENOMEM; > > > ep->outbound_addr = addr; > > > > > > - if (pci->link_gen < 1) > > > - pci->link_gen = of_pci_get_max_link_speed(np); > > > - > > > epc = devm_pci_epc_create(dev, &epc_ops); > > > if (IS_ERR(epc)) { > > > dev_err(dev, "Failed to create epc device\n"); > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > > > index ea923c25e12d..3ab6ae3712c4 100644 > > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > > > @@ -16,7 +16,6 @@ > > > #include <linux/pci_regs.h> > > > #include <linux/platform_device.h> > > > > > > -#include "../../pci.h" > > > #include "pcie-designware.h" > > > > > > static struct pci_ops dw_pcie_ops; > > > @@ -395,6 +394,10 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > > > > > > raw_spin_lock_init(&pp->lock); > > > > > > + ret = dw_pcie_get_resources(pci); > > > + if (ret) > > > + return ret; > > > + > > > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config"); > > > if (res) { > > > pp->cfg0_size = resource_size(res); > > > @@ -408,13 +411,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > > > return -ENODEV; > > > } > > > > > > - if (!pci->dbi_base) { > > > - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi"); > > > - pci->dbi_base = devm_pci_remap_cfg_resource(dev, res); > > > - if (IS_ERR(pci->dbi_base)) > > > - return PTR_ERR(pci->dbi_base); > > > - } > > > - > > > bridge = devm_pci_alloc_host_bridge(dev, 0); > > > if (!bridge) > > > return -ENOMEM; > > > @@ -429,9 +425,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > > > pp->io_base = pci_pio_to_address(win->res->start); > > > } > > > > > > - if (pci->link_gen < 1) > > > - pci->link_gen = of_pci_get_max_link_speed(np); > > > - > > > /* 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 9d78e7ca61e1..a8436027434d 100644 > > > --- a/drivers/pci/controller/dwc/pcie-designware.c > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c > > > @@ -11,6 +11,7 @@ > > > #include <linux/align.h> > > > #include <linux/bitops.h> > > > #include <linux/delay.h> > > > +#include <linux/ioport.h> > > > #include <linux/of.h> > > > #include <linux/of_platform.h> > > > #include <linux/sizes.h> > > > @@ -19,6 +20,59 @@ > > > #include "../../pci.h" > > > #include "pcie-designware.h" > > > > > > +int dw_pcie_get_resources(struct dw_pcie *pci) > > > +{ > > > + struct platform_device *pdev = to_platform_device(pci->dev); > > > + struct device_node *np = dev_of_node(pci->dev); > > > + struct resource *res; > > > + > > > + if (!pci->dbi_base) { > > > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi"); > > > + pci->dbi_base = devm_pci_remap_cfg_resource(pci->dev, res); > > > + if (IS_ERR(pci->dbi_base)) > > > + return PTR_ERR(pci->dbi_base); > > > + } > > > + > > > + /* DBI2 is mainly useful for the endpoint controller */ > > > + if (!pci->dbi_base2) { > > > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi2"); > > > + if (res) { > > > + pci->dbi_base2 = devm_pci_remap_cfg_resource(pci->dev, res); > > > + if (IS_ERR(pci->dbi_base2)) > > > + return PTR_ERR(pci->dbi_base2); > > > + } else { > > > + pci->dbi_base2 = pci->dbi_base + SZ_4K; > > > + } > > > + } > > > + > > > + /* For non-unrolled iATU/eDMA platforms this range will be ignored */ > > > + if (!pci->atu_base) { > > > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "atu"); > > > + if (res) { > > > + pci->atu_size = resource_size(res); > > > + pci->atu_base = devm_ioremap_resource(pci->dev, res); > > > + if (IS_ERR(pci->atu_base)) > > > + return PTR_ERR(pci->atu_base); > > > + } else { > > > + pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET; > > > + } > > > + } > > > + > > > + /* Set a default value suitable for at most 8 in and 8 out windows */ > > > + if (!pci->atu_size) > > > + pci->atu_size = SZ_4K; > > > + > > > + 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); > > > + > > > + if (of_property_read_bool(np, "snps,enable-cdm-check")) > > > + dw_pcie_cap_set(pci, CDM_CHECK); > > > + > > > + return 0; > > > +} > > > + > > > void dw_pcie_version_detect(struct dw_pcie *pci) > > > { > > > u32 ver; > > > @@ -639,25 +693,8 @@ static void dw_pcie_iatu_detect_regions(struct dw_pcie *pci) > > > > > > void dw_pcie_iatu_detect(struct dw_pcie *pci) > > > { > > > - struct platform_device *pdev = to_platform_device(pci->dev); > > > - > > > if (dw_pcie_iatu_unroll_enabled(pci)) { > > > dw_pcie_cap_set(pci, IATU_UNROLL); > > > - > > > - if (!pci->atu_base) { > > > - struct resource *res = > > > - platform_get_resource_byname(pdev, IORESOURCE_MEM, "atu"); > > > - if (res) { > > > - pci->atu_size = resource_size(res); > > > - pci->atu_base = devm_ioremap_resource(pci->dev, res); > > > - } > > > - if (!pci->atu_base || IS_ERR(pci->atu_base)) > > > - pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET; > > > - } > > > - > > > - if (!pci->atu_size) > > > - /* Pick a minimal default, enough for 8 in and 8 out windows */ > > > - pci->atu_size = SZ_4K; > > > } else { > > > pci->atu_base = pci->dbi_base + PCIE_ATU_VIEWPORT_BASE; > > > pci->atu_size = PCIE_ATU_VIEWPORT_SIZE; > > > @@ -675,7 +712,6 @@ void dw_pcie_iatu_detect(struct dw_pcie *pci) > > > > > > void dw_pcie_setup(struct dw_pcie *pci) > > > { > > > - struct device_node *np = pci->dev->of_node; > > > u32 val; > > > > > > if (pci->link_gen > 0) > > > @@ -703,14 +739,13 @@ void dw_pcie_setup(struct dw_pcie *pci) > > > val |= PORT_LINK_DLL_LINK_EN; > > > dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val); > > > > > > - if (of_property_read_bool(np, "snps,enable-cdm-check")) { > > > + if (dw_pcie_cap_is(pci, CDM_CHECK)) { > > > val = dw_pcie_readl_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS); > > > val |= PCIE_PL_CHK_REG_CHK_REG_CONTINUOUS | > > > PCIE_PL_CHK_REG_CHK_REG_START; > > > dw_pcie_writel_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS, 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; > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > > > index c6dddacee3b1..081f169e6021 100644 > > > --- a/drivers/pci/controller/dwc/pcie-designware.h > > > +++ b/drivers/pci/controller/dwc/pcie-designware.h > > > @@ -46,6 +46,7 @@ > > > > > > /* DWC PCIe controller capabilities */ > > > #define DW_PCIE_CAP_IATU_UNROLL 1 > > > +#define DW_PCIE_CAP_CDM_CHECK 2 > > > > > > #define dw_pcie_cap_is(_pci, _cap) \ > > > test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps) > > > @@ -338,6 +339,8 @@ struct dw_pcie { > > > #define to_dw_pcie_from_ep(endpoint) \ > > > container_of((endpoint), struct dw_pcie, ep) > > > > > > +int dw_pcie_get_resources(struct dw_pcie *pci); > > > + > > > void dw_pcie_version_detect(struct dw_pcie *pci); > > > > > > u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap); > > > -- > > > 2.38.1 > > > > > > > > > > -- > > மணிவண்ணன் சதாசிவம்
Hi Serge, On Sun, Nov 13, 2022 at 10:12:58PM +0300, Serge Semin wrote: > Currently the DW PCIe Root Port and Endpoint CSR spaces are retrieved in > the separate parts of the DW PCIe core driver. It doesn't really make > sense since the both controller types have identical set of the core CSR > regions: DBI, DBI CS2 and iATU/eDMA. Thus we can simplify the DW PCIe Host > and EP initialization methods by moving the platform-specific registers > space getting and mapping into a common method. It gets to be even more > justified seeing the CSRs base address pointers are preserved in the > common DW PCIe descriptor. Note all the OF-based common DW PCIe settings > initialization will be moved to the new method too in order to have a > single function for all the generic platform properties handling in single > place. > > A nice side-effect of this change is that the pcie-designware-host.c and > pcie-designware-ep.c drivers are cleaned up from all the direct dw_pcie > storage modification, which makes the DW PCIe core, Root Port and Endpoint > modules more coherent. Thanks for these new generic interfaces in the DWC core! And thanks for the changes in this patch to take advantage of them in the pcie-designware drivers. Do you plan similar changes to other drivers to take advantage of these DWC-generic data and interfaces? If you add generic things to the DWC core but only take advantage of them in your driver, I don't think they are really usefully generic. Bjorn
Hi Bjorn, On Wed, Nov 23, 2022 at 01:44:36PM -0600, Bjorn Helgaas wrote: > Hi Serge, > > On Sun, Nov 13, 2022 at 10:12:58PM +0300, Serge Semin wrote: > > Currently the DW PCIe Root Port and Endpoint CSR spaces are retrieved in > > the separate parts of the DW PCIe core driver. It doesn't really make > > sense since the both controller types have identical set of the core CSR > > regions: DBI, DBI CS2 and iATU/eDMA. Thus we can simplify the DW PCIe Host > > and EP initialization methods by moving the platform-specific registers > > space getting and mapping into a common method. It gets to be even more > > justified seeing the CSRs base address pointers are preserved in the > > common DW PCIe descriptor. Note all the OF-based common DW PCIe settings > > initialization will be moved to the new method too in order to have a > > single function for all the generic platform properties handling in single > > place. > > > > A nice side-effect of this change is that the pcie-designware-host.c and > > pcie-designware-ep.c drivers are cleaned up from all the direct dw_pcie > > storage modification, which makes the DW PCIe core, Root Port and Endpoint > > modules more coherent. > > Thanks for these new generic interfaces in the DWC core! And thanks > for the changes in this patch to take advantage of them in the > pcie-designware drivers. > > Do you plan similar changes to other drivers to take advantage of > these DWC-generic data and interfaces? If you add generic things to > the DWC core but only take advantage of them in your driver, I don't > think they are really usefully generic. Could you be more specific what generic things you are referring to? I am asking because the only part of the changes which is used in my low-level driver only is introduced in another patch of this series. It's < [PATCH v7 19/20] PCI: dwc: Introduce generic platform clocks and resets The new clock/reset request interface has been implemented the way it is due to reasons I in details described to Rob here: Link: https://lore.kernel.org/linux-pci/20220520160246.guczq52v2ycfgc6c@mobilestation To cut it short it can't be used by the most of the already available low-level drivers since they already have their own versions of the names for the clock and reset resources (or don't have any name defined at all). The only driver for which the interface could be utilized is Toshiba Visconti PCIe host controller driver. The device DT-bindings defines the clock names matching the generic names introduced in the patches of this series. If you find it appropriate enough I can provide a patch for that driver. Note the main goal of the patch [PATCH v7 19/20] PCI: dwc: Introduce generic platform clocks and resets was to create some interface to stop the developers of the new drivers from creating the platform-specific DT-bindings to the same clock and reset resources. Since the already defined DT-bindings can't be changed anyway I don't think it would worth risking to catch regressions on an attempt to provide a more complicated interface utilized in the old drivers too. -Serge(y) > > Bjorn
On Sun, Nov 27, 2022 at 04:10:05AM +0300, Serge Semin wrote: > On Wed, Nov 23, 2022 at 01:44:36PM -0600, Bjorn Helgaas wrote: > > On Sun, Nov 13, 2022 at 10:12:58PM +0300, Serge Semin wrote: > > Thanks for these new generic interfaces in the DWC core! And thanks > > for the changes in this patch to take advantage of them in the > > pcie-designware drivers. > > > > Do you plan similar changes to other drivers to take advantage of > > these DWC-generic data and interfaces? If you add generic things to > > the DWC core but only take advantage of them in your driver, I don't > > think they are really usefully generic. > > Could you be more specific what generic things you are referring to? I > am asking because the only part of the changes which is used in my > low-level driver only is introduced in another patch of this series. I asked because three of your patches mention "generic" things, but I didn't see any changes to drivers except pcie-designware: PCI: dwc: Introduce generic platform clocks and reset PCI: dwc: Introduce generic resources getter PCI: dwc: Introduce generic controller capabilities interface I hoped that we would be able to use these to remove some code from existing drivers, but if they only improve maintainability of future drivers, that's useful, too. Bjorn
On Tue, Nov 29, 2022 at 12:35:43PM -0600, Bjorn Helgaas wrote: > On Sun, Nov 27, 2022 at 04:10:05AM +0300, Serge Semin wrote: > > On Wed, Nov 23, 2022 at 01:44:36PM -0600, Bjorn Helgaas wrote: > > > On Sun, Nov 13, 2022 at 10:12:58PM +0300, Serge Semin wrote: > > > > Thanks for these new generic interfaces in the DWC core! And thanks > > > for the changes in this patch to take advantage of them in the > > > pcie-designware drivers. > > > > > > Do you plan similar changes to other drivers to take advantage of > > > these DWC-generic data and interfaces? If you add generic things to > > > the DWC core but only take advantage of them in your driver, I don't > > > think they are really usefully generic. > > > > Could you be more specific what generic things you are referring to? I > > am asking because the only part of the changes which is used in my > > low-level driver only is introduced in another patch of this series. > > I asked because three of your patches mention "generic" things, but I > didn't see any changes to drivers except pcie-designware: > > PCI: dwc: Introduce generic platform clocks and reset This patch introduces a method to request a generic platform clocks and resets by their names. As I already said these names are defined by the DT-bindings, which are platform-specific. That's why the most of the currently available drivers can't be converted to using it. Instead the new drivers are supposed to be encouraged to use the generic names (in accordance with the generic DW PCIe DT-schema) and the resources request interface (based on the generic DT-bindings) if it suits their design. Anyway I honestly tried to come up with an even more generic interface, which could be used by all the low-level drivers. But due to too much variations of the resource names and their sometimes too complex utilization in the drivers any solution looked too complex. After all of thoughts I decided to keep things simpler. > PCI: dwc: Introduce generic resources getter This patch defines a generic resource getter for the DW PCIe host and end-point drivers. That's why it's called generic. > PCI: dwc: Introduce generic controller capabilities interface This patch introduces an interface to set the device-specific capabilities. Since these capabilities can be marked as available by both the core driver (at least two of them already defined within this patchset) and low-level platform drivers the interface is called as generic. > > I hoped that we would be able to use these to remove some code from > existing drivers, but if they only improve maintainability of future > drivers, that's useful, too. Removing some code is possible for instance from the pcie-visconti.c driver by using the new generic clocks and resets request interface. I've scheduled to create a small patchset which would do that after the rest of my patches pass the review process. -Serge(y) > > Bjorn
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c index 237bb01d7852..f68d1ab83bb3 100644 --- a/drivers/pci/controller/dwc/pcie-designware-ep.c +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c @@ -13,8 +13,6 @@ #include <linux/pci-epc.h> #include <linux/pci-epf.h> -#include "../../pci.h" - void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) { struct pci_epc *epc = ep->epc; @@ -694,23 +692,9 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) INIT_LIST_HEAD(&ep->func_list); - if (!pci->dbi_base) { - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi"); - pci->dbi_base = devm_pci_remap_cfg_resource(dev, res); - if (IS_ERR(pci->dbi_base)) - return PTR_ERR(pci->dbi_base); - } - - if (!pci->dbi_base2) { - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi2"); - if (!res) { - pci->dbi_base2 = pci->dbi_base + SZ_4K; - } else { - pci->dbi_base2 = devm_pci_remap_cfg_resource(dev, res); - if (IS_ERR(pci->dbi_base2)) - return PTR_ERR(pci->dbi_base2); - } - } + ret = dw_pcie_get_resources(pci); + if (ret) + return ret; res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space"); if (!res) @@ -739,9 +723,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) return -ENOMEM; ep->outbound_addr = addr; - if (pci->link_gen < 1) - pci->link_gen = of_pci_get_max_link_speed(np); - epc = devm_pci_epc_create(dev, &epc_ops); if (IS_ERR(epc)) { dev_err(dev, "Failed to create epc device\n"); diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index ea923c25e12d..3ab6ae3712c4 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -16,7 +16,6 @@ #include <linux/pci_regs.h> #include <linux/platform_device.h> -#include "../../pci.h" #include "pcie-designware.h" static struct pci_ops dw_pcie_ops; @@ -395,6 +394,10 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) raw_spin_lock_init(&pp->lock); + ret = dw_pcie_get_resources(pci); + if (ret) + return ret; + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config"); if (res) { pp->cfg0_size = resource_size(res); @@ -408,13 +411,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) return -ENODEV; } - if (!pci->dbi_base) { - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi"); - pci->dbi_base = devm_pci_remap_cfg_resource(dev, res); - if (IS_ERR(pci->dbi_base)) - return PTR_ERR(pci->dbi_base); - } - bridge = devm_pci_alloc_host_bridge(dev, 0); if (!bridge) return -ENOMEM; @@ -429,9 +425,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) pp->io_base = pci_pio_to_address(win->res->start); } - if (pci->link_gen < 1) - pci->link_gen = of_pci_get_max_link_speed(np); - /* 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 9d78e7ca61e1..a8436027434d 100644 --- a/drivers/pci/controller/dwc/pcie-designware.c +++ b/drivers/pci/controller/dwc/pcie-designware.c @@ -11,6 +11,7 @@ #include <linux/align.h> #include <linux/bitops.h> #include <linux/delay.h> +#include <linux/ioport.h> #include <linux/of.h> #include <linux/of_platform.h> #include <linux/sizes.h> @@ -19,6 +20,59 @@ #include "../../pci.h" #include "pcie-designware.h" +int dw_pcie_get_resources(struct dw_pcie *pci) +{ + struct platform_device *pdev = to_platform_device(pci->dev); + struct device_node *np = dev_of_node(pci->dev); + struct resource *res; + + if (!pci->dbi_base) { + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi"); + pci->dbi_base = devm_pci_remap_cfg_resource(pci->dev, res); + if (IS_ERR(pci->dbi_base)) + return PTR_ERR(pci->dbi_base); + } + + /* DBI2 is mainly useful for the endpoint controller */ + if (!pci->dbi_base2) { + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi2"); + if (res) { + pci->dbi_base2 = devm_pci_remap_cfg_resource(pci->dev, res); + if (IS_ERR(pci->dbi_base2)) + return PTR_ERR(pci->dbi_base2); + } else { + pci->dbi_base2 = pci->dbi_base + SZ_4K; + } + } + + /* For non-unrolled iATU/eDMA platforms this range will be ignored */ + if (!pci->atu_base) { + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "atu"); + if (res) { + pci->atu_size = resource_size(res); + pci->atu_base = devm_ioremap_resource(pci->dev, res); + if (IS_ERR(pci->atu_base)) + return PTR_ERR(pci->atu_base); + } else { + pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET; + } + } + + /* Set a default value suitable for at most 8 in and 8 out windows */ + if (!pci->atu_size) + pci->atu_size = SZ_4K; + + 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); + + if (of_property_read_bool(np, "snps,enable-cdm-check")) + dw_pcie_cap_set(pci, CDM_CHECK); + + return 0; +} + void dw_pcie_version_detect(struct dw_pcie *pci) { u32 ver; @@ -639,25 +693,8 @@ static void dw_pcie_iatu_detect_regions(struct dw_pcie *pci) void dw_pcie_iatu_detect(struct dw_pcie *pci) { - struct platform_device *pdev = to_platform_device(pci->dev); - if (dw_pcie_iatu_unroll_enabled(pci)) { dw_pcie_cap_set(pci, IATU_UNROLL); - - if (!pci->atu_base) { - struct resource *res = - platform_get_resource_byname(pdev, IORESOURCE_MEM, "atu"); - if (res) { - pci->atu_size = resource_size(res); - pci->atu_base = devm_ioremap_resource(pci->dev, res); - } - if (!pci->atu_base || IS_ERR(pci->atu_base)) - pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET; - } - - if (!pci->atu_size) - /* Pick a minimal default, enough for 8 in and 8 out windows */ - pci->atu_size = SZ_4K; } else { pci->atu_base = pci->dbi_base + PCIE_ATU_VIEWPORT_BASE; pci->atu_size = PCIE_ATU_VIEWPORT_SIZE; @@ -675,7 +712,6 @@ void dw_pcie_iatu_detect(struct dw_pcie *pci) void dw_pcie_setup(struct dw_pcie *pci) { - struct device_node *np = pci->dev->of_node; u32 val; if (pci->link_gen > 0) @@ -703,14 +739,13 @@ void dw_pcie_setup(struct dw_pcie *pci) val |= PORT_LINK_DLL_LINK_EN; dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val); - if (of_property_read_bool(np, "snps,enable-cdm-check")) { + if (dw_pcie_cap_is(pci, CDM_CHECK)) { val = dw_pcie_readl_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS); val |= PCIE_PL_CHK_REG_CHK_REG_CONTINUOUS | PCIE_PL_CHK_REG_CHK_REG_START; dw_pcie_writel_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS, 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; diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h index c6dddacee3b1..081f169e6021 100644 --- a/drivers/pci/controller/dwc/pcie-designware.h +++ b/drivers/pci/controller/dwc/pcie-designware.h @@ -46,6 +46,7 @@ /* DWC PCIe controller capabilities */ #define DW_PCIE_CAP_IATU_UNROLL 1 +#define DW_PCIE_CAP_CDM_CHECK 2 #define dw_pcie_cap_is(_pci, _cap) \ test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps) @@ -338,6 +339,8 @@ struct dw_pcie { #define to_dw_pcie_from_ep(endpoint) \ container_of((endpoint), struct dw_pcie, ep) +int dw_pcie_get_resources(struct dw_pcie *pci); + void dw_pcie_version_detect(struct dw_pcie *pci); u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap);