Re: [PATCH 08/11] misc: rp1: RaspberryPi RP1 misc driver

From: Stefan Wahren
Date: Wed Aug 21 2024 - 12:21:33 EST


Hi Andrea,

Am 20.08.24 um 16:36 schrieb Andrea della Porta:
The RaspberryPi RP1 is ia PCI multi function device containing
peripherals ranging from Ethernet to USB controller, I2C, SPI
and others.
sorry, i cannot provide you a code review, but just some comments. multi
function device suggests "mfd" subsystem or at least "soc" . I won't
recommend misc driver here.
Implement a bare minimum driver to operate the RP1, leveraging
actual OF based driver implementations for the on-borad peripherals
by loading a devicetree overlay during driver probe.
Can you please explain why this should be a DT overlay? The RP1 is
assembled on the Raspberry Pi 5 PCB. DT overlays are typically for loose
connections like displays or HATs. I think a DTSI just for the RP1 would
fit better and is easier to read.
The peripherals are accessed by mapping MMIO registers starting
from PCI BAR1 region.
As a minimum driver, the peripherals will not be added to the
dtbo here, but in following patches.

Link: https://datasheets.raspberrypi.com/rp1/rp1-peripherals.pdf
Signed-off-by: Andrea della Porta <andrea.porta@xxxxxxxx>
---
MAINTAINERS | 2 +
arch/arm64/boot/dts/broadcom/rp1.dtso | 152 ++++++++++++
drivers/misc/Kconfig | 1 +
drivers/misc/Makefile | 1 +
drivers/misc/rp1/Kconfig | 20 ++
drivers/misc/rp1/Makefile | 3 +
drivers/misc/rp1/rp1-pci.c | 333 ++++++++++++++++++++++++++
drivers/misc/rp1/rp1-pci.dtso | 8 +
drivers/pci/quirks.c | 1 +
include/linux/pci_ids.h | 3 +
10 files changed, 524 insertions(+)
create mode 100644 arch/arm64/boot/dts/broadcom/rp1.dtso
create mode 100644 drivers/misc/rp1/Kconfig
create mode 100644 drivers/misc/rp1/Makefile
create mode 100644 drivers/misc/rp1/rp1-pci.c
create mode 100644 drivers/misc/rp1/rp1-pci.dtso

...
+
+ rp1_clocks: clocks@c040018000 {
+ compatible = "raspberrypi,rp1-clocks";
+ #clock-cells = <1>;
+ reg = <0xc0 0x40018000 0x0 0x10038>;
+ clocks = <&clk_xosc>;
+ clock-names = "xosc";
+
+ assigned-clocks = <&rp1_clocks RP1_PLL_SYS_CORE>,
+ <&rp1_clocks RP1_PLL_AUDIO_CORE>,
+ // RP1_PLL_VIDEO_CORE and dividers are now managed by VEC,DPI drivers
+ <&rp1_clocks RP1_PLL_SYS>,
+ <&rp1_clocks RP1_PLL_SYS_SEC>,
+ <&rp1_clocks RP1_PLL_SYS_PRI_PH>,
+ <&rp1_clocks RP1_CLK_ETH_TSU>;
+
+ assigned-clock-rates = <1000000000>, // RP1_PLL_SYS_CORE
+ <1536000000>, // RP1_PLL_AUDIO_CORE
+ <200000000>, // RP1_PLL_SYS
+ <125000000>, // RP1_PLL_SYS_SEC
+ <100000000>, // RP1_PLL_SYS_PRI_PH
+ <50000000>; // RP1_CLK_ETH_TSU
+ };
+
+ rp1_gpio: pinctrl@c0400d0000 {
+ reg = <0xc0 0x400d0000 0x0 0xc000>,
+ <0xc0 0x400e0000 0x0 0xc000>,
+ <0xc0 0x400f0000 0x0 0xc000>;
+ compatible = "raspberrypi,rp1-gpio";
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ interrupts = <RP1_INT_IO_BANK0 IRQ_TYPE_LEVEL_HIGH>,
+ <RP1_INT_IO_BANK1 IRQ_TYPE_LEVEL_HIGH>,
+ <RP1_INT_IO_BANK2 IRQ_TYPE_LEVEL_HIGH>;
+ gpio-line-names =
+ "ID_SDA", // GPIO0
+ "ID_SCL", // GPIO1
+ "GPIO2", // GPIO2
+ "GPIO3", // GPIO3
+ "GPIO4", // GPIO4
+ "GPIO5", // GPIO5
+ "GPIO6", // GPIO6
+ "GPIO7", // GPIO7
+ "GPIO8", // GPIO8
+ "GPIO9", // GPIO9
+ "GPIO10", // GPIO10
+ "GPIO11", // GPIO11
+ "GPIO12", // GPIO12
+ "GPIO13", // GPIO13
+ "GPIO14", // GPIO14
+ "GPIO15", // GPIO15
+ "GPIO16", // GPIO16
+ "GPIO17", // GPIO17
+ "GPIO18", // GPIO18
+ "GPIO19", // GPIO19
+ "GPIO20", // GPIO20
+ "GPIO21", // GPIO21
+ "GPIO22", // GPIO22
+ "GPIO23", // GPIO23
+ "GPIO24", // GPIO24
+ "GPIO25", // GPIO25
+ "GPIO26", // GPIO26
+ "GPIO27", // GPIO27
+ "PCIE_RP1_WAKE", // GPIO28
+ "FAN_TACH", // GPIO29
+ "HOST_SDA", // GPIO30
+ "HOST_SCL", // GPIO31
+ "ETH_RST_N", // GPIO32
+ "", // GPIO33
+ "CD0_IO0_MICCLK", // GPIO34
+ "CD0_IO0_MICDAT0", // GPIO35
+ "RP1_PCIE_CLKREQ_N", // GPIO36
+ "", // GPIO37
+ "CD0_SDA", // GPIO38
+ "CD0_SCL", // GPIO39
+ "CD1_SDA", // GPIO40
+ "CD1_SCL", // GPIO41
+ "USB_VBUS_EN", // GPIO42
+ "USB_OC_N", // GPIO43
+ "RP1_STAT_LED", // GPIO44
+ "FAN_PWM", // GPIO45
+ "CD1_IO0_MICCLK", // GPIO46
+ "2712_WAKE", // GPIO47
+ "CD1_IO1_MICDAT1", // GPIO48
+ "EN_MAX_USB_CUR", // GPIO49
+ "", // GPIO50
+ "", // GPIO51
+ "", // GPIO52
+ ""; // GPIO53
GPIO line names are board specific, so this should go to the Raspberry
Pi 5 file.
+ };
+ };
+ };
+ };
+};
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 41c3d2821a78..02405209e6c4 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -618,4 +618,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 c2f990862d2b..84bfa866fbee 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -71,3 +71,4 @@ obj-$(CONFIG_TPS6594_PFSM) += tps6594-pfsm.o
obj-$(CONFIG_NSM) += nsm.o
obj-$(CONFIG_MARVELL_CN10K_DPI) += mrvl_cn10k_dpi.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..050417ee09ae
--- /dev/null
+++ b/drivers/misc/rp1/Kconfig
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# RaspberryPi RP1 misc device
+#
+
+config MISC_RP1
+ tristate "RaspberryPi RP1 PCIe support"
+ depends on PCI && PCI_QUIRKS
+ select OF
+ select OF_OVERLAY
+ select IRQ_DOMAIN
+ select PCI_DYNAMIC_OF_NODES
+ help
+ Support for 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..e83854b4ed2c
--- /dev/null
+++ b/drivers/misc/rp1/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0-only
+rp1-pci-objs := rp1-pci.o rp1-pci.dtbo.o
+obj-$(CONFIG_MISC_RP1) += rp1-pci.o
diff --git a/drivers/misc/rp1/rp1-pci.c b/drivers/misc/rp1/rp1-pci.c
new file mode 100644
index 000000000000..a6093ba7e19a
--- /dev/null
+++ b/drivers/misc/rp1/rp1-pci.c
@@ -0,0 +1,333 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018-22 Raspberry Pi Ltd.
+ * All rights reserved.
+ */
+
+#include <linux/clk.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#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 <linux/reset.h>
+
+#include <dt-bindings/misc/rp1.h>
+
+#define RP1_B0_CHIP_ID 0x10001927
+#define RP1_C0_CHIP_ID 0x20001927
+
+#define RP1_PLATFORM_ASIC BIT(1)
+#define RP1_PLATFORM_FPGA BIT(0)
+
+#define RP1_DRIVER_NAME "rp1"
+
+#define RP1_ACTUAL_IRQS RP1_INT_END
+#define RP1_IRQS RP1_ACTUAL_IRQS
+#define RP1_HW_IRQ_MASK GENMASK(5, 0)
+
+#define RP1_SYSCLK_RATE 200000000
+#define RP1_SYSCLK_FPGA_RATE 60000000
+
+enum {
+ SYSINFO_CHIP_ID_OFFSET = 0,
+ SYSINFO_PLATFORM_OFFSET = 4,
+};
+
+#define REG_SET 0x800
+#define REG_CLR 0xc00
+
+/* MSIX 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_TEST BIT(1)
+#define MSIX_CFG_ENABLE BIT(0)
+
+#define INTSTATL 0x108
+#define INTSTATH 0x10c
+
+extern char __dtbo_rp1_pci_begin[];
+extern char __dtbo_rp1_pci_end[];
+
+struct rp1_dev {
+ struct pci_dev *pdev;
+ struct device *dev;
+ struct clk *sys_clk;
+ struct irq_domain *domain;
+ struct irq_data *pcie_irqds[64];
+ void __iomem *bar1;
+ int ovcs_id;
+ bool level_triggered_irq[RP1_ACTUAL_IRQS];
+};
+
+
...
+
+static const struct pci_device_id dev_id_table[] = {
+ { PCI_DEVICE(PCI_VENDOR_ID_RPI, PCI_DEVICE_ID_RP1_C0), },
+ { 0, }
+};
+
+static struct pci_driver rp1_driver = {
+ .name = RP1_DRIVER_NAME,
+ .id_table = dev_id_table,
+ .probe = rp1_probe,
+ .remove = rp1_remove,
+};
+
+module_pci_driver(rp1_driver);
+
+MODULE_AUTHOR("Phil Elwell <phil@xxxxxxxxxxxxxxx>");
Module author & Copyright doesn't seem to match with this patch author.
Please clarify/fix