On Mon, Jan 13, 2025 at 03:58:07PM +0100, Andrea della Porta wrote: > The RaspberryPi RP1 is a PCI multi function device containing > peripherals ranging from Ethernet to USB controller, I2C, SPI > and others. > > Implement a bare minimum driver to operate the RP1, leveraging > actual OF based driver implementations for the on-board peripherals > by loading a devicetree overlay during driver probe. > > The peripherals are accessed by mapping MMIO registers starting > from PCI BAR1 region. > > With the overlay approach we can achieve more generic and agnostic > approach to managing this chipset, being that it is a PCI endpoint > and could possibly be reused in other hw implementations. The > presented approach is also used by Bootlin's Microchip LAN966x > patchset (see link) as well, for a similar chipset. > > For reasons why this driver is contained in drivers/misc, please > check the links. Links aren't always around all the time, please document it here why this is needed, and then links can "add to" that summary. > This driver is heavily based on downstream code from RaspberryPi > Foundation, and the original author is Phil Elwell. > > Link: https://datasheets.raspberrypi.com/rp1/rp1-peripherals.pdf > Link: https://lore.kernel.org/all/20240612140208.GC1504919@xxxxxxxxxx/ > Link: https://lore.kernel.org/all/83f7fa09-d0e6-4f36-a27d-cee08979be2a@xxxxxxxxxxxxxxxx/ > Link: https://lore.kernel.org/all/2024081356-mutable-everyday-6f9d@gregkh/ > Link: https://lore.kernel.org/all/20240808154658.247873-1-herve.codina@xxxxxxxxxxx/ > > Signed-off-by: Andrea della Porta <andrea.porta@xxxxxxxx> > --- > MAINTAINERS | 1 + > drivers/misc/Kconfig | 1 + > drivers/misc/Makefile | 1 + > drivers/misc/rp1/Kconfig | 21 +++ > drivers/misc/rp1/Makefile | 3 + > drivers/misc/rp1/rp1-pci.dtso | 8 + > drivers/misc/rp1/rp1_pci.c | 305 ++++++++++++++++++++++++++++++++++ > drivers/misc/rp1/rp1_pci.h | 14 ++ > drivers/pci/quirks.c | 1 + > include/linux/pci_ids.h | 3 + > 10 files changed, 358 insertions(+) > create mode 100644 drivers/misc/rp1/Kconfig > create mode 100644 drivers/misc/rp1/Makefile > create mode 100644 drivers/misc/rp1/rp1-pci.dtso > create mode 100644 drivers/misc/rp1/rp1_pci.c > create mode 100644 drivers/misc/rp1/rp1_pci.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index fbdd8594aa7e..d67ba6d10aa8 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -19583,6 +19583,7 @@ F: Documentation/devicetree/bindings/misc/pci1de4,1.yaml > F: Documentation/devicetree/bindings/pci/pci-ep-bus.yaml > F: Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml > F: drivers/clk/clk-rp1.c > +F: drivers/misc/rp1/ > F: drivers/pinctrl/pinctrl-rp1.c > F: include/dt-bindings/clock/rp1.h > F: include/dt-bindings/misc/rp1.h > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > index 09cbe3f0ab1e..ffa4d8315c35 100644 > --- a/drivers/misc/Kconfig > +++ b/drivers/misc/Kconfig > @@ -651,4 +651,5 @@ source "drivers/misc/uacce/Kconfig" > source "drivers/misc/pvpanic/Kconfig" > source "drivers/misc/mchp_pci1xxxx/Kconfig" > source "drivers/misc/keba/Kconfig" > +source "drivers/misc/rp1/Kconfig" > endmenu > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > index 40bf953185c7..3b6b07a23aac 100644 > --- a/drivers/misc/Makefile > +++ b/drivers/misc/Makefile > @@ -74,3 +74,4 @@ lan966x-pci-objs := lan966x_pci.o > lan966x-pci-objs += lan966x_pci.dtbo.o > obj-$(CONFIG_MCHP_LAN966X_PCI) += lan966x-pci.o > obj-y += keba/ > +obj-$(CONFIG_MISC_RP1) += rp1/ > diff --git a/drivers/misc/rp1/Kconfig b/drivers/misc/rp1/Kconfig > new file mode 100644 > index 000000000000..15c443e13389 > --- /dev/null > +++ b/drivers/misc/rp1/Kconfig > @@ -0,0 +1,21 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +# > +# RaspberryPi RP1 misc device > +# > + > +config MISC_RP1 > + tristate "RaspberryPi RP1 PCIe support" > + depends on OF_IRQ && PCI_MSI && PCI_QUIRKS > + select OF_OVERLAY > + select PCI_DYNAMIC_OF_NODES > + help > + Support the RP1 peripheral chip found on Raspberry Pi 5 board. > + > + This device supports several sub-devices including e.g. Ethernet > + controller, USB controller, I2C, SPI and UART. > + > + The driver is responsible for enabling the DT node once the PCIe > + endpoint has been configured, and handling interrupts. > + > + This driver uses an overlay to load other drivers to support for > + RP1 internal sub-devices. > diff --git a/drivers/misc/rp1/Makefile b/drivers/misc/rp1/Makefile > new file mode 100644 > index 000000000000..508b4cb05627 > --- /dev/null > +++ b/drivers/misc/rp1/Makefile > @@ -0,0 +1,3 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +obj-$(CONFIG_MISC_RP1) += rp1-pci.o > +rp1-pci-objs := rp1_pci.o rp1-pci.dtbo.o > diff --git a/drivers/misc/rp1/rp1-pci.dtso b/drivers/misc/rp1/rp1-pci.dtso > new file mode 100644 > index 000000000000..0bf2f4bb18e6 > --- /dev/null > +++ b/drivers/misc/rp1/rp1-pci.dtso > @@ -0,0 +1,8 @@ > +// SPDX-License-Identifier: (GPL-2.0 OR MIT) > + > +/* the dts overlay is included from the dts directory so > + * it can be possible to check it with CHECK_DTBS while > + * also compile it from the driver source directory. > + */ > + > +#include "arm64/broadcom/rp1.dtso" > diff --git a/drivers/misc/rp1/rp1_pci.c b/drivers/misc/rp1/rp1_pci.c > new file mode 100644 > index 000000000000..3e8ba3fa7fd5 > --- /dev/null > +++ b/drivers/misc/rp1/rp1_pci.c > @@ -0,0 +1,305 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018-24 Raspberry Pi Ltd. > + * All rights reserved. > + */ > + > +#include <linux/err.h> > +#include <linux/interrupt.h> > +#include <linux/irq.h> > +#include <linux/irqchip/chained_irq.h> > +#include <linux/irqdomain.h> > +#include <linux/module.h> > +#include <linux/msi.h> > +#include <linux/of_platform.h> > +#include <linux/pci.h> > +#include <linux/platform_device.h> > + > +#include "rp1_pci.h" Why does a self-contained .c file need a .h file? Please put it all in here. > + > +#define RP1_DRIVER_NAME "rp1" KBUILD_MODNAME? > + > +#define RP1_HW_IRQ_MASK GENMASK(5, 0) > + > +#define REG_SET 0x800 > +#define REG_CLR 0xc00 > + > +/* MSI-X CFG registers start at 0x8 */ > +#define MSIX_CFG(x) (0x8 + (4 * (x))) > + > +#define MSIX_CFG_IACK_EN BIT(3) > +#define MSIX_CFG_IACK BIT(2) > +#define MSIX_CFG_ENABLE BIT(0) > + > +/* Address map */ > +#define RP1_PCIE_APBS_BASE 0x108000 > + > +/* Interrupts */ > +#define RP1_INT_END 61 > + > +struct rp1_dev { > + struct pci_dev *pdev; > + struct irq_domain *domain; > + struct irq_data *pcie_irqds[64]; > + void __iomem *bar1; > + int ovcs_id; /* overlay changeset id */ > + bool level_triggered_irq[RP1_INT_END]; > +}; > + > +static void msix_cfg_set(struct rp1_dev *rp1, unsigned int hwirq, u32 value) > +{ > + iowrite32(value, rp1->bar1 + RP1_PCIE_APBS_BASE + REG_SET + MSIX_CFG(hwirq)); Do your writes need a read to flush them properly? Or can they handle this automatically? thanks, greg k-h