Re: [PATCH v6 08/10] misc: rp1: RaspberryPi RP1 misc driver
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: 2025-01-17 11:47:29
Also in:
linux-clk, linux-devicetree, linux-gpio, linux-pci, lkml
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.
quoted hunk ↗ jump to hunk
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@google.com/ (local) Link: https://lore.kernel.org/all/83f7fa09-d0e6-4f36-a27d-cee08979be2a@app.fastmail.com/ (local) Link: https://lore.kernel.org/all/2024081356-mutable-everyday-6f9d@gregkh/ (local) Link: https://lore.kernel.org/all/20240808154658.247873-1-herve.codina@bootlin.com/ (local) Signed-off-by: Andrea della Porta <andrea.porta@suse.com> --- 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.hdiff --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.hdiff --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" endmenudiff --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.odiff --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