From patchwork Fri Nov 24 01:03:55 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: ching Huang X-Patchwork-Id: 10073751 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id C7BB76037F for ; Fri, 24 Nov 2017 09:04:26 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BDF642A37A for ; Fri, 24 Nov 2017 09:04:26 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B2A122A37C; Fri, 24 Nov 2017 09:04:26 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.4 required=2.0 tests=BAYES_00, DATE_IN_PAST_06_12, DKIM_SIGNED, DKIM_VALID, RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B55762A37A for ; Fri, 24 Nov 2017 09:04:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752420AbdKXJEG (ORCPT ); Fri, 24 Nov 2017 04:04:06 -0500 Received: from mail-io0-f193.google.com ([209.85.223.193]:42341 "EHLO mail-io0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752336AbdKXJEB (ORCPT ); Fri, 24 Nov 2017 04:04:01 -0500 Received: by mail-io0-f193.google.com with SMTP id u42so28909362ioi.9 for ; Fri, 24 Nov 2017 01:04:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=areca-com-tw.20150623.gappssmtp.com; s=20150623; h=subject:from:to:cc:in-reply-to:references:date:message-id :mime-version:content-transfer-encoding; bh=FqqQUw/877JNi0DSA+wxu9L6ut0VP0r6Z4J5mTuojFc=; b=ET/SSFVle8joXzBzGE63rETZdpVV/Q42z5IXETRWLXmWDcg8DF1H8usYnXymullqvf 1hj7AsVsllFAH3dM6a60TGdy2zblwcfHvZcXmxdrKg8v90Ia9AW3FthWbOFqi6FzHBZH nsV6/GrSKuaWpTks2/SnZghjS5MhR7oXjsSjggaHlSUgNhN1eznczuYyalblkYYGQPON 82XM4lTkj5W3rrpIlqsX4GGhwnkZleb/Z4XmjBF/7CjMrRR7Ewo6rwOJ2LqUt/2WFex1 pbYZWowqCvejbLWp3bII8BdXdCRTEcJNy+1NHVtYh4ZfFpqS37ePtFj1R7a7sx++OaSJ Zh7Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:in-reply-to:references:date :message-id:mime-version:content-transfer-encoding; bh=FqqQUw/877JNi0DSA+wxu9L6ut0VP0r6Z4J5mTuojFc=; b=oZ9KwaMY08kBXe1NJSFgPSimhqm7gIWFNrUox/+mA5a2WiOsDRSV26Ih/2ZDpwCJ/g JIXIXcHgqfeZS3+rDdMmBhk76A7QhroatWltZy1B9mZiC3uEK440qY1fcoX60qGSg/hx 3YqTMqXRYxWWciEufg7wn+uRhrx6US8wgEFF778aN0pSg7/GvQwQb9rpc4GfW9aur0O0 NKPLj9pRVdktmGC6KrvCgLL98r0FppVVu5uCA2gtrNt8BRSRvl/BxluYRXijT1cNjeBk ookQYuKtYEAAhj/WNfMlwZxj59Ncvnv+rTbeq+5k5/0rCrIjR+gRZ/NF/dTvoo8Otler lntg== X-Gm-Message-State: AJaThX6YKtsobEvzi4dagNcAPcKyKFdh9aIC8NzEtMwC5wxOfFPOmNLe rK/gwb+fHVi9J3HasnJddQhKJvbe X-Google-Smtp-Source: AGs4zMZyQKOLLf7lQJEHcbULS3aspJJxHGC3LBom5xnbQMdVboK3/J71XnZE6umBwVSmj9wLlx14Eg== X-Received: by 10.107.173.223 with SMTP id m92mr31852902ioo.28.1511514240707; Fri, 24 Nov 2017 01:04:00 -0800 (PST) Received: from [192.168.0.119] (60-248-88-209.HINET-IP.hinet.net. [60.248.88.209]) by smtp.gmail.com with ESMTPSA id c36sm6524552iod.11.2017.11.24.01.03.57 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 24 Nov 2017 01:03:59 -0800 (PST) Subject: Re: [PATCH 1/3] scsi: arcmsr: Add driver module parameter msi_enable From: Ching Huang To: Dan Carpenter Cc: martin.petersen@oracle.com, James.Bottomley@HansenPartnership.com, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, jthumshirn@suse.de, hare@suse.de, hch@infradead.org In-Reply-To: <1511469909.4370.14.camel@Centos6.3-64> References: <1511400439.9832.19.camel@Centos6.3-64> <20171123104408.jkslvj2axy4svt4a@mwanda> <1511469909.4370.14.camel@Centos6.3-64> Date: Fri, 24 Nov 2017 09:03:55 +0800 Message-ID: <1511485435.4370.19.camel@Centos6.3-64> Mime-Version: 1.0 X-Mailer: Evolution 2.32.3 (2.32.3-37.el6) Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, 2017-11-24 at 04:45 +0800, Ching Huang wrote: > Hello Dan, > > On Thu, 2017-11-23 at 13:44 +0300, Dan Carpenter wrote: > > On Thu, Nov 23, 2017 at 09:27:19AM +0800, Ching Huang wrote: > > > From: Ching Huang > > > > > > Add module parameter msi_enable to has a chance to disable msi interrupt if it does not work properly. > > > > > > Signed-off-by: Ching Huang > > > --- > > > > > > diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c > > > --- a/drivers/scsi/arcmsr/arcmsr_hba.c 2017-11-23 14:29:26.000000000 +0800 > > > +++ b/drivers/scsi/arcmsr/arcmsr_hba.c 2017-11-23 16:02:28.000000000 +0800 > > > @@ -75,6 +75,10 @@ MODULE_DESCRIPTION("Areca ARC11xx/12xx/1 > > > MODULE_LICENSE("Dual BSD/GPL"); > > > MODULE_VERSION(ARCMSR_DRIVER_VERSION); > > > > > > +static int msi_enable = 1; > > > +module_param(msi_enable, int, S_IRUGO); > > ^^^^^^^ > > checkpatch.pl will complain that this should be 0444 > S_IRUGO value is 00444, defined in -> . > A. It will be not a issue. > > > > > +MODULE_PARM_DESC(msi_enable, " Enable MSI interrupt(0 ~ 1), msi_enable=1(enable), =0(disable)"); > > ^ > > Remove the extra space > OK > > > > > + > > > static int host_can_queue = ARCMSR_DEFAULT_OUTSTANDING_CMD; > > > module_param(host_can_queue, int, S_IRUGO); > > > MODULE_PARM_DESC(host_can_queue, " adapter queue depth(32 ~ 1024), default is 128"); > > > @@ -831,11 +835,15 @@ arcmsr_request_irq(struct pci_dev *pdev, > > > pr_info("arcmsr%d: msi-x enabled\n", acb->host->host_no); > > > flags = 0; > > > } else { > > > - nvec = pci_alloc_irq_vectors(pdev, 1, 1, > > > - PCI_IRQ_MSI | PCI_IRQ_LEGACY); > > > + if (msi_enable == 1) > > > + nvec = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI); > > > + else > > > + nvec = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_LEGACY); > > > if (nvec < 1) > > > return FAILED; > > > > I feel like we should try PCI_IRQ_MSI then if it fails we could fall > > back to PCI_IRQ_LEGACY. Originally, it worked like this and now it just > > fails unless you toggle the module param. It's a regression. > update as below > --- > unsigned int irq_flag; > irq_flag = PCI_IRQ_LEGACY; > if (msi_enable == 1) > irq_flag |= PCI_IRQ_MSI; > nvec = pci_alloc_irq_vectors(pdev, 1, 1, irq_flag); > > > > > > + if (msi_enable == 1) > > > + pr_info("arcmsr%d: msi enabled\n", acb->host->host_no); > > > > This printk could be improved. Use dev_info(&pdev->dev, for a start. > > I know that the other prints don't use this, but we could use it one > > time then slowly add more users until more are using dev_info() than > > pr_info() and then someone will decide to clean up the old users. > update as below > --- > if (msi_enable == 1) > dev_info(&pdev->dev, "msi enabled\n"); > > > > > regards, > > dan carpenter > > > Dan,. This new patch apply to mkp/scsi.git 4.16/scsi-queue diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c --- a/drivers/scsi/arcmsr/arcmsr_hba.c 2017-11-23 14:29:26.000000000 +0800 +++ b/drivers/scsi/arcmsr/arcmsr_hba.c 2017-11-24 15:16:20.000000000 +0800 @@ -75,6 +75,10 @@ MODULE_DESCRIPTION("Areca ARC11xx/12xx/1 MODULE_LICENSE("Dual BSD/GPL"); MODULE_VERSION(ARCMSR_DRIVER_VERSION); +static int msi_enable = 1; +module_param(msi_enable, int, S_IRUGO); +MODULE_PARM_DESC(msi_enable, "Enable MSI interrupt(0 ~ 1), msi_enable=1(enable), =0(disable)"); + static int host_can_queue = ARCMSR_DEFAULT_OUTSTANDING_CMD; module_param(host_can_queue, int, S_IRUGO); MODULE_PARM_DESC(host_can_queue, " adapter queue depth(32 ~ 1024), default is 128"); @@ -831,11 +835,17 @@ arcmsr_request_irq(struct pci_dev *pdev, pr_info("arcmsr%d: msi-x enabled\n", acb->host->host_no); flags = 0; } else { - nvec = pci_alloc_irq_vectors(pdev, 1, 1, - PCI_IRQ_MSI | PCI_IRQ_LEGACY); + if (msi_enable == 1) { + nvec = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI); + if (nvec == 1) { + dev_info(&pdev->dev, "msi enabled\n"); + goto msi_int1; + } + } + nvec = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_LEGACY); if (nvec < 1) return FAILED; - +msi_int1: flags = IRQF_SHARED; }