From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161040AbcFBLoG (ORCPT ); Thu, 2 Jun 2016 07:44:06 -0400 Received: from mout.kundenserver.de ([212.227.126.187]:63691 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1161016AbcFBLoD (ORCPT ); Thu, 2 Jun 2016 07:44:03 -0400 From: Arnd Bergmann To: Bjorn Helgaas Cc: Bjorn Helgaas , Heiko Stuebner , Wenrui Li , Doug Anderson , linux-pci@vger.kernel.org, linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org, Shawn Lin , Thomas Petazzoni , linux-arm-kernel@lists.infradead.org, Jingoo Han , Pratyush Anand , Hannes Reinecke , Alex Williamson Subject: Re: [PATCH 1/3] pci: introduce read_bridge/write_bridge pci ops Date: Wed, 01 Jun 2016 22:37:28 +0200 Message-ID: <4967020.J4dsRYGugq@wuerfel> User-Agent: KMail/5.1.3 (Linux/4.4.0-22-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: <20160601190430.GA19546@localhost> References: <1464784332-3775650-1-git-send-email-arnd@arndb.de> <4890460.9IGW89muCc@wuerfel> <20160601190430.GA19546@localhost> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:mEhSsz6SVTDcQUd9UcWZZqHncOPTw5jqCy2Z9h6p8I6nee7SXex FgOozCC+KMB6ImK1k0Yoh5OzkktTEJ4SLO6wdmup5uRgZvTosX81fdZEPkPUHGfn3hTGK1Z bycn1IwxH1BrZJqXpP7bXmOjS9gRKE1ww5NWXcXnlXWBbwOvpU2+6PCWUr7fzRzQuU3n7zy s9DS4t79IbOllrk9MJqjg== X-UI-Out-Filterresults: notjunk:1;V01:K0:oPSlVAsM7Aw=:hx623V5UbGZRQhQqVCeO4V XLdHU/gZXGdjlcFE72/1cL0TwMQN8k8IR/YE/FP0YBaVyxZ5KHqyPCMqJx2fxIBlwD7sdTkoT tPC4FpgusR1kStq1I67CORcacgeqt3yJnePJYBJ+qo1xE+qc33ZRViqmYhu/UCx0fXckE85QB sD52frMQ/Qpk82nk63eIa/2p/ZyQ1hvLfDIuwKk0YzuOcjC7gofLsAYgURF2ckoqKGr0Hc+qW FCW0gBlFhk4O/5dlu7Bk5H2om0gkmfCBZxgU3k+G/An9vBn7PgujPIzAnAbfb+3uSTpFDPzps FFUg3ny40xYobUHBtu641AmiR0pI81VnXvqnSSQ2n6n7fDxzLAeyMdACcXtSTLrWhNkkEYQZc GP2qcnnJJSAC/cJdL5k9PIMp/mrAx5WQFfh5FGNBKkNRR5+d0l72BJWakbjTN3awbX/FLLjUO I695QdHTC47CxJ/PlzSlP7/H38qHQ1tKCJ+RRGY3Mb0+/5mE49eS/gu4Hi8c5DX5m+/tU9f2F lb/1k00GGfrMMCJWoB7CO3nHE0jT//qgD454ubqSKCq2sJu0arYaWKaZzxwY0IXup2qgiFLSz DBs7elK654jGTFRe2OBQn9Logkqnlu3UQV+qRzV/B02mAJETl06TlpR6AY00esESzdPmvOHdm CQiwe8lVgquD+GiKBSd2fElUTLh4dIrfn4bxsscYu9seq3s4bW0qKvwif3cpXVMl/kh+S2nYC H1JhhizPElqAMYlU Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday, June 1, 2016 2:04:30 PM CEST Bjorn Helgaas wrote: > On Wed, Jun 01, 2016 at 05:41:53PM +0200, Arnd Bergmann wrote: > > On Wednesday, June 1, 2016 10:09:29 AM CEST Bjorn Helgaas wrote: > > > Hi Arnd, > > > > > > On Wed, Jun 01, 2016 at 02:31:22PM +0200, Arnd Bergmann wrote: > > > > A lot of PCI host bridges require different methods for initiating > > > > type 0 and type 1 config space accesses, leading to duplication of > > > > code. > > > > > > > > This adds support for the two different kinds at the pci_ops > > > > level, with the newly added map_bridge/read_bridge/write_bridge > > > > operations for type 1 accesses. > > > > > > > > When these are not set, we fall back to the regular map_bus/read/write > > > > operations, so all existing drivers keep working, and bridges that > > > > have identical operations continue to only require one set. > > > > > > This adds new config accessor functions to struct pci_ops and makes > > > the callers responsible for figuring out which one to use. The > > > benefit is to reduce code duplication in some host bridge drivers > > > (DesignWare and MVEBU so far). > > > > > > From a design perspective, I'm not comfortable with moving this burden > > > from the host bridge drivers to the callers of the config accessors. > > ... > > > Maybe we can simply change them to use the normal API and come up with > > a way to make the pci_ops harder to misuse? Would it make you feel better > > if we also renamed .read/.write into .read_type0/.write_type0 or something > > like that? > > I'm trying to get a better feel for the tradeoff here. It seems like > an API complication vs. code duplication. > > I don't really think the callers should have to figure out which > accessor to use. How much of a benefit do we really gain by > complicating the callers? We've managed for quite a few years with > the current scheme, and it seems like only a couple new ARM platforms > would benefit. I just did a count of the implementations of pci_ops: I found 107 instances of 'struct pci_ops', and 67 of them treat type0 and type1 access differently in some form. I'd estimate that about half of them, or roughly a third of the total instances would benefit from my change, if we were to do them again. Clearly there is no need to change the existing code here when it works, unless the benefit is very clear and the code is actively maintained. In some cases, the difference is only that the root bus has a limited set of devices that are allowed to be accessed, so there would likely be no benefit of this, compared to e.g. yet another callback that checks the validity. Some other instances have type0 registers at a different memory location from type1, some use different layout inside of that space, and some are completely different. Arnd