Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753321AbdFSA37 (ORCPT ); Sun, 18 Jun 2017 20:29:59 -0400 Received: from mail-wr0-f178.google.com ([209.85.128.178]:34280 "EHLO mail-wr0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753002AbdFSA3z (ORCPT ); Sun, 18 Jun 2017 20:29:55 -0400 Date: Mon, 19 Jun 2017 01:26:52 +0100 From: Piotr Gregor To: Bjorn Helgaas Cc: Alex Williamson , Bjorn Helgaas , "Michael S. Tsirkin" , Greg Kroah-Hartman , Neo Jia , Kirti Wankhede , Vlad Tsyrklevich , Arvind Yadav , Yongji Xie , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org Subject: Re: [PATCH] PCI: Move test of INTx masking to pci_setup_device Message-ID: <20170619002651.GA29796@westernst> References: <20170526210222.GA22204@westernst> <20170616225446.GG11129@bhelgaas-glaptop.roam.corp.google.com> <20170617174543.GA1723@westernst> <20170618011433.GK11129@bhelgaas-glaptop.roam.corp.google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170618011433.GK11129@bhelgaas-glaptop.roam.corp.google.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1481 Lines: 44 On Sat, Jun 17, 2017 at 08:14:33PM -0500, Bjorn Helgaas wrote: > On Sat, Jun 17, 2017 at 06:45:44PM +0100, Piotr Gregor wrote: > > Hi Bjorn, > > > > The pci_cfg_access_lock is most likely not needed there. > > The assignment by return type is indeed preferred in this case. > > > > However, you have changed the meaning of returned boolean information > > by pci_intx_mask_broken leaving pci_intx_mask_supported unchanged. > > The test should be: > > > > if (new != toggle) /* the test failed */ > > return 1; > > return 0; > > Oh, you're absolutely right, thanks for catching that! I updated my > pci/enumeration branch. > > > Regarding v2.3 - do you think it is worth to apply the check > > so we would have something like > > > > if ((new == toggle) || PCI_VERSION_PRIOR_TO_23) /* test OK or PCI prior to r2.3 */ > > return 0; > > return 1; > > I'm not sure how to test for r2.3 compliance. But even if we could, I > guess I think the current code is probably better because it actually > checks the property we care about, not a spec revision that is one > step removed from the property. > > Bjorn Hi Bjorn, You are right, having if ((new == toggle) || PCI_VERSION_PRIOR_TO_23) /* test OK or PCI prior to r2.3 */ return 0; return 1; would be incorrect, as if new != toggle then PCI_COMMAND_INTX_DISABLE is not writable so INTx masking support should be considered broken (regardless of PCI version). Piotr