Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756374Ab3JGSSB (ORCPT ); Mon, 7 Oct 2013 14:18:01 -0400 Received: from mail-qa0-f51.google.com ([209.85.216.51]:41052 "EHLO mail-qa0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753218Ab3JGSRz (ORCPT ); Mon, 7 Oct 2013 14:17:55 -0400 Date: Mon, 7 Oct 2013 14:17:49 -0400 From: Tejun Heo To: Alexander Gordeev Cc: linux-kernel@vger.kernel.org, Bjorn Helgaas , Ralf Baechle , Michael Ellerman , Benjamin Herrenschmidt , Martin Schwidefsky , Ingo Molnar , Dan Williams , Andy King , Jon Mason , Matt Porter , linux-pci@vger.kernel.org, linux-doc@vger.kernel.org, linux-mips@linux-mips.org, linuxppc-dev@lists.ozlabs.org, linux390@de.ibm.com, linux-s390@vger.kernel.org, x86@kernel.org, linux-ide@vger.kernel.org, iss_storagedev@hp.com, linux-nvme@lists.infradead.org, linux-rdma@vger.kernel.org, netdev@vger.kernel.org, e1000-devel@lists.sourceforge.net, linux-driver@qlogic.com, Solarflare linux maintainers , "VMware, Inc." , linux-scsi@vger.kernel.org Subject: Re: [PATCH RFC 07/77] PCI/MSI: Re-design MSI/MSI-X interrupts enablement pattern Message-ID: <20131007181749.GB27396@htj.dyndns.org> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1953 Lines: 66 Hello, On Wed, Oct 02, 2013 at 12:48:23PM +0200, Alexander Gordeev wrote: > +static int foo_driver_enable_msi(struct foo_adapter *adapter, int nvec) > +{ > + rc = pci_get_msi_cap(adapter->pdev); > + if (rc < 0) > + return rc; > + > + nvec = min(nvec, rc); > + if (nvec < FOO_DRIVER_MINIMUM_NVEC) { > + return -ENOSPC; > + > + rc = pci_enable_msi_block(adapter->pdev, nvec); > + return rc; > +} If there are many which duplicate the above pattern, it'd probably be worthwhile to provide a helper? It's usually a good idea to reduce the amount of boilerplate code in drivers. > static int foo_driver_enable_msix(struct foo_adapter *adapter, int nvec) > { > + rc = pci_msix_table_size(adapter->pdev); > + if (rc < 0) > + return rc; > + > + nvec = min(nvec, rc); > + if (nvec < FOO_DRIVER_MINIMUM_NVEC) { > + return -ENOSPC; > + > + for (i = 0; i < nvec; i++) > + adapter->msix_entries[i].entry = i; > + > + rc = pci_enable_msix(adapter->pdev, adapter->msix_entries, nvec); > + return rc; > } Ditto. > @@ -975,7 +951,7 @@ int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec) > if (nr_entries < 0) > return nr_entries; > if (nvec > nr_entries) > - return nr_entries; > + return -EINVAL; > > /* Check for any invalid entries */ > for (i = 0; i < nvec; i++) { If we do things this way, it breaks all drivers using this interface until they're converted, right? Also, it probably isn't the best idea to flip the behavior like this as this can go completely unnoticed (no compiler warning or anything, the same function just behaves differently). Maybe it'd be a better idea to introduce a simpler interface that most can be converted to? Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/