From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755558AbbKRJud (ORCPT ); Wed, 18 Nov 2015 04:50:33 -0500 Received: from mout.kundenserver.de ([212.227.126.187]:63981 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755207AbbKRJua (ORCPT ); Wed, 18 Nov 2015 04:50:30 -0500 From: Arnd Bergmann To: Marc Zyngier Cc: Ray Jui , Bjorn Helgaas , Hauke Mehrtens , linux-kernel@vger.kernel.org, bcm-kernel-feedback-list@broadcom.com, linux-pci@vger.kernel.org Subject: Re: [PATCH 4/5] PCI: iproc: Add iProc PCIe MSI support Date: Wed, 18 Nov 2015 10:50:18 +0100 Message-ID: <6698299.zIsv0CNpOi@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <20151118084845.49ba6304@arm.com> References: <1447806715-30043-1-git-send-email-rjui@broadcom.com> <1447806715-30043-5-git-send-email-rjui@broadcom.com> <20151118084845.49ba6304@arm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:QM7hlVWABUHbhYcDDKH9FEt4rfVNvGnd/r5xwRNV7S1xs5We5gr 6D9iUi3RYkc04q9jULVsD5/qZNxT/Etn5GCsH+rj0G/99IU29pfLWJzt/VCEx8NYdXDyIqi UpZnmxvl1cHhwVeonKeWMIbgvTBMGZA6TZFNBBmc8dmLHpMB+0Xv+lDyjlxi08bugS00I6l 6sdKHmfRSMIt4khvskUhQ== X-UI-Out-Filterresults: notjunk:1;V01:K0:dzcp5Ra52AI=:aJSIWNi1uFxwyROaXI9kX1 ahsEnZ8vqymRFDHLAcDMThJw8T0sU7GYrWEtxKufbiTgZ/RN0izNX4lW4H9B4LjL/5Gkz5fLh k5M+PQyJpJqFoKCi+Vi6snbm5MgeRGEMzquwW1NMXpJQ3uM+JdgxWnhWYDCZ+1l3f6yW4pUXF 7K5abQQVbDY19v6x6LHIO0uKxfcbXQuFJ/cDQwLwwwtGck1c2ZNGN4SacQpORFfgXdn67BTNL cZ+D9pBdj/Kazfi21lkHauU5uqSxdRRkTepLoV5Pzv2J9VhgMDFJTbEORmt2sBJrdqBUbkpjU M+1ZKp1ECOIu2+w/b7mx6V3SNqHLaTE7+yajfuVbSVYrUPT4xbpb3OmnfvWqigSUPstqnbEYl zMO5wpT7pvWkLujB7t1amv6QpDqkb6ZH/Knob4nz3OdXt4pnpBDT4EphFBlb7IbdR7NrUTSCR Vp1b3wkjgqrpgmzAyUcE68b+rY5nkIxPxdYB4w6I8MBjTPptW4Z/fblce3DbrggG7AVbO3RYy 4cCIbwjRnMno9S+fN/UdwB2f9a5nU4tLkuAV9oOyukKl9Vdh3I0hH65A8eC6I6+t0daUNZ2qC omLbNo2YUJ6Tt7TQigvLO4GKA/srQzbNbHL2O+4KJa1T2bndzSxI9KG8pgCMO3YgF1lnyP69c 5hOEKFlC6ydiHp/Uc+YAMUw1beO1AHbnDEhH0HgMFjZ8GxNwxpLegQwcKxU+Tt5wzKOE2X1JC iDKc0kv3jhtGzLE0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday 18 November 2015 08:48:45 Marc Zyngier wrote: > > +static inline u32 iproc_msi_read_reg(struct iproc_msi *msi, > > + enum iproc_msi_reg reg, > > + unsigned int eq) > > +{ > > + struct iproc_pcie *pcie = msi->pcie; > > + > > + return readl(pcie->base + msi->reg_offsets[eq][reg]); > > Do you need the extra barrier implied by readl? readl_relaxed should be > enough. I suspect this is the one place where it's needed for a lot of drivers: when the PCI device sends DMA data followed by the MSI message, the device driver can safely assume that the DMA data has arrived in memory even without doing another readl() from the device itself. It really depends on how the MSI implementation here interacts with the memory controller, and we should probably have a comment to explain this either way. > > +static inline void iproc_msi_write_reg(struct iproc_msi *msi, > > + enum iproc_msi_reg reg, > > + int eq, u32 val) > > +{ > > + struct iproc_pcie *pcie = msi->pcie; > > + > > + writel(val, pcie->base + msi->reg_offsets[eq][reg]); > > Same here for writel vs writel_relaxed. We probably want writel_relaxed() when calling this from iproc_msi_handler(), but not when calling from iproc_msi_enable(), which should default to a normal writel(), so we can be sure it's actually configured right at the time we return from iproc_msi_init(). You could try to prove that using writel_relaxed is correct here, but using writel makes it so much easier. Arnd