From patchwork Thu Jan 22 20:33:36 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 5687891 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id E2EF69F358 for ; Thu, 22 Jan 2015 20:33:45 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C42B720265 for ; Thu, 22 Jan 2015 20:33:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 64AE420251 for ; Thu, 22 Jan 2015 20:33:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754051AbbAVUdm (ORCPT ); Thu, 22 Jan 2015 15:33:42 -0500 Received: from mail-oi0-f45.google.com ([209.85.218.45]:48855 "EHLO mail-oi0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753849AbbAVUdk (ORCPT ); Thu, 22 Jan 2015 15:33:40 -0500 Received: by mail-oi0-f45.google.com with SMTP id g201so3326002oib.4 for ; Thu, 22 Jan 2015 12:33:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=H4GPP5MaHeX3uVEbtzZqEhHi4Knqs0aTI+7CHf57opI=; b=hIc+nVbXe4gMzEOg6jzUbAfIa1Jt7yD2wqxlq/0RcZYIpEL3F3cCkbD2rLxv2a8ex3 QquvR8cGtaJvvKSqwTPaUZ1xArYEsJmC1bPFZeDhYT64l7fSJy8QCvBXqlnE4xTa6wiZ +LJ9NHaTmyl8kyJuWIDnzpiGXGjbYlgaiD0ebQTl57OmyRYrzv2I/l3gxf6YcLv6y74V QpNW9dmcT98Z5lyCAe+6c1wy80DrOdrNQUT4YkX9dSzA7IoGRJkMKgkmkfy4//RXHeUK iiyP7pIJGi0PKuxSUoXUKsDzKdDU7vPSJeL+3yQLQ/vhozbtSNYvhyw8L5X3KNsovjT1 dbLA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=H4GPP5MaHeX3uVEbtzZqEhHi4Knqs0aTI+7CHf57opI=; b=Jcyc5Vz8u4IMHCyGmDIKIdNl5BtfQYwoDv/ndHHQXVqXcC00Kfpneh/TSZpD2gqkVa JAay0qYTQZeXSk/7I9UiwM/mI4DQToH9Xzr0xGzitIwTgjVmg5M+g+pq4egQOzumrdit 6eYqV4+sZgm0+sLvsSu+vAsuAidccFedGCmv4IPdlnKMa4FtJMWyp4onFQuq4Np9jvM9 P6+yBA95y0tzTIcHCJHKral/BIbKXmO0ScVxuF1yF+lW4w4Ara1SBE+G4Suw/1ffJBEJ bH1QuNIxhMQMlTHVNcrdvkiTcotovxQ84K8f7CjWH4iji4OnuMyA+atkUclHsUbtGACQ rjvA== X-Gm-Message-State: ALoCoQmLzZNrFJSn/mOuVAyO9iANBpwecFt+n2FOiS+16llo7Rk+hWeVrokP1hdMa7jtmcXFo0oM X-Received: by 10.202.56.84 with SMTP id f81mr1974535oia.101.1421958819930; Thu, 22 Jan 2015 12:33:39 -0800 (PST) Received: from google.com ([69.71.1.1]) by mx.google.com with ESMTPSA id s5sm2082633oie.16.2015.01.22.12.33.38 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Thu, 22 Jan 2015 12:33:39 -0800 (PST) Date: Thu, 22 Jan 2015 14:33:36 -0600 From: Bjorn Helgaas To: Linus Walleij Cc: Arnd Bergmann , Rob Herring , "linux-kernel@vger.kernel.org" , linux-pci , Russell King , "linux-arm-kernel@lists.infradead.org" Subject: Re: [PATCH 07/16] ARM: integrator: convert PCI to use generic config accesses Message-ID: <20150122203336.GD13072@google.com> References: <1420857290-8373-1-git-send-email-robh@kernel.org> <1420857290-8373-8-git-send-email-robh@kernel.org> <4938579.0KIk3QtTCP@wuerfel> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-3.4 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FSL_HELO_FAKE, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Jan 12, 2015 at 01:05:12AM +0100, Linus Walleij wrote: > On Sat, Jan 10, 2015 at 10:53 PM, Arnd Bergmann wrote: > > On Saturday 10 January 2015 22:40:22 Linus Walleij wrote: > >> > static int v3_read_config(struct pci_bus *bus, unsigned int devfn, int where, > >> > int size, u32 *val) > >> > { > >> > - addr = v3_open_config_window(bus, devfn, where); > >> > + int ret = pci_generic_config_read(bus, devfn, where, size, val); > >> > v3_close_config_window(); > >> > + return ret; > >> > } > >> > > >> > static int v3_write_config(struct pci_bus *bus, unsigned int devfn, int where, > >> > int size, u32 val) > >> > { > >> > + int ret = pci_generic_config_write(bus, devfn, where, size, val); > >> > v3_close_config_window(); > >> > - raw_spin_unlock_irqrestore(&v3_lock, flags); > >> > + return ret; > >> > } > >> > > >> > static struct pci_ops pci_v3_ops = { > >> > + .map_bus = v3_open_config_window, > >> > .read = v3_read_config, > >> > .write = v3_write_config, > >> > >> So .map_bus is called before every .read/.write operation I take it. > >> > >> Wouldn't it be proper to call the v3_close_config_window() from a > >> matching .unmap_bus() callback for symmetry? > > > > It would be nicer for integrator but useless for anything else, so > > I'd vote for leaving it the way Rob posted. > > OK I buy that, throw in a comment about it in the code if there > is some time for iterating the patch. Would you prefer something like the following instead? It keeps the v3_open/close symmetry, but it does break apart and duplicate some of the logic from v3_open_config_window(). I can't build or test this. commit 3e0c269a560968002298d22ac47bde4bd45aea8e Author: Rob Herring Date: Fri Jan 9 20:34:41 2015 -0600 ARM: integrator: Convert PCI to use generic config accessors Convert the integrator PCI driver to use the generic config access functions. This changes accesses from __raw_readX/__raw_writeX to readX/writeX variants. The spinlock is removed because it is unnecessary. The config read and write functions are already protected with a spinlock and no access can occur during the .pre_init function. Signed-off-by: Rob Herring Signed-off-by: Bjorn Helgaas Acked-by: Linus Walleij CC: Russell King CC: linux-arm-kernel@lists.infradead.org --- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/arch/arm/mach-integrator/pci_v3.c b/arch/arm/mach-integrator/pci_v3.c index c186a17c2cff..c1098729cf8a 100644 --- a/arch/arm/mach-integrator/pci_v3.c +++ b/arch/arm/mach-integrator/pci_v3.c @@ -324,7 +324,7 @@ static u64 pre_mem_pci_sz; * configuration address space, you present the V3 with the following pattern * (which is very nearly a type 1 (except that the lower two bits are 00 and * not 01). In order for this mapping to work you need to set up one of - * the local to PCI aperatures to 16Mbytes in length translating to + * the local to PCI apertures to 16Mbytes in length translating to * PCI configuration space starting at 0x0000.0000. * * PCI configuration cycles look like this: @@ -356,15 +356,14 @@ static u64 pre_mem_pci_sz; * 7:2 register number * */ -static DEFINE_RAW_SPINLOCK(v3_lock); #undef V3_LB_BASE_PREFETCH #define V3_LB_BASE_PREFETCH 0 -static void __iomem *v3_open_config_window(struct pci_bus *bus, - unsigned int devfn, int offset) +static void v3_open_config_window(struct pci_bus *bus, unsigned int devfn, + int offset) { - unsigned int address, mapaddress, busnr; + unsigned int mapaddress, busnr; busnr = bus->number; @@ -381,14 +380,10 @@ static void __iomem *v3_open_config_window(struct pci_bus *bus, /* * local bus segment so need a type 0 config cycle * - * build the PCI configuration "address" with one-hot in - * A31-A11 - * * mapaddress: * 3:1 = config cycle (101) * 0 = PCI A1 & A0 are 0 (0) */ - address = PCI_FUNC(devfn) << 8; mapaddress = V3_LB_MAP_TYPE_CONFIG; if (slot > 12) @@ -396,26 +391,15 @@ static void __iomem *v3_open_config_window(struct pci_bus *bus, * high order bits are handled by the MAP register */ mapaddress |= 1 << (slot - 5); - else - /* - * low order bits handled directly in the address - */ - address |= 1 << (slot + 11); } else { /* * not the local bus segment so need a type 1 config cycle * - * address: - * 23:16 = bus number - * 15:11 = slot number (7:3 of devfn) - * 10:8 = func number (2:0 of devfn) - * * mapaddress: * 3:1 = config cycle (101) * 0 = PCI A1 & A0 from host bus (1) */ mapaddress = V3_LB_MAP_TYPE_CONFIG | V3_LB_MAP_AD_LOW_EN; - address = (busnr << 16) | (devfn << 8); } /* @@ -432,8 +416,6 @@ static void __iomem *v3_open_config_window(struct pci_bus *bus, v3_writel(V3_LB_BASE1, v3_addr_to_lb_base(conf_mem.start) | V3_LB_BASE_ADR_SIZE_16MB | V3_LB_BASE_ENABLE); v3_writew(V3_LB_MAP1, mapaddress); - - return PCI_CONFIG_VADDR + address + offset; } static void v3_close_config_window(void) @@ -454,70 +436,66 @@ static void v3_close_config_window(void) V3_LB_BASE_ADR_SIZE_256MB | V3_LB_BASE_ENABLE); } -static int v3_read_config(struct pci_bus *bus, unsigned int devfn, int where, - int size, u32 *val) +static void __iomem *v3_map_bus(struct pci_bus *bus, + unsigned int devfn, int offset) { - void __iomem *addr; - unsigned long flags; - u32 v; + unsigned int busnr = bus->number; + unsigned int address; - raw_spin_lock_irqsave(&v3_lock, flags); - addr = v3_open_config_window(bus, devfn, where); - - switch (size) { - case 1: - v = __raw_readb(addr); - break; + if (busnr == 0) { + int slot = PCI_SLOT(devfn); - case 2: - v = __raw_readw(addr); - break; + /* + * build the PCI configuration "address" with function in + * A10-A8 and device one-hot in A31-A11 + */ + address = PCI_FUNC(devfn) << 8; - default: - v = __raw_readl(addr); - break; + /* + * high order bits are handled by the MAP register; + * low order bits handled directly in the address + */ + if (slot <= 12) + address |= 1 << (slot + 11); + } else { + /* + * not the local bus segment so need a type 1 config cycle + * + * address: + * 23:16 = bus number + * 15:11 = slot number (7:3 of devfn) + * 10:8 = func number (2:0 of devfn) + */ + address = (busnr << 16) | (devfn << 8); } - v3_close_config_window(); - raw_spin_unlock_irqrestore(&v3_lock, flags); + return PCI_CONFIG_VADDR + address + offset; +} + +static int v3_read_config(struct pci_bus *bus, unsigned int devfn, int where, + int size, u32 *val) +{ + int ret; - *val = v; - return PCIBIOS_SUCCESSFUL; + v3_open_config_window(); + ret = pci_generic_config_read(bus, devfn, where, size, val); + v3_close_config_window(); + return ret; } static int v3_write_config(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val) { - void __iomem *addr; - unsigned long flags; - - raw_spin_lock_irqsave(&v3_lock, flags); - addr = v3_open_config_window(bus, devfn, where); - - switch (size) { - case 1: - __raw_writeb((u8)val, addr); - __raw_readb(addr); - break; - - case 2: - __raw_writew((u16)val, addr); - __raw_readw(addr); - break; - - case 4: - __raw_writel(val, addr); - __raw_readl(addr); - break; - } + int ret; + v3_open_config_window(); + ret = pci_generic_config_write(bus, devfn, where, size, val); v3_close_config_window(); - raw_spin_unlock_irqrestore(&v3_lock, flags); - - return PCIBIOS_SUCCESSFUL; + return ret; } static struct pci_ops pci_v3_ops = { + .map_bus = v3_map_bus, .read = v3_read_config, .write = v3_write_config, }; @@ -672,8 +650,6 @@ static void __init pci_v3_preinit(void) hook_fault_code(8, v3_pci_fault, SIGBUS, 0, "external abort on non-linefetch"); hook_fault_code(10, v3_pci_fault, SIGBUS, 0, "external abort on non-linefetch"); - raw_spin_lock_irqsave(&v3_lock, flags); - /* * Unlock V3 registers, but only if they were previously locked. */ @@ -736,8 +712,6 @@ static void __init pci_v3_preinit(void) v3_writew(V3_LB_CFG, v3_readw(V3_LB_CFG) | (1 << 10)); v3_writeb(V3_LB_IMASK, 0x28); __raw_writel(3, ap_syscon_base + INTEGRATOR_SC_PCIENABLE_OFFSET); - - raw_spin_unlock_irqrestore(&v3_lock, flags); } static void __init pci_v3_postinit(void)