From: Hannes Reinecke <hare@suse.de>
To: Achim Leubner <Achim.Leubner@pmcs.com>,
Mahesh Rajashekhara <Mahesh.Rajashekhara@pmcs.com>,
"JBottomley@Parallels.com" <JBottomley@Parallels.com>,
"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>
Subject: Re: [PATCH 4/7] aacraid: MSI-x support
Date: Wed, 18 Mar 2015 12:18:44 +0100 [thread overview]
Message-ID: <55095F14.8070102@suse.de> (raw)
In-Reply-To: <307F48E420013C4E85C75C93E532197AB8092042@BBYEXM01.pmc-sierra.internal>
On 03/17/2015 04:27 PM, Achim Leubner wrote:
> Reviewed-by: Achim Leubner <Achim.Leubner@pmcs.com>
>
>
> -----Original Message-----
> From: Mahesh Rajashekhara
> Sent: Wednesday, March 4, 2015 9:39 AM
> To: JBottomley@Parallels.com; linux-scsi@vger.kernel.org
> Cc: aacraid@pmc-sierra.com; Harry Yang; Achim Leubner; Rajinikanth Pandurangan; Rich Bono; Mahesh Rajashekhara
> Subject: [PATCH 4/7] aacraid: MSI-x support
>
> Add MSI-x interrupt mode support.
>
> Signed-off-by: Mahesh Rajashekhara <Mahesh.Rajashekhara@pmcs.com>
> ---
> drivers/scsi/aacraid/aachba.c | 6 +-
> drivers/scsi/aacraid/aacraid.h | 79 ++++++++-
> drivers/scsi/aacraid/comminit.c | 86 +++++++++-
> drivers/scsi/aacraid/commsup.c | 19 ++-
> drivers/scsi/aacraid/dpcsup.c | 9 +-
> drivers/scsi/aacraid/linit.c | 18 ++-
> drivers/scsi/aacraid/src.c | 365 +++++++++++++++++++++++++++++----------
> 7 files changed, 473 insertions(+), 109 deletions(-)
>
> diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c index 0819644..eb524e6 100644
> --- a/drivers/scsi/aacraid/aachba.c
> +++ b/drivers/scsi/aacraid/aachba.c
> @@ -473,7 +473,7 @@ static void get_container_name_callback(void *context, struct fib * fibptr)
> if ((le32_to_cpu(get_name_reply->status) == CT_OK)
> && (get_name_reply->data[0] != '\0')) {
> char *sp = get_name_reply->data;
> - sp[sizeof(((struct aac_get_name_resp *)NULL)->data)-1] = '\0';
> + sp[sizeof(((struct aac_get_name_resp *)NULL)->data)] = '\0';
> while (*sp == ' ')
> ++sp;
> if (*sp) {
> @@ -613,7 +613,9 @@ static void _aac_probe_container1(void * context, struct fib * fibptr)
> int status;
>
> dresp = (struct aac_mount *) fib_data(fibptr);
> - dresp->mnt[0].capacityhigh = 0;
> + if (!(fibptr->dev->supplement_adapter_info.SupportedOptions2 &
> + AAC_OPTION_VARIABLE_BLOCK_SIZE))
> + dresp->mnt[0].capacityhigh = 0;
> if ((le32_to_cpu(dresp->status) != ST_OK) ||
> (le32_to_cpu(dresp->mnt[0].vol) != CT_NONE)) {
> _aac_probe_container2(context, fibptr); diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h index 93579f3..c162a65 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -6,11 +6,61 @@
> #define nblank(x) _nblank(x)[0]
>
> #include <linux/interrupt.h>
> +#include <linux/pci.h>
>
> /*------------------------------------------------------------------------------
> * D E F I N E S
> *----------------------------------------------------------------------------*/
>
> +#define AAC_MAX_MSIX 32 /* vectors */
> +#define AAC_PCI_MSI_ENABLE 0x8000
> +
> +enum {
> + AAC_ENABLE_INTERRUPT = 0x0,
> + AAC_DISABLE_INTERRUPT,
> + AAC_ENABLE_MSIX,
> + AAC_DISABLE_MSIX,
> + AAC_CLEAR_AIF_BIT,
> + AAC_CLEAR_SYNC_BIT,
> + AAC_ENABLE_INTX
> +};
> +
> +#define AAC_INT_MODE_INTX (1<<0)
> +#define AAC_INT_MODE_MSI (1<<1)
> +#define AAC_INT_MODE_AIF (1<<2)
> +#define AAC_INT_MODE_SYNC (1<<3)
> +
> +#define AAC_INT_ENABLE_TYPE1_INTX 0xfffffffb
> +#define AAC_INT_ENABLE_TYPE1_MSIX 0xfffffffa
> +#define AAC_INT_DISABLE_ALL 0xffffffff
> +
> +/* Bit definitions in IOA->Host Interrupt Register */
> +#define PMC_TRANSITION_TO_OPERATIONAL (0x80000000 >> 0)
> +#define PMC_IOARCB_TRANSFER_FAILED (0x80000000 >> 3)
> +#define PMC_IOA_UNIT_CHECK (0x80000000 >> 4)
> +#define PMC_NO_HOST_RRQ_FOR_CMD_RESPONSE (0x80000000 >> 5)
> +#define PMC_CRITICAL_IOA_OP_IN_PROGRESS (0x80000000 >> 6)
> +#define PMC_IOARRIN_LOST (0x80000000 >> 27)
> +#define PMC_SYSTEM_BUS_MMIO_ERROR (0x80000000 >> 28)
> +#define PMC_IOA_PROCESSOR_IN_ERROR_STATE (0x80000000 >> 29)
> +#define PMC_HOST_RRQ_VALID (0x80000000 >> 30)
> +#define PMC_OPERATIONAL_STATUS (0x80000000 >> 0)
> +#define PMC_ALLOW_MSIX_VECTOR0 (0x80000000 >> 31)
> +
How positively curious.
Typically you define a git mask by shifting _left_, not right.
> +#define PMC_IOA_ERROR_INTERRUPTS (PMC_IOARCB_TRANSFER_FAILED | \
> + PMC_IOA_UNIT_CHECK | \
> + PMC_NO_HOST_RRQ_FOR_CMD_RESPONSE | \
> + PMC_IOARRIN_LOST | \
> + PMC_SYSTEM_BUS_MMIO_ERROR | \
> + PMC_IOA_PROCESSOR_IN_ERROR_STATE)
> +
> +#define PMC_ALL_INTERRUPT_BITS (PMC_IOA_ERROR_INTERRUPTS | \
> + PMC_HOST_RRQ_VALID | \
> + PMC_TRANSITION_TO_OPERATIONAL | \
> + PMC_ALLOW_MSIX_VECTOR0)
> +#define PMC_GLOBAL_INT_BIT2 0x00000004
> +#define PMC_GLOBAL_INT_BIT0 0x00000001
> +
> #ifndef AAC_DRIVER_BUILD
> # define AAC_DRIVER_BUILD 30300
> # define AAC_DRIVER_BRANCH "-ms"
> @@ -36,6 +86,7 @@
> #define CONTAINER_TO_ID(cont) (cont)
> #define CONTAINER_TO_LUN(cont) (0)
>
> +#define PMC_DEVICE_S6 0x28b
> #define PMC_DEVICE_S7 0x28c
> #define PMC_DEVICE_S8 0x28d
> #define PMC_DEVICE_S9 0x28f
> @@ -434,7 +485,7 @@ enum fib_xfer_state { struct aac_init {
> __le32 InitStructRevision;
> - __le32 MiniPortRevision;
> + __le32 NoOfMSIXVectors;
> __le32 fsrev;
> __le32 CommHeaderAddress;
> __le32 FastIoCommAreaAddress;
> @@ -755,7 +806,8 @@ struct rkt_registers {
>
> struct src_mu_registers {
> /* PCI*| Name */
> - __le32 reserved0[8]; /* 00h | Reserved */
> + __le32 reserved0[6]; /* 00h | Reserved */
> + __le32 IOAR[2]; /* 18h | IOA->host interrupt register */
> __le32 IDR; /* 20h | Inbound Doorbell Register */
> __le32 IISR; /* 24h | Inbound Int. Status Register */
> __le32 reserved1[3]; /* 28h | Reserved */
> @@ -767,17 +819,18 @@ struct src_mu_registers {
> __le32 OMR; /* bch | Outbound Message Register */
> __le32 IQ_L; /* c0h | Inbound Queue (Low address) */
> __le32 IQ_H; /* c4h | Inbound Queue (High address) */
> + __le32 ODR_MSI; /* c8h | MSI register for sync./AIF */
> };
>
> struct src_registers {
> - struct src_mu_registers MUnit; /* 00h - c7h */
> + struct src_mu_registers MUnit; /* 00h - cbh */
> union {
> struct {
> - __le32 reserved1[130790]; /* c8h - 7fc5fh */
> + __le32 reserved1[130789]; /* cch - 7fc5fh */
> struct src_inbound IndexRegs; /* 7fc60h */
> } tupelo;
> struct {
> - __le32 reserved1[974]; /* c8h - fffh */
> + __le32 reserved1[973]; /* cch - fffh */
> struct src_inbound IndexRegs; /* 1000h */
> } denali;
> } u;
> @@ -1028,6 +1081,11 @@ struct aac_bus_info_response {
> #define AAC_OPT_NEW_COMM_TYPE3 cpu_to_le32(1<<30)
> #define AAC_OPT_NEW_COMM_TYPE4 cpu_to_le32(1<<31)
>
> +/* MSIX context */
> +struct aac_msix_ctx {
> + int vector_no;
> + struct aac_dev *dev;
> +};
>
> struct aac_dev
> {
> @@ -1083,8 +1141,9 @@ struct aac_dev
> * if AAC_COMM_MESSAGE_TYPE1 */
>
> dma_addr_t host_rrq_pa; /* phys. address */
> - u32 host_rrq_idx; /* index into rrq buffer */
> -
> + u32 host_rrq_idx[AAC_MAX_MSIX]; /* index into rrq buffer */
> + atomic_t rrq_outstanding[AAC_MAX_MSIX];
> + u32 fibs_pushed_no;
> struct pci_dev *pdev; /* Our PCI interface */
> void * printfbuf; /* pointer to buffer used for printf's from the adapter */
> void * comm_addr; /* Base address of Comm area */
> @@ -1153,6 +1212,11 @@ struct aac_dev
> int sync_mode;
> struct fib *sync_fib;
> struct list_head sync_fib_list;
> + u32 max_msix; /* max. MSI-X vectors */
> + u32 vector_cap; /* MSI-X vector capab.*/
> + int msi_enabled; /* MSI/MSI-X enabled */
> + struct msix_entry msixentry[AAC_MAX_MSIX];
> + struct aac_msix_ctx aac_msix[AAC_MAX_MSIX]; /* context */
> };
>
> #define aac_adapter_interrupt(dev) \
> @@ -2035,6 +2099,7 @@ void aac_consumer_free(struct aac_dev * dev, struct aac_queue * q, u32 qnum); int aac_fib_complete(struct fib * context); #define fib_data(fibctx) ((void *)(fibctx)->hw_fib_va->data) struct aac_dev *aac_init_adapter(struct aac_dev *dev);
> +void aac_src_access_devreg(struct aac_dev *dev, int mode);
> int aac_get_config_status(struct aac_dev *dev, int commit_flag); int aac_get_containers(struct aac_dev *dev); int aac_scsi_cmd(struct scsi_cmnd *cmd); diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c index 177b094..29c35c8 100644
> --- a/drivers/scsi/aacraid/comminit.c
> +++ b/drivers/scsi/aacraid/comminit.c
> @@ -43,6 +43,8 @@
>
> #include "aacraid.h"
>
> +static void aac_define_int_mode(struct aac_dev *dev);
> +
> struct aac_common aac_config = {
> .irq_mod = 1
> };
> @@ -91,7 +93,7 @@ static int aac_alloc_comm(struct aac_dev *dev, void **commaddr, unsigned long co
> init->InitStructRevision = cpu_to_le32(ADAPTER_INIT_STRUCT_REVISION);
> if (dev->max_fib_size != sizeof(struct hw_fib))
> init->InitStructRevision = cpu_to_le32(ADAPTER_INIT_STRUCT_REVISION_4);
> - init->MiniPortRevision = cpu_to_le32(Sa_MINIPORT_REVISION);
> + init->NoOfMSIXVectors = cpu_to_le32(Sa_MINIPORT_REVISION);
> init->fsrev = cpu_to_le32(dev->fsrev);
>
> /*
Now that you switched the field definition from 'MiniPortRevision'
to 'NoOfMSIXVectors', shouldn't you rather introduce a
'Sa_MSIXVECTORS' here instead of using the old definitions?
> @@ -140,7 +142,7 @@ static int aac_alloc_comm(struct aac_dev *dev, void **commaddr, unsigned long co
> INITFLAGS_NEW_COMM_TYPE2_SUPPORTED | INITFLAGS_FAST_JBOD_SUPPORTED);
> init->HostRRQ_AddrHigh = cpu_to_le32((u32)((u64)dev->host_rrq_pa >> 32));
> init->HostRRQ_AddrLow = cpu_to_le32((u32)(dev->host_rrq_pa & 0xffffffff));
> - init->MiniPortRevision = cpu_to_le32(0L); /* number of MSI-X */
> + init->NoOfMSIXVectors = cpu_to_le32(dev->max_msix); /* number of MSI-X */
> dprintk((KERN_WARNING"aacraid: New Comm Interface type2 enabled\n"));
> }
>
> @@ -228,6 +230,11 @@ int aac_send_shutdown(struct aac_dev * dev)
> /* FIB should be freed only after getting the response from the F/W */
> if (status != -ERESTARTSYS)
> aac_fib_free(fibctx);
> + if ((dev->pdev->device == PMC_DEVICE_S7 ||
> + dev->pdev->device == PMC_DEVICE_S8 ||
> + dev->pdev->device == PMC_DEVICE_S9) &&
> + dev->msi_enabled)
> + aac_src_access_devreg(dev, AAC_ENABLE_INTX);
> return status;
> }
>
> @@ -388,6 +395,8 @@ struct aac_dev *aac_init_adapter(struct aac_dev *dev)
> }
> }
> }
> + dev->max_msix = 0;
> + dev->msi_enabled = 0;
> if ((!aac_adapter_sync_cmd(dev, GET_COMM_PREFERRED_SETTINGS,
> 0, 0, 0, 0, 0, 0,
> status+0, status+1, status+2, status+3, status+4)) @@ -461,6 +470,11 @@ struct aac_dev *aac_init_adapter(struct aac_dev *dev)
> if (host->can_queue > AAC_NUM_IO_FIB)
> host->can_queue = AAC_NUM_IO_FIB;
>
> + if (dev->pdev->device == PMC_DEVICE_S6 ||
> + dev->pdev->device == PMC_DEVICE_S7 ||
> + dev->pdev->device == PMC_DEVICE_S8 ||
> + dev->pdev->device == PMC_DEVICE_S9)
> + aac_define_int_mode(dev);
> /*
> * Ok now init the communication subsystem
> */
> @@ -489,4 +503,70 @@ struct aac_dev *aac_init_adapter(struct aac_dev *dev)
> return dev;
> }
>
> -
> +static void aac_define_int_mode(struct aac_dev *dev) {
> +
> + int i, msi_count;
> +
> + /* max. vectors from GET_COMM_PREFERRED_SETTINGS */
> + if (dev->max_msix == 0 ||
> + dev->pdev->device == PMC_DEVICE_S6 ||
> + dev->sync_mode) {
> + dev->max_msix = 1;
> + dev->vector_cap = dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB;
> + return;
> + }
> +
> + msi_count = min(dev->max_msix,
> + (unsigned int)num_online_cpus());
> +
> + dev->max_msix = msi_count;
> +
> + if (msi_count > AAC_MAX_MSIX)
> + msi_count = AAC_MAX_MSIX;
> +
> + for (i = 0; i < msi_count; i++)
> + dev->msixentry[i].entry = i;
> +
> + if (msi_count > 1 && pci_find_capability(dev->pdev, PCI_CAP_ID_MSIX))
> +{
> +
Braces should be at the same line as the condition.
> + i = pci_enable_msix(dev->pdev, dev->msixentry, msi_count);
> + /* Check how many MSIX vectors are allocated */
> + if (i >= 0) {
> + dev->msi_enabled = 1;
> + if (i) {
> + msi_count = i;
> + if (pci_enable_msix(dev->pdev, dev->msixentry, msi_count)) {
> + dev->msi_enabled = 0;
> + printk(KERN_ERR "%s%d: MSIX not supported!! Will try MSI 0x%x.\n",
> + dev->name, dev->id, i);
> + }
> + }
> + } else {
> + dev->msi_enabled = 0;
> + printk(KERN_ERR "%s%d: MSIX not supported!! Will try MSI 0x%x.\n",
> + dev->name, dev->id, i);
> + }
> + }
> +
> + if (!dev->msi_enabled) {
> + msi_count = 1;
> + i = !pci_enable_msi(dev->pdev);
> +
Yikes. Please don't do that; use (!i) in the condition instead.
> + if (i) {
> + dev->msi_enabled = 1;
> + dev->msi = 1;
> + } else {
> + printk(KERN_ERR "%s%d: MSI not supported!! Will try INTx 0x%x.\n",
> + dev->name, dev->id, i);
> + }
> + }
> +
> + if (!dev->msi_enabled)
> + dev->max_msix = msi_count = 1;
> + else {
> + if (dev->max_msix > msi_count)
> + dev->max_msix = msi_count;
> + }
> + dev->vector_cap = (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB) /
> +msi_count; }
Closing braces should be on an individual line.
Also here: please remember to run checkpatch before submitting.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2015-03-18 11:18 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-04 8:38 [PATCH 0/7] aacraid driver updates Mahesh Rajashekhara
2015-03-04 8:38 ` [PATCH 1/7] aacraid: AIF support for SES device add/remove Mahesh Rajashekhara
2015-03-17 7:05 ` Murthy Bhat
2015-03-17 15:24 ` Achim Leubner
2015-03-18 11:02 ` Hannes Reinecke
2015-03-04 8:38 ` [PATCH 2/7] aacraid: IOCTL pass-through command fix Mahesh Rajashekhara
2015-03-17 7:06 ` Murthy Bhat
2015-03-17 15:26 ` Achim Leubner
2015-03-18 11:03 ` Hannes Reinecke
2015-03-04 8:38 ` [PATCH 3/7] aacraid: 4KB sector support Mahesh Rajashekhara
2015-03-17 7:06 ` Murthy Bhat
2015-03-17 15:26 ` Achim Leubner
2015-03-18 11:06 ` Hannes Reinecke
2015-03-04 8:38 ` [PATCH 4/7] aacraid: MSI-x support Mahesh Rajashekhara
2015-03-17 7:06 ` Murthy Bhat
2015-03-17 15:27 ` Achim Leubner
2015-03-18 11:18 ` Hannes Reinecke [this message]
2015-03-04 8:38 ` [PATCH 5/7] aacraid: vpd page code 0x83 support Mahesh Rajashekhara
2015-03-17 7:06 ` Murthy Bhat
2015-03-17 15:27 ` Achim Leubner
2015-03-18 11:26 ` Hannes Reinecke
2015-03-04 8:38 ` [PATCH 6/7] aacraid: performance improvement changes Mahesh Rajashekhara
2015-03-17 7:07 ` Murthy Bhat
2015-03-17 15:27 ` Achim Leubner
2015-03-18 11:27 ` Hannes Reinecke
2015-03-04 8:38 ` [PATCH 7/7] aacraid: AIF raw device remove support Mahesh Rajashekhara
2015-03-17 7:07 ` Murthy Bhat
2015-03-17 15:28 ` Achim Leubner
2015-03-18 11:29 ` Hannes Reinecke
2015-03-26 14:43 ` Mahesh Rajashekhara
2015-03-16 11:24 ` [PATCH 0/7] aacraid driver updates Mahesh Rajashekhara
2015-03-16 12:49 ` James Bottomley
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=55095F14.8070102@suse.de \
--to=hare@suse.de \
--cc=Achim.Leubner@pmcs.com \
--cc=JBottomley@Parallels.com \
--cc=Mahesh.Rajashekhara@pmcs.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.