From: Dan Williams <dan.j.williams@intel.com>
To: Jeff Garzik <jeff@garzik.org>
Cc: "james.bottomley@suse.de" <james.bottomley@suse.de>,
"Jiang, Dave" <dave.jiang@intel.com>,
"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
"Danecki, Jacek" <jacek.danecki@intel.com>,
"Ciechanowski, Ed" <ed.ciechanowski@intel.com>,
"Skirvin, Jeffrey D" <jeffrey.d.skirvin@intel.com>,
"Nadolski, Edmund" <edmund.nadolski@intel.com>
Subject: Re: [RFC PATCH 1/6] isci: initialization
Date: Fri, 18 Feb 2011 16:12:13 -0800 [thread overview]
Message-ID: <1298074333.19161.28.camel@dwillia2-linux> (raw)
In-Reply-To: <4D5CDADB.6040508@garzik.org>
On Thu, 2011-02-17 at 00:22 -0800, Jeff Garzik wrote:
> Obviously my grep-fu is failing me... where is interrupt_handler
> assigned a value? Creating a pointer for the interrupt handler, rather
> than directly registering the proper callback with the kernel, seems a
> bit odd.
>
Yes... the following will be showing up in the git tree shortly along
with some other interrupt fixes.
>From bb17c26248fad2e897fb5a351f36bba2d0dcb077 Mon Sep 17 00:00:00 2001
From: Dan Williams <dan.j.williams@intel.com>
Date: Fri, 18 Feb 2011 09:25:05 -0800
Subject: [PATCH] isci: bypass scic_controller_get_handler_methods()
The indirection is unnecessary and broken in the current case that assigns the
handlers based on a not up-to-date pdev->msix_enabled value.
Route the handlers directly to the requisite core routines.
Todo: hook up error interrupt handling
Reported-by: Jeff Garzik <jeff@garzik.org>
Cc: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Edmund Nadolski <edmund.nadolski@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/core/scic_sds_controller.c | 10 +-
drivers/scsi/isci/host.c | 158 ++++++--------------------
drivers/scsi/isci/host.h | 12 --
drivers/scsi/isci/init.c | 4 +-
drivers/scsi/isci/isci.h | 7 +-
5 files changed, 45 insertions(+), 146 deletions(-)
diff --git a/drivers/scsi/isci/core/scic_sds_controller.c b/drivers/scsi/isci/core/scic_sds_controller.c
index 6a32d91..cd8017f 100644
--- a/drivers/scsi/isci/core/scic_sds_controller.c
+++ b/drivers/scsi/isci/core/scic_sds_controller.c
@@ -1898,8 +1898,7 @@ static void scic_sds_controller_single_vector_completion_handler(
*
* bool true if an interrupt is processed false if no interrupt was processed
*/
-static bool scic_sds_controller_normal_vector_interrupt_handler(
- struct scic_sds_controller *scic)
+bool scic_sds_controller_isr(struct scic_sds_controller *scic)
{
if (scic_sds_controller_completion_queue_has_entries(scic)) {
return true;
@@ -1925,8 +1924,7 @@ static bool scic_sds_controller_normal_vector_interrupt_handler(
* This is the method provided to handle the completions for a normal MSIX
* message.
*/
-static void scic_sds_controller_normal_vector_completion_handler(
- struct scic_sds_controller *scic)
+void scic_sds_controller_completion_handler(struct scic_sds_controller *scic)
{
/* Empty out the completion queue */
if (scic_sds_controller_completion_queue_has_entries(scic))
@@ -2582,9 +2580,9 @@ enum sci_status scic_controller_get_handler_methods(
status = SCI_SUCCESS;
} else if (message_count == 2) {
handler_methods[0].interrupt_handler
- = scic_sds_controller_normal_vector_interrupt_handler;
+ = scic_sds_controller_isr;
handler_methods[0].completion_handler
- = scic_sds_controller_normal_vector_completion_handler;
+ = scic_sds_controller_completion_handler;
handler_methods[1].interrupt_handler
= scic_sds_controller_error_vector_interrupt_handler;
diff --git a/drivers/scsi/isci/host.c b/drivers/scsi/isci/host.c
index 6f16f4d..b66e620 100644
--- a/drivers/scsi/isci/host.c
+++ b/drivers/scsi/isci/host.c
@@ -62,80 +62,49 @@
#include "request.h"
#include "host.h"
-/**
- * isci_isr() - This function is the interrupt service routine for the
- * controller. It schedules the tasklet and returns.
- * @vec: This parameter specifies the interrupt vector.
- * @data: This parameter specifies the ISCI host object.
- *
- * IRQ_HANDLED if out interrupt otherwise, IRQ_NONE
- */
-irqreturn_t isci_isr(int vec, void *data)
+irqreturn_t isci_msix_isr(int vec, void *data)
{
- struct isci_host *isci_host
- = (struct isci_host *)data;
- struct scic_controller_handler_methods *handlers
- = &isci_host->scic_irq_handlers[SCI_MSIX_NORMAL_VECTOR];
- irqreturn_t ret = IRQ_NONE;
-
- if (isci_host_get_state(isci_host) != isci_starting
- && handlers->interrupt_handler) {
-
- if (handlers->interrupt_handler(isci_host->core_controller)) {
- if (isci_host_get_state(isci_host) != isci_stopped) {
- tasklet_schedule(
- &isci_host->completion_tasklet);
- } else
- dev_dbg(&isci_host->pdev->dev,
- "%s: controller stopped\n",
- __func__);
- ret = IRQ_HANDLED;
+ struct isci_host *ihost = data;
+ struct scic_sds_controller *scic = ihost->core_controller;
+
+ if (isci_host_get_state(ihost) != isci_starting) {
+ if (scic_sds_controller_isr(scic)) {
+ if (isci_host_get_state(ihost) != isci_stopped)
+ tasklet_schedule(&ihost->completion_tasklet);
+ else
+ dev_dbg(&ihost->pdev->dev,
+ "%s: controller stopped\n", __func__);
}
- } else
- dev_warn(&isci_host->pdev->dev,
- "%s: get_handler_methods failed, "
- "isci_host->status = 0x%x\n",
- __func__,
- isci_host_get_state(isci_host));
+ }
- return ret;
+ return IRQ_HANDLED;
}
-irqreturn_t isci_legacy_isr(int vec, void *data)
+irqreturn_t isci_intx_isr(int vec, void *data)
{
struct pci_dev *pdev = data;
- struct isci_host *isci_host;
- struct scic_controller_handler_methods *handlers;
+ struct isci_host *ihost;
irqreturn_t ret = IRQ_NONE;
- /*
- * Since this is a legacy interrupt, either or both
- * controllers could have triggered it. Thus, we have to call
- * the legacy interrupt handler for all controllers on the
- * PCI function.
- */
- for_each_isci_host(isci_host, pdev) {
- handlers = &isci_host->scic_irq_handlers[SCI_MSIX_NORMAL_VECTOR];
-
- if (isci_host_get_state(isci_host) != isci_starting
- && handlers->interrupt_handler) {
-
- if (handlers->interrupt_handler(isci_host->core_controller)) {
- if (isci_host_get_state(isci_host) != isci_stopped) {
- tasklet_schedule(
- &isci_host->completion_tasklet);
- } else
- dev_dbg(&isci_host->pdev->dev,
+ for_each_isci_host(ihost, pdev) {
+ struct scic_sds_controller *scic = ihost->core_controller;
+
+ if (isci_host_get_state(ihost) != isci_starting) {
+ if (scic_sds_controller_isr(scic)) {
+ if (isci_host_get_state(ihost) != isci_stopped)
+ tasklet_schedule(&ihost->completion_tasklet);
+ else
+ dev_dbg(&ihost->pdev->dev,
"%s: controller stopped\n",
__func__);
ret = IRQ_HANDLED;
}
} else
- dev_warn(&isci_host->pdev->dev,
+ dev_warn(&ihost->pdev->dev,
"%s: get_handler_methods failed, "
- "isci_host->status = 0x%x\n",
+ "ihost->status = 0x%x\n",
__func__,
- isci_host_get_state(isci_host));
+ isci_host_get_state(ihost));
}
return ret;
}
@@ -166,34 +135,9 @@ void isci_host_start_complete(
}
-
-
-/**
- * isci_host_scan_finished() - This function is one of the SCSI Host Template
- * functions. The SCSI midlayer calls this function during a target scan,
- * approx. once every 10 millisecs.
- * @shost: This parameter specifies the SCSI host being scanned
- * @time: This parameter specifies the number of ticks since the scan started.
- *
- * scan status, zero indicates the SCSI midlayer should continue to poll,
- * otherwise assume controller is ready.
- */
-int isci_host_scan_finished(
- struct Scsi_Host *shost,
- unsigned long time)
+int isci_host_scan_finished(struct Scsi_Host *shost, unsigned long time)
{
- struct isci_host *isci_host
- = isci_host_from_sas_ha(SHOST_TO_SAS_HA(shost));
-
- struct scic_controller_handler_methods *handlers
- = &isci_host->scic_irq_handlers[SCI_MSIX_NORMAL_VECTOR];
-
- if (handlers->interrupt_handler == NULL) {
- dev_err(&isci_host->pdev->dev,
- "%s: scic_controller_get_handler_methods failed\n",
- __func__);
- return 1;
- }
+ struct isci_host *isci_host = isci_host_from_sas_ha(SHOST_TO_SAS_HA(shost));
/**
* check interrupt_handler's status and call completion_handler if true,
@@ -204,8 +148,8 @@ int isci_host_scan_finished(
* continue to return zero from thee scan_finished routine until
* the scic_cb_controller_start_complete() call comes from the core.
**/
- if (handlers->interrupt_handler(isci_host->core_controller))
- handlers->completion_handler(isci_host->core_controller);
+ if (scic_sds_controller_isr(isci_host->core_controller))
+ scic_sds_controller_completion_handler(isci_host->core_controller);
if (isci_starting == isci_host_get_state(isci_host)
&& time < (HZ * 10)) {
@@ -363,8 +307,6 @@ static int isci_host_mdl_allocate_coherent(
static void isci_host_completion_routine(unsigned long data)
{
struct isci_host *isci_host = (struct isci_host *)data;
- struct scic_controller_handler_methods *handlers
- = &isci_host->scic_irq_handlers[SCI_MSIX_NORMAL_VECTOR];
struct list_head completed_request_list;
struct list_head aborted_request_list;
struct list_head *current_position;
@@ -378,11 +320,8 @@ static void isci_host_completion_routine(unsigned long data)
spin_lock_irq(&isci_host->scic_lock);
- if (handlers->completion_handler) {
- handlers->completion_handler(
- isci_host->core_controller
- );
- }
+ scic_sds_controller_completion_handler(isci_host->core_controller);
+
/* Take the lists of completed I/Os from the host. */
list_splice_init(&isci_host->requests_to_complete,
&completed_request_list);
@@ -544,8 +483,6 @@ int isci_host_init(struct isci_host *isci_host)
enum sci_status status;
struct scic_sds_controller *controller;
struct scic_sds_port *scic_port;
- struct scic_controller_handler_methods *handlers
- = &isci_host->scic_irq_handlers[0];
union scic_oem_parameters scic_oem_params;
union scic_user_parameters scic_user_params;
const struct firmware *fw = NULL;
@@ -691,35 +628,8 @@ int isci_host_init(struct isci_host *isci_host)
goto out;
}
- /* @todo: use both MSI-X interrupts, and don't do indirect
- * calls to the handlers just register direct calls
- */
- if (isci_host->pdev->msix_enabled) {
- status = scic_controller_get_handler_methods(
- SCIC_MSIX_INTERRUPT_TYPE,
- SCI_MSIX_DOUBLE_VECTOR,
- handlers
- );
- } else {
- status = scic_controller_get_handler_methods(
- SCIC_LEGACY_LINE_INTERRUPT_TYPE,
- 0,
- handlers
- );
- }
-
- if (status != SCI_SUCCESS) {
- handlers->interrupt_handler = NULL;
- handlers->completion_handler = NULL;
- dev_err(&isci_host->pdev->dev,
- "%s: scic_controller_get_handler_methods failed\n",
- __func__);
- }
-
tasklet_init(&isci_host->completion_tasklet,
- isci_host_completion_routine,
- (unsigned long)isci_host
- );
+ isci_host_completion_routine, (unsigned long)isci_host);
INIT_LIST_HEAD(&(isci_host->mdl_struct_list));
diff --git a/drivers/scsi/isci/host.h b/drivers/scsi/isci/host.h
index 4f4b99d..421d3de 100644
--- a/drivers/scsi/isci/host.h
+++ b/drivers/scsi/isci/host.h
@@ -53,13 +53,6 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
-/**
- * This file contains the isci_module initialization routines.
- *
- * host.h
- */
-
-
#if !defined(_SCI_HOST_H_)
#define _SCI_HOST_H_
@@ -79,10 +72,6 @@
#define SCI_SCU_BAR_SIZE (4*1024*1024)
#define SCI_IO_SPACE_BAR0 2
#define SCI_IO_SPACE_BAR1 3
-#define SCI_MSIX_NORMAL_VECTOR 0
-#define SCI_MSIX_ERROR_VECTOR 1
-#define SCI_MSIX_SINGLE_VECTOR 1
-#define SCI_MSIX_DOUBLE_VECTOR 2
#define ISCI_CAN_QUEUE_VAL 250 /* < SCI_MAX_IO_REQUESTS ? */
#define SCIC_CONTROLLER_STOP_TIMEOUT 5000
@@ -96,7 +85,6 @@ struct coherent_memory_info {
struct isci_host {
struct scic_sds_controller *core_controller;
- struct scic_controller_handler_methods scic_irq_handlers[SCI_NUM_MSI_X_INT];
union scic_oem_parameters oem_parameters;
int id; /* unique within a given pci device */
diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
index e3d9b15..f2bd92b 100644
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -334,7 +334,7 @@ static int isci_setup_interrupts(struct pci_dev *pdev)
BUG_ON(!isci_host);
/* @todo: need to handle error case. */
- err = devm_request_irq(&pdev->dev, msix->vector, isci_isr, 0,
+ err = devm_request_irq(&pdev->dev, msix->vector, isci_msix_isr, 0,
DRV_NAME"-msix", isci_host);
if (!err)
continue;
@@ -353,7 +353,7 @@ static int isci_setup_interrupts(struct pci_dev *pdev)
return 0;
intx:
- err = devm_request_irq(&pdev->dev, pdev->irq, isci_legacy_isr,
+ err = devm_request_irq(&pdev->dev, pdev->irq, isci_intx_isr,
IRQF_SHARED, DRV_NAME"-intx", pdev);
return err;
diff --git a/drivers/scsi/isci/isci.h b/drivers/scsi/isci/isci.h
index 6aee3c9..3dc0f6c 100644
--- a/drivers/scsi/isci/isci.h
+++ b/drivers/scsi/isci/isci.h
@@ -113,8 +113,11 @@ struct isci_firmware {
u8 sas_addrs_size;
};
-irqreturn_t isci_isr(int vec, void *data);
-irqreturn_t isci_legacy_isr(int vec, void *data);
+irqreturn_t isci_msix_isr(int vec, void *data);
+irqreturn_t isci_intx_isr(int vec, void *data);
+
+bool scic_sds_controller_isr(struct scic_sds_controller *scic);
+void scic_sds_controller_completion_handler(struct scic_sds_controller *scic);
enum sci_status isci_parse_oem_parameters(
union scic_oem_parameters *oem_params,
--
1.7.2.3
next prev parent reply other threads:[~2011-02-18 23:51 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-02-07 0:34 [RFC PATCH 0/6] isci: initial driver release (part1: intro and lldd) Dan Williams
2011-02-07 0:34 ` [RFC PATCH 1/6] isci: initialization Dan Williams
2011-02-17 8:22 ` Jeff Garzik
2011-02-19 0:12 ` Dan Williams [this message]
2011-02-17 8:25 ` Christoph Hellwig
2011-02-19 0:23 ` Dan Williams
2011-03-04 23:35 ` James Bottomley
2011-03-08 1:51 ` Dan Williams
2011-03-18 16:51 ` Christoph Hellwig
2011-02-07 0:34 ` [RFC PATCH 2/6] isci: task (libsas interface support) Dan Williams
2011-02-09 15:01 ` David Milburn
2011-02-14 7:14 ` Dan Williams
2011-02-16 18:48 ` David Milburn
2011-02-16 19:35 ` David Milburn
2011-02-07 0:34 ` [RFC PATCH 3/6] isci: request (core request infrastructure) Dan Williams
2011-03-18 16:41 ` Christoph Hellwig
2011-02-07 0:34 ` [RFC PATCH 4/6] isci: hardware / topology event handling Dan Williams
2011-03-18 16:18 ` Christoph Hellwig
2011-03-23 8:15 ` Dan Williams
2011-03-23 8:40 ` Christoph Hellwig
2011-03-23 9:04 ` Dan Williams
2011-03-23 9:08 ` Christoph Hellwig
2011-03-24 0:07 ` Dan Williams
2011-03-24 6:26 ` Christoph Hellwig
2011-03-25 0:57 ` Dan Williams
2011-03-25 19:45 ` Christoph Hellwig
2011-03-25 21:39 ` Dan Williams
2011-03-25 22:07 ` Christoph Hellwig
2011-03-25 22:34 ` Dan Williams
2011-03-27 22:28 ` Christoph Hellwig
2011-03-29 1:11 ` Dan Williams
2011-03-30 0:37 ` Dan Williams
2011-02-07 0:35 ` [RFC PATCH 5/6] isci: phy, port, and remote device Dan Williams
2011-02-07 0:35 ` [RFC PATCH 6/6] isci: sata support and phy settings via request_firmware() Dan Williams
2011-02-07 7:58 ` [RFC PATCH 1/6] isci: initialization jack_wang
2011-02-14 7:49 ` Dan Williams
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1298074333.19161.28.camel@dwillia2-linux \
--to=dan.j.williams@intel.com \
--cc=dave.jiang@intel.com \
--cc=ed.ciechanowski@intel.com \
--cc=edmund.nadolski@intel.com \
--cc=jacek.danecki@intel.com \
--cc=james.bottomley@suse.de \
--cc=jeff@garzik.org \
--cc=jeffrey.d.skirvin@intel.com \
--cc=linux-scsi@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.