LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 1/3] scsi: arcmsr: Add driver module parameter msi_enable
@ 2017-11-23 1:27 Ching Huang
2017-11-23 10:44 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Ching Huang @ 2017-11-23 1:27 UTC (permalink / raw)
To: martin.petersen, James.Bottomley, linux-scsi, linux-kernel,
jthumshirn, hare, dan.carpenter, hch
From: Ching Huang <ching2048@areca.com.tw>
Add module parameter msi_enable to has a chance to disable msi interrupt if it does not work properly.
Signed-off-by: Ching Huang <ching2048@areca.com.tw>
---
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);
+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,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;
+ if (msi_enable == 1)
+ pr_info("arcmsr%d: msi enabled\n", acb->host->host_no);
flags = IRQF_SHARED;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/3] scsi: arcmsr: Add driver module parameter msi_enable
2017-11-23 1:27 [PATCH 1/3] scsi: arcmsr: Add driver module parameter msi_enable Ching Huang
@ 2017-11-23 10:44 ` Dan Carpenter
2017-11-23 20:45 ` Ching Huang
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2017-11-23 10:44 UTC (permalink / raw)
To: Ching Huang
Cc: martin.petersen, James.Bottomley, linux-scsi, linux-kernel,
jthumshirn, hare, hch
On Thu, Nov 23, 2017 at 09:27:19AM +0800, Ching Huang wrote:
> From: Ching Huang <ching2048@areca.com.tw>
>
> Add module parameter msi_enable to has a chance to disable msi interrupt if it does not work properly.
>
> Signed-off-by: Ching Huang <ching2048@areca.com.tw>
> ---
>
> 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
> +MODULE_PARM_DESC(msi_enable, " Enable MSI interrupt(0 ~ 1), msi_enable=1(enable), =0(disable)");
^
Remove the extra space
> +
> 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.
>
> + 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.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/3] scsi: arcmsr: Add driver module parameter msi_enable
2017-11-23 10:44 ` Dan Carpenter
@ 2017-11-23 20:45 ` Ching Huang
2017-11-24 1:03 ` Ching Huang
0 siblings, 1 reply; 4+ messages in thread
From: Ching Huang @ 2017-11-23 20:45 UTC (permalink / raw)
To: Dan Carpenter
Cc: martin.petersen, James.Bottomley, linux-scsi, linux-kernel,
jthumshirn, hare, hch
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 <ching2048@areca.com.tw>
> >
> > Add module parameter msi_enable to has a chance to disable msi interrupt if it does not work properly.
> >
> > Signed-off-by: Ching Huang <ching2048@areca.com.tw>
> > ---
> >
> > 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 <linux/stat.h> -> <uapi/linux/stat.h>.
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/3] scsi: arcmsr: Add driver module parameter msi_enable
2017-11-23 20:45 ` Ching Huang
@ 2017-11-24 1:03 ` Ching Huang
0 siblings, 0 replies; 4+ messages in thread
From: Ching Huang @ 2017-11-24 1:03 UTC (permalink / raw)
To: Dan Carpenter
Cc: martin.petersen, James.Bottomley, linux-scsi, linux-kernel,
jthumshirn, hare, hch
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 <ching2048@areca.com.tw>
> > >
> > > Add module parameter msi_enable to has a chance to disable msi interrupt if it does not work properly.
> > >
> > > Signed-off-by: Ching Huang <ching2048@areca.com.tw>
> > > ---
> > >
> > > 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 <linux/stat.h> -> <uapi/linux/stat.h>.
> 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;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-11-24 9:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-23 1:27 [PATCH 1/3] scsi: arcmsr: Add driver module parameter msi_enable Ching Huang
2017-11-23 10:44 ` Dan Carpenter
2017-11-23 20:45 ` Ching Huang
2017-11-24 1:03 ` Ching Huang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).