| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <98c570cb-c2ca-4816-9ca4-94033f7fb3fb@gmx.net>
Date: Wed, 21 Aug 2024 18:20:29 +0200
From: Stefan Wahren <wahrenst@....net>
To: Andrea della Porta <andrea.porta@...e.com>,
Michael Turquette <mturquette@...libre.com>, Stephen Boyd
<sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Florian Fainelli <florian.fainelli@...adcom.com>,
Broadcom internal kernel review list
<bcm-kernel-feedback-list@...adcom.com>,
Linus Walleij <linus.walleij@...aro.org>,
Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
Derek Kiernan <derek.kiernan@....com>, Dragan Cvetic
<dragan.cvetic@....com>, Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Nicolas Ferre <nicolas.ferre@...rochip.com>,
Claudiu Beznea <claudiu.beznea@...on.dev>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Saravana Kannan <saravanak@...gle.com>, Bjorn Helgaas <bhelgaas@...gle.com>,
linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
linux-rpi-kernel@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org,
netdev@...r.kernel.org, linux-pci@...r.kernel.org,
linux-arch@...r.kernel.org, Lee Jones <lee@...nel.org>,
Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH 08/11] misc: rp1: RaspberryPi RP1 misc driver
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@...e.com>
> ---
> 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@...0018000 {
> + 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@...00d0000 {
> + 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@...pberrypi.com>");
Module author & Copyright doesn't seem to match with this patch author.
Please clarify/fix
Powered by blists - more mailing lists