Hi Lorenzo Le 16/01/2018 à 17:07, Lorenzo Pieralisi a écrit : > On Wed, Jan 10, 2018 at 11:47:35PM +0100, Cyrille Pitchen wrote: >> This patch adds support to the Cadence PCIe controller in host mode. >> >> Signed-off-by: Cyrille Pitchen <[email protected]> >> --- >> MAINTAINERS | 7 + >> drivers/pci/Kconfig | 1 + >> drivers/pci/Makefile | 1 + >> drivers/pci/cadence/Kconfig | 16 ++ >> drivers/pci/cadence/Makefile | 3 + >> drivers/pci/cadence/pcie-cadence-host.c | 336 >> ++++++++++++++++++++++++++++++++ >> drivers/pci/cadence/pcie-cadence.c | 68 +++++++ >> drivers/pci/cadence/pcie-cadence.h | 196 +++++++++++++++++++ > > [...] > >> +void cdns_pcie_reset_outbound_region(struct cdns_pcie *pcie, u32 r) >> +{ >> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR0(r), 0); >> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR1(r), 0); >> + >> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC0(r), 0); >> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC1(r), 0); >> + >> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR0(r), 0); >> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR1(r), 0); >> +} > > Unused function, please remove it.
done in v4. > >> diff --git a/drivers/pci/cadence/pcie-cadence.h >> b/drivers/pci/cadence/pcie-cadence.h >> new file mode 100644 >> index 000000000000..3a15b4016352 >> --- /dev/null >> +++ b/drivers/pci/cadence/pcie-cadence.h >> @@ -0,0 +1,196 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +// Copyright (c) 2017 Cadence >> +// Cadence PCIe controller driver. >> +// Author: Cyrille Pitchen <[email protected]> >> + >> +#ifndef _PCIE_CADENCE_H >> +#define _PCIE_CADENCE_H >> + >> +#include <linux/kernel.h> >> +#include <linux/pci.h> >> + >> +/* >> + * Local Management Registers >> + */ >> +#define CDNS_PCIE_LM_BASE 0x00100000 >> + >> +/* Vendor ID Register */ >> +#define CDNS_PCIE_LM_ID (CDNS_PCIE_LM_BASE + 0x0044) >> +#define CDNS_PCIE_LM_ID_VENDOR_MASK GENMASK(15, 0) >> +#define CDNS_PCIE_LM_ID_VENDOR_SHIFT 0 >> +#define CDNS_PCIE_LM_ID_VENDOR(vid) \ >> + (((vid) << CDNS_PCIE_LM_ID_VENDOR_SHIFT) & CDNS_PCIE_LM_ID_VENDOR_MASK) >> +#define CDNS_PCIE_LM_ID_SUBSYS_MASK GENMASK(31, 16) >> +#define CDNS_PCIE_LM_ID_SUBSYS_SHIFT 16 >> +#define CDNS_PCIE_LM_ID_SUBSYS(sub) \ >> + (((sub) << CDNS_PCIE_LM_ID_SUBSYS_SHIFT) & CDNS_PCIE_LM_ID_SUBSYS_MASK) >> + >> +/* Root Port Requestor ID Register */ >> +#define CDNS_PCIE_LM_RP_RID (CDNS_PCIE_LM_BASE + 0x0228) >> +#define CDNS_PCIE_LM_RP_RID_MASK GENMASK(15, 0) >> +#define CDNS_PCIE_LM_RP_RID_SHIFT 0 >> +#define CDNS_PCIE_LM_RP_RID_(rid) \ >> + (((rid) << CDNS_PCIE_LM_RP_RID_SHIFT) & CDNS_PCIE_LM_RP_RID_MASK) >> + >> +/* Root Complex BAR Configuration Register */ >> +#define CDNS_PCIE_LM_RC_BAR_CFG (CDNS_PCIE_LM_BASE + 0x0300) >> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR0_APERTURE_MASK GENMASK(5, 0) >> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR0_APERTURE(a) \ >> + (((a) << 0) & CDNS_PCIE_LM_RC_BAR_CFG_BAR0_APERTURE_MASK) >> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR0_CTRL_MASK GENMASK(8, 6) >> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR0_CTRL(c) \ >> + (((c) << 6) & CDNS_PCIE_LM_RC_BAR_CFG_BAR0_CTRL_MASK) >> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR1_APERTURE_MASK GENMASK(13, 9) >> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR1_APERTURE(a) \ >> + (((a) << 9) & CDNS_PCIE_LM_RC_BAR_CFG_BAR1_APERTURE_MASK) >> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR1_CTRL_MASK GENMASK(16, 14) >> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR1_CTRL(c) \ >> + (((c) << 14) & CDNS_PCIE_LM_RC_BAR_CFG_BAR1_CTRL_MASK) >> +#define CDNS_PCIE_LM_RC_BAR_CFG_PREFETCH_MEM_ENABLE BIT(17) >> +#define CDNS_PCIE_LM_RC_BAR_CFG_PREFETCH_MEM_32BITS 0 >> +#define CDNS_PCIE_LM_RC_BAR_CFG_PREFETCH_MEM_64BITS BIT(18) >> +#define CDNS_PCIE_LM_RC_BAR_CFG_IO_ENABLE BIT(19) >> +#define CDNS_PCIE_LM_RC_BAR_CFG_IO_16BITS 0 >> +#define CDNS_PCIE_LM_RC_BAR_CFG_IO_32BITS BIT(20) >> +#define CDNS_PCIE_LM_RC_BAR_CFG_CHECK_ENABLE BIT(31) >> + >> +/* BAR control values applicable to both Endpoint Function and Root Complex >> */ >> +#define CDNS_PCIE_LM_BAR_CFG_CTRL_DISABLED 0x0 >> +#define CDNS_PCIE_LM_BAR_CFG_CTRL_IO_32BITS 0x1 >> +#define CDNS_PCIE_LM_BAR_CFG_CTRL_MEM_32BITS 0x4 >> +#define CDNS_PCIE_LM_BAR_CFG_CTRL_PREFETCH_MEM_32BITS 0x5 >> +#define CDNS_PCIE_LM_BAR_CFG_CTRL_MEM_64BITS 0x6 >> +#define CDNS_PCIE_LM_BAR_CFG_CTRL_PREFETCH_MEM_64BITS 0x7 >> + >> + >> +/* >> + * Root Port Registers (PCI configuration space for the root port function) >> + */ >> +#define CDNS_PCIE_RP_BASE 0x00200000 >> + >> + >> +/* >> + * Address Translation Registers >> + */ >> +#define CDNS_PCIE_AT_BASE 0x00400000 >> + >> +/* Region r Outbound AXI to PCIe Address Translation Register 0 */ >> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0(r) \ >> + (CDNS_PCIE_AT_BASE + 0x0000 + ((r) & 0x1f) * 0x0020) >> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_NBITS_MASK GENMASK(5, 0) >> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_NBITS(nbits) \ >> + (((nbits) - 1) & CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_NBITS_MASK) >> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_DEVFN_MASK GENMASK(19, 12) >> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_DEVFN(devfn) \ >> + (((devfn) << 12) & CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_DEVFN_MASK) >> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_BUS_MASK GENMASK(27, 20) >> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_BUS(bus) \ >> + (((bus) << 20) & CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_BUS_MASK) >> + >> +/* Region r Outbound AXI to PCIe Address Translation Register 1 */ >> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR1(r) \ >> + (CDNS_PCIE_AT_BASE + 0x0004 + ((r) & 0x1f) * 0x0020) >> + >> +/* Region r Outbound PCIe Descriptor Register 0 */ >> +#define CDNS_PCIE_AT_OB_REGION_DESC0(r) \ >> + (CDNS_PCIE_AT_BASE + 0x0008 + ((r) & 0x1f) * 0x0020) >> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_MASK GENMASK(3, 0) >> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_MEM 0x2 >> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_IO 0x6 >> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_CONF_TYPE0 0xa >> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_CONF_TYPE1 0xb >> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_NORMAL_MSG 0xc >> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_VENDOR_MSG 0xd >> +/* Bit 23 MUST be set in RC mode. */ >> +#define CDNS_PCIE_AT_OB_REGION_DESC0_HARDCODED_RID BIT(23) >> +#define CDNS_PCIE_AT_OB_REGION_DESC0_DEVFN_MASK GENMASK(31, 24) >> +#define CDNS_PCIE_AT_OB_REGION_DESC0_DEVFN(devfn) \ >> + (((devfn) << 24) & CDNS_PCIE_AT_OB_REGION_DESC0_DEVFN_MASK) >> + >> +/* Region r Outbound PCIe Descriptor Register 1 */ >> +#define CDNS_PCIE_AT_OB_REGION_DESC1(r) \ >> + (CDNS_PCIE_AT_BASE + 0x000c + ((r) & 0x1f) * 0x0020) >> +#define CDNS_PCIE_AT_OB_REGION_DESC1_BUS_MASK GENMASK(7, 0) >> +#define CDNS_PCIE_AT_OB_REGION_DESC1_BUS(bus) \ >> + ((bus) & CDNS_PCIE_AT_OB_REGION_DESC1_BUS_MASK) >> + >> +/* Region r AXI Region Base Address Register 0 */ >> +#define CDNS_PCIE_AT_OB_REGION_CPU_ADDR0(r) \ >> + (CDNS_PCIE_AT_BASE + 0x0018 + ((r) & 0x1f) * 0x0020) >> +#define CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS_MASK GENMASK(5, 0) >> +#define CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS(nbits) \ >> + (((nbits) - 1) & CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS_MASK) >> + >> +/* Region r AXI Region Base Address Register 1 */ >> +#define CDNS_PCIE_AT_OB_REGION_CPU_ADDR1(r) \ >> + (CDNS_PCIE_AT_BASE + 0x001c + ((r) & 0x1f) * 0x0020) >> + >> +/* Root Port BAR Inbound PCIe to AXI Address Translation Register */ >> +#define CDNS_PCIE_AT_IB_RP_BAR_ADDR0(bar) \ >> + (CDNS_PCIE_AT_BASE + 0x0800 + (bar) * 0x0008) >> +#define CDNS_PCIE_AT_IB_RP_BAR_ADDR0_NBITS_MASK GENMASK(5, 0) >> +#define CDNS_PCIE_AT_IB_RP_BAR_ADDR0_NBITS(nbits) \ >> + (((nbits) - 1) & CDNS_PCIE_AT_IB_RP_BAR_ADDR0_NBITS_MASK) >> +#define CDNS_PCIE_AT_IB_RP_BAR_ADDR1(bar) \ >> + (CDNS_PCIE_AT_BASE + 0x0804 + (bar) * 0x0008) >> + >> +enum cdns_pcie_rp_bar { >> + RP_BAR0, >> + RP_BAR1, >> + RP_NO_BAR >> +}; >> + >> +/** >> + * struct cdns_pcie - private data for Cadence PCIe controller drivers >> + * @reg_base: IO mapped register base >> + * @mem_res: start/end offsets in the physical system memory to map PCI >> accesses >> + * @is_rc: tell whether the PCIe controller mode is Root Complex or >> Endpoint. >> + * @bus: In Root Complex mode, the bus number >> + */ >> +struct cdns_pcie { >> + void __iomem *reg_base; >> + struct resource *mem_res; >> + bool is_rc; >> + u8 bus; >> +}; >> + >> +/* Register access */ >> +static inline void cdns_pcie_writeb(struct cdns_pcie *pcie, u32 reg, u8 >> value) >> +{ >> + writeb(value, pcie->reg_base + reg); >> +} >> + >> +static inline void cdns_pcie_writew(struct cdns_pcie *pcie, u32 reg, u16 >> value) >> +{ >> + writew(value, pcie->reg_base + reg); >> +} >> + >> +static inline void cdns_pcie_writel(struct cdns_pcie *pcie, u32 reg, u32 >> value) >> +{ >> + writel(value, pcie->reg_base + reg); >> +} >> + >> +static inline u32 cdns_pcie_readl(struct cdns_pcie *pcie, u32 reg) >> +{ >> + return readl(pcie->reg_base + reg); >> +} >> + >> +/* Root Port register access */ >> +static inline void cdns_pcie_rp_writeb(struct cdns_pcie *pcie, >> + u32 reg, u8 value) >> +{ >> + writeb(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg); >> +} >> + >> +static inline void cdns_pcie_rp_writew(struct cdns_pcie *pcie, >> + u32 reg, u16 value) >> +{ >> + writew(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg); >> +} >> + >> +void cdns_pcie_set_outbound_region(struct cdns_pcie *pcie, u32 r, bool >> is_io, >> + u64 cpu_addr, u64 pci_addr, size_t size); >> + >> +void cdns_pcie_reset_outbound_region(struct cdns_pcie *pcie, u32 r); > > Ditto. I understand it is hard to untangle code that was written with > both EP and host bridge support in mind but, as I state below every > patch has to make sense on its own. done in v4. > >> + >> +#endif /* _PCIE_CADENCE_H */ > > Technically speaking, this 3-file (.c .h) split is not necessary and can > turn out unnecessary churn if we never merge the endpoint code - > actually you would not even need a special cadence directory, just a > host bridge driver file. > > Bottom line, as you know: every patch has to be self contained. > > It is already quite late -rc, I understand that changes this late > are error prone so I will accept that this file splitting is necessary > for "future" patches - if you manage to squash changes into a file I'd > appreciate though, it does not make sense to export functions that are > used in just one compilation unit (and some that are not used at all). Sorry, I didn't have time to squash the 3 files into a single one. I can do it in v5 if you think it's better. I plan to work on the endpoint driver with multi-functions this week-end, hence I hope I will be able to add this driver back in v5. I've just been sending v4 anyway so at least you will have former patch 3 from v3 split into new patches 3 and 4 in v4 as you requested. So if you want to merge at least the new generic patches [1 - 4] for v4.16, they are available :) Best regards, Cyrille > > Thanks, > Lorenzo > -- Cyrille Pitchen, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
