Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754867AbdFXCB5 (ORCPT ); Fri, 23 Jun 2017 22:01:57 -0400 Received: from mail-pf0-f196.google.com ([209.85.192.196]:34233 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754175AbdFXCBx (ORCPT ); Fri, 23 Jun 2017 22:01:53 -0400 Date: Fri, 23 Jun 2017 19:01:51 -0700 (PDT) X-Google-Original-Date: Fri, 23 Jun 2017 18:47:04 PDT (-0700) From: Palmer Dabbelt To: Arnd Bergmann CC: hch@infradead.org CC: geert@linux-m68k.org CC: linux-arch@vger.kernel.org CC: linux-kernel@vger.kernel.org CC: Olof Johansson CC: albert@sifive.com CC: patches@groups.riscv.org CC: bhelgaas@google.com CC: linux-pci@vger.kernel.org Subject: Re: [PATCH 06/17] pci: Add generic pcibios_{fixup_bus,align_resource} In-Reply-To: Message-ID: Mime-Version: 1.0 (MHng) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2456 Lines: 56 On Thu, 08 Jun 2017 01:35:29 PDT (-0700), Arnd Bergmann wrote: > On Thu, Jun 8, 2017 at 10:12 AM, Christoph Hellwig wrote: >> On Wed, Jun 07, 2017 at 09:19:49AM +0200, Geert Uytterhoeven wrote: >>> CC pci folks >> >> Ok, replying with pci folks in Cc then :) >> >> Weak symbols have (rightly) gotten a bad reputation, so maybe >> we should approach this without them. > > Agreed, I would almost never recommend using a __weak symbol, > but they are already widely used in the PCI subsystem, so I suggested > using them here for consistency. > > We have a struct pci_host_bridge now, and we should eventually > move most of the 38 pci specific weak per-architecture symbols > into per-host driver callbacks, but that I think that would be too much > to ask for when adding an architecture port. I agree :) >> It seems we have a large number of emptry pcibios_fixup_bus calls >> alreayd, so I think we should simply have the architectures >> that do define it define a Kconfig or header symbol and not call >> it at all otherwise. > > I would argue that most of them should not be per-architecture > in the first place, the current state is mostly an artifact of the > times when each architecture had just one PCI implementation. > > The ones that have multiple implementations (arm, powerpc, ...) > tend to actually override the weak functions with their own > per-host multiplexers again. > >> For the ones that exist as lot just seem to call pci_read_bridge_bases >> and/or pcibios_fixup_device_resources in one form or another, >> and I wonder why we even need the arch indirection for that. >> >> Similarly for pcibios_align_resource: a lot of architetures seem >> to have a noop, and the once that don't mostly seem copy and >> paste code, so we should again have a symbol for architectures >> to opt into it, and we probably should have a generic helper >> for the VGA window mirroring code instead of duplicating it multiple >> times. > > I now remember that we already have a host_bridge->align_resource > callback pointer, so the generic function should definitely try > to use that. > > We could just use the version from arch/mips/pci/pci-generic.c > and remove that in the process. I'm not sure about why mips calls > pci_read_bridge_bases() in pcibios_fixup_bus() though, or whether > this make sense to put in the generic version. I'm splitting this off into another patch set and sending it to a bunch of PCI people.