On Mon, Dec 09, 2024 at 03:19:57PM +0800, Chen Wang wrote: > From: Chen Wang <unicorn_wang@xxxxxxxxxxx> > > Add support for PCIe controller in SG2042 SoC. The controller > uses the Cadence PCIe core programmed by pcie-cadence*.c. The > PCIe controller will work in host mode only. > +++ b/drivers/pci/controller/cadence/Makefile > @@ -4,3 +4,4 @@ obj-$(CONFIG_PCIE_CADENCE_HOST) += pcie-cadence-host.o > obj-$(CONFIG_PCIE_CADENCE_EP) += pcie-cadence-ep.o > obj-$(CONFIG_PCIE_CADENCE_PLAT) += pcie-cadence-plat.o > obj-$(CONFIG_PCI_J721E) += pci-j721e.o > +obj-$(CONFIG_PCIE_SG2042) += pcie-sg2042.o > \ No newline at end of file Add the newline. > +++ b/drivers/pci/controller/cadence/pcie-sg2042.c > +#include "../../../irqchip/irq-msi-lib.h" This is the only file outside drivers/irqchip/ that includes this. What's special about this driver? Maybe this is a hint that something here belongs in drivers/irqchip/? > +#ifdef CONFIG_SMP No other drivers test CONFIG_SMP, why should this be different? > +static int sg2042_pcie_msi_irq_set_affinity(struct irq_data *d, > + const struct cpumask *mask, > + bool force) > +{ > + if (d->parent_data) > + return irq_chip_set_affinity_parent(d, mask, force); > + > + return -EINVAL; > +} > +#endif /* CONFIG_SMP */ > +static int sg2042_pcie_init_msi_data(struct sg2042_pcie *pcie) > +{ > + struct device *dev = pcie->cdns_pcie->dev; > + u32 value; > + int ret; > + > + raw_spin_lock_init(&pcie->msi_lock); > + > + /* > + * Though the PCIe controller can address >32-bit address space, to > + * facilitate endpoints that support only 32-bit MSI target address, > + * the mask is set to 32-bit to make sure that MSI target address is > + * always a 32-bit address > + */ > + ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32)); Not sure this is needed. Does DT dma-ranges not cover this? > +static int sg2042_pcie_setup_msi(struct sg2042_pcie *pcie, struct device_node *msi_node) Wrap to fit in 80 columns like the rest. > +/* > + * SG2042 only support 4-byte aligned access, so for the rootbus (i.e. to read > + * the PCIe controller itself, read32 is required. For non-rootbus (i.e. to read s/PCIe controller/Root Port/ > + * the PCIe peripheral registers, supports 1/2/4 byte aligned access, so > + * directly use read should be fine. s/use read/using read/ > +static int sg2042_pcie_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + struct pci_host_bridge *bridge; > + struct device_node *np_syscon; > + struct device_node *msi_node; > + struct cdns_pcie *cdns_pcie; > + struct sg2042_pcie *pcie; > + struct cdns_pcie_rc *rc; > + struct regmap *syscon; > + int ret; > + > + if (!IS_ENABLED(CONFIG_PCIE_CADENCE_HOST)) > + return -ENODEV; I don't think this is needed since CONFIG_PCIE_SG2042 selects PCIE_CADENCE_HOST. Bjorn