From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 4C9E1C52D7C for ; Wed, 21 Aug 2024 16:57:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To: Content-Type:MIME-Version:Message-ID:Subject:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:References: List-Owner; bh=yF8SsSrxaH5T1XSkGJV8IQe0bZ/QP9RDc+M7hbG3C3A=; b=01Ss3kwAfhepq+ zSG5XtgwnI00zhPP4HOzMGWKaJGtWCGXy2RhAKRoYlA7CG1aUQJ13s08a9e4mx++CgwxfIpN0hhPS xnAXm33DSm8amJxpQSAJAmuf+Iu6ugDVsBd8Z+iXH7Dgw5a69cAUU7ZvJO90pofJrPoZsh5UTDPS5 7o//KaSNeeAEeKS+Q+WKH5vV7GmiWEuReRKWoBwFOt5UE1mTlQ39FY1WPawpq2RsWlEmoLlZJiE66 +JMAC6F6m7B7YbhPGFmR87uCy1S7bHXLxm4D8xJ2OhU5FyNHOQmc+Q7D2HY02vSh41+LxtrMRGxcI 8AF+Qrku40+FFIgVotIQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sgoeU-00000009q2P-1wkL; Wed, 21 Aug 2024 16:57:42 +0000 Received: from nyc.source.kernel.org ([147.75.193.91]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sgoca-00000009pYu-3pCv; Wed, 21 Aug 2024 16:55:46 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 1101BA4119E; Wed, 21 Aug 2024 16:55:37 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 10041C32786; Wed, 21 Aug 2024 16:55:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1724259343; bh=8v5Y7+Bdr6Vrg6fg/jjVkWGIt4zD/dKPaTA7XMhn+n8=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=UWI3DEoYXhlo5tEJ8+b6j2jglrtnyX7EMyzxBQALxCZIKK3Vv0INTGGsDg0AIv69+ dgiGf1Ndqkr6QIBOzlmEBV2saM1kC/PTaM2XyM1owiQqsS3DmG5j7ptA+pt6yZn0rT bVQWFDEHzMQc/KqhtAKT+P2IYSaiwR3ritbtAxsLsU9KiSsFA479LcSG/bspzSQlnL bQ23dcB92e9qtISrNTjWVChljaVmucocnYAFGkViVKxHEVMhyT8L5xb7rPvVD2RPSC 4Z+oJgI0Mzw41s+SeDEFxx3HsPTyi1kEsW+9eV/s4/h8WQV98g0lwy0pZxbI1dCYJg 9Y92grUkNfw5w== Date: Wed, 21 Aug 2024 11:55:41 -0500 From: Bjorn Helgaas To: Andrea della Porta Subject: Re: [PATCH 08/11] misc: rp1: RaspberryPi RP1 misc driver Message-ID: <20240821165541.GA254124@bhelgaas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5954e4dccc0e158cf434d2c281ad57120538409b.1724159867.git.andrea.porta@suse.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240821_095545_112727_8786CE44 X-CRM114-Status: GOOD ( 25.99 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Andrew Lunn , Catalin Marinas , Michael Turquette , Claudiu Beznea , Eric Dumazet , Dragan Cvetic , Will Deacon , linux-clk@vger.kernel.org, linux-arch@vger.kernel.org, Rob Herring , Florian Fainelli , Lee Jones , Saravana Kannan , Broadcom internal kernel review list , linux-pci@vger.kernel.org, Jakub Kicinski , Paolo Abeni , Linus Walleij , devicetree@vger.kernel.org, Conor Dooley , Arnd Bergmann , linux-gpio@vger.kernel.org, linux-rpi-kernel@lists.infradead.org, Bjorn Helgaas , linux-arm-kernel@lists.infradead.org, Derek Kiernan , Stephen Boyd , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, Stefan Wahren , netdev@vger.kernel.org, Krzysztof Kozlowski , "David S. Miller" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Aug 20, 2024 at 04:36:10PM +0200, Andrea della Porta wrote: > The RaspberryPi RP1 is ia PCI multi function device containing s/ia/a/ > peripherals ranging from Ethernet to USB controller, I2C, SPI > and others. Add blank lines between paragraphs. > Implement a bare minimum driver to operate the RP1, leveraging > actual OF based driver implementations for the on-borad peripherals s/on-borad/on-board/ > by loading a devicetree overlay during driver probe. > 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. > +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. s/support for/support/ Add blank lines between paragraphs. Consider wrapping to fit in 80 columns. Current width of 86 seems random. > 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. s/22/24/ ? > +#define RP1_B0_CHIP_ID 0x10001927 > +#define RP1_C0_CHIP_ID 0x20001927 Drop; both unused. > +#define RP1_PLATFORM_ASIC BIT(1) > +#define RP1_PLATFORM_FPGA BIT(0) Drop; both unused. > +#define RP1_SYSCLK_RATE 200000000 > +#define RP1_SYSCLK_FPGA_RATE 60000000 Drop; both unused. > +enum { > + SYSINFO_CHIP_ID_OFFSET = 0, > + SYSINFO_PLATFORM_OFFSET = 4, > +}; Drop; unused. > +/* MSIX CFG registers start at 0x8 */ s/MSIX/MSI-X/ > +#define MSIX_CFG_TEST BIT(1) Unused. > +#define INTSTATL 0x108 > +#define INTSTATH 0x10c Drop; both unused. > +static void dump_bar(struct pci_dev *pdev, unsigned int bar) > +{ > + dev_info(&pdev->dev, > + "bar%d len 0x%llx, start 0x%llx, end 0x%llx, flags, 0x%lx\n", %pR does most of this for you. > +static int rp1_irq_set_type(struct irq_data *irqd, unsigned int type) > +{ > + struct rp1_dev *rp1 = irqd->domain->host_data; > + unsigned int hwirq = (unsigned int)irqd->hwirq; > + int ret = 0; > + > + switch (type) { > + case IRQ_TYPE_LEVEL_HIGH: > + dev_dbg(rp1->dev, "MSIX IACK EN for irq %d\n", hwirq); > + msix_cfg_set(rp1, hwirq, MSIX_CFG_IACK_EN); > + rp1->level_triggered_irq[hwirq] = true; > + break; > + case IRQ_TYPE_EDGE_RISING: > + msix_cfg_clr(rp1, hwirq, MSIX_CFG_IACK_EN); > + rp1->level_triggered_irq[hwirq] = false; > + break; > + default: > + ret = -EINVAL; If you "return -EINVAL" directly here, I think you can drop "ret" and just "return 0" below. > + break; > + } > + > + return ret; > +} > +static int rp1_irq_xlate(struct irq_domain *d, struct device_node *node, > + const u32 *intspec, unsigned int intsize, > + unsigned long *out_hwirq, unsigned int *out_type) > +{ > + struct rp1_dev *rp1 = d->host_data; > + struct irq_data *pcie_irqd; > + unsigned long hwirq; > + int pcie_irq; > + int ret; > + > + ret = irq_domain_xlate_twocell(d, node, intspec, intsize, > + &hwirq, out_type); > + if (!ret) { > + pcie_irq = pci_irq_vector(rp1->pdev, hwirq); > + pcie_irqd = irq_get_irq_data(pcie_irq); > + rp1->pcie_irqds[hwirq] = pcie_irqd; > + *out_hwirq = hwirq; > + } > + > + return ret; if (ret) return ret; ... return 0; would make this easier to read and unindent the normal path. > + rp1->bar1 = pci_iomap(pdev, 1, 0); pcim_iomap() > + if (!rp1->bar1) { > + dev_err(&pdev->dev, "Cannot map PCI bar\n"); s/bar/BAR/ > +#define PCI_VENDOR_ID_RPI 0x1de4 > +#define PCI_DEVICE_ID_RP1_C0 0x0001 Device ID should include "RPI" as well.