| Message ID | 20220822184701.25246-1-Sergey.Semin@baikalelectronics.ru |
|---|---|
| Headers | show |
| Series | PCI: dwc: Add generic resources and Baikal-T1 support | expand |
[+Robin, Will - please jump to DMA mask set-up] On Mon, Aug 22, 2022 at 09:47:01PM +0300, Serge Semin wrote: > Baikal-T1 SoC is equipped with DWC PCIe v4.60a host controller. It can be > trained to work up to Gen.3 speed over up to x4 lanes. The host controller > is attached to the DW PCIe 3.0 PCS via the PIPE-4 interface, which in its > turn is connected to the DWC 10G PHY. The whole system is supposed to be > fed up with four clock sources: DBI peripheral clock, AXI application > clocks and external PHY/core reference clock generating the 100MHz signal. > In addition to that the platform provide a way to reset each part of the > controller: sticky/non-sticky bits, host controller core, PIPE interface, > PCS/PHY and Hot/Power reset signal. The driver also provides a way to > handle the GPIO-based PERST# signal. > > Note due to the Baikal-T1 MMIO peculiarity we have to implement the DBI > interface accessors which make sure the IO operations are dword-aligned. > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > --- > > Changelog v2: > - Rename 'syscon' property to 'baikal,bt1-syscon'. > > Changelog v3: > - Use the clocks/resets handlers defined in the DW PCIe core descriptor. > (@Rob) > - Redefine PCI host bridge config space accessors with the generic > pci_generic_config_read32() and pci_generic_config_write32() methods. > (@Rob) > > Changelog v4: > - Drop PCIBIOS_* macros usage. (@Rob) > - Add "static const" to the dw_pcie_ops and dw_pcie_host_ops structure > instances. (@Bjorn) > - Rename bt1_pcie_dw_ops to bt1_pcie_ops. (@Bjorn) > - Rename bt1_pcie_ops to bt1_pci_ops. (@Bjorn) > - Use start_link/stop_link suffixes in the corresponding callbacks. > (@Bjorn) > - Change the get_res() method suffix to being get_resources(). (@Bjorn) > - Change *_{add,del}_dw_port() method to *_{add,del}_port(). (@Bjorn) > - Drop dma_coerce_mask_and_coherent() applied to the PCI host bridge > kernel device instance. (@Bjorn) > - Add the comment above the dma_set_mask_and_coherent() method usage > regarding the controller eDMA feature. (@Bjorn) > - Fix the comment above the core reset controls assertion. (@Bjorn) > - Replace delays and timeout numeric literals with macros. (@Bjorn) > --- > drivers/pci/controller/dwc/Kconfig | 9 + > drivers/pci/controller/dwc/Makefile | 1 + > drivers/pci/controller/dwc/pcie-bt1.c | 653 ++++++++++++++++++++++++++ > 3 files changed, 663 insertions(+) > create mode 100644 drivers/pci/controller/dwc/pcie-bt1.c > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig > index 62ce3abf0f19..771b8b146623 100644 > --- a/drivers/pci/controller/dwc/Kconfig > +++ b/drivers/pci/controller/dwc/Kconfig > @@ -222,6 +222,15 @@ config PCIE_ARTPEC6_EP > Enables support for the PCIe controller in the ARTPEC-6 SoC to work in > endpoint mode. This uses the DesignWare core. > > +config PCIE_BT1 > + tristate "Baikal-T1 PCIe controller" > + depends on MIPS_BAIKAL_T1 || COMPILE_TEST > + depends on PCI_MSI_IRQ_DOMAIN > + select PCIE_DW_HOST > + help > + Enables support for the PCIe controller in the Baikal-T1 SoC to work > + in host mode. It's based on the Synopsys DWC PCIe v4.60a IP-core. > + > config PCIE_ROCKCHIP_DW_HOST > bool "Rockchip DesignWare PCIe controller" > select PCIE_DW > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile > index 8ba7b67f5e50..bf5c311875a1 100644 > --- a/drivers/pci/controller/dwc/Makefile > +++ b/drivers/pci/controller/dwc/Makefile > @@ -3,6 +3,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o > obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o > obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o > obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o > +obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o > obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o > obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o > obj-$(CONFIG_PCIE_FU740) += pcie-fu740.o > diff --git a/drivers/pci/controller/dwc/pcie-bt1.c b/drivers/pci/controller/dwc/pcie-bt1.c > new file mode 100644 > index 000000000000..86b230575ddc > --- /dev/null > +++ b/drivers/pci/controller/dwc/pcie-bt1.c > @@ -0,0 +1,653 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2021 BAIKAL ELECTRONICS, JSC > + * > + * Authors: > + * Vadim Vlasov <Vadim.Vlasov@baikalelectronics.ru> > + * Serge Semin <Sergey.Semin@baikalelectronics.ru> > + * > + * Baikal-T1 PCIe controller driver > + */ > + > +#include <linux/bitfield.h> > +#include <linux/bits.h> > +#include <linux/clk.h> > +#include <linux/delay.h> > +#include <linux/gpio/consumer.h> > +#include <linux/kernel.h> > +#include <linux/mfd/syscon.h> > +#include <linux/module.h> > +#include <linux/pci.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > +#include <linux/reset.h> > +#include <linux/types.h> > + > +#include "pcie-designware.h" > + > +/* Baikal-T1 System CCU control registers */ > +#define BT1_CCU_PCIE_CLKC 0x140 > +#define BT1_CCU_PCIE_REQ_PCS_CLK BIT(16) > +#define BT1_CCU_PCIE_REQ_MAC_CLK BIT(17) > +#define BT1_CCU_PCIE_REQ_PIPE_CLK BIT(18) > + > +#define BT1_CCU_PCIE_RSTC 0x144 > +#define BT1_CCU_PCIE_REQ_LINK_RST BIT(13) > +#define BT1_CCU_PCIE_REQ_SMLH_RST BIT(14) > +#define BT1_CCU_PCIE_REQ_PHY_RST BIT(16) > +#define BT1_CCU_PCIE_REQ_CORE_RST BIT(24) > +#define BT1_CCU_PCIE_REQ_STICKY_RST BIT(26) > +#define BT1_CCU_PCIE_REQ_NSTICKY_RST BIT(27) > + > +#define BT1_CCU_PCIE_PMSC 0x148 > +#define BT1_CCU_PCIE_LTSSM_STATE_MASK GENMASK(5, 0) > +#define BT1_CCU_PCIE_LTSSM_DET_QUIET 0x00 > +#define BT1_CCU_PCIE_LTSSM_DET_ACT 0x01 > +#define BT1_CCU_PCIE_LTSSM_POLL_ACT 0x02 > +#define BT1_CCU_PCIE_LTSSM_POLL_COMP 0x03 > +#define BT1_CCU_PCIE_LTSSM_POLL_CONF 0x04 > +#define BT1_CCU_PCIE_LTSSM_PRE_DET_QUIET 0x05 > +#define BT1_CCU_PCIE_LTSSM_DET_WAIT 0x06 > +#define BT1_CCU_PCIE_LTSSM_CFG_LNKWD_START 0x07 > +#define BT1_CCU_PCIE_LTSSM_CFG_LNKWD_ACEPT 0x08 > +#define BT1_CCU_PCIE_LTSSM_CFG_LNNUM_WAIT 0x09 > +#define BT1_CCU_PCIE_LTSSM_CFG_LNNUM_ACEPT 0x0a > +#define BT1_CCU_PCIE_LTSSM_CFG_COMPLETE 0x0b > +#define BT1_CCU_PCIE_LTSSM_CFG_IDLE 0x0c > +#define BT1_CCU_PCIE_LTSSM_RCVR_LOCK 0x0d > +#define BT1_CCU_PCIE_LTSSM_RCVR_SPEED 0x0e > +#define BT1_CCU_PCIE_LTSSM_RCVR_RCVRCFG 0x0f > +#define BT1_CCU_PCIE_LTSSM_RCVR_IDLE 0x10 > +#define BT1_CCU_PCIE_LTSSM_L0 0x11 > +#define BT1_CCU_PCIE_LTSSM_L0S 0x12 > +#define BT1_CCU_PCIE_LTSSM_L123_SEND_IDLE 0x13 > +#define BT1_CCU_PCIE_LTSSM_L1_IDLE 0x14 > +#define BT1_CCU_PCIE_LTSSM_L2_IDLE 0x15 > +#define BT1_CCU_PCIE_LTSSM_L2_WAKE 0x16 > +#define BT1_CCU_PCIE_LTSSM_DIS_ENTRY 0x17 > +#define BT1_CCU_PCIE_LTSSM_DIS_IDLE 0x18 > +#define BT1_CCU_PCIE_LTSSM_DISABLE 0x19 > +#define BT1_CCU_PCIE_LTSSM_LPBK_ENTRY 0x1a > +#define BT1_CCU_PCIE_LTSSM_LPBK_ACTIVE 0x1b > +#define BT1_CCU_PCIE_LTSSM_LPBK_EXIT 0x1c > +#define BT1_CCU_PCIE_LTSSM_LPBK_EXIT_TOUT 0x1d > +#define BT1_CCU_PCIE_LTSSM_HOT_RST_ENTRY 0x1e > +#define BT1_CCU_PCIE_LTSSM_HOT_RST 0x1f > +#define BT1_CCU_PCIE_LTSSM_RCVR_EQ0 0x20 > +#define BT1_CCU_PCIE_LTSSM_RCVR_EQ1 0x21 > +#define BT1_CCU_PCIE_LTSSM_RCVR_EQ2 0x22 > +#define BT1_CCU_PCIE_LTSSM_RCVR_EQ3 0x23 You could make this an enum and define only the states that are actually used. [...] > + > +/* Generic Baikal-T1 PCIe interface resources */ > +#define BT1_PCIE_NUM_APP_CLKS ARRAY_SIZE(bt1_pcie_app_clks) > +#define BT1_PCIE_NUM_CORE_CLKS ARRAY_SIZE(bt1_pcie_core_clks) > +#define BT1_PCIE_NUM_APP_RSTS ARRAY_SIZE(bt1_pcie_app_rsts) > +#define BT1_PCIE_NUM_CORE_RSTS ARRAY_SIZE(bt1_pcie_core_rsts) > + > +/* PCIe bus setup delays and timeouts */ > +#define BT1_PCIE_RST_DELAY_MS 100 > +#define BT1_PCIE_RUN_DELAY_US 100 > +#define BT1_PCIE_REQ_DELAY_US 1 > +#define BT1_PCIE_REQ_TIMEOUT_US 1000 > +#define BT1_PCIE_LNK_DELAY_US 1000 > +#define BT1_PCIE_LNK_TIMEOUT_US 1000000 > + > +static const enum dw_pcie_app_clk bt1_pcie_app_clks[] = { > + DW_PCIE_DBI_CLK, DW_PCIE_MSTR_CLK, DW_PCIE_SLV_CLK, > +}; > + > +static const enum dw_pcie_core_clk bt1_pcie_core_clks[] = { > + DW_PCIE_REF_CLK, > +}; > + > +static const enum dw_pcie_app_rst bt1_pcie_app_rsts[] = { > + DW_PCIE_MSTR_RST, DW_PCIE_SLV_RST, > +}; > + > +static const enum dw_pcie_core_rst bt1_pcie_core_rsts[] = { > + DW_PCIE_NON_STICKY_RST, DW_PCIE_STICKY_RST, DW_PCIE_CORE_RST, > + DW_PCIE_PIPE_RST, DW_PCIE_PHY_RST, DW_PCIE_HOT_RST, DW_PCIE_PWR_RST, > +}; I wonder whether we could allocate the rst/clks in DWC dynamically, by using these configuration arrays. > + > +struct bt1_pcie { > + struct dw_pcie dw; > + struct platform_device *pdev; > + struct regmap *sys_regs; > +}; > +#define to_bt1_pcie(_dw) container_of(_dw, struct bt1_pcie, dw) > + > +/* > + * Baikal-T1 MMIO space must be read/written by the dword-aligned > + * instructions. Note the methods are optimized to have the dword operations > + * performed with minimum overhead as the most frequently used ones. > + */ > +static int bt1_pcie_read_mmio(void __iomem *addr, int size, u32 *val) > +{ > + unsigned int ofs = (uintptr_t)addr & 0x3; > + > + if (!IS_ALIGNED((uintptr_t)addr, size)) > + return -EINVAL; > + > + *val = readl(addr - ofs) >> ofs * BITS_PER_BYTE; Is it always safe to read more than requested ? > + if (size == 4) { > + return 0; > + } else if (size == 2) { > + *val &= 0xffff; > + return 0; > + } else if (size == 1) { > + *val &= 0xff; > + return 0; > + } > + > + return -EINVAL; > +} > + > +static int bt1_pcie_write_mmio(void __iomem *addr, int size, u32 val) > +{ > + unsigned int ofs = (uintptr_t)addr & 0x3; > + u32 tmp, mask; > + > + if (!IS_ALIGNED((uintptr_t)addr, size)) > + return -EINVAL; > + > + if (size == 4) { > + writel(val, addr); > + return 0; > + } else if (size == 2 || size == 1) { > + mask = GENMASK(size * BITS_PER_BYTE - 1, 0); > + tmp = readl(addr - ofs) & ~(mask << ofs * BITS_PER_BYTE); > + tmp |= (val & mask) << ofs * BITS_PER_BYTE; > + writel(tmp, addr - ofs); > + return 0; > + } Same question read/modify/write, is it always safe to do it regardless of size ? > + > + return -EINVAL; > +} > + > +static u32 bt1_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base, u32 reg, > + size_t size) > +{ > + int ret; > + u32 val; > + > + ret = bt1_pcie_read_mmio(base + reg, size, &val); > + if (ret) { > + dev_err(pci->dev, "Read DBI address failed\n"); > + return ~0U; Is this a special magic value the DWC core is expecting ? Does it clash with a _valid_ return value ? > + } > + > + return val; > +} > + > +static void bt1_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base, u32 reg, > + size_t size, u32 val) > +{ > + int ret; > + > + ret = bt1_pcie_write_mmio(base + reg, size, val); > + if (ret) > + dev_err(pci->dev, "Write DBI address failed\n"); > +} > + > +static void bt1_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base, u32 reg, > + size_t size, u32 val) > +{ > + struct bt1_pcie *btpci = to_bt1_pcie(pci); > + int ret; > + > + regmap_update_bits(btpci->sys_regs, BT1_CCU_PCIE_GENC, > + BT1_CCU_PCIE_DBI2_MODE, BT1_CCU_PCIE_DBI2_MODE); > + > + ret = bt1_pcie_write_mmio(base + reg, size, val); > + if (ret) > + dev_err(pci->dev, "Write DBI2 address failed\n"); > + > + regmap_update_bits(btpci->sys_regs, BT1_CCU_PCIE_GENC, > + BT1_CCU_PCIE_DBI2_MODE, 0); IIUC the regmap_update_bits() set up decoding for DBI2. Hopefully the DBI/DBI2 writes are sequentialized, this is a question valid also for other DWC controllers. What I want to say is, the regmap update in this function sets the DWC HW in a way that can decode DBI2 (please correct me if I am wrong), between the two _update_bits() no DBI access should happen because it just would not work. It is a question. > +static int bt1_pcie_host_init(struct dw_pcie_rp *pp) > +{ > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > + struct bt1_pcie *btpci = to_bt1_pcie(pci); > + int ret; > + > + ret = bt1_pcie_get_resources(btpci); > + if (ret) > + return ret; > + > + bt1_pcie_full_stop_bus(btpci, true); > + > + return bt1_pcie_cold_start_bus(btpci); > +} > + > +static void bt1_pcie_host_deinit(struct dw_pcie_rp *pp) > +{ > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > + struct bt1_pcie *btpci = to_bt1_pcie(pci); > + > + bt1_pcie_full_stop_bus(btpci, false); > +} > + > +static const struct dw_pcie_host_ops bt1_pcie_host_ops = { > + .host_init = bt1_pcie_host_init, > + .host_deinit = bt1_pcie_host_deinit, > +}; > + > +static struct bt1_pcie *bt1_pcie_create_data(struct platform_device *pdev) > +{ > + struct bt1_pcie *btpci; > + > + btpci = devm_kzalloc(&pdev->dev, sizeof(*btpci), GFP_KERNEL); > + if (!btpci) > + return ERR_PTR(-ENOMEM); > + > + btpci->pdev = pdev; > + > + platform_set_drvdata(pdev, btpci); > + > + return btpci; > +} > + > +static int bt1_pcie_add_port(struct bt1_pcie *btpci) > +{ > + struct device *dev = &btpci->pdev->dev; > + int ret; > + > + /* > + * DW PCIe Root Port controller is equipped with eDMA capable of > + * working with the 64-bit memory addresses. > + */ > + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); > + if (ret) > + return ret; Is this the right place to set the DMA mask for the host controller embedded DMA controller (actually, the dev pointer is the _host_ controller device) ? How this is going to play when combined with: https://lore.kernel.org/linux-pci/1e63a581-14ae-b4b5-a5bf-ca8f09c33af6@arm.com It is getting a bit confusing. I believe the code in the link above sets the mask so that through the DMA API we are capable of getting an MSI doorbell virtual address whose physical address can be addressed by the endpoint; this through the DMA API. This patch is setting the DMA mask for a different reason, namely setting the host controller embedded DMA controller addressing capabilities. AFAICS - both approaches set the mask for the same device - now the question is about which one is legitimate and how to handle the other. > + > + btpci->dw.version = DW_PCIE_VER_460A; > + btpci->dw.dev = dev; > + btpci->dw.ops = &bt1_pcie_ops; > + > + btpci->dw.pp.num_vectors = MAX_MSI_IRQS; > + btpci->dw.pp.ops = &bt1_pcie_host_ops; > + > + dw_pcie_cap_set(&btpci->dw, REQ_RES); > + > + ret = dw_pcie_host_init(&btpci->dw.pp); > + if (ret) > + dev_err_probe(dev, ret, "Failed to initialize DWC PCIe host\n"); > + > + return ret; > +} > + > +static void bt1_pcie_del_port(struct bt1_pcie *btpci) > +{ > + dw_pcie_host_deinit(&btpci->dw.pp); > +} > + > +static int bt1_pcie_probe(struct platform_device *pdev) > +{ > + struct bt1_pcie *btpci; > + > + btpci = bt1_pcie_create_data(pdev); Do we really need a function for that ? I am not too bothered but I think it is overkill. Thanks, Lorenzo > + if (IS_ERR(btpci)) > + return PTR_ERR(btpci); > + > + return bt1_pcie_add_port(btpci); > +} > + > +static int bt1_pcie_remove(struct platform_device *pdev) > +{ > + struct bt1_pcie *btpci = platform_get_drvdata(pdev); > + > + bt1_pcie_del_port(btpci); > + > + return 0; > +} > + > +static const struct of_device_id bt1_pcie_of_match[] = { > + { .compatible = "baikal,bt1-pcie" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, bt1_pcie_of_match); > + > +static struct platform_driver bt1_pcie_driver = { > + .probe = bt1_pcie_probe, > + .remove = bt1_pcie_remove, > + .driver = { > + .name = "bt1-pcie", > + .of_match_table = bt1_pcie_of_match, > + }, > +}; > +module_platform_driver(bt1_pcie_driver); > + > +MODULE_AUTHOR("Serge Semin <Sergey.Semin@baikalelectronics.ru>"); > +MODULE_DESCRIPTION("Baikal-T1 PCIe driver"); > +MODULE_LICENSE("GPL"); > -- > 2.35.1 >
On 08/29/2022, Lorenzo Pieralisi wrote: > [+Robin, Will - please jump to DMA mask set-up] > > On Mon, Aug 22, 2022 at 09:47:01PM +0300, Serge Semin wrote: > > Baikal-T1 SoC is equipped with DWC PCIe v4.60a host controller. It can be > > trained to work up to Gen.3 speed over up to x4 lanes. The host controller > > is attached to the DW PCIe 3.0 PCS via the PIPE-4 interface, which in its > > turn is connected to the DWC 10G PHY. The whole system is supposed to be > > fed up with four clock sources: DBI peripheral clock, AXI application > > clocks and external PHY/core reference clock generating the 100MHz signal. > > In addition to that the platform provide a way to reset each part of the > > controller: sticky/non-sticky bits, host controller core, PIPE interface, > > PCS/PHY and Hot/Power reset signal. The driver also provides a way to > > handle the GPIO-based PERST# signal. > > > > Note due to the Baikal-T1 MMIO peculiarity we have to implement the DBI > > interface accessors which make sure the IO operations are dword-aligned. > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > > --- > > > > Changelog v2: > > - Rename 'syscon' property to 'baikal,bt1-syscon'. > > > > Changelog v3: > > - Use the clocks/resets handlers defined in the DW PCIe core descriptor. > > (@Rob) > > - Redefine PCI host bridge config space accessors with the generic > > pci_generic_config_read32() and pci_generic_config_write32() methods. > > (@Rob) > > > > Changelog v4: > > - Drop PCIBIOS_* macros usage. (@Rob) > > - Add "static const" to the dw_pcie_ops and dw_pcie_host_ops structure > > instances. (@Bjorn) > > - Rename bt1_pcie_dw_ops to bt1_pcie_ops. (@Bjorn) > > - Rename bt1_pcie_ops to bt1_pci_ops. (@Bjorn) > > - Use start_link/stop_link suffixes in the corresponding callbacks. > > (@Bjorn) > > - Change the get_res() method suffix to being get_resources(). (@Bjorn) > > - Change *_{add,del}_dw_port() method to *_{add,del}_port(). (@Bjorn) > > - Drop dma_coerce_mask_and_coherent() applied to the PCI host bridge > > kernel device instance. (@Bjorn) > > - Add the comment above the dma_set_mask_and_coherent() method usage > > regarding the controller eDMA feature. (@Bjorn) > > - Fix the comment above the core reset controls assertion. (@Bjorn) > > - Replace delays and timeout numeric literals with macros. (@Bjorn) > > --- > > drivers/pci/controller/dwc/Kconfig | 9 + > > drivers/pci/controller/dwc/Makefile | 1 + > > drivers/pci/controller/dwc/pcie-bt1.c | 653 ++++++++++++++++++++++++++ > > 3 files changed, 663 insertions(+) > > create mode 100644 drivers/pci/controller/dwc/pcie-bt1.c > > > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig > > index 62ce3abf0f19..771b8b146623 100644 > > --- a/drivers/pci/controller/dwc/Kconfig > > +++ b/drivers/pci/controller/dwc/Kconfig > > @@ -222,6 +222,15 @@ config PCIE_ARTPEC6_EP > > Enables support for the PCIe controller in the ARTPEC-6 SoC to work in > > endpoint mode. This uses the DesignWare core. > > > > +config PCIE_BT1 > > + tristate "Baikal-T1 PCIe controller" > > + depends on MIPS_BAIKAL_T1 || COMPILE_TEST > > + depends on PCI_MSI_IRQ_DOMAIN > > + select PCIE_DW_HOST > > + help > > + Enables support for the PCIe controller in the Baikal-T1 SoC to work > > + in host mode. It's based on the Synopsys DWC PCIe v4.60a IP-core. > > + > > config PCIE_ROCKCHIP_DW_HOST > > bool "Rockchip DesignWare PCIe controller" > > select PCIE_DW > > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile > > index 8ba7b67f5e50..bf5c311875a1 100644 > > --- a/drivers/pci/controller/dwc/Makefile > > +++ b/drivers/pci/controller/dwc/Makefile > > @@ -3,6 +3,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o > > obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o > > obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o > > obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o > > +obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o > > obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o > > obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o > > obj-$(CONFIG_PCIE_FU740) += pcie-fu740.o > > diff --git a/drivers/pci/controller/dwc/pcie-bt1.c b/drivers/pci/controller/dwc/pcie-bt1.c > > new file mode 100644 > > index 000000000000..86b230575ddc > > --- /dev/null > > +++ b/drivers/pci/controller/dwc/pcie-bt1.c > > @@ -0,0 +1,653 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (C) 2021 BAIKAL ELECTRONICS, JSC > > + * > > + * Authors: > > + * Vadim Vlasov <Vadim.Vlasov@baikalelectronics.ru> > > + * Serge Semin <Sergey.Semin@baikalelectronics.ru> > > + * > > + * Baikal-T1 PCIe controller driver > > + */ > > + > > +#include <linux/bitfield.h> > > +#include <linux/bits.h> > > +#include <linux/clk.h> > > +#include <linux/delay.h> > > +#include <linux/gpio/consumer.h> > > +#include <linux/kernel.h> > > +#include <linux/mfd/syscon.h> > > +#include <linux/module.h> > > +#include <linux/pci.h> > > +#include <linux/platform_device.h> > > +#include <linux/regmap.h> > > +#include <linux/reset.h> > > +#include <linux/types.h> > > + > > +#include "pcie-designware.h" > > + > > +/* Baikal-T1 System CCU control registers */ > > +#define BT1_CCU_PCIE_CLKC 0x140 > > +#define BT1_CCU_PCIE_REQ_PCS_CLK BIT(16) > > +#define BT1_CCU_PCIE_REQ_MAC_CLK BIT(17) > > +#define BT1_CCU_PCIE_REQ_PIPE_CLK BIT(18) > > + > > +#define BT1_CCU_PCIE_RSTC 0x144 > > +#define BT1_CCU_PCIE_REQ_LINK_RST BIT(13) > > +#define BT1_CCU_PCIE_REQ_SMLH_RST BIT(14) > > +#define BT1_CCU_PCIE_REQ_PHY_RST BIT(16) > > +#define BT1_CCU_PCIE_REQ_CORE_RST BIT(24) > > +#define BT1_CCU_PCIE_REQ_STICKY_RST BIT(26) > > +#define BT1_CCU_PCIE_REQ_NSTICKY_RST BIT(27) > > + > > +#define BT1_CCU_PCIE_PMSC 0x148 > > +#define BT1_CCU_PCIE_LTSSM_STATE_MASK GENMASK(5, 0) > > +#define BT1_CCU_PCIE_LTSSM_DET_QUIET 0x00 > > +#define BT1_CCU_PCIE_LTSSM_DET_ACT 0x01 > > +#define BT1_CCU_PCIE_LTSSM_POLL_ACT 0x02 > > +#define BT1_CCU_PCIE_LTSSM_POLL_COMP 0x03 > > +#define BT1_CCU_PCIE_LTSSM_POLL_CONF 0x04 > > +#define BT1_CCU_PCIE_LTSSM_PRE_DET_QUIET 0x05 > > +#define BT1_CCU_PCIE_LTSSM_DET_WAIT 0x06 > > +#define BT1_CCU_PCIE_LTSSM_CFG_LNKWD_START 0x07 > > +#define BT1_CCU_PCIE_LTSSM_CFG_LNKWD_ACEPT 0x08 > > +#define BT1_CCU_PCIE_LTSSM_CFG_LNNUM_WAIT 0x09 > > +#define BT1_CCU_PCIE_LTSSM_CFG_LNNUM_ACEPT 0x0a > > +#define BT1_CCU_PCIE_LTSSM_CFG_COMPLETE 0x0b > > +#define BT1_CCU_PCIE_LTSSM_CFG_IDLE 0x0c > > +#define BT1_CCU_PCIE_LTSSM_RCVR_LOCK 0x0d > > +#define BT1_CCU_PCIE_LTSSM_RCVR_SPEED 0x0e > > +#define BT1_CCU_PCIE_LTSSM_RCVR_RCVRCFG 0x0f > > +#define BT1_CCU_PCIE_LTSSM_RCVR_IDLE 0x10 > > +#define BT1_CCU_PCIE_LTSSM_L0 0x11 > > +#define BT1_CCU_PCIE_LTSSM_L0S 0x12 > > +#define BT1_CCU_PCIE_LTSSM_L123_SEND_IDLE 0x13 > > +#define BT1_CCU_PCIE_LTSSM_L1_IDLE 0x14 > > +#define BT1_CCU_PCIE_LTSSM_L2_IDLE 0x15 > > +#define BT1_CCU_PCIE_LTSSM_L2_WAKE 0x16 > > +#define BT1_CCU_PCIE_LTSSM_DIS_ENTRY 0x17 > > +#define BT1_CCU_PCIE_LTSSM_DIS_IDLE 0x18 > > +#define BT1_CCU_PCIE_LTSSM_DISABLE 0x19 > > +#define BT1_CCU_PCIE_LTSSM_LPBK_ENTRY 0x1a > > +#define BT1_CCU_PCIE_LTSSM_LPBK_ACTIVE 0x1b > > +#define BT1_CCU_PCIE_LTSSM_LPBK_EXIT 0x1c > > +#define BT1_CCU_PCIE_LTSSM_LPBK_EXIT_TOUT 0x1d > > +#define BT1_CCU_PCIE_LTSSM_HOT_RST_ENTRY 0x1e > > +#define BT1_CCU_PCIE_LTSSM_HOT_RST 0x1f > > +#define BT1_CCU_PCIE_LTSSM_RCVR_EQ0 0x20 > > +#define BT1_CCU_PCIE_LTSSM_RCVR_EQ1 0x21 > > +#define BT1_CCU_PCIE_LTSSM_RCVR_EQ2 0x22 > > +#define BT1_CCU_PCIE_LTSSM_RCVR_EQ3 0x23 > > You could make this an enum and define only the states > that are actually used. > > [...] > > > + > > +/* Generic Baikal-T1 PCIe interface resources */ > > +#define BT1_PCIE_NUM_APP_CLKS ARRAY_SIZE(bt1_pcie_app_clks) > > +#define BT1_PCIE_NUM_CORE_CLKS ARRAY_SIZE(bt1_pcie_core_clks) > > +#define BT1_PCIE_NUM_APP_RSTS ARRAY_SIZE(bt1_pcie_app_rsts) > > +#define BT1_PCIE_NUM_CORE_RSTS ARRAY_SIZE(bt1_pcie_core_rsts) > > + > > +/* PCIe bus setup delays and timeouts */ > > +#define BT1_PCIE_RST_DELAY_MS 100 > > +#define BT1_PCIE_RUN_DELAY_US 100 > > +#define BT1_PCIE_REQ_DELAY_US 1 > > +#define BT1_PCIE_REQ_TIMEOUT_US 1000 > > +#define BT1_PCIE_LNK_DELAY_US 1000 > > +#define BT1_PCIE_LNK_TIMEOUT_US 1000000 > > + > > +static const enum dw_pcie_app_clk bt1_pcie_app_clks[] = { > > + DW_PCIE_DBI_CLK, DW_PCIE_MSTR_CLK, DW_PCIE_SLV_CLK, > > +}; > > + > > +static const enum dw_pcie_core_clk bt1_pcie_core_clks[] = { > > + DW_PCIE_REF_CLK, > > +}; > > + > > +static const enum dw_pcie_app_rst bt1_pcie_app_rsts[] = { > > + DW_PCIE_MSTR_RST, DW_PCIE_SLV_RST, > > +}; > > + > > +static const enum dw_pcie_core_rst bt1_pcie_core_rsts[] = { > > + DW_PCIE_NON_STICKY_RST, DW_PCIE_STICKY_RST, DW_PCIE_CORE_RST, > > + DW_PCIE_PIPE_RST, DW_PCIE_PHY_RST, DW_PCIE_HOT_RST, DW_PCIE_PWR_RST, > > +}; > > I wonder whether we could allocate the rst/clks in DWC dynamically, > by using these configuration arrays. > > > + > > +struct bt1_pcie { > > + struct dw_pcie dw; > > + struct platform_device *pdev; > > + struct regmap *sys_regs; > > +}; > > +#define to_bt1_pcie(_dw) container_of(_dw, struct bt1_pcie, dw) > > + > > +/* > > + * Baikal-T1 MMIO space must be read/written by the dword-aligned > > + * instructions. Note the methods are optimized to have the dword operations > > + * performed with minimum overhead as the most frequently used ones. > > + */ > > +static int bt1_pcie_read_mmio(void __iomem *addr, int size, u32 *val) > > +{ > > + unsigned int ofs = (uintptr_t)addr & 0x3; > > + > > + if (!IS_ALIGNED((uintptr_t)addr, size)) > > + return -EINVAL; > > + > > + *val = readl(addr - ofs) >> ofs * BITS_PER_BYTE; > > Is it always safe to read more than requested ? > > > + if (size == 4) { > > + return 0; > > + } else if (size == 2) { > > + *val &= 0xffff; > > + return 0; > > + } else if (size == 1) { > > + *val &= 0xff; > > + return 0; > > + } > > + > > + return -EINVAL; > > +} > > + > > +static int bt1_pcie_write_mmio(void __iomem *addr, int size, u32 val) > > +{ > > + unsigned int ofs = (uintptr_t)addr & 0x3; > > + u32 tmp, mask; > > + > > + if (!IS_ALIGNED((uintptr_t)addr, size)) > > + return -EINVAL; > > + > > + if (size == 4) { > > + writel(val, addr); > > + return 0; > > + } else if (size == 2 || size == 1) { > > + mask = GENMASK(size * BITS_PER_BYTE - 1, 0); > > + tmp = readl(addr - ofs) & ~(mask << ofs * BITS_PER_BYTE); > > + tmp |= (val & mask) << ofs * BITS_PER_BYTE; > > + writel(tmp, addr - ofs); > > + return 0; > > + } > > Same question read/modify/write, is it always safe to do it > regardless of size ? > > > + > > + return -EINVAL; > > +} > > + > > +static u32 bt1_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base, u32 reg, > > + size_t size) > > +{ > > + int ret; > > + u32 val; > > + > > + ret = bt1_pcie_read_mmio(base + reg, size, &val); > > + if (ret) { > > + dev_err(pci->dev, "Read DBI address failed\n"); > > + return ~0U; > > Is this a special magic value the DWC core is expecting ? > > Does it clash with a _valid_ return value ? > > > + } > > + > > + return val; > > +} > > + > > +static void bt1_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base, u32 reg, > > + size_t size, u32 val) > > +{ > > + int ret; > > + > > + ret = bt1_pcie_write_mmio(base + reg, size, val); > > + if (ret) > > + dev_err(pci->dev, "Write DBI address failed\n"); > > +} > > + > > +static void bt1_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base, u32 reg, > > + size_t size, u32 val) > > +{ > > + struct bt1_pcie *btpci = to_bt1_pcie(pci); > > + int ret; > > + > > + regmap_update_bits(btpci->sys_regs, BT1_CCU_PCIE_GENC, > > + BT1_CCU_PCIE_DBI2_MODE, BT1_CCU_PCIE_DBI2_MODE); > > + > > + ret = bt1_pcie_write_mmio(base + reg, size, val); > > + if (ret) > > + dev_err(pci->dev, "Write DBI2 address failed\n"); > > + > > + regmap_update_bits(btpci->sys_regs, BT1_CCU_PCIE_GENC, > > + BT1_CCU_PCIE_DBI2_MODE, 0); > > IIUC the regmap_update_bits() set up decoding for DBI2. Hopefully the > DBI/DBI2 writes are sequentialized, this is a question valid also > for other DWC controllers. > > What I want to say is, the regmap update in this function sets the > DWC HW in a way that can decode DBI2 (please correct me if I am wrong), > between the two _update_bits() no DBI access should happen because > it just would not work. > > It is a question. > > > +static int bt1_pcie_host_init(struct dw_pcie_rp *pp) > > +{ > > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > > + struct bt1_pcie *btpci = to_bt1_pcie(pci); > > + int ret; > > + > > + ret = bt1_pcie_get_resources(btpci); > > + if (ret) > > + return ret; > > + > > + bt1_pcie_full_stop_bus(btpci, true); > > + > > + return bt1_pcie_cold_start_bus(btpci); > > +} > > + > > +static void bt1_pcie_host_deinit(struct dw_pcie_rp *pp) > > +{ > > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > > + struct bt1_pcie *btpci = to_bt1_pcie(pci); > > + > > + bt1_pcie_full_stop_bus(btpci, false); > > +} > > + > > +static const struct dw_pcie_host_ops bt1_pcie_host_ops = { > > + .host_init = bt1_pcie_host_init, > > + .host_deinit = bt1_pcie_host_deinit, > > +}; > > + > > +static struct bt1_pcie *bt1_pcie_create_data(struct platform_device *pdev) > > +{ > > + struct bt1_pcie *btpci; > > + > > + btpci = devm_kzalloc(&pdev->dev, sizeof(*btpci), GFP_KERNEL); > > + if (!btpci) > > + return ERR_PTR(-ENOMEM); > > + > > + btpci->pdev = pdev; > > + > > + platform_set_drvdata(pdev, btpci); > > + > > + return btpci; > > +} > > + > > +static int bt1_pcie_add_port(struct bt1_pcie *btpci) > > +{ > > + struct device *dev = &btpci->pdev->dev; > > + int ret; > > + > > + /* > > + * DW PCIe Root Port controller is equipped with eDMA capable of > > + * working with the 64-bit memory addresses. > > + */ > > + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); > > + if (ret) > > + return ret; > > Is this the right place to set the DMA mask for the host controller > embedded DMA controller (actually, the dev pointer is the _host_ > controller device) ? > > How this is going to play when combined with: > > https://lore.kernel.org/linux-pci/1e63a581-14ae-b4b5-a5bf-ca8f09c33af6@arm.com > > It is getting a bit confusing. I believe the code in the link > above sets the mask so that through the DMA API we are capable > of getting an MSI doorbell virtual address whose physical address > can be addressed by the endpoint; this through the DMA API. > > This patch is setting the DMA mask for a different reason, namely > setting the host controller embedded DMA controller addressing > capabilities. > > AFAICS - both approaches set the mask for the same device - now > the question is about which one is legitimate and how to handle > the other. Since the mask is being set before calling dw_pcie_host_init() and the msi_host_init op is not set, setting the mask here won't have any affect due to the fact that dw_pcie_msi_host_init() unconditionally sets the DMA mask. AFAIK, you don't technically need a 64-bit address, right? If your platform disables 32-bit allocations, then you'll need to set the MSI capabilities flag to support a 64-bit allocation for when the 32-bit allocation fails in the host controller driver. Here is how I'm doing that with the Exynos PCIe driver during probe before calling dw_pcie_host_init(): dw_pcie_dbi_ro_wr_en(exynos_pcie->pci); offset = dw_pcie_find_capability(exynos_pcie->pci, PCI_CAP_ID_MSI); cap = dw_pcie_readw_dbi(exynos_pcie->pci, offset + PCI_MSI_FLAGS); /* Enable MSI with 64-bit support */ cap |= PCI_MSI_FLAGS_ENABLE | PCI_MSI_FLAGS_64BIT; dw_pcie_writew_dbi(exynos_pcie->pci, offset + PCI_MSI_FLAGS, cap); dw_pcie_dbi_ro_wr_dis(exynos_pcie->pci); I hope that helps! Regards, Will > > > + > > + btpci->dw.version = DW_PCIE_VER_460A; > > + btpci->dw.dev = dev; > > + btpci->dw.ops = &bt1_pcie_ops; > > + > > + btpci->dw.pp.num_vectors = MAX_MSI_IRQS; > > + btpci->dw.pp.ops = &bt1_pcie_host_ops; > > + > > + dw_pcie_cap_set(&btpci->dw, REQ_RES); > > + > > + ret = dw_pcie_host_init(&btpci->dw.pp); > > + if (ret) > > + dev_err_probe(dev, ret, "Failed to initialize DWC PCIe host\n"); > > + > > + return ret; > > +} > > + > > +static void bt1_pcie_del_port(struct bt1_pcie *btpci) > > +{ > > + dw_pcie_host_deinit(&btpci->dw.pp); > > +} > > + > > +static int bt1_pcie_probe(struct platform_device *pdev) > > +{ > > + struct bt1_pcie *btpci; > > + > > + btpci = bt1_pcie_create_data(pdev); > > Do we really need a function for that ? I am not too > bothered but I think it is overkill. > > Thanks, > Lorenzo > > > + if (IS_ERR(btpci)) > > + return PTR_ERR(btpci); > > + > > + return bt1_pcie_add_port(btpci); > > +} > > + > > +static int bt1_pcie_remove(struct platform_device *pdev) > > +{ > > + struct bt1_pcie *btpci = platform_get_drvdata(pdev); > > + > > + bt1_pcie_del_port(btpci); > > + > > + return 0; > > +} > > + > > +static const struct of_device_id bt1_pcie_of_match[] = { > > + { .compatible = "baikal,bt1-pcie" }, > > + {}, > > +}; > > +MODULE_DEVICE_TABLE(of, bt1_pcie_of_match); > > + > > +static struct platform_driver bt1_pcie_driver = { > > + .probe = bt1_pcie_probe, > > + .remove = bt1_pcie_remove, > > + .driver = { > > + .name = "bt1-pcie", > > + .of_match_table = bt1_pcie_of_match, > > + }, > > +}; > > +module_platform_driver(bt1_pcie_driver); > > + > > +MODULE_AUTHOR("Serge Semin <Sergey.Semin@baikalelectronics.ru>"); > > +MODULE_DESCRIPTION("Baikal-T1 PCIe driver"); > > +MODULE_LICENSE("GPL"); > > -- > > 2.35.1 > >
On Mon, Aug 22, 2022 at 09:46:43PM +0300, Serge Semin wrote: > In accordance with the way the device DT-node is actually defined in > arch/arm64/boot/dts/toshiba/tmpv7708.dtsi and the way the device is probed > by the DW PCIe driver there are two IRQs it actually has. It's MSI IRQ the > DT-bindings lack. Let's extend the interrupts property constraints then > and fix the schema example so one would be acceptable by the actual device > DT-bindings. > > Fixes: 17c1b16340f0 ("dt-bindings: pci: Add DT binding for Toshiba Visconti PCIe controller") > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > --- > > Changelog v5: > - This is a new patch added on the v5 release of the patchset. > --- > .../devicetree/bindings/pci/toshiba,visconti-pcie.yaml | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) No need for this to be in this series. Acked-by: Rob Herring <robh@kernel.org>
On 2022-08-29 16:28, Lorenzo Pieralisi wrote: [...] >> +static int bt1_pcie_add_port(struct bt1_pcie *btpci) >> +{ >> + struct device *dev = &btpci->pdev->dev; >> + int ret; >> + >> + /* >> + * DW PCIe Root Port controller is equipped with eDMA capable of >> + * working with the 64-bit memory addresses. >> + */ >> + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); >> + if (ret) >> + return ret; > > Is this the right place to set the DMA mask for the host controller > embedded DMA controller (actually, the dev pointer is the _host_ > controller device) ? > > How this is going to play when combined with: > > https://lore.kernel.org/linux-pci/1e63a581-14ae-b4b5-a5bf-ca8f09c33af6@arm.com > > It is getting a bit confusing. I believe the code in the link > above sets the mask so that through the DMA API we are capable > of getting an MSI doorbell virtual address whose physical address > can be addressed by the endpoint; this through the DMA API. > > This patch is setting the DMA mask for a different reason, namely > setting the host controller embedded DMA controller addressing > capabilities. > > AFAICS - both approaches set the mask for the same device - now > the question is about which one is legitimate and how to handle > the other. Assuming the dw-edma-pcie driver is the relevant one, that already sets its own masks on its own device, so I also don't see why this is here. Thanks, Robin.
On Mon, Aug 22, 2022 at 09:46:54PM +0300, Serge Semin wrote: > As the DT-bindings description states the Rockchip PCIe controller is > based on the DW PCIe RP IP-core thus its DT-nodes are supposed to be > compatible with the common DW PCIe controller schema. Let's make sure they > are evaluated against it by referring to the snps,dw-pcie.yaml schema in > the allOf sub-schemas composition. > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > --- > > Changelog v3: > - This is a new patch created on v3 lap of the series. > > Changelog v5: > - Apply snps,dw-pcie.yaml instead of the snps,dw-pcie-common.yaml schema. > --- > Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Shouldn't this come before/after patch 7? Reviewed-by: Rob Herring <robh@kernel.org>
On Wed, Aug 31, 2022 at 04:26:31PM -0500, Rob Herring wrote: > On Mon, Aug 22, 2022 at 09:46:54PM +0300, Serge Semin wrote: > > As the DT-bindings description states the Rockchip PCIe controller is > > based on the DW PCIe RP IP-core thus its DT-nodes are supposed to be > > compatible with the common DW PCIe controller schema. Let's make sure they > > are evaluated against it by referring to the snps,dw-pcie.yaml schema in > > the allOf sub-schemas composition. > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > > --- > > > > Changelog v3: > > - This is a new patch created on v3 lap of the series. > > > > Changelog v5: > > - Apply snps,dw-pcie.yaml instead of the snps,dw-pcie-common.yaml schema. > > --- > > Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > Shouldn't this come before/after patch 7? It must be applied after the patch [PATCH v5 11/20] dt-bindings: PCI: dwc: Add clocks/resets common properties and after the rest of the resource-related patches submitted before that one. -Sergey > > Reviewed-by: Rob Herring <robh@kernel.org>
On Mon, Aug 29, 2022 at 12:09:24PM +0200, Lorenzo Pieralisi wrote: > On Mon, Aug 22, 2022 at 09:46:41PM +0300, Serge Semin wrote: > > This patchset is a third one in the series created in the framework of > > my Baikal-T1 PCIe/eDMA-related work: > > > > [1: Done v5] PCI: dwc: Various fixes and cleanups > > Link: https://lore.kernel.org/linux-pci/20220624143428.8334-1-Sergey.Semin@baikalelectronics.ru/ > > Merged: kernel 6.0-rc1 > > [2: Done v4] PCI: dwc: Add hw version and dma-ranges support > > Link: https://lore.kernel.org/linux-pci/20220624143947.8991-1-Sergey.Semin@baikalelectronics.ru > > Merged: kernel 6.0-rc1 > > [3: In-review v5] PCI: dwc: Add generic resources and Baikal-T1 support > > Link: ---you are looking at it--- > > [4: Done v4] dmaengine: dw-edma: Add RP/EP local DMA support > > Link: https://lore.kernel.org/linux-pci/20220728142841.12305-1-Sergey.Semin@baikalelectronics.ru/ > > > > Note it is very recommended to merge the patchsets in the same order as > > they are listed in the set above in order to have them applied smoothly. > > Nothing prevents them from being reviewed synchronously though. > > > > Originally the patches submitted in this patchset were a part of the series: > > Link: https://lore.kernel.org/linux-pci/20220503214638.1895-1-Sergey.Semin@baikalelectronics.ru/ > > but due to the reviewers requests the series was expanded to about 30 > > patches which made it too bulky for a comfortable review. So I decided to > > split it up into two patchsets: 2. and 3. in the table above. > > > > Regarding the series content. This patchset is mainly about adding new DW > > PCIe platform support - Baikal-T1 PCIe of DW PCIe v4.60a IP-core. But a > > set of feature-reach preparations are done first. It starts from > > converting the currently available DT-schema into a more flexible schemas > > hierarchy with separately defined regs, clocks, resets and interrupts > > properties. As a result the common schema can be easily re-used by all the > > currently available platforms while the named properties above can be > > either re-defined or used as is if the platforms support they. In the > > framework of that modification we also suggest to add a set of generic > > regs, clocks, resets and interrupts resource names in accordance with what > > the DW PCIe hardware reference manual describes and what the DW PCIe core > > driver already expects to be specified. Thus the new platform driver will > > be able to re-use the common resources infrastructure. > > > > Link: https://lore.kernel.org/linux-pci/20220324013734.18234-1-Sergey.Semin@baikalelectronics.ru/ > > Changelog v2: > > - Rename 'syscon' property to 'baikal,bt1-syscon'. (@Rob) > > - Move the iATU region selection procedure into a helper function (@Rob). > > - Rebase from kernel v5.17 onto v5.18-rc3 since the later kernel has > > already DT bindings converted. (@Rob) > > - Use 'definitions' property instead of the '$defs' one. It fixes the > > dt-validate error: 'X is not of type array.' > > - Drop 'interrupts' and 'interrupt-names' property from being required > > for the native DW PCIe host. > > - Evaluate the 'snps,dw-pcie-common.yaml' schema in the > > 'socionext,uniphier-pcie-ep.yaml' DT-bindings since the later has > > platform-specific names defined. > > > > Link: https://lore.kernel.org/linux-pci/20220503225104.12108-1-Sergey.Semin@baikalelectronics.ru > > Changelog v3: > > - Split up the patch "dt-bindings: PCI: dwc: Define common and native DT > > bindings" into a series of modifications. (@Rob) > > - Detach this series of the patches into a dedicated patchset. > > - Add a new feature patch: "PCI: dwc: Introduce generic controller > > capabilities interface". > > - Add a new feature patch: "PCI: dwc: Introduce generic resources getter". > > - Add a new cleanup patch: "PCI: dwc: Combine iATU detection procedures". > > - Add a method to at least request the generic clocks and resets. (@Rob) > > - Add GPIO-based PERST# signal support to the core module. > > - Redefine Baikal-T1 PCIe host bridge config space accessors with the > > pci_generic_config_read32() and pci_generic_config_write32() methods. > > (@Rob) > > - Drop synonymous from the names list in the common DT-schema since the > > device sub-schemas create their own enumerations anyway. > > - Rebase onto kernel v5.18. > > > > Link: https://lore.kernel.org/linux-pci/20220610085706.15741-1-Sergey.Semin@baikalelectronics.ru/ > > Changelog v4: > > - Drop PCIBIOS_* macros usage. (@Rob) > > - Add "static const" to the dw_pcie_ops and dw_pcie_host_ops structure > > instances. (@Bjorn) > > - Rename bt1_pcie_dw_ops to bt1_pcie_ops. (@Bjorn) > > - Rename bt1_pcie_ops to bt1_pci_ops. (@Bjorn) > > - Use start_link/stop_link suffixes in the Baikal-T1 PCIe > > start/stop link callbacks. (@Bjorn) > > - Change the get_res() method suffix to being get_resources(). (@Bjorn) > > - Change *_{add,del}_dw_port() method to *_{add,del}_port(). (@Bjorn) > > - Drop dma_coerce_mask_and_coherent() applied to the PCI host bridge > > kernel device instance. (@Bjorn) > > - Add the comment above the dma_set_mask_and_coherent() method usage > > regarding the controller eDMA feature. (@Bjorn) > > - Fix the comment above the core reset controls assertion. (@Bjorn) > > - Replace delays and timeout numeric literals with macros. (@Bjorn) > > - Convert the method name from dw_pcie_get_res() to > > dw_pcie_get_resources(). (@Bjorn) > > - Rebase onto the kernel v5.19-rcX. > > > > Link: https://lore.kernel.org/linux-pci/20220728143427.13617-1-Sergey.Semin@baikalelectronics.ru/ > > Changelog v5: > > - Add a note about having line-based PHY phandles order. (@Rob) > > - Prefer 'pcie[0-9]+' PHY-names over the rest of the cases. (@Rob) > > - Drop generic fallback names from the Baikal-T1 compatible property > > constraints. (@Rob) > > - Define ordered {reg,interrupt,clock,reset}-names Baikal-T1 PCIe > > properties. (@Rob) > > - Drop minItems from the Baikal-T1 PCIe clocks and reset properties, > > since it equals to the maxItems for them. > > - Drop num-ob-windows and num-ib-windows properties constraint from > > Baikal-T1 PCIe bindings. (@Rob) > > - Add a note about having line-based PHY phandles order. (@Rob) > > - Prefer 'pcie[0-9]+' PHY-names over the rest of the cases. (@Rob) > > - Add platform-specific reg/interrupt/clock/reset names to the generic > > schema, but mark them as deprecated. > > - Add new patches: > > dt-bindings: visconti-pcie: Fix interrupts array max constraints > > dt-bindings: imx6q-pcie: Fix clock names for imx6sx and imx8mq > > Are these two new patches linked to the remainder of the series ? If they weren't I would have submitted them separately. They are required for the DW PCIe DT-part of the series to work correctly. -Sergey > > Thanks, > Lorenzo > > > - Move the patch: > > PCI: dwc: Introduce dma-ranges property support for RC-host > > from the previous patchset in here. (@Bjorn) > > - Rebase onto the kernel v6.0-rc2. > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> > > Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru> > > Cc: "Krzysztof Wilczyński" <kw@linux.com> > > Cc: Frank Li <Frank.Li@nxp.com> > > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > Cc: linux-pci@vger.kernel.org > > Cc: devicetree@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org > > > > Serge Semin (20): > > dt-bindings: imx6q-pcie: Fix clock names for imx6sx and imx8mq > > dt-bindings: visconti-pcie: Fix interrupts array max constraints > > dt-bindings: PCI: dwc: Detach common RP/EP DT bindings > > dt-bindings: PCI: dwc: Remove bus node from the examples > > dt-bindings: PCI: dwc: Add phys/phy-names common properties > > dt-bindings: PCI: dwc: Add max-link-speed common property > > dt-bindings: PCI: dwc: Apply generic schema for generic device only > > dt-bindings: PCI: dwc: Add max-functions EP property > > dt-bindings: PCI: dwc: Add interrupts/interrupt-names common > > properties > > dt-bindings: PCI: dwc: Add reg/reg-names common properties > > dt-bindings: PCI: dwc: Add clocks/resets common properties > > dt-bindings: PCI: dwc: Add dma-coherent property > > dt-bindings: PCI: dwc: Apply common schema to Rockchip DW PCIe nodes > > dt-bindings: PCI: dwc: Add Baikal-T1 PCIe Root Port bindings > > PCI: dwc: Introduce dma-ranges property support for RC-host > > PCI: dwc: Introduce generic controller capabilities interface > > PCI: dwc: Introduce generic resources getter > > PCI: dwc: Combine iATU detection procedures > > PCI: dwc: Introduce generic platform clocks and resets > > PCI: dwc: Add Baikal-T1 PCIe controller support > > > > .../bindings/pci/baikal,bt1-pcie.yaml | 153 ++++ > > .../bindings/pci/fsl,imx6q-pcie.yaml | 47 +- > > .../bindings/pci/rockchip-dw-pcie.yaml | 4 +- > > .../bindings/pci/snps,dw-pcie-common.yaml | 327 +++++++++ > > .../bindings/pci/snps,dw-pcie-ep.yaml | 169 +++-- > > .../devicetree/bindings/pci/snps,dw-pcie.yaml | 236 +++++-- > > .../bindings/pci/toshiba,visconti-pcie.yaml | 7 +- > > drivers/pci/controller/dwc/Kconfig | 9 + > > drivers/pci/controller/dwc/Makefile | 1 + > > drivers/pci/controller/dwc/pcie-bt1.c | 653 ++++++++++++++++++ > > .../pci/controller/dwc/pcie-designware-ep.c | 30 +- > > .../pci/controller/dwc/pcie-designware-host.c | 47 +- > > drivers/pci/controller/dwc/pcie-designware.c | 262 +++++-- > > drivers/pci/controller/dwc/pcie-designware.h | 63 +- > > 14 files changed, 1785 insertions(+), 223 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml > > create mode 100644 Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml > > create mode 100644 drivers/pci/controller/dwc/pcie-bt1.c > > > > -- > > 2.35.1 > >
On Mon, Aug 29, 2022 at 05:28:01PM +0200, Lorenzo Pieralisi wrote: > [+Robin, Will - please jump to DMA mask set-up] > > On Mon, Aug 22, 2022 at 09:47:01PM +0300, Serge Semin wrote: > > Baikal-T1 SoC is equipped with DWC PCIe v4.60a host controller. It can be > > trained to work up to Gen.3 speed over up to x4 lanes. The host controller > > is attached to the DW PCIe 3.0 PCS via the PIPE-4 interface, which in its > > turn is connected to the DWC 10G PHY. The whole system is supposed to be > > fed up with four clock sources: DBI peripheral clock, AXI application > > clocks and external PHY/core reference clock generating the 100MHz signal. > > In addition to that the platform provide a way to reset each part of the > > controller: sticky/non-sticky bits, host controller core, PIPE interface, > > PCS/PHY and Hot/Power reset signal. The driver also provides a way to > > handle the GPIO-based PERST# signal. > > > > Note due to the Baikal-T1 MMIO peculiarity we have to implement the DBI > > interface accessors which make sure the IO operations are dword-aligned. > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > > --- > > > > Changelog v2: > > - Rename 'syscon' property to 'baikal,bt1-syscon'. > > > > Changelog v3: > > - Use the clocks/resets handlers defined in the DW PCIe core descriptor. > > (@Rob) > > - Redefine PCI host bridge config space accessors with the generic > > pci_generic_config_read32() and pci_generic_config_write32() methods. > > (@Rob) > > > > Changelog v4: > > - Drop PCIBIOS_* macros usage. (@Rob) > > - Add "static const" to the dw_pcie_ops and dw_pcie_host_ops structure > > instances. (@Bjorn) > > - Rename bt1_pcie_dw_ops to bt1_pcie_ops. (@Bjorn) > > - Rename bt1_pcie_ops to bt1_pci_ops. (@Bjorn) > > - Use start_link/stop_link suffixes in the corresponding callbacks. > > (@Bjorn) > > - Change the get_res() method suffix to being get_resources(). (@Bjorn) > > - Change *_{add,del}_dw_port() method to *_{add,del}_port(). (@Bjorn) > > - Drop dma_coerce_mask_and_coherent() applied to the PCI host bridge > > kernel device instance. (@Bjorn) > > - Add the comment above the dma_set_mask_and_coherent() method usage > > regarding the controller eDMA feature. (@Bjorn) > > - Fix the comment above the core reset controls assertion. (@Bjorn) > > - Replace delays and timeout numeric literals with macros. (@Bjorn) > > --- > > drivers/pci/controller/dwc/Kconfig | 9 + > > drivers/pci/controller/dwc/Makefile | 1 + > > drivers/pci/controller/dwc/pcie-bt1.c | 653 ++++++++++++++++++++++++++ > > 3 files changed, 663 insertions(+) > > create mode 100644 drivers/pci/controller/dwc/pcie-bt1.c > > > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig > > index 62ce3abf0f19..771b8b146623 100644 > > --- a/drivers/pci/controller/dwc/Kconfig > > +++ b/drivers/pci/controller/dwc/Kconfig > > @@ -222,6 +222,15 @@ config PCIE_ARTPEC6_EP > > Enables support for the PCIe controller in the ARTPEC-6 SoC to work in > > endpoint mode. This uses the DesignWare core. > > > > +config PCIE_BT1 > > + tristate "Baikal-T1 PCIe controller" > > + depends on MIPS_BAIKAL_T1 || COMPILE_TEST > > + depends on PCI_MSI_IRQ_DOMAIN > > + select PCIE_DW_HOST > > + help > > + Enables support for the PCIe controller in the Baikal-T1 SoC to work > > + in host mode. It's based on the Synopsys DWC PCIe v4.60a IP-core. > > + > > config PCIE_ROCKCHIP_DW_HOST > > bool "Rockchip DesignWare PCIe controller" > > select PCIE_DW > > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile > > index 8ba7b67f5e50..bf5c311875a1 100644 > > --- a/drivers/pci/controller/dwc/Makefile > > +++ b/drivers/pci/controller/dwc/Makefile > > @@ -3,6 +3,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o > > obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o > > obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o > > obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o > > +obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o > > obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o > > obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o > > obj-$(CONFIG_PCIE_FU740) += pcie-fu740.o > > diff --git a/drivers/pci/controller/dwc/pcie-bt1.c b/drivers/pci/controller/dwc/pcie-bt1.c > > new file mode 100644 > > index 000000000000..86b230575ddc > > --- /dev/null > > +++ b/drivers/pci/controller/dwc/pcie-bt1.c > > @@ -0,0 +1,653 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (C) 2021 BAIKAL ELECTRONICS, JSC > > + * > > + * Authors: > > + * Vadim Vlasov <Vadim.Vlasov@baikalelectronics.ru> > > + * Serge Semin <Sergey.Semin@baikalelectronics.ru> > > + * > > + * Baikal-T1 PCIe controller driver > > + */ > > + > > +#include <linux/bitfield.h> > > +#include <linux/bits.h> > > +#include <linux/clk.h> > > +#include <linux/delay.h> > > +#include <linux/gpio/consumer.h> > > +#include <linux/kernel.h> > > +#include <linux/mfd/syscon.h> > > +#include <linux/module.h> > > +#include <linux/pci.h> > > +#include <linux/platform_device.h> > > +#include <linux/regmap.h> > > +#include <linux/reset.h> > > +#include <linux/types.h> > > + > > +#include "pcie-designware.h" > > + > > +/* Baikal-T1 System CCU control registers */ > > +#define BT1_CCU_PCIE_CLKC 0x140 > > +#define BT1_CCU_PCIE_REQ_PCS_CLK BIT(16) > > +#define BT1_CCU_PCIE_REQ_MAC_CLK BIT(17) > > +#define BT1_CCU_PCIE_REQ_PIPE_CLK BIT(18) > > + > > +#define BT1_CCU_PCIE_RSTC 0x144 > > +#define BT1_CCU_PCIE_REQ_LINK_RST BIT(13) > > +#define BT1_CCU_PCIE_REQ_SMLH_RST BIT(14) > > +#define BT1_CCU_PCIE_REQ_PHY_RST BIT(16) > > +#define BT1_CCU_PCIE_REQ_CORE_RST BIT(24) > > +#define BT1_CCU_PCIE_REQ_STICKY_RST BIT(26) > > +#define BT1_CCU_PCIE_REQ_NSTICKY_RST BIT(27) > > + > > +#define BT1_CCU_PCIE_PMSC 0x148 > > +#define BT1_CCU_PCIE_LTSSM_STATE_MASK GENMASK(5, 0) > > +#define BT1_CCU_PCIE_LTSSM_DET_QUIET 0x00 > > +#define BT1_CCU_PCIE_LTSSM_DET_ACT 0x01 > > +#define BT1_CCU_PCIE_LTSSM_POLL_ACT 0x02 > > +#define BT1_CCU_PCIE_LTSSM_POLL_COMP 0x03 > > +#define BT1_CCU_PCIE_LTSSM_POLL_CONF 0x04 > > +#define BT1_CCU_PCIE_LTSSM_PRE_DET_QUIET 0x05 > > +#define BT1_CCU_PCIE_LTSSM_DET_WAIT 0x06 > > +#define BT1_CCU_PCIE_LTSSM_CFG_LNKWD_START 0x07 > > +#define BT1_CCU_PCIE_LTSSM_CFG_LNKWD_ACEPT 0x08 > > +#define BT1_CCU_PCIE_LTSSM_CFG_LNNUM_WAIT 0x09 > > +#define BT1_CCU_PCIE_LTSSM_CFG_LNNUM_ACEPT 0x0a > > +#define BT1_CCU_PCIE_LTSSM_CFG_COMPLETE 0x0b > > +#define BT1_CCU_PCIE_LTSSM_CFG_IDLE 0x0c > > +#define BT1_CCU_PCIE_LTSSM_RCVR_LOCK 0x0d > > +#define BT1_CCU_PCIE_LTSSM_RCVR_SPEED 0x0e > > +#define BT1_CCU_PCIE_LTSSM_RCVR_RCVRCFG 0x0f > > +#define BT1_CCU_PCIE_LTSSM_RCVR_IDLE 0x10 > > +#define BT1_CCU_PCIE_LTSSM_L0 0x11 > > +#define BT1_CCU_PCIE_LTSSM_L0S 0x12 > > +#define BT1_CCU_PCIE_LTSSM_L123_SEND_IDLE 0x13 > > +#define BT1_CCU_PCIE_LTSSM_L1_IDLE 0x14 > > +#define BT1_CCU_PCIE_LTSSM_L2_IDLE 0x15 > > +#define BT1_CCU_PCIE_LTSSM_L2_WAKE 0x16 > > +#define BT1_CCU_PCIE_LTSSM_DIS_ENTRY 0x17 > > +#define BT1_CCU_PCIE_LTSSM_DIS_IDLE 0x18 > > +#define BT1_CCU_PCIE_LTSSM_DISABLE 0x19 > > +#define BT1_CCU_PCIE_LTSSM_LPBK_ENTRY 0x1a > > +#define BT1_CCU_PCIE_LTSSM_LPBK_ACTIVE 0x1b > > +#define BT1_CCU_PCIE_LTSSM_LPBK_EXIT 0x1c > > +#define BT1_CCU_PCIE_LTSSM_LPBK_EXIT_TOUT 0x1d > > +#define BT1_CCU_PCIE_LTSSM_HOT_RST_ENTRY 0x1e > > +#define BT1_CCU_PCIE_LTSSM_HOT_RST 0x1f > > +#define BT1_CCU_PCIE_LTSSM_RCVR_EQ0 0x20 > > +#define BT1_CCU_PCIE_LTSSM_RCVR_EQ1 0x21 > > +#define BT1_CCU_PCIE_LTSSM_RCVR_EQ2 0x22 > > +#define BT1_CCU_PCIE_LTSSM_RCVR_EQ3 0x23 > > You could make this an enum and define only the states > that are actually used. I'd prefer to leave it as is. > > [...] > > > + > > +/* Generic Baikal-T1 PCIe interface resources */ > > +#define BT1_PCIE_NUM_APP_CLKS ARRAY_SIZE(bt1_pcie_app_clks) > > +#define BT1_PCIE_NUM_CORE_CLKS ARRAY_SIZE(bt1_pcie_core_clks) > > +#define BT1_PCIE_NUM_APP_RSTS ARRAY_SIZE(bt1_pcie_app_rsts) > > +#define BT1_PCIE_NUM_CORE_RSTS ARRAY_SIZE(bt1_pcie_core_rsts) > > + > > +/* PCIe bus setup delays and timeouts */ > > +#define BT1_PCIE_RST_DELAY_MS 100 > > +#define BT1_PCIE_RUN_DELAY_US 100 > > +#define BT1_PCIE_REQ_DELAY_US 1 > > +#define BT1_PCIE_REQ_TIMEOUT_US 1000 > > +#define BT1_PCIE_LNK_DELAY_US 1000 > > +#define BT1_PCIE_LNK_TIMEOUT_US 1000000 > > + > > +static const enum dw_pcie_app_clk bt1_pcie_app_clks[] = { > > + DW_PCIE_DBI_CLK, DW_PCIE_MSTR_CLK, DW_PCIE_SLV_CLK, > > +}; > > + > > +static const enum dw_pcie_core_clk bt1_pcie_core_clks[] = { > > + DW_PCIE_REF_CLK, > > +}; > > + > > +static const enum dw_pcie_app_rst bt1_pcie_app_rsts[] = { > > + DW_PCIE_MSTR_RST, DW_PCIE_SLV_RST, > > +}; > > + > > +static const enum dw_pcie_core_rst bt1_pcie_core_rsts[] = { > > + DW_PCIE_NON_STICKY_RST, DW_PCIE_STICKY_RST, DW_PCIE_CORE_RST, > > + DW_PCIE_PIPE_RST, DW_PCIE_PHY_RST, DW_PCIE_HOT_RST, DW_PCIE_PWR_RST, > > +}; > > I wonder whether we could allocate the rst/clks in DWC dynamically, > by using these configuration arrays. The resets and clocks are now requested by means of the bulk API in the dw_pcie_get_resources() method. So they are already dynamically allocated. What these config arrays are needed for is to make sure the required resets and clocks have been retrieved and in order to access the requested handlers. > > > + > > +struct bt1_pcie { > > + struct dw_pcie dw; > > + struct platform_device *pdev; > > + struct regmap *sys_regs; > > +}; > > +#define to_bt1_pcie(_dw) container_of(_dw, struct bt1_pcie, dw) > > + > > +/* > > + * Baikal-T1 MMIO space must be read/written by the dword-aligned > > + * instructions. Note the methods are optimized to have the dword operations > > + * performed with minimum overhead as the most frequently used ones. > > + */ > > +static int bt1_pcie_read_mmio(void __iomem *addr, int size, u32 *val) > > +{ > > + unsigned int ofs = (uintptr_t)addr & 0x3; > > + > > + if (!IS_ALIGNED((uintptr_t)addr, size)) > > + return -EINVAL; > > + > > + *val = readl(addr - ofs) >> ofs * BITS_PER_BYTE; > > Is it always safe to read more than requested ? This question is kind of contradicting. No matter whether it's safe or not we just can't perform the IOs with size other than of the dword size. Doing otherwise will cause the bus access error. > > > + if (size == 4) { > > + return 0; > > + } else if (size == 2) { > > + *val &= 0xffff; > > + return 0; > > + } else if (size == 1) { > > + *val &= 0xff; > > + return 0; > > + } > > + > > + return -EINVAL; > > +} > > + > > +static int bt1_pcie_write_mmio(void __iomem *addr, int size, u32 val) > > +{ > > + unsigned int ofs = (uintptr_t)addr & 0x3; > > + u32 tmp, mask; > > + > > + if (!IS_ALIGNED((uintptr_t)addr, size)) > > + return -EINVAL; > > + > > + if (size == 4) { > > + writel(val, addr); > > + return 0; > > + } else if (size == 2 || size == 1) { > > + mask = GENMASK(size * BITS_PER_BYTE - 1, 0); > > + tmp = readl(addr - ofs) & ~(mask << ofs * BITS_PER_BYTE); > > + tmp |= (val & mask) << ofs * BITS_PER_BYTE; > > + writel(tmp, addr - ofs); > > + return 0; > > + } > > Same question read/modify/write, is it always safe to do it > regardless of size ? ditto > > > + > > + return -EINVAL; > > +} > > + > > +static u32 bt1_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base, u32 reg, > > + size_t size) > > +{ > > + int ret; > > + u32 val; > > + > > + ret = bt1_pcie_read_mmio(base + reg, size, &val); > > + if (ret) { > > + dev_err(pci->dev, "Read DBI address failed\n"); > > + return ~0U; > > Is this a special magic value the DWC core is expecting ? > > Does it clash with a _valid_ return value ? It's a normal return value if the PCIe IO wasn't successful. In this particular case there is no actual PCIe-bus IO though, but there are conditions which can cause the errors. So the error status is still sanity checked. This part was already commented by Rob here: https://lore.kernel.org/linux-pci/20220615171045.GD1413880-robh@kernel.org/ my response was: https://lore.kernel.org/linux-pci/20220619203904.h7q2eb7e4ctsujsk@mobilestation/ > > > + } > > + > > + return val; > > +} > > + > > +static void bt1_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base, u32 reg, > > + size_t size, u32 val) > > +{ > > + int ret; > > + > > + ret = bt1_pcie_write_mmio(base + reg, size, val); > > + if (ret) > > + dev_err(pci->dev, "Write DBI address failed\n"); > > +} > > + > > +static void bt1_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base, u32 reg, > > + size_t size, u32 val) > > +{ > > + struct bt1_pcie *btpci = to_bt1_pcie(pci); > > + int ret; > > + > > + regmap_update_bits(btpci->sys_regs, BT1_CCU_PCIE_GENC, > > + BT1_CCU_PCIE_DBI2_MODE, BT1_CCU_PCIE_DBI2_MODE); > > + > > + ret = bt1_pcie_write_mmio(base + reg, size, val); > > + if (ret) > > + dev_err(pci->dev, "Write DBI2 address failed\n"); > > + > > + regmap_update_bits(btpci->sys_regs, BT1_CCU_PCIE_GENC, > > + BT1_CCU_PCIE_DBI2_MODE, 0); > > IIUC the regmap_update_bits() set up decoding for DBI2. Right and then switches it back off. > Hopefully the > DBI/DBI2 writes are sequentialized, this is a question valid also > for other DWC controllers. In general you are right, but not in particular case of the DW PCIe Root Ports. So the concurrent access to DBI and DBI2 won't cause any problem. > > What I want to say is, the regmap update in this function sets the > DWC HW in a way that can decode DBI2 (please correct me if I am wrong), Right. > between the two _update_bits() no DBI access should happen because > it just would not work. No. Because in case of the DW PCIe Root Ports, DBI and DBI2 are almost identical. The difference is only in two CSR fields which turn to be R/W in DBI2 instead of being RO in DBI. Other than that the DBI and DBI2 spaces are identical. That's why we don't need any software-based synchronization between the DBI/DBI2 accesses. Moreover we won't need to worry about the synchronisation at all if DBI2 is mapped via a separate reg-space (see dw_pcie.dbi_base2 field) because any concurrency is resolved behind the scene by means of the DBI bus HW implementation. > > It is a question. The situation gets to be more complex in case of DW PCIe End-points because some of the DBI CSRs change semantics in DBI2. At the very least it concerns the TYPE0_HDR.{BAR0-BAR5} registers, which determine the corresponding BARx size and whether it is enabled in DBI2 (see the reset_bar() and set_bar() methods implementation in drivers/pci/controller/dwc/pcie-designware-ep.c). But my controller is the Root Port controller, so the denoted peculiarity doesn't concern it. > > > +static int bt1_pcie_host_init(struct dw_pcie_rp *pp) > > +{ > > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > > + struct bt1_pcie *btpci = to_bt1_pcie(pci); > > + int ret; > > + > > + ret = bt1_pcie_get_resources(btpci); > > + if (ret) > > + return ret; > > + > > + bt1_pcie_full_stop_bus(btpci, true); > > + > > + return bt1_pcie_cold_start_bus(btpci); > > +} > > + > > +static void bt1_pcie_host_deinit(struct dw_pcie_rp *pp) > > +{ > > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > > + struct bt1_pcie *btpci = to_bt1_pcie(pci); > > + > > + bt1_pcie_full_stop_bus(btpci, false); > > +} > > + > > +static const struct dw_pcie_host_ops bt1_pcie_host_ops = { > > + .host_init = bt1_pcie_host_init, > > + .host_deinit = bt1_pcie_host_deinit, > > +}; > > + > > +static struct bt1_pcie *bt1_pcie_create_data(struct platform_device *pdev) > > +{ > > + struct bt1_pcie *btpci; > > + > > + btpci = devm_kzalloc(&pdev->dev, sizeof(*btpci), GFP_KERNEL); > > + if (!btpci) > > + return ERR_PTR(-ENOMEM); > > + > > + btpci->pdev = pdev; > > + > > + platform_set_drvdata(pdev, btpci); > > + > > + return btpci; > > +} > > + > > +static int bt1_pcie_add_port(struct bt1_pcie *btpci) > > +{ > > + struct device *dev = &btpci->pdev->dev; > > + int ret; > > + > > + /* > > + * DW PCIe Root Port controller is equipped with eDMA capable of > > + * working with the 64-bit memory addresses. > > + */ > > + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); > > + if (ret) > > + return ret; > > Is this the right place to set the DMA mask for the host controller > embedded DMA controller (actually, the dev pointer is the _host_ > controller device) ? Yes it's. The DMA controller is embedded into the PCIe Root Port controller. It CSRs are accessed via either the same CSR space or via a separate space but synchronously clocked by the same clock source (it's called unrolled iATU/eDMA mode). The memory range the controller is capable to reach is platform specific. So the glue driver is the best place to set the device DMA-mask. (For instance the DW PCIe master AXI-bus width is selected by means of the MASTER_BUS_ADDR_WIDTH parameter of the DW PCIe IP-core.) > > How this is going to play when combined with: > > https://lore.kernel.org/linux-pci/1e63a581-14ae-b4b5-a5bf-ca8f09c33af6@arm.com > > It is getting a bit confusing. I believe the code in the link > above sets the mask so that through the DMA API we are capable > of getting an MSI doorbell virtual address whose physical address > can be addressed by the endpoint; this through the DMA API. I don't really understand why the code in the link above tries to analyze the MSI capability of the DW PCIe Root Port in the framework of the dw_pcie_msi_host_init() method. The method utilizes the iMSI-RX engine which is specific to the DW PCIe AXI-bus controller implementation. It has nothing to do with the PCIe MSI capability normally available over the standard PCIe config space. As Rob correctly noted here https://lore.kernel.org/all/CAL_JsqJh=d-B51b6yPBRq0tOwbChN=AFPr-a19U1QdQZAE7c1A@mail.gmail.com MSI TLPs never reaches the system memory. (But I would add that this only concerns the iMSI-RX engine.) So no matter which memory allocated and where, the only thing that matters is the PCIe-bus address specified to the PCIE_MSI_ADDR_LO and PCIE_MSI_ADDR_HI CSRs, which are the DW PCIe-specific and both are always available thus supporting the 64-bit messages in any case. So if we had a way to just reserve a PCIe-bus address range which at the same time wouldn't have a system memory behind, we could have used the reserved range to initialize the iMSI-RX MSI-address without need to allocate any DMA-able memory at all. That's why the commit 07940c369a6b ("PCI: dwc: Fix MSI page leakage in suspend/resume") was absolutely correct. > > This patch is setting the DMA mask for a different reason, namely > setting the host controller embedded DMA controller addressing > capabilities. AFAIU what is done in that patch is incorrect. > > AFAICS - both approaches set the mask for the same device - now > the question is about which one is legitimate and how to handle > the other. That's simple. Mine is legitimate for sure. Another one isn't. > > > + > > + btpci->dw.version = DW_PCIE_VER_460A; > > + btpci->dw.dev = dev; > > + btpci->dw.ops = &bt1_pcie_ops; > > + > > + btpci->dw.pp.num_vectors = MAX_MSI_IRQS; > > + btpci->dw.pp.ops = &bt1_pcie_host_ops; > > + > > + dw_pcie_cap_set(&btpci->dw, REQ_RES); > > + > > + ret = dw_pcie_host_init(&btpci->dw.pp); > > + if (ret) > > + dev_err_probe(dev, ret, "Failed to initialize DWC PCIe host\n"); > > + > > + return ret; > > +} > > + > > +static void bt1_pcie_del_port(struct bt1_pcie *btpci) > > +{ > > + dw_pcie_host_deinit(&btpci->dw.pp); > > +} > > + > > +static int bt1_pcie_probe(struct platform_device *pdev) > > +{ > > + struct bt1_pcie *btpci; > > + > > + btpci = bt1_pcie_create_data(pdev); > > Do we really need a function for that ? I am not too > bothered but I think it is overkill. I prefer splitting the probe method up into a set of small and coherent methods. It IMO improves the code readability for just no price since the compiler will embed the single-time used static methods anyway. -Sergey > > Thanks, > Lorenzo > > > + if (IS_ERR(btpci)) > > + return PTR_ERR(btpci); > > + > > + return bt1_pcie_add_port(btpci); > > +} > > + > > +static int bt1_pcie_remove(struct platform_device *pdev) > > +{ > > + struct bt1_pcie *btpci = platform_get_drvdata(pdev); > > + > > + bt1_pcie_del_port(btpci); > > + > > + return 0; > > +} > > + > > +static const struct of_device_id bt1_pcie_of_match[] = { > > + { .compatible = "baikal,bt1-pcie" }, > > + {}, > > +}; > > +MODULE_DEVICE_TABLE(of, bt1_pcie_of_match); > > + > > +static struct platform_driver bt1_pcie_driver = { > > + .probe = bt1_pcie_probe, > > + .remove = bt1_pcie_remove, > > + .driver = { > > + .name = "bt1-pcie", > > + .of_match_table = bt1_pcie_of_match, > > + }, > > +}; > > +module_platform_driver(bt1_pcie_driver); > > + > > +MODULE_AUTHOR("Serge Semin <Sergey.Semin@baikalelectronics.ru>"); > > +MODULE_DESCRIPTION("Baikal-T1 PCIe driver"); > > +MODULE_LICENSE("GPL"); > > -- > > 2.35.1 > >
On Wed, Aug 31, 2022 at 09:36:46AM +0100, Robin Murphy wrote: > On 2022-08-29 16:28, Lorenzo Pieralisi wrote: > [...] > > > +static int bt1_pcie_add_port(struct bt1_pcie *btpci) > > > +{ > > > + struct device *dev = &btpci->pdev->dev; > > > + int ret; > > > + > > > + /* > > > + * DW PCIe Root Port controller is equipped with eDMA capable of > > > + * working with the 64-bit memory addresses. > > > + */ > > > + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); > > > + if (ret) > > > + return ret; > > > > Is this the right place to set the DMA mask for the host controller > > embedded DMA controller (actually, the dev pointer is the _host_ > > controller device) ? > > > > How this is going to play when combined with: > > > > https://lore.kernel.org/linux-pci/1e63a581-14ae-b4b5-a5bf-ca8f09c33af6@arm.com > > > > It is getting a bit confusing. I believe the code in the link > > above sets the mask so that through the DMA API we are capable > > of getting an MSI doorbell virtual address whose physical address > > can be addressed by the endpoint; this through the DMA API. > > > > This patch is setting the DMA mask for a different reason, namely > > setting the host controller embedded DMA controller addressing > > capabilities. > > > > AFAICS - both approaches set the mask for the same device - now > > the question is about which one is legitimate and how to handle > > the other. > > Assuming the dw-edma-pcie driver is the relevant one, that already sets its > own masks on its own device, so I also don't see why this is here. No. dw-edma-pcie has nothing to do with this driver. -Sergey > > Thanks, > Robin.
On Mon, Sep 12, 2022 at 03:02:11AM +0300, Serge Semin wrote: [...] > I prefer splitting the probe method up into a set of small and > coherent methods. It IMO improves the code readability for just no > price since the compiler will embed the single-time used static > methods anyway. I will get back to this thread at -rc7 - we will decide a merge strategy then. Lorenzo > -Sergey > > > > > Thanks, > > Lorenzo > > > > > + if (IS_ERR(btpci)) > > > + return PTR_ERR(btpci); > > > + > > > + return bt1_pcie_add_port(btpci); > > > +} > > > + > > > +static int bt1_pcie_remove(struct platform_device *pdev) > > > +{ > > > + struct bt1_pcie *btpci = platform_get_drvdata(pdev); > > > + > > > + bt1_pcie_del_port(btpci); > > > + > > > + return 0; > > > +} > > > + > > > +static const struct of_device_id bt1_pcie_of_match[] = { > > > + { .compatible = "baikal,bt1-pcie" }, > > > + {}, > > > +}; > > > +MODULE_DEVICE_TABLE(of, bt1_pcie_of_match); > > > + > > > +static struct platform_driver bt1_pcie_driver = { > > > + .probe = bt1_pcie_probe, > > > + .remove = bt1_pcie_remove, > > > + .driver = { > > > + .name = "bt1-pcie", > > > + .of_match_table = bt1_pcie_of_match, > > > + }, > > > +}; > > > +module_platform_driver(bt1_pcie_driver); > > > + > > > +MODULE_AUTHOR("Serge Semin <Sergey.Semin@baikalelectronics.ru>"); > > > +MODULE_DESCRIPTION("Baikal-T1 PCIe driver"); > > > +MODULE_LICENSE("GPL"); > > > -- > > > 2.35.1 > > >
On Mon, Sep 26, 2022 at 04:31:28PM +0200, Christoph Hellwig wrote: > On Mon, Sep 26, 2022 at 03:49:24PM +0300, Serge Semin wrote: > > @Christoph, @Marek, @Bjorn, @Rob could you please join to the > > DMA-mask related discussion. @Lorenzo can't decide which driver should > > initialize the device DMA-mask. > > The driver that does the actual DMA mapping or allocation functions > need to set it. But even with your comments on the questions I'm > still confused what struct device you are even talking about. Can > you explain this a bit better? We are talking about the DW PCIe Root Port controller with DW eDMA engine embedded. It' simplified structure can be represented as follows: +---------------+ +--------+ | System memory | | CPU(s) | +---------------+ +--------+ ^ | | ^ | ... System bus ... | ... | | ... | v v | +------------+------+--------+----------+------+ | DW PCIe RP | AXI-m| | AXI-s/DBI| | | +------+ +----------+ | | ^ ^ | | | +------+----+ | CSRs | | v v v | | +-------+ +---------+ +----------+ | | | eDMA | | in-iATU | | out-iATU | | | +-------+ +---------+ +----------+ | | ^ ^ ^ | | +--------+--+---+-------+ | +------------------| PIPE |--------------------+ +------+ | ^ v | PCIe bus The DW PCIe controller device is instantiated as a platform device defined in the system DT source file. The device is probed by the DW PCIe low-level driver, which after the platform-specific setups initiates the generic DW PCIe host-controller registration. On the way of that procedure the DW PCIe core tries to auto-detect the DW eDMA engine availability. If the engine is found, the DW eDMA probe method is called in order to register the DMA-engine device. After that the PCIe host bridge is registered. Both the PCIe host-bridge and DMA-engine devices will have the DW PCIe platform device as parent. Getting back to the sketch above. Here is a short description of the content: 1. DW eDMA is capable of performing the data transfers from/to System memory to/from PCIe bus memory. 2. in-iATU is the Inbound Address Translation Unit, which is responsible for the PCIe bus peripheral devices to access the system memory. The "dma-ranges" DT-property is used to initialize the PCIe<->Sys memory mapping. (@William note the In-iATU setup doesn't affect the eDMA transfers.) 3. out-iATU is responsible for the CPU(s) to access the PCIe bus peripheral devices memory/cfg-space. So eDMA and in-iATU are using the same AXI-master interface to access the system memory. Thus the DMAable memory capability is the same for both of them (Though in-iATU may have some specific mapping based on the "dma-ranges" DT-property setup). Neither DW eDMA nor DW PCIe Root Port CSRs region have any register to auto-detect the AXI-m interface address bus width. It's selected during the IP-core synthesize and is platform-specific. The question is: "What driver/code is supposed to set the DMA-mask of the DW PCIe platform device?" Seeing the parental platform device is used to perform the memory-mapping for both DW eDMA clients and PCIe-bus peripheral device drivers, and seeing the AXI-m interface parameters aren't auto-detectable and are platform-specific, the only place it should be done in is the DW PCIe low-level device driver. I don't really see any alternative... What is your opinion? -Sergey
On 09/26/2022, Serge Semin wrote: > On Mon, Sep 26, 2022 at 04:31:28PM +0200, Christoph Hellwig wrote: > > On Mon, Sep 26, 2022 at 03:49:24PM +0300, Serge Semin wrote: > > > @Christoph, @Marek, @Bjorn, @Rob could you please join to the > > > DMA-mask related discussion. @Lorenzo can't decide which driver should > > > initialize the device DMA-mask. > > > > > The driver that does the actual DMA mapping or allocation functions > > need to set it. But even with your comments on the questions I'm > > still confused what struct device you are even talking about. Can > > you explain this a bit better? > > We are talking about the DW PCIe Root Port controller with DW eDMA engine > embedded. It' simplified structure can be represented as follows: > > +---------------+ +--------+ > | System memory | | CPU(s) | > +---------------+ +--------+ > ^ | | ^ > | ... System bus ... | > ... | | ... > | v v | > +------------+------+--------+----------+------+ > | DW PCIe RP | AXI-m| | AXI-s/DBI| | > | +------+ +----------+ | > | ^ ^ | | > | +------+----+ | CSRs | > | v v v | > | +-------+ +---------+ +----------+ | > | | eDMA | | in-iATU | | out-iATU | | > | +-------+ +---------+ +----------+ | > | ^ ^ ^ | > | +--------+--+---+-------+ | > +------------------| PIPE |--------------------+ > +------+ > | ^ > v | > PCIe bus > > The DW PCIe controller device is instantiated as a platform device > defined in the system DT source file. The device is probed by the > DW PCIe low-level driver, which after the platform-specific setups > initiates the generic DW PCIe host-controller registration. On the way > of that procedure the DW PCIe core tries to auto-detect the DW eDMA > engine availability. If the engine is found, the DW eDMA probe method > is called in order to register the DMA-engine device. After that the > PCIe host bridge is registered. Both the PCIe host-bridge and > DMA-engine devices will have the DW PCIe platform device as parent. > > Getting back to the sketch above. Here is a short description of the > content: > 1. DW eDMA is capable of performing the data transfers from/to System > memory to/from PCIe bus memory. > 2. in-iATU is the Inbound Address Translation Unit, which is > responsible for the PCIe bus peripheral devices to access the system > memory. The "dma-ranges" DT-property is used to initialize the > PCIe<->Sys memory mapping. (@William note the In-iATU setup doesn't > affect the eDMA transfers.) > 3. out-iATU is responsible for the CPU(s) to access the PCIe bus > peripheral devices memory/cfg-space. > > So eDMA and in-iATU are using the same AXI-master interface to access > the system memory. Thus the DMAable memory capability is the same for > both of them (Though in-iATU may have some specific mapping based on > the "dma-ranges" DT-property setup). Neither DW eDMA nor DW PCIe Root > Port CSRs region have any register to auto-detect the AXI-m interface > address bus width. It's selected during the IP-core synthesize and is > platform-specific. The question is: "What driver/code is supposed to > set the DMA-mask of the DW PCIe platform device?" Seeing the parental > platform device is used to perform the memory-mapping for both DW eDMA > clients and PCIe-bus peripheral device drivers, and seeing the AXI-m > interface parameters aren't auto-detectable and are platform-specific, > the only place it should be done in is the DW PCIe low-level device > driver. I don't really see any alternative... What is your opinion? > > -Sergey I believe this eDMA implementation is new for an upstream DW PCIe device driver, right? If so, this will require some refactoring of the DMA mask code, but you need to also make sure you don't break the MSI target address use case that prompted this 32-bit DMA mask change -- [1]. My changes were directly related to allowing the DW PCIe device driver to fallback to a 64-bit DMA mask for the MSI target address if there are no 32-bit allocations available. For that use-case, using a 32-bit mask doesn't have any perf impact here since there is no actual DMAs happening. Would it be possible for the DW PCIe device driver to set a capabilities flag that the PCIe host controller can read and set the mask accordingly. This way you don't need to go fix up any drivers that require a 32-bit DMA'able address for the MSI target address. For example, I see several of the PCI capability features have 64-bit flags, e.g. PCI_MSI_FLAGS_64BIT and PCI_X_STATUS_64BIT. If not, then you're going to have to re-work the host controller driver and DW PCIe device drivers that require a 32-bit MSI target address. [1] https://lore.kernel.org/all/20201117165312.25847-1-vidyas@nvidia.com/ Thanks, Will
This patchset is a third one in the series created in the framework of my Baikal-T1 PCIe/eDMA-related work: [1: Done v5] PCI: dwc: Various fixes and cleanups Link: https://lore.kernel.org/linux-pci/20220624143428.8334-1-Sergey.Semin@baikalelectronics.ru/ Merged: kernel 6.0-rc1 [2: Done v4] PCI: dwc: Add hw version and dma-ranges support Link: https://lore.kernel.org/linux-pci/20220624143947.8991-1-Sergey.Semin@baikalelectronics.ru Merged: kernel 6.0-rc1 [3: In-review v5] PCI: dwc: Add generic resources and Baikal-T1 support Link: ---you are looking at it--- [4: Done v4] dmaengine: dw-edma: Add RP/EP local DMA support Link: https://lore.kernel.org/linux-pci/20220728142841.12305-1-Sergey.Semin@baikalelectronics.ru/ Note it is very recommended to merge the patchsets in the same order as they are listed in the set above in order to have them applied smoothly. Nothing prevents them from being reviewed synchronously though. Originally the patches submitted in this patchset were a part of the series: Link: https://lore.kernel.org/linux-pci/20220503214638.1895-1-Sergey.Semin@baikalelectronics.ru/ but due to the reviewers requests the series was expanded to about 30 patches which made it too bulky for a comfortable review. So I decided to split it up into two patchsets: 2. and 3. in the table above. Regarding the series content. This patchset is mainly about adding new DW PCIe platform support - Baikal-T1 PCIe of DW PCIe v4.60a IP-core. But a set of feature-reach preparations are done first. It starts from converting the currently available DT-schema into a more flexible schemas hierarchy with separately defined regs, clocks, resets and interrupts properties. As a result the common schema can be easily re-used by all the currently available platforms while the named properties above can be either re-defined or used as is if the platforms support they. In the framework of that modification we also suggest to add a set of generic regs, clocks, resets and interrupts resource names in accordance with what the DW PCIe hardware reference manual describes and what the DW PCIe core driver already expects to be specified. Thus the new platform driver will be able to re-use the common resources infrastructure. Link: https://lore.kernel.org/linux-pci/20220324013734.18234-1-Sergey.Semin@baikalelectronics.ru/ Changelog v2: - Rename 'syscon' property to 'baikal,bt1-syscon'. (@Rob) - Move the iATU region selection procedure into a helper function (@Rob). - Rebase from kernel v5.17 onto v5.18-rc3 since the later kernel has already DT bindings converted. (@Rob) - Use 'definitions' property instead of the '$defs' one. It fixes the dt-validate error: 'X is not of type array.' - Drop 'interrupts' and 'interrupt-names' property from being required for the native DW PCIe host. - Evaluate the 'snps,dw-pcie-common.yaml' schema in the 'socionext,uniphier-pcie-ep.yaml' DT-bindings since the later has platform-specific names defined. Link: https://lore.kernel.org/linux-pci/20220503225104.12108-1-Sergey.Semin@baikalelectronics.ru Changelog v3: - Split up the patch "dt-bindings: PCI: dwc: Define common and native DT bindings" into a series of modifications. (@Rob) - Detach this series of the patches into a dedicated patchset. - Add a new feature patch: "PCI: dwc: Introduce generic controller capabilities interface". - Add a new feature patch: "PCI: dwc: Introduce generic resources getter". - Add a new cleanup patch: "PCI: dwc: Combine iATU detection procedures". - Add a method to at least request the generic clocks and resets. (@Rob) - Add GPIO-based PERST# signal support to the core module. - Redefine Baikal-T1 PCIe host bridge config space accessors with the pci_generic_config_read32() and pci_generic_config_write32() methods. (@Rob) - Drop synonymous from the names list in the common DT-schema since the device sub-schemas create their own enumerations anyway. - Rebase onto kernel v5.18. Link: https://lore.kernel.org/linux-pci/20220610085706.15741-1-Sergey.Semin@baikalelectronics.ru/ Changelog v4: - Drop PCIBIOS_* macros usage. (@Rob) - Add "static const" to the dw_pcie_ops and dw_pcie_host_ops structure instances. (@Bjorn) - Rename bt1_pcie_dw_ops to bt1_pcie_ops. (@Bjorn) - Rename bt1_pcie_ops to bt1_pci_ops. (@Bjorn) - Use start_link/stop_link suffixes in the Baikal-T1 PCIe start/stop link callbacks. (@Bjorn) - Change the get_res() method suffix to being get_resources(). (@Bjorn) - Change *_{add,del}_dw_port() method to *_{add,del}_port(). (@Bjorn) - Drop dma_coerce_mask_and_coherent() applied to the PCI host bridge kernel device instance. (@Bjorn) - Add the comment above the dma_set_mask_and_coherent() method usage regarding the controller eDMA feature. (@Bjorn) - Fix the comment above the core reset controls assertion. (@Bjorn) - Replace delays and timeout numeric literals with macros. (@Bjorn) - Convert the method name from dw_pcie_get_res() to dw_pcie_get_resources(). (@Bjorn) - Rebase onto the kernel v5.19-rcX. Link: https://lore.kernel.org/linux-pci/20220728143427.13617-1-Sergey.Semin@baikalelectronics.ru/ Changelog v5: - Add a note about having line-based PHY phandles order. (@Rob) - Prefer 'pcie[0-9]+' PHY-names over the rest of the cases. (@Rob) - Drop generic fallback names from the Baikal-T1 compatible property constraints. (@Rob) - Define ordered {reg,interrupt,clock,reset}-names Baikal-T1 PCIe properties. (@Rob) - Drop minItems from the Baikal-T1 PCIe clocks and reset properties, since it equals to the maxItems for them. - Drop num-ob-windows and num-ib-windows properties constraint from Baikal-T1 PCIe bindings. (@Rob) - Add a note about having line-based PHY phandles order. (@Rob) - Prefer 'pcie[0-9]+' PHY-names over the rest of the cases. (@Rob) - Add platform-specific reg/interrupt/clock/reset names to the generic schema, but mark them as deprecated. - Add new patches: dt-bindings: visconti-pcie: Fix interrupts array max constraints dt-bindings: imx6q-pcie: Fix clock names for imx6sx and imx8mq - Move the patch: PCI: dwc: Introduce dma-ranges property support for RC-host from the previous patchset in here. (@Bjorn) - Rebase onto the kernel v6.0-rc2. Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru> Cc: "Krzysztof Wilczyński" <kw@linux.com> Cc: Frank Li <Frank.Li@nxp.com> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Cc: linux-pci@vger.kernel.org Cc: devicetree@vger.kernel.org Cc: linux-kernel@vger.kernel.org Serge Semin (20): dt-bindings: imx6q-pcie: Fix clock names for imx6sx and imx8mq dt-bindings: visconti-pcie: Fix interrupts array max constraints dt-bindings: PCI: dwc: Detach common RP/EP DT bindings dt-bindings: PCI: dwc: Remove bus node from the examples dt-bindings: PCI: dwc: Add phys/phy-names common properties dt-bindings: PCI: dwc: Add max-link-speed common property dt-bindings: PCI: dwc: Apply generic schema for generic device only dt-bindings: PCI: dwc: Add max-functions EP property dt-bindings: PCI: dwc: Add interrupts/interrupt-names common properties dt-bindings: PCI: dwc: Add reg/reg-names common properties dt-bindings: PCI: dwc: Add clocks/resets common properties dt-bindings: PCI: dwc: Add dma-coherent property dt-bindings: PCI: dwc: Apply common schema to Rockchip DW PCIe nodes dt-bindings: PCI: dwc: Add Baikal-T1 PCIe Root Port bindings PCI: dwc: Introduce dma-ranges property support for RC-host PCI: dwc: Introduce generic controller capabilities interface PCI: dwc: Introduce generic resources getter PCI: dwc: Combine iATU detection procedures PCI: dwc: Introduce generic platform clocks and resets PCI: dwc: Add Baikal-T1 PCIe controller support .../bindings/pci/baikal,bt1-pcie.yaml | 153 ++++ .../bindings/pci/fsl,imx6q-pcie.yaml | 47 +- .../bindings/pci/rockchip-dw-pcie.yaml | 4 +- .../bindings/pci/snps,dw-pcie-common.yaml | 327 +++++++++ .../bindings/pci/snps,dw-pcie-ep.yaml | 169 +++-- .../devicetree/bindings/pci/snps,dw-pcie.yaml | 236 +++++-- .../bindings/pci/toshiba,visconti-pcie.yaml | 7 +- drivers/pci/controller/dwc/Kconfig | 9 + drivers/pci/controller/dwc/Makefile | 1 + drivers/pci/controller/dwc/pcie-bt1.c | 653 ++++++++++++++++++ .../pci/controller/dwc/pcie-designware-ep.c | 30 +- .../pci/controller/dwc/pcie-designware-host.c | 47 +- drivers/pci/controller/dwc/pcie-designware.c | 262 +++++-- drivers/pci/controller/dwc/pcie-designware.h | 63 +- 14 files changed, 1785 insertions(+), 223 deletions(-) create mode 100644 Documentation/devicetree/bindings/pci/baikal,bt1-pcie.yaml create mode 100644 Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml create mode 100644 drivers/pci/controller/dwc/pcie-bt1.c