On 03/17/2015 04:27 PM, Achim Leubner wrote:
> Reviewed-by: Achim Leubner <Achim.Leubner@xxxxxxxx>
>
>
> -----Original Message-----
> From: Mahesh Rajashekhara
> Sent: Wednesday, March 4, 2015 9:39 AM
> To: JBottomley@xxxxxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx
> Cc: aacraid@xxxxxxxxxxxxxx; 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@xxxxxxxx>
> ---
> 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@xxxxxxx +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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
