RE: [PATCH 3/7] megaraid_sas : Do not process IOCTLs and SCSI commands during driver removal

2014-11-12 Thread Kashyap Desai
> -Original Message-
> From: Kashyap Desai [mailto:kashyap.de...@avagotech.com]
> Sent: Wednesday, November 12, 2014 4:39 PM
> To: 'Tomas Henzl'; Sumit Saxena; 'linux-scsi@vger.kernel.org'
> Cc: 'martin.peter...@oracle.com'; 'h...@infradead.org';
> 'jbottom...@parallels.com'; 'aradf...@gmail.com'
> Subject: RE: [PATCH 3/7] megaraid_sas : Do not process IOCTLs and SCSI
> commands during driver removal
>
>
>
> > -Original Message-
> > From: Tomas Henzl [mailto:the...@redhat.com]
> > Sent: Tuesday, November 11, 2014 7:13 PM
> > To: sumit.sax...@avagotech.com; linux-scsi@vger.kernel.org
> > Cc: martin.peter...@oracle.com; h...@infradead.org;
> > jbottom...@parallels.com; kashyap.de...@avagotech.com;
> > aradf...@gmail.com
> > Subject: Re: [PATCH 3/7] megaraid_sas : Do not process IOCTLs and SCSI
> > commands during driver removal
> >
> > On 11/10/2014 01:21 PM, sumit.sax...@avagotech.com wrote:
> > > Do not process any SCSI and IOCTL command further(return them with
> > > appropriate return values to callers), while driver removal is in
> > > progress/PCI
> > shutdown is invoked.
> > >
> > > Signed-off-by: Sumit Saxena 
> > > Signed-off-by: Kashyap Desai 
> > > ---
> > >  drivers/scsi/megaraid/megaraid_sas_base.c |   20 --
> --
> > >  1 files changed, 16 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
> > > b/drivers/scsi/megaraid/megaraid_sas_base.c
> > > index f6a69a3..7754eeb 100644
> > > --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> > > +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> > > @@ -1572,6 +1572,12 @@ megasas_queue_command(struct Scsi_Host
> > *shost, struct scsi_cmnd *scmd)
> > >   instance = (struct megasas_instance *)
> > >   scmd->device->host->hostdata;
> > >
> > > + if (instance->unload == 1) {
> > > + scmd->result = DID_NO_CONNECT << 16;
> > > + scmd->scsi_done(scmd);
> > > + return 0;
> > > + }
> > > +
> > >   if (instance->issuepend_done == 0)
> > >   return SCSI_MLQUEUE_HOST_BUSY;
> > >
> > > @@ -4957,10 +4963,6 @@ static int megasas_io_attach(struct
> > megasas_instance *instance)
> > >   return -ENODEV;
> > >   }
> > >
> > > - /*
> > > -  * Trigger SCSI to scan our drives
> > > -  */
> > > - scsi_scan_host(host);
> > >   return 0;
> > >  }
> > >
> > > @@ -5288,6 +5290,10 @@ retry_irq_register:
> > >   goto fail_io_attach;
> > >
> > >   instance->unload = 0;
> > > + /*
> > > +  * Trigger SCSI to scan our drives
> > > +  */
> > > + scsi_scan_host(host);
> > >
> > >   /*
> > >* Initiate AEN (Asynchronous Event Notification) @@ -6051,6
> > > +6057,12 @@ megasas_mgmt_fw_ioctl(struct megasas_instance
> > *instance,
> > >   megasas_issue_blocked_cmd(instance, cmd, 0);
> > >   cmd->sync_cmd = 0;
> >
> > I've expected that you'll not send the command to the card, you first
> > issue blocked command and after that test unload state.
> > Shouldn't it be reversed?
> Actually, there was a two part of this Patch for IOCTL path. One will
not allow
> to send IOCTL while driver removal.
> Another will not use already return IOCTL from FW to avoid illegal
memory
> access on MFI frame.
> Driver wakeup all blocked command in unload path and MFI/MPT frames are
> freed as well, so this check is good to have.
>
> As you suggested, we missed to have another check in IOCTL path, before
> driver takes semaphore. It was planned, but somehow we missed while
> submitting this patch set.
> We will send incremental patch for that part.

Correction - First of all sorry for confusion.
megasas_mgmt_ioctl_fw and megasas_mgmt_ioctl_aen already has a check for
instance->unload. We don't need any changes for this patch.

Tomas -
We are planning to provide resend patch series which will have changes for
below two patch.
[PATCH 1/7] megaraid_sas : Driver version upgrade and
[PATCH 4/7] megaraid_sas : Online Firmware upgrade support for Extended VD
feature


` Kashyap

>
> >
> > >
> > > + if (instance->unload == 1) {
> > > + dev_info(&instance->pdev->dev, "we are doing unload so no
> > need"
> > > + "to submit data to application. This may be force
exit"
> > > + "from driver\n");
> >
> > I'm not a native speaker, so I'll not comment on the wording, but you
> > should add a space at the line breaks.
>
> Agree. will address this part.
>
> > +   dev_info(&instance->pdev->dev, "we are doing unload so no
> > need "
> > +   "to submit data to application. This may be force
exit
> > "
> > +   "from driver\n");
> >
> > > + goto out;
> > > + }
> > >   /*
> > >* copy out the kernel buffers to user buffers
> > >*/
> >
> > Aren't there other ioctl paths too - for example
> > megasas_mgmt_ioctl_aen - isn't a block needed there too?
> This path will be cleanup from megasas_shutdown_controller() call.   AEN
> and MAP SYNC command will be aborted explicitly.
>
> >
--
To unsubscribe from this list: send the 

RE: BUG in scsi_lib.c due to a bad commit

2014-11-12 Thread Elliott, Robert (Server Storage)


> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel-
> ow...@vger.kernel.org] On Behalf Of Barto
> Sent: Wednesday, November 12, 2014 9:28 PM
> To: Guenter Roeck; Bjorn Helgaas
> Cc: linux-ker...@vger.kernel.org; linux-scsi@vger.kernel.org; Joe
> Perches
> Subject: Re: BUG in scsi_lib.c due to a bad commit
> 
> reverting your commit 045065d8a300a37218c is a solution, but it's just a
> temporary solution,
> 
> it's better to search why your commit can create a random hang on boot
> on some PC configurations,
> 
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1774,7 +1774,7 @@ static void scsi_request_fn(struct request_queue
> *q)
> blk_requeue_request(q, req);
> atomic_dec(&sdev->device_busy);
> out_delay:
> - if (atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev))
> + if (!atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev))
> blk_delay_queue(q, SCSI_QUEUE_DELAY);
> }
> 
> perhaps the atomic_read() function doesn't make the expected job on some
> rare circonstances, I have the same doubts about the blk_delay_queue()
> function

Were you running with scsi_mod.use_blk_mq=Y or =N?

device_busy is the active queue depth for the device (e.g.
5 means there are 5 commands submitted but not yet completed).

The function reaches this code if it has run out of tags, the host
has reached its limit of outstanding commands, or the target has
reached its limit.  It requeus the request:
* with delay if device_busy is zero
* without delay if device_busy is non_zero

I think this is the reasoning:
If device_busy is zero, trying to process the request again will
probably run into the same problem; a delay gives time for the
situation to change.  If device_busy is non-zero, then the 
requeued command goes behind others and might get a different
result.

With the polarity backwards, the lack of delay hung PA-RISC 
and SPARC64 systems), not just QEMU.  So, I don't think reverting
the fix is good.

Changing it to an unconditional delay might be safe - delay
regardless of device_busy (until the root cause is understood).

Also, SCSI_QUEUE_DELAY seems like an arbitrary magic number; 
maybe that value isn't working correctly anymore?




Re: BUG in scsi_lib.c due to a bad commit

2014-11-12 Thread Barto
reverting your commit 045065d8a300a37218c is a solution, but it's just a
temporary solution,

it's better to search why your commit can create a random hang on boot
on some PC configurations,

--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1774,7 +1774,7 @@ static void scsi_request_fn(struct request_queue *q)
blk_requeue_request(q, req);
atomic_dec(&sdev->device_busy);
out_delay:
- if (atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev))
+ if (!atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev))
blk_delay_queue(q, SCSI_QUEUE_DELAY);
}

perhaps the atomic_read() function doesn't make the expected job on some
rare circonstances, I have the same doubts about the blk_delay_queue()
function


Le 12/11/2014 03:53, Guenter Roeck a écrit :> On 11/11/2014 04:17 PM,
Bjorn Helgaas wrote:
>> [+cc Guenter, linux-scsi]
>>
>> On Tue, Nov 11, 2014 at 4:33 PM, Barto 
>> wrote:
>>> Hello everyone,
>>>
>>> I notice a bug since kernel 3.17 ( and also with 3.18 branch ), a random
>>> hang at boot on some PC configurations, I did a "git bisect" and I found
>>> that the culprit is  :
>>>
>>> [045065d8a300a37218c548e9aa7becd581c6a0e8] [SCSI] fix qemu boot hang
>>> problem
>>>
>>>
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=045065d8a300a37218c548e9aa7becd581c6a0e8
>>>
>>>
>>> the author of this commit has choosen to inverse the logic of the if
>>> statement in the file  drivers/scsi/scsi_lib.c in order to solve an
>>> issue with qemu :
>>>
>>> --- a/drivers/scsi/scsi_lib.c
>>> +++ b/drivers/scsi/scsi_lib.c
>>> @@ -1774,7 +1774,7 @@ static void scsi_request_fn(struct
>>> request_queue *q)
>>> blk_requeue_request(q, req);
>>> atomic_dec(&sdev->device_busy);
>>> out_delay:
>>> - if (atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev))
>>> + if (!atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev))
>>> blk_delay_queue(q, SCSI_QUEUE_DELAY);
>>> }
>>>
>
> The above commit was a follow-up to commit 71e75c97f97a, which did the
> following:
>
> -   if (sdev->device_busy == 0 && !scsi_device_blocked(sdev))
> +   if (atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev))
>
> meaning it reversed the polarity of the check in the original code.
> Since that commit created problems as observed in qemu, I thought it
> would be obvious that restoring the original polarity would make sense.
> This is explained in my patch, so I am somewhat at loss why ...
>
>>> this change triggers a bug on my PC ( I don't have SCSI devices, but
>>> only 3 SATA harddisks and 2 IDE harddisks, SATA disks are on an ICH7
>>> sata controler on the motherboard gigabyte GA-P31-DS3L, and IDE disk on
>>> a JMicron JMB363/368 Sata/IDE PCIe card ), every 5~10 boots the boot
>>> stops suddenly because of this commit,
>>>
>>> If I revert this commit then the bug is gone, more details can be found
>>> here, where I created a patch who reverts commit 045065d8 :
>>>
>>> https://bugzilla.kernel.org/show_bug.cgi?id=87581
>>>
>>> my question: why Guenter Roeck ( the author of the bad commit ) has
>>> choosen to inverse the logic in the if statement ?
>
> this question is asked.
>
> Having said that, I don't really care one way or another. I am fine
> with reverting my commit; if that is done, I'll simply remove the
> related scsi tests from my qemu test runs.
>
> Guenter
>
>>> before his commit the if statement was like this :
>>>
>>> if (atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev))
>>> blk_delay_queue(q, SCSI_QUEUE_DELAY);
>>>
>>> if his decision to inverse the logic of
>>> "atomic_read(&sdev->device_busy)" is acceptable then the real bug is
>>> probably located elsewhere in the scsi source code, and we must solve
>>> this mistery because there is obviously a bug regression in SCSI code
>>> because with older kernels ( 3.16.7 and lower ) I don't have the random
>>> hang boot bug with my configuration,
>>>
>>> another user in archlinux forums has also this bug and he has a more
>>> modern PC ( intel i7 core cpu, SSD device ), my fear is when linux
>>> distros will move to kernel 3.17 then more users will have this weird
>>> random bug who can occur only on boot and only with a specific PC
>>> configuration, if the boot step is passed despite the random bug then
>>> the bug will not occur, it occurs only during the boot process, which
>>> probably means that the faulty source code is only called during the
>>> boot process,
>>>
>>> thanks for anyone who wants to dig this problem with me
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe
>>> linux-kernel" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at  http://www.tux.org/lkml/
>>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] st: implement sysfs based tape statistics

2014-11-12 Thread Seymour, Shane M
It's been a year since my last attempt at doing this as I got distracted by 
some other things. Comments are appreciated and any questions will be answered.

The following implements sysfs based per device tape statistics files with one 
file per statistic and a method of trying to allow a consistent set of 
statistics to be gathered. Included this time (see very end) is also 
documentation about the changes. These patches should apply on top of 
yesterdays changes to update the struct device_driver owner field. Tested with 
kernel version 3.18.0-rc4-next-20141112+.
---
Signed-off-by: Shane Seymour 
--- a/drivers/scsi/st.c 2014-11-12 11:31:54.416289214 -0600
+++ b/drivers/scsi/st.c 2014-11-12 11:44:24.243238146 -0600
@@ -20,6 +20,7 @@
 static const char *verstr = "20101219";
 
 #include 
+#include 
 
 #include 
 #include 
@@ -226,6 +227,20 @@ static DEFINE_SPINLOCK(st_index_lock);
 static DEFINE_SPINLOCK(st_use_lock);
 static DEFINE_IDR(st_index_idr);
 
+static inline void st_stats_remove_files(struct scsi_tape *);
+static inline void st_stats_create_files(struct scsi_tape *);
+static ssize_t st_tape_attr_show(struct kobject *, struct attribute *, char *);
+static ssize_t st_tape_attr_store(struct kobject *, struct attribute *,
+   const char *, size_t);
+static void st_release_stats_kobj(struct kobject *);
+static const struct sysfs_ops st_stats_sysfs_ops = {
+   .show   = st_tape_attr_show,
+   .store  = st_tape_attr_store,
+};
+static struct kobj_type st_stats_ktype = {
+   .release = st_release_stats_kobj,
+   .sysfs_ops = &st_stats_sysfs_ops,
+};
 
 

 #include "osst_detect.h"
@@ -476,10 +491,21 @@ static void st_scsi_execute_end(struct r
struct st_request *SRpnt = req->end_io_data;
struct scsi_tape *STp = SRpnt->stp;
struct bio *tmp;
+   u64 ticks = get_jiffies_64();
 
STp->buffer->cmdstat.midlevel_result = SRpnt->result = req->errors;
STp->buffer->cmdstat.residual = req->resid_len;
 
+   if (STp->stats != NULL) {
+   STp->stats->in_flight--;
+   ticks -= STp->stats->stamp;
+   STp->stats->io_ticks += ticks;
+   if (req->cmd[0] == WRITE_6)
+   STp->stats->write_ticks += ticks;
+   else if (req->cmd[0] == READ_6)
+   STp->stats->read_ticks += ticks;
+   }
+
tmp = SRpnt->bio;
if (SRpnt->waiting)
complete(SRpnt->waiting);
@@ -496,6 +522,7 @@ static int st_scsi_execute(struct st_req
struct rq_map_data *mdata = &SRpnt->stp->buffer->map_data;
int err = 0;
int write = (data_direction == DMA_TO_DEVICE);
+   struct scsi_tape *STp = SRpnt->stp;
 
req = blk_get_request(SRpnt->stp->device->request_queue, write,
  GFP_KERNEL);
@@ -516,6 +543,20 @@ static int st_scsi_execute(struct st_req
}
}
 
+   if (STp->stats != NULL) {
+   if (cmd[0] == WRITE_6) {
+   STp->stats->write_cnt++;
+   STp->stats->write_byte_cnt += bufflen;
+   } else if (cmd[0] == READ_6) {
+   STp->stats->read_cnt++;
+   STp->stats->read_byte_cnt += bufflen;
+   } else {
+   STp->stats->other_cnt++;
+   }
+   STp->stats->stamp = get_jiffies_64();
+   STp->stats->in_flight++;
+   }
+
SRpnt->bio = req->bio;
req->cmd_len = COMMAND_SIZE(cmd[0]);
memset(req->cmd, 0, BLK_MAX_CDB);
@@ -4064,6 +4105,8 @@ out:
 static int create_cdevs(struct scsi_tape *tape)
 {
int mode, error;
+   struct scsi_tape_stats *tmp;
+
for (mode = 0; mode < ST_NBR_MODES; ++mode) {
error = create_one_cdev(tape, mode, 0);
if (error)
@@ -4072,6 +4115,26 @@ static int create_cdevs(struct scsi_tape
if (error)
return error;
}
+/* Create statistics directory under device, if it fails we dont
+   have statistics. */
+   if (tape->stats != NULL) {
+   kobject_init(&tape->stats->statistics, &st_stats_ktype);
+   error = kobject_add(&tape->stats->statistics,
+   &tape->device->sdev_gendev.kobj,
+   "statistics");
+   if (error) {
+   kobject_del(&tape->stats->statistics);
+   tmp = tape->stats;
+   tape->stats = NULL;
+   kfree(tmp);
+   } else {
+   st_stats_create_files(tape);
+   }
+   } else {
+   tmp = tape->stats;
+ 

RE: [PATCH 05/10] scsi: use external buffer for command logging

2014-11-12 Thread Elliott, Robert (Server Storage)


> -Original Message-
> From: Hannes Reinecke [mailto:h...@suse.de]
> Sent: Thursday, 06 November, 2014 2:31 AM
> To: James Bottomley
> Cc: Christoph Hellwig; Ewan Milne; Elliott, Robert (Server Storage);
> linux-scsi@vger.kernel.org; Hannes Reinecke
> Subject: [PATCH 05/10] scsi: use external buffer for command logging
> 
...
> diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
> index 4f502f9..ed0d10d 100644
> --- a/drivers/scsi/ch.c
> +++ b/drivers/scsi/ch.c
> @@ -195,8 +195,10 @@ ch_do_scsi(scsi_changer *ch, unsigned char *cmd,
> int cmd_len,
>   retry:
>   errno = 0;
>   if (debug) {
> - DPRINTK("command: ");
> - __scsi_print_command(cmd, cmd_len);
> + char logbuf[SCSI_LOG_BUFSIZE];
> +
> + __scsi_format_command(logbuf, SCSI_LOG_BUFSIZE, cmd,
> cmd_len);

Use sizeof(logbuf) rather than SCSI_LOG_BUFSIZE to avoid 
the macro name ever mismatching.

> + DPRINTK("command: %s", logbuf);
>   }
> 
>   result = scsi_execute_req(ch->device, cmd, direction, buffer,
...
> diff --git a/drivers/scsi/scsi_logging.c
> b/drivers/scsi/scsi_logging.c
> index dca45fe..d166d12 100644
> --- a/drivers/scsi/scsi_logging.c
> +++ b/drivers/scsi/scsi_logging.c
> @@ -13,10 +13,10 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
> 
>  #define SCSI_LOG_SPOOLSIZE 4096
> -#define SCSI_LOG_BUFSIZE 128
> 
>  struct scsi_log_buf {
>   char buffer[SCSI_LOG_SPOOLSIZE];
> @@ -64,6 +64,21 @@ static void scsi_log_release_buffer(char *bufptr)
>   preempt_enable();
>  }
> 
> +static size_t scmd_format_header(char *logbuf, size_t logbuf_len,
> +  struct gendisk *disk, int tag)
> +{
> + size_t off = 0;
> +
> + if (disk)
> + off += scnprintf(logbuf + off, logbuf_len - off,
> +  "[%s] ", disk->disk_name);
> +
> + if (tag >= 0)
> + off += scnprintf(logbuf + off, logbuf_len - off,
> +  "tag#%d ", tag);

If the first scnprintf consumes the full logbuf_len, then
logbuf_len - off will cause an unsigned wrap (since it's
using size_t types; it'd go negative if it used signed types), 
triggering this from vsnprintf (called by scnprintf):
if (WARN_ON_ONCE((int) size < 0))
return 0;

It might be prudent to check that it hasn't gone too far
before calling scnprintf again.

> + return off;
> +}
> +
>  int sdev_prefix_printk(const char *level, const struct scsi_device
> *sdev,
>  const char *name, const char *fmt, ...)
>  {
> @@ -106,13 +121,8 @@ int scmd_printk(const char *level, const struct
> scsi_cmnd *scmd,
>   logbuf = scsi_log_reserve_buffer(&logbuf_len);
>   if (!logbuf)
>   return 0;
> - if (disk)
> - off += scnprintf(logbuf + off, logbuf_len - off,
> -  "[%s] ", disk->disk_name);
> -
> - if (scmd->request->tag >= 0)
> - off += scnprintf(logbuf + off, logbuf_len - off,
> -  "tag#%d ", scmd->request->tag);
> + off = scmd_format_header(logbuf, logbuf_len, disk,
> +  scmd->request->tag);
>   va_start(args, fmt);

Same comment about ensuring off hasn't gone too far.

>   off += vscnprintf(logbuf + off, logbuf_len - off, fmt, args);
>   va_end(args);
> @@ -121,3 +131,121 @@ int scmd_printk(const char *level, const struct
> scsi_cmnd *scmd,
>   return ret;
>  }
>  EXPORT_SYMBOL_GPL(scmd_printk);
> +
> +static size_t scsi_format_opcode_name(char *buffer, size_t buf_len,
> +   const unsigned char *cdbp)
> +{
> + int sa, cdb0;
> + const char *cdb_name = NULL, *sa_name = NULL;
> + size_t off;
> +
> + cdb0 = cdbp[0];
> + if (cdb0 == VARIABLE_LENGTH_CMD) {
> + int len = scsi_varlen_cdb_length(cdbp);
> +
> + if (len < 10) {
> + off = scnprintf(buffer, buf_len,
> + "short variable length command,
> len=%d",
> + len);
> + return off;
> + }
> + sa = (cdbp[8] << 8) + cdbp[9];
> + } else
> + sa = cdbp[1] & 0x1f;
> +
> + if (!scsi_opcode_sa_name(cdb0, sa, &cdb_name, &sa_name)) {
> + if (cdb_name)
> + off = scnprintf(buffer, buf_len, "%s", cdb_name);
> + else {
> + off = scnprintf(buffer, buf_len, "cdb[0]=0x%x",
> cdb0);

Since the service action is printed using "sa=" below,
consider using "op=" or "opcode=" rather than "cdb[0]=".
That might be clearer to readers who don't know the CDB
layout but do have an opcode table handy.


> + if (cdb0 > VENDOR_SPECIFIC_CDB)

The > should be >= since that define is 0xc0 (which itself
is vendor-specific).

> + off += scnprintf(buffer + off, buf_len -
> off,
> + 

RE: [PATCH 03/10] scsi: Implement per-cpu logging buffer

2014-11-12 Thread Elliott, Robert (Server Storage)
One more comment on this one (which was PATCH 01 in v1 of the series,
to which I sent most of my comments)...

> -Original Message-
> From: Hannes Reinecke [mailto:h...@suse.de]
> Sent: Thursday, 06 November, 2014 2:31 AM
> To: James Bottomley
> Cc: Christoph Hellwig; Ewan Milne; Elliott, Robert (Server Storage);
> linux-scsi@vger.kernel.org; Hannes Reinecke
> Subject: [PATCH 03/10] scsi: Implement per-cpu logging buffer
> 
> +++ b/drivers/scsi/scsi_logging.c
> @@ -0,0 +1,119 @@
...
> +static DEFINE_PER_CPU(struct scsi_log_buf, scsi_format_log);
> +
> +static char *scsi_log_reserve_buffer(size_t *len)
> +{
> + struct scsi_log_buf *buf;
> + unsigned long map_bits = SCSI_LOG_SPOOLSIZE / SCSI_LOG_BUFSIZE;

sizeof(buf->buffer) would be a bit safer than 
SCSI_LOG_SPOOLSIZE - adapts automatically if the real value
used in the structure definition ever changes.


---
Rob ElliottHP Server Storage




--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 25/37] sun3_scsi: Convert to platform device

2014-11-12 Thread Finn Thain
Convert sun3_scsi to platform device and eliminate scsi_register().

Signed-off-by: Finn Thain 
Reviewed-by: Hannes Reinecke 
Acked-by: Geert Uytterhoeven 

---

Changes since v1:
- Use NO_IRQ instead of IRQ_NONE.
- Move device IRQ and address constants to platform resources.
- Test idprom->id_machtype before registering platform device instead of
  during platform driver probe.
- Omit pointless instance->n_io_port assignment.

Changes since v2:
- Give more precise addresses in platform resource initializers and
  move PAGE_SIZE back to ioremap() arguments.

Changes since v3:
- Use resource_size() in ioremap() arguments instead of PAGE_SIZE.

---
 arch/m68k/sun3/config.c  |   60 +++
 drivers/scsi/sun3_scsi.c |  390 ++-
 drivers/scsi/sun3_scsi.h |   17 --
 3 files changed, 245 insertions(+), 222 deletions(-)

Index: linux/arch/m68k/sun3/config.c
===
--- linux.orig/arch/m68k/sun3/config.c  2014-11-13 11:19:22.0 +1100
+++ linux/arch/m68k/sun3/config.c   2014-11-13 11:20:10.0 +1100
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -27,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -169,3 +171,61 @@ static void __init sun3_sched_init(irq_h
 intersil_clear();
 }
 
+#ifdef CONFIG_SUN3_SCSI
+
+static const struct resource sun3_scsi_vme_rsrc[] __initconst = {
+   {
+   .flags = IORESOURCE_IRQ,
+   .start = SUN3_VEC_VMESCSI0,
+   .end   = SUN3_VEC_VMESCSI0,
+   }, {
+   .flags = IORESOURCE_MEM,
+   .start = 0xff20,
+   .end   = 0xff200021,
+   }, {
+   .flags = IORESOURCE_IRQ,
+   .start = SUN3_VEC_VMESCSI1,
+   .end   = SUN3_VEC_VMESCSI1,
+   }, {
+   .flags = IORESOURCE_MEM,
+   .start = 0xff204000,
+   .end   = 0xff204021,
+   },
+};
+
+/*
+ * Int: level 2 autovector
+ * IO: type 1, base 0x0014, 5 bits phys space: A<4..0>
+ */
+static const struct resource sun3_scsi_rsrc[] __initconst = {
+   {
+   .flags = IORESOURCE_IRQ,
+   .start = 2,
+   .end   = 2,
+   }, {
+   .flags = IORESOURCE_MEM,
+   .start = 0x0014,
+   .end   = 0x0014001f,
+   },
+};
+
+int __init sun3_platform_init(void)
+{
+   switch (idprom->id_machtype) {
+   case SM_SUN3 | SM_3_160:
+   case SM_SUN3 | SM_3_260:
+   platform_device_register_simple("sun3_scsi_vme", -1,
+   sun3_scsi_vme_rsrc, ARRAY_SIZE(sun3_scsi_vme_rsrc));
+   break;
+   case SM_SUN3 | SM_3_50:
+   case SM_SUN3 | SM_3_60:
+   platform_device_register_simple("sun3_scsi", -1,
+   sun3_scsi_rsrc, ARRAY_SIZE(sun3_scsi_rsrc));
+   break;
+   }
+   return 0;
+}
+
+arch_initcall(sun3_platform_init);
+
+#endif
Index: linux/drivers/scsi/sun3_scsi.c
===
--- linux.orig/drivers/scsi/sun3_scsi.c 2014-11-13 11:19:57.0 +1100
+++ linux/drivers/scsi/sun3_scsi.c  2014-11-13 11:20:10.0 +1100
@@ -23,22 +23,15 @@
  */
 
 #include 
-#include 
-#include 
 #include 
-
 #include 
-#include 
 #include 
 #include 
 #include 
+#include 
 
 #include 
-
-#include 
 #include 
-#include 
-#include 
 
 /* dma on! */
 #define REAL_DMA
@@ -59,8 +52,6 @@ extern int sun3_map_test(unsigned long,
 #endif
 
 
-static irqreturn_t scsi_sun3_intr(int irq, void *dummy);
-
 static int setup_can_queue = -1;
 module_param(setup_can_queue, int, 0);
 static int setup_cmd_per_lun = -1;
@@ -89,15 +80,14 @@ static struct scsi_cmnd *sun3_dma_setup_
 /* minimum number of bytes to do dma on */
 #define SUN3_DMA_MINSIZE 128
 
-static volatile unsigned char *sun3_scsi_regp;
+static unsigned char *sun3_scsi_regp;
 static volatile struct sun3_dma_regs *dregs;
-#ifndef SUN3_SCSI_VME
-static struct sun3_udc_regs *udc_regs = NULL;
-#endif
+static struct sun3_udc_regs *udc_regs;
 static unsigned char *sun3_dma_orig_addr = NULL;
 static unsigned long sun3_dma_orig_count = 0;
 static int sun3_dma_active = 0;
 static unsigned long last_residual = 0;
+static struct Scsi_Host *default_instance;
 
 /*
  * NCR 5380 register access functions
@@ -105,12 +95,12 @@ static unsigned long last_residual = 0;
 
 static inline unsigned char sun3scsi_read(int reg)
 {
-   return( sun3_scsi_regp[reg] );
+   return in_8(sun3_scsi_regp + reg);
 }
 
 static inline void sun3scsi_write(int reg, int value)
 {
-   sun3_scsi_regp[reg] = value;
+   out_8(sun3_scsi_regp + reg, value);
 }
 
 #ifndef SUN3_SCSI_VME
@@ -137,192 +127,7 @@ static inline void sun3_udc_write(unsign
 }
 #endif
 
-/*
- * XXX: status debug
- */
-static struct Scsi_Host *default_instance;
-
-/*
- * Function : in

[PATCH net-next] cxgb4i/cxgb4 : Refactor macros to conform to uniform standards

2014-11-12 Thread Anish Bhatt
Refactored all macros used in cxgb4i as part of previously started cxgb4 macro
names cleanup. Makes them more uniform and avoids namespace collision.
Minor changes in other drivers where required as some of these macros are used
 by multiple drivers, affected drivers are iw_cxgb4, cxgb4(vf) & csiostor

Signed-off-by: Anish Bhatt 
---
 drivers/infiniband/hw/cxgb4/cm.c| 104 ++--
 drivers/infiniband/hw/cxgb4/mem.c   |  20 ++--
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c |   4 +-
 drivers/net/ethernet/chelsio/cxgb4/l2t.c|   2 +-
 drivers/net/ethernet/chelsio/cxgb4/sge.c|   2 +-
 drivers/net/ethernet/chelsio/cxgb4/t4_msg.h | 120 +---
 drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h   |   6 +-
 drivers/net/ethernet/chelsio/cxgb4vf/sge.c  |   2 +-
 drivers/scsi/csiostor/csio_lnode.c  |   2 +-
 drivers/scsi/csiostor/csio_scsi.c   |   2 +-
 drivers/scsi/cxgbi/cxgb4i/cxgb4i.c  |  78 +++
 11 files changed, 200 insertions(+), 142 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index a07d8e124a80..83fa16fa4644 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -649,31 +649,31 @@ static int send_connect(struct c4iw_ep *ep)
 * remainder will be specified in the rx_data_ack.
 */
win = ep->rcv_win >> 10;
-   if (win > RCV_BUFSIZ_MASK)
-   win = RCV_BUFSIZ_MASK;
+   if (win > RCV_BUFSIZ_M)
+   win = RCV_BUFSIZ_M;
 
opt0 = (nocong ? NO_CONG(1) : 0) |
-  KEEP_ALIVE(1) |
+  KEEP_ALIVE_F |
   DELACK(1) |
-  WND_SCALE(wscale) |
-  MSS_IDX(mtu_idx) |
-  L2T_IDX(ep->l2t->idx) |
-  TX_CHAN(ep->tx_chan) |
-  SMAC_SEL(ep->smac_idx) |
+  WND_SCALE_V(wscale) |
+  MSS_IDX_V(mtu_idx) |
+  L2T_IDX_V(ep->l2t->idx) |
+  TX_CHAN_V(ep->tx_chan) |
+  SMAC_SEL_V(ep->smac_idx) |
   DSCP(ep->tos) |
-  ULP_MODE(ULP_MODE_TCPDDP) |
-  RCV_BUFSIZ(win);
-   opt2 = RX_CHANNEL(0) |
+  ULP_MODE_V(ULP_MODE_TCPDDP) |
+  RCV_BUFSIZ_V(win);
+   opt2 = RX_CHANNEL_V(0) |
   CCTRL_ECN(enable_ecn) |
-  RSS_QUEUE_VALID | RSS_QUEUE(ep->rss_qid);
+  RSS_QUEUE_VALID_F | RSS_QUEUE_V(ep->rss_qid);
if (enable_tcp_timestamps)
opt2 |= TSTAMPS_EN(1);
if (enable_tcp_sack)
opt2 |= SACK_EN(1);
if (wscale && enable_tcp_window_scaling)
-   opt2 |= WND_SCALE_EN(1);
+   opt2 |= WND_SCALE_EN_F;
if (is_t5(ep->com.dev->rdev.lldi.adapter_type)) {
-   opt2 |= T5_OPT_2_VALID;
+   opt2 |= T5_OPT_2_VALID_F;
opt2 |= V_CONG_CNTRL(CONG_ALG_TAHOE);
opt2 |= CONG_CNTRL_VALID; /* OPT_2_ISS for T5 */
}
@@ -736,7 +736,7 @@ static int send_connect(struct c4iw_ep *ep)
t5_req->local_ip = la->sin_addr.s_addr;
t5_req->peer_ip = ra->sin_addr.s_addr;
t5_req->opt0 = cpu_to_be64(opt0);
-   t5_req->params = cpu_to_be64(V_FILTER_TUPLE(
+   t5_req->params = cpu_to_be64(FILTER_TUPLE_V(
 cxgb4_select_ntuple(
 ep->com.dev->rdev.lldi.ports[0],
 ep->l2t)));
@@ -762,7 +762,7 @@ static int send_connect(struct c4iw_ep *ep)
t5_req6->peer_ip_lo = *((__be64 *)
(ra6->sin6_addr.s6_addr + 8));
t5_req6->opt0 = cpu_to_be64(opt0);
-   t5_req6->params = cpu_to_be64(V_FILTER_TUPLE(
+   t5_req6->params = cpu_to_be64(FILTER_TUPLE_V(
cxgb4_select_ntuple(
ep->com.dev->rdev.lldi.ports[0],
ep->l2t)));
@@ -1249,15 +1249,15 @@ static int update_rx_credits(struct c4iw_ep *ep, u32 
credits)
 * due to the limit in the number of bits in the RCV_BUFSIZ field,
 * then add the overage in to the credits returned.
 */
-   if (ep->rcv_win > RCV_BUFSIZ_MASK * 1024)
-   credits += ep->rcv_win - RCV_BUFSIZ_MASK * 1024;
+   if (ep->rcv_win > RCV_BUFSIZ_M * 1024)
+   credits += ep->rcv_win - RCV_BUFSIZ_M * 1024;
 
req = (struct cpl_rx_data_ack *) skb_put(skb, wrlen);
memset(req, 0, wrlen);
INIT_TP_WR(req, ep->hwtid);
OPCODE_TID(req) = cpu_to_be32(MK_OPCODE_TID(CPL_RX_DATA_ACK,
ep->hwtid));
- 

Re: tag handling refactor V2

2014-11-12 Thread James Bottomley
On Mon, 2014-11-10 at 16:56 +0100, Christoph Hellwig wrote:
> - for those drivers looking at the command tagged information we'd need
>to quiesce the LUN.  No driver but the 53c700 driver does that, and the
>53c700 does it at a target-level, which despite a comment claiming it's
>needed doesn't seem to make sense given the code.

I can answer that from ancient memory.  The mid-layer tag tracking
wasn't sufficient to track the negotiated state of the tags.  the 53c700
was constructed before the slave alloc/configure routines came along, so
it used a per target tracker for the tag negotiation (i.e. it was all on
or all off per target).  The reason for the quiesce is so as not to have
to try to work out if any LUN has any tagged commands outstanding when
changing to untagged.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: making the queue_type attribute read only, was: Re: tag handling refactor V2

2014-11-12 Thread Elliott, Robert (Server Storage)


> -Original Message-
> From: Christoph Hellwig [mailto:h...@infradead.org]
> Sent: Wednesday, 12 November, 2014 11:42 AM
> To: Christoph Hellwig
> Cc: linux-scsi@vger.kernel.org; James Bottomley; Elliott, Robert
> (Server Storage); Hannes Reinecke; Martin K. Petersen; Bart van
> Assche; Mike Christie
> Subject: making the queue_type attribute read only, was: Re: tag
> handling refactor V2
> 
> >  - how is the change_queue_type API supposed to be used for most
> >drivers?  It only changes the tag type from none to simple
> >or back, but except for the special implementation in the
> >53c700 driver doesn't change the queue depth,
> >which might cause it to issue multiple non-tagged command.
> >Fortunately most drivers never look at this information, but
> >then again changing the queue type is useless to start with.
> >  - for those drivers looking at the command tagged information
> >we'd need to quiesce the LUN.  No driver but the 53c700
> >driver does that, and the 53c700 does it at a target-level,
> >which despite a comment claiming it's needed doesn't seem
> >to make sense given the code.  If we can make sure
> >to quience all LUNs we could avoid the per-command flag for
> >tagged commands and always look at the scsi_device flag.
> 
> I've not merged the series, but ondering the above two issues I've
> become convinved that we should make the queue_type sysfs attribute
> read only.
> 
> Switching off tagging isn't really something we should leave to
> users, as the hardware knows much better.  Changing the tag type
> might have made sense during the times of the ordered tags for
> barriers insanity, but now that it's just tagged vs untagged
> it's pretty pointless.
> Especially given that only a single driver ever got the
> implementation right.
> 
> Does anyone have objections to removing the change_queue_type method?

A queue_type of "none" disables the SCSI midlayer automatic queue depth
adjustments based on TASK SET FULL vs. other status values, while
"simple" enables them.  That's the feature where TASK SET FULL reception
causes the queue depth to be lowered, while several GOOD status values
in a row causes it to be increased.

That logic doesn't work very well right now, so that sysfs file
might be the best way to keep controlling the automatic adjustment
feature even if it no longer controls whether tags exist.

One problem is it adjusts the queue depths for all devices sharing
the same target number; we talked about letting the host choose
the policy (http://marc.info/?l=linux-scsi&m=140440001619574&w=2).

Even if that is corrected, the ramp-up/ramp-down algorithms
and controls are not quite right for the full range of device 
types (SSDs to HDDs).  With SSDs, I saw the queue_depth:
* ramp down by about 10 every 2 seconds
* with the default queue_ramp_up_period of 12: never ramp up
* with a queue_ramp_up_period of 120: ramp up by 16 ever 2 seconds

At 1M IOPS, 2 seconds is far too long to decide anything; some
algorithm changes are needed.

---
Rob ElliottHP Server Storage



--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


mpt2sas,mpt3sas: SATA affiliations

2014-11-12 Thread Douglas Gilbert

From a correspondent and my own testing I have seen way
too many of these messages in the log:
   log_info(0x3116): originator(PL), code(0x16), sub_code(0x)

That comes from either the mpt2sas or mpt3sas driver and may be
a problem with their interaction with the SCSI EH. In one case,
those messages go on forever, requiring a reboot; in my testing
(with sg_readcap) the command timeout (60 seconds) stopped them.


How they occur needs a bit of explaining: ATA disks are designed
to have only only initiator (host). So if you build a SAS fabric
including at least two initiators, an expander and one SATA disk,
then there is potentially a problem which SAS expanders address
with "affiliations". An affiliation is a mechanism for the
expander to remember the SAS address of the initiator (host)
that first "grabbed" the SATA disk, and rejecting any other
initiator that tries to access that SATA disk.

That rejection, in the link layer in SAS for the STP protocol,
is a OPEN_REJECT (STP RESOURCES BUSY) response. That is *not*
a retry-able error (so the use of "busy" is unfortunate).
FreeBSD handles this correctly, Linux in some cases retries
which results in chaos plus bloated logs.

There are mechanisms for the owner of the affiliation to clear
it so another initiator can claim it. However affiliations are
designed to thwart brute force attempts by non-owners. At best
non-owners should get one log message along the lines of:
  cannot access SATA disk  since another machine/HBA is
  affiliated with it

Linux properly handles SATA affiliations when it comes across
them in normal device discovery. It is the "surprise"
disappearance of an affiliation that causes instability. That
surprise is caused by a utility like smp_phy_control telling
the expander to clear the affiliation and doing a rescan on
the other machine to claim the affiliation.

Doug Gilbert


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] Discard update for 3.19

2014-11-12 Thread Martin K. Petersen
> "Ewan" == Ewan Milne  writes:

Ewan> The changes in [PATCH 2/3] sd: Disable discard_zeroes_data for
Ewan> UNMAP look fine to me.  I was wondering, though, if the changes to
Ewan> add the "devices_handle_discard_safely" module parameter to the MD
Ewan> raid drivers are really needed if this is fixed.

I was expecting Neil to either revert or toggle the default once this
change was in place.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] Discard update for 3.19

2014-11-12 Thread Ewan Milne
On Mon, 2014-11-10 at 06:19 -0800, Christoph Hellwig wrote:
> Looks like there is no real dependency between these patches, so we
> might take on each through the libata, scsi and block trees.
> 
> Can I get another review for the sd patch, please?
> 

The changes in [PATCH 2/3] sd: Disable discard_zeroes_data for UNMAP
look fine to me.  I was wondering, though, if the changes to add the
"devices_handle_discard_safely" module parameter to the MD raid drivers
are really needed if this is fixed.  (It's great to be able to disable
the use of discard if desired, but how is an administrator actually
supposed to know if the devices they have *really* work properly?)
Maybe the default value of this parameter should be changed, or the
parameter should be changed to have an inverse sense, i.e. "disable
use of discard"...

-Ewan


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] scsi: remove scsi_driver owner field

2014-11-12 Thread Ewan Milne
On Wed, 2014-11-12 at 18:34 +0100, Christoph Hellwig wrote:
> The driver core driver structure has grown an owner field and now
> requires it to be set for all modular drivers.  Set it up for
> all scsi_driver instances and get rid of the now superflous
> scsi_driver owner field.
> 
> Signed-off-by: Christoph Hellwig 
> Reported-by: Shane M Seymour 
> ---
>  drivers/scsi/ch.c  | 2 +-
>  drivers/scsi/osd/osd_uld.c | 2 +-
>  drivers/scsi/osst.c| 2 +-
>  drivers/scsi/scsi_scan.c   | 9 -
>  drivers/scsi/sd.c  | 2 +-
>  drivers/scsi/ses.c | 2 +-
>  drivers/scsi/sr.c  | 2 +-
>  drivers/scsi/st.c  | 2 +-
>  include/scsi/scsi_driver.h | 1 -
>  9 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
> index 4f502f9..6bac8a7 100644
> --- a/drivers/scsi/ch.c
> +++ b/drivers/scsi/ch.c
> @@ -972,9 +972,9 @@ static int ch_remove(struct device *dev)
>  }
>  
>  static struct scsi_driver ch_template = {
> - .owner  = THIS_MODULE,
>   .gendrv = {
>   .name   = "ch",
> + .owner  = THIS_MODULE,
>   .probe  = ch_probe,
>   .remove = ch_remove,
>   },
> diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
> index 92cdd4b..243eab3 100644
> --- a/drivers/scsi/osd/osd_uld.c
> +++ b/drivers/scsi/osd/osd_uld.c
> @@ -540,9 +540,9 @@ static int osd_remove(struct device *dev)
>   */
>  
>  static struct scsi_driver osd_driver = {
> - .owner  = THIS_MODULE,
>   .gendrv = {
>   .name   = osd_name,
> + .owner  = THIS_MODULE,
>   .probe  = osd_probe,
>   .remove = osd_remove,
>   }
> diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c
> index 8c38464..5033223 100644
> --- a/drivers/scsi/osst.c
> +++ b/drivers/scsi/osst.c
> @@ -172,9 +172,9 @@ static int osst_probe(struct device *);
>  static int osst_remove(struct device *);
>  
>  static struct scsi_driver osst_template = {
> - .owner  = THIS_MODULE,
>   .gendrv = {
>   .name   =  "osst",
> + .owner  = THIS_MODULE,
>   .probe  = osst_probe,
>   .remove = osst_remove,
>   }
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index d97597e..0cda53a 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -1593,16 +1593,15 @@ EXPORT_SYMBOL(scsi_add_device);
>  
>  void scsi_rescan_device(struct device *dev)
>  {
> - struct scsi_driver *drv;
> - 
>   if (!dev->driver)
>   return;
>  
> - drv = to_scsi_driver(dev->driver);
> - if (try_module_get(drv->owner)) {
> + if (try_module_get(dev->driver->owner)) {
> + struct scsi_driver *drv = to_scsi_driver(dev->driver);
> +
>   if (drv->rescan)
>   drv->rescan(dev);
> - module_put(drv->owner);
> + module_put(dev->driver->owner);
>   }
>  }
>  EXPORT_SYMBOL(scsi_rescan_device);
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 95bfb7b..2ac603f8 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -510,9 +510,9 @@ static const struct dev_pm_ops sd_pm_ops = {
>  };
>  
>  static struct scsi_driver sd_template = {
> - .owner  = THIS_MODULE,
>   .gendrv = {
>   .name   = "sd",
> + .owner  = THIS_MODULE,
>   .probe  = sd_probe,
>   .remove = sd_remove,
>   .shutdown   = sd_shutdown,
> diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
> index 80bfece..b7e79e7 100644
> --- a/drivers/scsi/ses.c
> +++ b/drivers/scsi/ses.c
> @@ -693,9 +693,9 @@ static struct class_interface ses_interface = {
>  };
>  
>  static struct scsi_driver ses_template = {
> - .owner  = THIS_MODULE,
>   .gendrv = {
>   .name   = "ses",
> + .owner  = THIS_MODULE,
>   .probe  = ses_probe,
>   .remove = ses_remove,
>   },
> diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
> index 3d5399e..20a1096 100644
> --- a/drivers/scsi/sr.c
> +++ b/drivers/scsi/sr.c
> @@ -88,9 +88,9 @@ static struct dev_pm_ops sr_pm_ops = {
>  };
>  
>  static struct scsi_driver sr_template = {
> - .owner  = THIS_MODULE,
>   .gendrv = {
>   .name   = "sr",
> + .owner  = THIS_MODULE,
>   .probe  = sr_probe,
>   .remove = sr_remove,
>   .pm = &sr_pm_ops,
> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
> index e46e02b2..128d3b5 100644
> --- a/drivers/scsi/st.c
> +++ b/drivers/scsi/st.c
> @@ -202,9 +202,9 @@ static int do_create_sysfs_files(void);
>  static void do_remove_sysfs_files(void

making the queue_type attribute read only, was: Re: tag handling refactor V2

2014-11-12 Thread Christoph Hellwig
>  - how is the change_queue_type API supposed to be used for most drivers?  It
>only changes the tag type from none to simple or back, but except for the
>special implementation in the 53c700 driver doesn't change the queue depth,
>which might cause it to issue multiple non-tagged command.  Fortunately
>most drivers never look at this information, but then again changing the
>queue type is useless to start with.
>  - for those drivers looking at the command tagged information we'd need
>to quiesce the LUN.  No driver but the 53c700 driver does that, and the
>53c700 does it at a target-level, which despite a comment claiming it's
>needed doesn't seem to make sense given the code.  If we can make sure
>to quience all LUNs we could avoid the per-command flag for tagged commands
>and always look at the scsi_device flag.

I've not merged the series, but ondering the above two issues I've
become convinved that we should make the queue_type sysfs attribute read
only.

Switching off tagging isn't really something we should leave to users,
as the hardware knows much better.  Changing the tag type might have
made sense during the times of the ordered tags for barriers insanity,
but now that it's just tagged vs untagged it's pretty pointless.
Especially given that only a single driver ever got the implementation
right.

Does anyone have objections to removing the change_queue_type method?

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] scsi: remove scsi_driver owner field

2014-11-12 Thread Christoph Hellwig
The driver core driver structure has grown an owner field and now
requires it to be set for all modular drivers.  Set it up for
all scsi_driver instances and get rid of the now superflous
scsi_driver owner field.

Signed-off-by: Christoph Hellwig 
Reported-by: Shane M Seymour 
---
 drivers/scsi/ch.c  | 2 +-
 drivers/scsi/osd/osd_uld.c | 2 +-
 drivers/scsi/osst.c| 2 +-
 drivers/scsi/scsi_scan.c   | 9 -
 drivers/scsi/sd.c  | 2 +-
 drivers/scsi/ses.c | 2 +-
 drivers/scsi/sr.c  | 2 +-
 drivers/scsi/st.c  | 2 +-
 include/scsi/scsi_driver.h | 1 -
 9 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
index 4f502f9..6bac8a7 100644
--- a/drivers/scsi/ch.c
+++ b/drivers/scsi/ch.c
@@ -972,9 +972,9 @@ static int ch_remove(struct device *dev)
 }
 
 static struct scsi_driver ch_template = {
-   .owner  = THIS_MODULE,
.gendrv = {
.name   = "ch",
+   .owner  = THIS_MODULE,
.probe  = ch_probe,
.remove = ch_remove,
},
diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index 92cdd4b..243eab3 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -540,9 +540,9 @@ static int osd_remove(struct device *dev)
  */
 
 static struct scsi_driver osd_driver = {
-   .owner  = THIS_MODULE,
.gendrv = {
.name   = osd_name,
+   .owner  = THIS_MODULE,
.probe  = osd_probe,
.remove = osd_remove,
}
diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c
index 8c38464..5033223 100644
--- a/drivers/scsi/osst.c
+++ b/drivers/scsi/osst.c
@@ -172,9 +172,9 @@ static int osst_probe(struct device *);
 static int osst_remove(struct device *);
 
 static struct scsi_driver osst_template = {
-   .owner  = THIS_MODULE,
.gendrv = {
.name   =  "osst",
+   .owner  = THIS_MODULE,
.probe  = osst_probe,
.remove = osst_remove,
}
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index d97597e..0cda53a 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1593,16 +1593,15 @@ EXPORT_SYMBOL(scsi_add_device);
 
 void scsi_rescan_device(struct device *dev)
 {
-   struct scsi_driver *drv;
-   
if (!dev->driver)
return;
 
-   drv = to_scsi_driver(dev->driver);
-   if (try_module_get(drv->owner)) {
+   if (try_module_get(dev->driver->owner)) {
+   struct scsi_driver *drv = to_scsi_driver(dev->driver);
+
if (drv->rescan)
drv->rescan(dev);
-   module_put(drv->owner);
+   module_put(dev->driver->owner);
}
 }
 EXPORT_SYMBOL(scsi_rescan_device);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 95bfb7b..2ac603f8 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -510,9 +510,9 @@ static const struct dev_pm_ops sd_pm_ops = {
 };
 
 static struct scsi_driver sd_template = {
-   .owner  = THIS_MODULE,
.gendrv = {
.name   = "sd",
+   .owner  = THIS_MODULE,
.probe  = sd_probe,
.remove = sd_remove,
.shutdown   = sd_shutdown,
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index 80bfece..b7e79e7 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -693,9 +693,9 @@ static struct class_interface ses_interface = {
 };
 
 static struct scsi_driver ses_template = {
-   .owner  = THIS_MODULE,
.gendrv = {
.name   = "ses",
+   .owner  = THIS_MODULE,
.probe  = ses_probe,
.remove = ses_remove,
},
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 3d5399e..20a1096 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -88,9 +88,9 @@ static struct dev_pm_ops sr_pm_ops = {
 };
 
 static struct scsi_driver sr_template = {
-   .owner  = THIS_MODULE,
.gendrv = {
.name   = "sr",
+   .owner  = THIS_MODULE,
.probe  = sr_probe,
.remove = sr_remove,
.pm = &sr_pm_ops,
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index e46e02b2..128d3b5 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -202,9 +202,9 @@ static int do_create_sysfs_files(void);
 static void do_remove_sysfs_files(void);
 
 static struct scsi_driver st_template = {
-   .owner  = THIS_MODULE,
.gendrv = {
.name   = "st",
+   .owner  = THIS_MODULE,
.probe  

Re: [V2 PATCH 3/4] scsi:stex.c Add reboot support

2014-11-12 Thread Christoph Hellwig
> +static int stex_reboot_callback(struct notifier_block *self,
> +   unsigned long val,
> +   void *data)
> +{
> + if (val == SYS_RESTART)
> + isRestart = 1;
> + return NOTIFY_OK;
> +}
> 
> @@ -1832,7 +1859,14 @@ static void stex_shutdown(struct pci_dev *pdev)
>  {
>   struct st_hba *hba = pci_get_drvdata(pdev);
> 
> - stex_hba_stop(hba);
> + if (hba->yellowstone == 1)
> + stex_hba_stop(hba, ST_IGNORED);
> + else {
> + if (isRestart)
> + stex_hba_stop(hba, ST_S6);
> + else
> + stex_hba_stop(hba, ST_S5);
> + }

This sort of check for reboot vs restart isn't really something
we want in drivers.  I don't really know how we could find this
out assuming we even want drivers to behave differently.

Maybe Greg or someone on lkml has an idea how to best handle this case.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] st: set owner in struct device_driver

2014-11-12 Thread Christoph Hellwig
On Wed, Nov 12, 2014 at 12:01:31PM +, Seymour, Shane M wrote:
> I can, but at this point it will be tomorrow (11pm where I am).

Looks like none of th scsi_driver intances sets the owner field.  Let me
sort this out in a generic way, and thanks for the initial patch!

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 04/10] scsi: log request tag for scmd_printk()

2014-11-12 Thread Elliott, Robert (Server Storage)


> -Original Message-
> From: Hannes Reinecke [mailto:h...@suse.de]
> Sent: Thursday, 06 November, 2014 2:31 AM
> To: James Bottomley
> Cc: Christoph Hellwig; Ewan Milne; Elliott, Robert (Server Storage);
> linux-scsi@vger.kernel.org; Hannes Reinecke
> Subject: [PATCH 04/10] scsi: log request tag for scmd_printk()
> 
> The request tag provides a concise identification of a SCSI
> command, so we should be printing that out for scmd_printk().
> 
> Suggested-by: Christoph Hellwig 
> Reviewed-by: Christoph Hellwig 
> Signed-off-by: Hannes Reinecke 
> ---
>  drivers/scsi/scsi_logging.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_logging.c
> b/drivers/scsi/scsi_logging.c
> index 4a76796..dca45fe 100644
> --- a/drivers/scsi/scsi_logging.c
> +++ b/drivers/scsi/scsi_logging.c
> @@ -109,6 +109,10 @@ int scmd_printk(const char *level, const struct
> scsi_cmnd *scmd,
>   if (disk)
>   off += scnprintf(logbuf + off, logbuf_len - off,
>"[%s] ", disk->disk_name);
> +
> + if (scmd->request->tag >= 0)
> + off += scnprintf(logbuf + off, logbuf_len - off,
> +  "tag#%d ", scmd->request->tag);
>   va_start(args, fmt);
>   off += vscnprintf(logbuf + off, logbuf_len - off, fmt, args);
>   va_end(args);
> --
> 1.8.5.2

Reviewed-by: Robert Elliott 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 02/10] scsi: Add SPC-3 command definitions

2014-11-12 Thread Elliott, Robert (Server Storage)


> -Original Message-
> From: Hannes Reinecke [mailto:h...@suse.de]
> Sent: Thursday, 06 November, 2014 2:31 AM
> To: James Bottomley
> Cc: Christoph Hellwig; Ewan Milne; Elliott, Robert (Server Storage);
> linux-scsi@vger.kernel.org; Hannes Reinecke
> Subject: [PATCH 02/10] scsi: Add SPC-3 command definitions
> 
> SPC-3 defines SERVICE ACTION IN(12), SERVICE_ACTION OUT(12),
> SERVICE ACTION OUT(16), and SERVICE ACTION BIDIRECTIONAL.
> And READ MEDIA SERIAL NUMBER has long since been deprecated.
> So update callers to refer to the new cdb name.
> 
> Reviewed-by: Christoph Hellwig 
> Signed-off-by: Hannes Reinecke 
> ---
>  drivers/scsi/constants.c   | 4 
>  drivers/target/target_core_pr.c| 2 +-
>  include/scsi/scsi.h| 6 +-
>  tools/lib/traceevent/plugin_scsi.c | 5 -
>  4 files changed, 10 insertions(+), 7 deletions(-)
> 
...
> diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
> index b354c0d..df00fd1 100644
> --- a/include/scsi/scsi.h
> +++ b/include/scsi/scsi.h
> @@ -128,8 +128,10 @@ enum scsi_timeouts {
>  #define MOVE_MEDIUM   0xa5
>  #define EXCHANGE_MEDIUM   0xa6
>  #define READ_12   0xa8
> +#define SERVICE_ACTION_OUT_12 0xa9
>  #define WRITE_12  0xaa
> -#define READ_MEDIA_SERIAL_NUMBER 0xab
> +#define READ_MEDIA_SERIAL_NUMBER 0xab /* Obsolete with SPC-2 */
> +#define SERVICE_ACTION_IN_12 0xab

That needs one more space before 0xab to line up with the others.

...
> diff --git a/tools/lib/traceevent/plugin_scsi.c
> b/tools/lib/traceevent/plugin_scsi.c
> index c699f47..63aba97 100644
> --- a/tools/lib/traceevent/plugin_scsi.c
> +++ b/tools/lib/traceevent/plugin_scsi.c
> @@ -85,8 +85,9 @@ typedef unsigned int u32;
>  #define MOVE_MEDIUM  0xa5
>  #define EXCHANGE_MEDIUM  0xa6
>  #define READ_12  0xa8
> +#define SERVICE_ACTION_out_120xa9
...

out should be capitalized

Reviewed-by: Robert Elliott 


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 4/7] megaraid_sas : Online Firmware upgrade suppport for Extended VD feature

2014-11-12 Thread Kashyap Desai
> -Original Message-
> From: Tomas Henzl [mailto:the...@redhat.com]
> Sent: Wednesday, November 12, 2014 7:19 PM
> To: Kashyap Desai; Sumit Saxena; linux-scsi@vger.kernel.org
> Cc: martin.peter...@oracle.com; h...@infradead.org;
> jbottom...@parallels.com; aradf...@gmail.com
> Subject: Re: [PATCH 4/7] megaraid_sas : Online Firmware upgrade suppport
> for Extended VD feature
>
> On 11/12/2014 12:25 PM, Kashyap Desai wrote:
> >> -Original Message-
> >> From: Tomas Henzl [mailto:the...@redhat.com]
> >> Sent: Tuesday, November 11, 2014 7:43 PM
> >> To: sumit.sax...@avagotech.com; linux-scsi@vger.kernel.org
> >> Cc: martin.peter...@oracle.com; h...@infradead.org;
> >> jbottom...@parallels.com; kashyap.de...@avagotech.com;
> >> aradf...@gmail.com
> >> Subject: Re: [PATCH 4/7] megaraid_sas : Online Firmware upgrade
> >> suppport for Extended VD feature
> >>
> >> On 11/10/2014 01:21 PM, sumit.sax...@avagotech.com wrote:
> >>> This patch provides driver compatibility for updating firmware
> >>> online to upgrade legacy(64 VD) firmware to Extended VD firmware and
> >>> viceversa. Currently, at driver load time only, driver will check
> >>> whether Firmware is legacy or 240 VD. If legacy Firmware is upgraded
> > to
> >> Extended VD firmware without unloading-loading driver, driver will
> >> never come to know the underlying firmware is changed and it will
> >> always acts
> > as
> >> firmware type is same, which was queried at driver load time.
> >>> Below is the descriptions of code changes done-
> >>>
> >>> 1)This patch has added functionality to get controller information
> >>> from OCR(Online Controller Rest) path(which is called after firmware
> >> upgrade) and update the paramter of firmwware type flashed on
> > controller-
> >> Extended VD firmware or Legacy VD firmware.
> >>> Driver will not change any other parameter except for this one.
> >>>
> >>> 2)Also added few code optimizations/fixes[memset after
> >>> get_free_pages, and reduced redundant parameters in
> megasas_ctrl_info() function.
> >>>
> >>> Signed-off-by: Sumit Saxena 
> >>> Signed-off-by: Kashyap Desai 
> >>> ---
> >>>  drivers/scsi/megaraid/megaraid_sas.h|3 +-
> >>>  drivers/scsi/megaraid/megaraid_sas_base.c   |   80
> >> +++
> >>>  drivers/scsi/megaraid/megaraid_sas_fusion.c |   55
> > +--
> >>>  3 files changed, 83 insertions(+), 55 deletions(-)
> >>>
> >>> diff --git a/drivers/scsi/megaraid/megaraid_sas.h
> >>> b/drivers/scsi/megaraid/megaraid_sas.h
> >>> index a49914d..0408dda 100644
> >>> --- a/drivers/scsi/megaraid/megaraid_sas.h
> >>> +++ b/drivers/scsi/megaraid/megaraid_sas.h
> >>> @@ -1931,8 +1931,7 @@ u16 get_updated_dev_handle(struct
> >> megasas_instance *instance,
> >>>   struct LD_LOAD_BALANCE_INFO *lbInfo, struct IO_REQUEST_INFO
> >>> *in_info);  void mr_update_load_balance_params(struct
> >> MR_DRV_RAID_MAP_ALL *map,
> >>>   struct LD_LOAD_BALANCE_INFO *lbInfo); -int
> >>> megasas_get_ctrl_info(struct megasas_instance *instance,
> >>> - struct megasas_ctrl_info *ctrl_info);
> >>> +int megasas_get_ctrl_info(struct megasas_instance *instance);
> >>>  int megasas_set_crash_dump_params(struct megasas_instance
> >> *instance,
> >>>   u8 crash_buf_state);
> >>>  void megasas_free_host_crash_buffer(struct megasas_instance
> >>> *instance); diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
> >>> b/drivers/scsi/megaraid/megaraid_sas_base.c
> >>> index 7754eeb..9d5db5f 100644
> >>> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> >>> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> >>> @@ -4034,25 +4034,81 @@ megasas_ld_list_query(struct
> >> megasas_instance *instance, u8 query_type)
> >>>   return ret;
> >>>  }
> >>>
> >>> +/*
> >>> + * megasas_update_ext_vd_details : Update details w.r.t Extended VD
> >>> + * instance   : Controller's instance
> >>> +*/
> >>> +static void megasas_update_ext_vd_details(struct megasas_instance
> >>> +*instance) {
> >>> + struct fusion_context *fusion;
> >>> +
> >>> + fusion = instance->ctrl_context;
> >>> + /* For MFI based controllers return dummy success */
> >>> + if (!fusion)
> >>> + return;
> >>> +
> >>> + instance->supportmax256vd =
> >>> + instance->ctrl_info->adapterOperations3.supportMaxExtLDs;
> >>> + /* Below is additional check to address future FW enhancement */
> >>> + if (instance->ctrl_info->max_lds > 64)
> >>> + instance->supportmax256vd = 1;
> >>> +
> >>> + instance->drv_supported_vd_count =
> >> MEGASAS_MAX_LD_CHANNELS
> >>> + *
> >> MEGASAS_MAX_DEV_PER_CHANNEL;
> >>> + instance->drv_supported_pd_count =
> >> MEGASAS_MAX_PD_CHANNELS
> >>> + *
> >> MEGASAS_MAX_DEV_PER_CHANNEL;
> >>> + if (instance->supportmax256vd) {
> >>> + instance->fw_supported_vd_count =
> >> MAX_LOGICAL_DRIVES_EXT;
> >>> + instance->fw_supported_pd_count =
> >> MAX_PHYSICAL_DEVICES;
> >>> + } else {
> >>> + ins

Re: [PATCH 1/7] megaraid_sas : Driver version upgrade

2014-11-12 Thread Tomas Henzl
On 11/12/2014 12:13 PM, Kashyap Desai wrote:
>> -Original Message-
>> From: Tomas Henzl [mailto:the...@redhat.com]
>> Sent: Tuesday, November 11, 2014 7:14 PM
>> To: sumit.sax...@avagotech.com; linux-scsi@vger.kernel.org
>> Cc: martin.peter...@oracle.com; h...@infradead.org;
>> jbottom...@parallels.com; kashyap.de...@avagotech.com;
>> aradf...@gmail.com
>> Subject: Re: [PATCH 1/7] megaraid_sas : Driver version upgrade
>>
>> On 11/10/2014 01:21 PM, sumit.sax...@avagotech.com wrote:
>>> megaraid_sas driver version upgrade
>>>
>>> Signed-off-by: Sumit Saxena 
>>> ---
>>>  drivers/scsi/megaraid/megaraid_sas.h  | 2 +-
>>>  drivers/scsi/megaraid/megaraid_sas_base.c | 2 +-
>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/scsi/megaraid/megaraid_sas.h
>> b/drivers/scsi/megaraid/megaraid_sas.h
>>> index a49914d..f3eccf1 100644
>>> --- a/drivers/scsi/megaraid/megaraid_sas.h
>>> +++ b/drivers/scsi/megaraid/megaraid_sas.h
>>> @@ -33,7 +33,7 @@
>>>  /*
>>>   * MegaRAID SAS Driver meta data
>>>   */
>>> -#define MEGASAS_VERSION"06.805.06.00-
>> rc1"
>>> +#define MEGASAS_VERSION"06.805.06.01-
>> rc1"
>>>  #define MEGASAS_RELDATE"Sep. 4, 2014"
>>>  #define MEGASAS_EXT_VERSION"Thu. Sep. 4
> 17:00:00
>> PDT 2014"
>>
>> Usually you have changed the dates too, but this not important, feel
> free to
>> ignore  it.
> Agree that we always keep  version/date sync-up in same patch.
> I think having RELDATE is not really helpful and I see only megaraid_sas
> driver use this.
>
> Probably we should keep only VERSION and remove RELDATE and EXT_VERSION

EXT_VERSION is used to print a message to log - you can change it as you wish,
I haven't noticed something similar to RELDATE in other drivers, so again
I've no problem with any change.

>
>>> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
>> b/drivers/scsi/megaraid/megaraid_sas_base.c
>>> index f6a69a3..36f463c 100644
>>> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
>>> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
>>> @@ -18,7 +18,7 @@
>>>   *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
> 02111-1307
>> USA
>>>   *
>>>   *  FILE: megaraid_sas_base.c
>>> - *  Version : 06.805.06.00-rc1
>>> + *  Version : 06.805.06.01-rc1
>>>   *
>>>   *  Authors: LSI Corporation
>>>   *   Sreenivas Bagalkote
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/7] megaraid_sas : Online Firmware upgrade suppport for Extended VD feature

2014-11-12 Thread Tomas Henzl
On 11/12/2014 12:25 PM, Kashyap Desai wrote:
>> -Original Message-
>> From: Tomas Henzl [mailto:the...@redhat.com]
>> Sent: Tuesday, November 11, 2014 7:43 PM
>> To: sumit.sax...@avagotech.com; linux-scsi@vger.kernel.org
>> Cc: martin.peter...@oracle.com; h...@infradead.org;
>> jbottom...@parallels.com; kashyap.de...@avagotech.com;
>> aradf...@gmail.com
>> Subject: Re: [PATCH 4/7] megaraid_sas : Online Firmware upgrade suppport
>> for Extended VD feature
>>
>> On 11/10/2014 01:21 PM, sumit.sax...@avagotech.com wrote:
>>> This patch provides driver compatibility for updating firmware online
>>> to upgrade legacy(64 VD) firmware to Extended VD firmware and
>>> viceversa. Currently, at driver load time only, driver will check
>>> whether Firmware is legacy or 240 VD. If legacy Firmware is upgraded
> to
>> Extended VD firmware without unloading-loading driver, driver will never
>> come to know the underlying firmware is changed and it will always acts
> as
>> firmware type is same, which was queried at driver load time.
>>> Below is the descriptions of code changes done-
>>>
>>> 1)This patch has added functionality to get controller information
>>> from OCR(Online Controller Rest) path(which is called after firmware
>> upgrade) and update the paramter of firmwware type flashed on
> controller-
>> Extended VD firmware or Legacy VD firmware.
>>> Driver will not change any other parameter except for this one.
>>>
>>> 2)Also added few code optimizations/fixes[memset after get_free_pages,
>>> and reduced redundant parameters in megasas_ctrl_info() function.
>>>
>>> Signed-off-by: Sumit Saxena 
>>> Signed-off-by: Kashyap Desai 
>>> ---
>>>  drivers/scsi/megaraid/megaraid_sas.h|3 +-
>>>  drivers/scsi/megaraid/megaraid_sas_base.c   |   80
>> +++
>>>  drivers/scsi/megaraid/megaraid_sas_fusion.c |   55
> +--
>>>  3 files changed, 83 insertions(+), 55 deletions(-)
>>>
>>> diff --git a/drivers/scsi/megaraid/megaraid_sas.h
>>> b/drivers/scsi/megaraid/megaraid_sas.h
>>> index a49914d..0408dda 100644
>>> --- a/drivers/scsi/megaraid/megaraid_sas.h
>>> +++ b/drivers/scsi/megaraid/megaraid_sas.h
>>> @@ -1931,8 +1931,7 @@ u16 get_updated_dev_handle(struct
>> megasas_instance *instance,
>>> struct LD_LOAD_BALANCE_INFO *lbInfo, struct IO_REQUEST_INFO
>>> *in_info);  void mr_update_load_balance_params(struct
>> MR_DRV_RAID_MAP_ALL *map,
>>> struct LD_LOAD_BALANCE_INFO *lbInfo); -int
>>> megasas_get_ctrl_info(struct megasas_instance *instance,
>>> -   struct megasas_ctrl_info *ctrl_info);
>>> +int megasas_get_ctrl_info(struct megasas_instance *instance);
>>>  int megasas_set_crash_dump_params(struct megasas_instance
>> *instance,
>>> u8 crash_buf_state);
>>>  void megasas_free_host_crash_buffer(struct megasas_instance
>>> *instance); diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
>>> b/drivers/scsi/megaraid/megaraid_sas_base.c
>>> index 7754eeb..9d5db5f 100644
>>> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
>>> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
>>> @@ -4034,25 +4034,81 @@ megasas_ld_list_query(struct
>> megasas_instance *instance, u8 query_type)
>>> return ret;
>>>  }
>>>
>>> +/*
>>> + * megasas_update_ext_vd_details : Update details w.r.t Extended VD
>>> + * instance : Controller's instance
>>> +*/
>>> +static void megasas_update_ext_vd_details(struct megasas_instance
>>> +*instance) {
>>> +   struct fusion_context *fusion;
>>> +
>>> +   fusion = instance->ctrl_context;
>>> +   /* For MFI based controllers return dummy success */
>>> +   if (!fusion)
>>> +   return;
>>> +
>>> +   instance->supportmax256vd =
>>> +   instance->ctrl_info->adapterOperations3.supportMaxExtLDs;
>>> +   /* Below is additional check to address future FW enhancement */
>>> +   if (instance->ctrl_info->max_lds > 64)
>>> +   instance->supportmax256vd = 1;
>>> +
>>> +   instance->drv_supported_vd_count =
>> MEGASAS_MAX_LD_CHANNELS
>>> +   *
>> MEGASAS_MAX_DEV_PER_CHANNEL;
>>> +   instance->drv_supported_pd_count =
>> MEGASAS_MAX_PD_CHANNELS
>>> +   *
>> MEGASAS_MAX_DEV_PER_CHANNEL;
>>> +   if (instance->supportmax256vd) {
>>> +   instance->fw_supported_vd_count =
>> MAX_LOGICAL_DRIVES_EXT;
>>> +   instance->fw_supported_pd_count =
>> MAX_PHYSICAL_DEVICES;
>>> +   } else {
>>> +   instance->fw_supported_vd_count =
>> MAX_LOGICAL_DRIVES;
>>> +   instance->fw_supported_pd_count =
>> MAX_PHYSICAL_DEVICES;
>>> +   }
>>> +   dev_info(&instance->pdev->dev, "Firmware supports %d VD %d
>> PD\n",
>>> +   instance->fw_supported_vd_count,
>>> +   instance->fw_supported_pd_count);
>>> +   dev_info(&instance->pdev->dev, "Driver supports %d VD  %d PD\n",
>>> +   instance->drv_supported_vd_count,
>>> +   instance->drv_supported_pd_count);
>>> +
>>> +   fusion->old_map_sz =  sizeof(struct MR_FW_RAID_MAP)

RE: [PATCH] st: set owner in struct device_driver

2014-11-12 Thread Seymour, Shane M
I can, but at this point it will be tomorrow (11pm where I am).
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 4/7] megaraid_sas : Online Firmware upgrade suppport for Extended VD feature

2014-11-12 Thread Kashyap Desai
> -Original Message-
> From: Tomas Henzl [mailto:the...@redhat.com]
> Sent: Tuesday, November 11, 2014 7:43 PM
> To: sumit.sax...@avagotech.com; linux-scsi@vger.kernel.org
> Cc: martin.peter...@oracle.com; h...@infradead.org;
> jbottom...@parallels.com; kashyap.de...@avagotech.com;
> aradf...@gmail.com
> Subject: Re: [PATCH 4/7] megaraid_sas : Online Firmware upgrade suppport
> for Extended VD feature
>
> On 11/10/2014 01:21 PM, sumit.sax...@avagotech.com wrote:
> > This patch provides driver compatibility for updating firmware online
> > to upgrade legacy(64 VD) firmware to Extended VD firmware and
> > viceversa. Currently, at driver load time only, driver will check
> > whether Firmware is legacy or 240 VD. If legacy Firmware is upgraded
to
> Extended VD firmware without unloading-loading driver, driver will never
> come to know the underlying firmware is changed and it will always acts
as
> firmware type is same, which was queried at driver load time.
> > Below is the descriptions of code changes done-
> >
> > 1)This patch has added functionality to get controller information
> > from OCR(Online Controller Rest) path(which is called after firmware
> upgrade) and update the paramter of firmwware type flashed on
controller-
> Extended VD firmware or Legacy VD firmware.
> > Driver will not change any other parameter except for this one.
> >
> > 2)Also added few code optimizations/fixes[memset after get_free_pages,
> > and reduced redundant parameters in megasas_ctrl_info() function.
> >
> > Signed-off-by: Sumit Saxena 
> > Signed-off-by: Kashyap Desai 
> > ---
> >  drivers/scsi/megaraid/megaraid_sas.h|3 +-
> >  drivers/scsi/megaraid/megaraid_sas_base.c   |   80
> +++
> >  drivers/scsi/megaraid/megaraid_sas_fusion.c |   55
+--
> >  3 files changed, 83 insertions(+), 55 deletions(-)
> >
> > diff --git a/drivers/scsi/megaraid/megaraid_sas.h
> > b/drivers/scsi/megaraid/megaraid_sas.h
> > index a49914d..0408dda 100644
> > --- a/drivers/scsi/megaraid/megaraid_sas.h
> > +++ b/drivers/scsi/megaraid/megaraid_sas.h
> > @@ -1931,8 +1931,7 @@ u16 get_updated_dev_handle(struct
> megasas_instance *instance,
> > struct LD_LOAD_BALANCE_INFO *lbInfo, struct IO_REQUEST_INFO
> > *in_info);  void mr_update_load_balance_params(struct
> MR_DRV_RAID_MAP_ALL *map,
> > struct LD_LOAD_BALANCE_INFO *lbInfo); -int
> > megasas_get_ctrl_info(struct megasas_instance *instance,
> > -   struct megasas_ctrl_info *ctrl_info);
> > +int megasas_get_ctrl_info(struct megasas_instance *instance);
> >  int megasas_set_crash_dump_params(struct megasas_instance
> *instance,
> > u8 crash_buf_state);
> >  void megasas_free_host_crash_buffer(struct megasas_instance
> > *instance); diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
> > b/drivers/scsi/megaraid/megaraid_sas_base.c
> > index 7754eeb..9d5db5f 100644
> > --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> > +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> > @@ -4034,25 +4034,81 @@ megasas_ld_list_query(struct
> megasas_instance *instance, u8 query_type)
> > return ret;
> >  }
> >
> > +/*
> > + * megasas_update_ext_vd_details : Update details w.r.t Extended VD
> > + * instance : Controller's instance
> > +*/
> > +static void megasas_update_ext_vd_details(struct megasas_instance
> > +*instance) {
> > +   struct fusion_context *fusion;
> > +
> > +   fusion = instance->ctrl_context;
> > +   /* For MFI based controllers return dummy success */
> > +   if (!fusion)
> > +   return;
> > +
> > +   instance->supportmax256vd =
> > +   instance->ctrl_info->adapterOperations3.supportMaxExtLDs;
> > +   /* Below is additional check to address future FW enhancement */
> > +   if (instance->ctrl_info->max_lds > 64)
> > +   instance->supportmax256vd = 1;
> > +
> > +   instance->drv_supported_vd_count =
> MEGASAS_MAX_LD_CHANNELS
> > +   *
> MEGASAS_MAX_DEV_PER_CHANNEL;
> > +   instance->drv_supported_pd_count =
> MEGASAS_MAX_PD_CHANNELS
> > +   *
> MEGASAS_MAX_DEV_PER_CHANNEL;
> > +   if (instance->supportmax256vd) {
> > +   instance->fw_supported_vd_count =
> MAX_LOGICAL_DRIVES_EXT;
> > +   instance->fw_supported_pd_count =
> MAX_PHYSICAL_DEVICES;
> > +   } else {
> > +   instance->fw_supported_vd_count =
> MAX_LOGICAL_DRIVES;
> > +   instance->fw_supported_pd_count =
> MAX_PHYSICAL_DEVICES;
> > +   }
> > +   dev_info(&instance->pdev->dev, "Firmware supports %d VD %d
> PD\n",
> > +   instance->fw_supported_vd_count,
> > +   instance->fw_supported_pd_count);
> > +   dev_info(&instance->pdev->dev, "Driver supports %d VD  %d PD\n",
> > +   instance->drv_supported_vd_count,
> > +   instance->drv_supported_pd_count);
> > +
> > +   fusion->old_map_sz =  sizeof(struct MR_FW_RAID_MAP) +
> > +   (sizeof(struct MR_LD_SPAN_MAP) *
> > +  

Re: [PATCH v3 25/37] sun3_scsi: Convert to platform device

2014-11-12 Thread Finn Thain

On Wed, 12 Nov 2014, Geert Uytterhoeven wrote:
> 
> >   move PAGE_SIZE back to ioremap() arguments.
> 
> Sorry, I think I didn't make myself clear, and you thus misunderstood.

I thought your email was clear. Also, no-one objected to
ioremap(..., PAGE_SIZE) in the v1 patch.

Oh well.

> I meant PAGE_SIZE makes sense when ioremap()ing a small region at a 
> hardcoded address, where you don't care about the exact size, as it will 
> be rounded up to PAGE_SIZE anyway.
> 
> When ioremap()ing resources, please use the size provided by the resource.
> 
> > +static int __init sun3_scsi_probe(struct platform_device *pdev)
> > +{
> > +   struct Scsi_Host *instance;
> > +   int error;
> > +   struct resource *irq, *mem;
> > +   unsigned char *ioaddr;
> > +#ifdef SUN3_SCSI_VME
> > +   int i;
> > +#endif
> > +
> > +   if (setup_can_queue > 0)
> > +   sun3_scsi_template.can_queue = setup_can_queue;
> > +   if (setup_cmd_per_lun > 0)
> > +   sun3_scsi_template.cmd_per_lun = setup_cmd_per_lun;
> > +   if (setup_sg_tablesize >= 0)
> > +   sun3_scsi_template.sg_tablesize = setup_sg_tablesize;
> > +   if (setup_hostid >= 0)
> > +   sun3_scsi_template.this_id = setup_hostid & 7;
> > +
> > +#ifdef SUPPORT_TAGS
> > +   if (setup_use_tagged_queuing < 0)
> > +   setup_use_tagged_queuing = 1;
> > +#endif
> > +
> > +#ifdef SUN3_SCSI_VME
> > +   ioaddr = NULL;
> > +   for (i = 0; i < 2; i++) {
> > +   unsigned char x;
> > +
> > +   irq = platform_get_resource(pdev, IORESOURCE_IRQ, i);
> > +   mem = platform_get_resource(pdev, IORESOURCE_MEM, i);
> > +   if (!irq || !mem)
> > +   break;
> > +
> > +   ioaddr = sun3_ioremap(mem->start, PAGE_SIZE,
> 
> Here ...

OK.

> 
> > + SUN3_PAGE_TYPE_VME16);
> > +   dregs = (struct sun3_dma_regs *)(ioaddr + 8);
> > +
> > +   if (sun3_map_test((unsigned long)dregs, &x)) {
> > +   unsigned short oldcsr;
> > +
> > +   oldcsr = dregs->csr;
> > +   dregs->csr = 0;
> > +   udelay(SUN3_DMA_DELAY);
> > +   if (dregs->csr == 0x1400)
> > +   break;
> > +
> > +   dregs->csr = oldcsr;
> > +   }
> > +
> > +   iounmap(ioaddr);
> > +   ioaddr = NULL;
> > +   }
> > +   if (!ioaddr)
> > +   return -ENODEV;
> > +#else
> > +   irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > +   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +   if (!irq || !mem)
> > +   return -ENODEV;
> > +
> > +   ioaddr = ioremap(mem->start, PAGE_SIZE);
> 
> and here.
> 

OK.

-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/7] megaraid_sas : Driver version upgrade

2014-11-12 Thread Kashyap Desai
> -Original Message-
> From: Tomas Henzl [mailto:the...@redhat.com]
> Sent: Tuesday, November 11, 2014 7:14 PM
> To: sumit.sax...@avagotech.com; linux-scsi@vger.kernel.org
> Cc: martin.peter...@oracle.com; h...@infradead.org;
> jbottom...@parallels.com; kashyap.de...@avagotech.com;
> aradf...@gmail.com
> Subject: Re: [PATCH 1/7] megaraid_sas : Driver version upgrade
>
> On 11/10/2014 01:21 PM, sumit.sax...@avagotech.com wrote:
> > megaraid_sas driver version upgrade
> >
> > Signed-off-by: Sumit Saxena 
> > ---
> >  drivers/scsi/megaraid/megaraid_sas.h  | 2 +-
> >  drivers/scsi/megaraid/megaraid_sas_base.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/scsi/megaraid/megaraid_sas.h
> b/drivers/scsi/megaraid/megaraid_sas.h
> > index a49914d..f3eccf1 100644
> > --- a/drivers/scsi/megaraid/megaraid_sas.h
> > +++ b/drivers/scsi/megaraid/megaraid_sas.h
> > @@ -33,7 +33,7 @@
> >  /*
> >   * MegaRAID SAS Driver meta data
> >   */
> > -#define MEGASAS_VERSION"06.805.06.00-
> rc1"
> > +#define MEGASAS_VERSION"06.805.06.01-
> rc1"
> >  #define MEGASAS_RELDATE"Sep. 4, 2014"
> >  #define MEGASAS_EXT_VERSION"Thu. Sep. 4
17:00:00
> PDT 2014"
>
> Usually you have changed the dates too, but this not important, feel
free to
> ignore  it.

Agree that we always keep  version/date sync-up in same patch.
I think having RELDATE is not really helpful and I see only megaraid_sas
driver use this.

Probably we should keep only VERSION and remove RELDATE and EXT_VERSION

>
> >
> > diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
> b/drivers/scsi/megaraid/megaraid_sas_base.c
> > index f6a69a3..36f463c 100644
> > --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> > +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> > @@ -18,7 +18,7 @@
> >   *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
02111-1307
> USA
> >   *
> >   *  FILE: megaraid_sas_base.c
> > - *  Version : 06.805.06.00-rc1
> > + *  Version : 06.805.06.01-rc1
> >   *
> >   *  Authors: LSI Corporation
> >   *   Sreenivas Bagalkote
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 3/7] megaraid_sas : Do not process IOCTLs and SCSI commands during driver removal

2014-11-12 Thread Kashyap Desai
> -Original Message-
> From: Tomas Henzl [mailto:the...@redhat.com]
> Sent: Tuesday, November 11, 2014 7:13 PM
> To: sumit.sax...@avagotech.com; linux-scsi@vger.kernel.org
> Cc: martin.peter...@oracle.com; h...@infradead.org;
> jbottom...@parallels.com; kashyap.de...@avagotech.com;
> aradf...@gmail.com
> Subject: Re: [PATCH 3/7] megaraid_sas : Do not process IOCTLs and SCSI
> commands during driver removal
>
> On 11/10/2014 01:21 PM, sumit.sax...@avagotech.com wrote:
> > Do not process any SCSI and IOCTL command further(return them with
> > appropriate return values to callers), while driver removal is in
progress/PCI
> shutdown is invoked.
> >
> > Signed-off-by: Sumit Saxena 
> > Signed-off-by: Kashyap Desai 
> > ---
> >  drivers/scsi/megaraid/megaraid_sas_base.c |   20 
> >  1 files changed, 16 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
> > b/drivers/scsi/megaraid/megaraid_sas_base.c
> > index f6a69a3..7754eeb 100644
> > --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> > +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> > @@ -1572,6 +1572,12 @@ megasas_queue_command(struct Scsi_Host
> *shost, struct scsi_cmnd *scmd)
> > instance = (struct megasas_instance *)
> > scmd->device->host->hostdata;
> >
> > +   if (instance->unload == 1) {
> > +   scmd->result = DID_NO_CONNECT << 16;
> > +   scmd->scsi_done(scmd);
> > +   return 0;
> > +   }
> > +
> > if (instance->issuepend_done == 0)
> > return SCSI_MLQUEUE_HOST_BUSY;
> >
> > @@ -4957,10 +4963,6 @@ static int megasas_io_attach(struct
> megasas_instance *instance)
> > return -ENODEV;
> > }
> >
> > -   /*
> > -* Trigger SCSI to scan our drives
> > -*/
> > -   scsi_scan_host(host);
> > return 0;
> >  }
> >
> > @@ -5288,6 +5290,10 @@ retry_irq_register:
> > goto fail_io_attach;
> >
> > instance->unload = 0;
> > +   /*
> > +* Trigger SCSI to scan our drives
> > +*/
> > +   scsi_scan_host(host);
> >
> > /*
> >  * Initiate AEN (Asynchronous Event Notification) @@ -6051,6
> > +6057,12 @@ megasas_mgmt_fw_ioctl(struct megasas_instance
> *instance,
> > megasas_issue_blocked_cmd(instance, cmd, 0);
> > cmd->sync_cmd = 0;
>
> I've expected that you'll not send the command to the card, you first
issue
> blocked command and after that test unload state.
> Shouldn't it be reversed?
Actually, there was a two part of this Patch for IOCTL path. One will not
allow to send IOCTL while driver removal.
Another will not use already return IOCTL from FW to avoid illegal memory
access on MFI frame.
Driver wakeup all blocked command in unload path and MFI/MPT frames are
freed as well, so this check is good to have.

As you suggested, we missed to have another check in IOCTL path, before
driver takes semaphore. It was planned, but somehow we missed while
submitting this patch set.
We will send incremental patch for that part.

>
> >
> > +   if (instance->unload == 1) {
> > +   dev_info(&instance->pdev->dev, "we are doing unload so no
> need"
> > +   "to submit data to application. This may be force
exit"
> > +   "from driver\n");
>
> I'm not a native speaker, so I'll not comment on the wording, but you
should
> add a space at the line breaks.

Agree. will address this part.

> + dev_info(&instance->pdev->dev, "we are doing unload so no
> need "
> + "to submit data to application. This may be force
exit
> "
> + "from driver\n");
>
> > +   goto out;
> > +   }
> > /*
> >  * copy out the kernel buffers to user buffers
> >  */
>
> Aren't there other ioctl paths too - for example megasas_mgmt_ioctl_aen
-
> isn't a block needed there too?
This path will be cleanup from megasas_shutdown_controller() call.   AEN
and MAP SYNC command will be aborted explicitly.

>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 09/11] IB/srp: Use block layer tags

2014-11-12 Thread Christoph Hellwig
FYI, I've updated this to use .use_blk_tags = 1 isntead of
scsi_activate_tcq now that I've rebased the drivers-for-3.19 branch
over the tcq rework.  Please verify that it's workign as intented.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [V2 PATCH 1/4] scsi:stex.c Support to Pegasus series.

2014-11-12 Thread Christoph Hellwig
> + u8  yellowstone;

Calling this flag yellowstone seems a bit confusing as there already
is a st_yel card type, and this only seems a subset of those.  Maybe
a ->support_pm flag or similar would be useful?

> + u32 subID;

We don't use camelCaps, but I don't think you even need this variable at
all..

> 
>   hba->cardtype = (unsigned int) id->driver_data;
>   ci = &stex_card_info[hba->cardtype];
> + subID = id->subdevice;
> + if ((subID == 0x4221 || subID == 0x4222 ||
> + subID == 0x4223 || subID == 0x4224 ||
> + subID == 0x4225 || subID == 0x4226 ||
> + subID == 0x4227 || subID == 0x4261 ||
> + subID == 0x4262 || subID == 0x4263 ||
> + subID == 0x4264 || subID == 0x4265) &&
> + (hba->cardtype == st_yel)) {
> + hba->yellowstone = 1;
> + } else if (hba->cardtype == st_yel) {
> + hba->yellowstone = 0;
> + }

Just write this as

switch (id->subdevice) {
case 0x4221:
case 0x4222:
case 0x4223:
case 0x4224:
case 0x4225:
case 0x4226:
case 0x4227:
case 0x4261:
case 0x4262:
case 0x4263:
case 0x4264:
case 0x4265:
if (hba->cardtype == st_yel) {
hba->supports_pm = 1;
break;
}
default:
hba->supports_pm = 0;
}

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] st: set owner in struct device_driver

2014-11-12 Thread Christoph Hellwig
Looks good to me.  Can you also send the equivalent patch for the osst
and ch drivers?
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 25/37] sun3_scsi: Convert to platform device

2014-11-12 Thread Geert Uytterhoeven
Hi Finn,

On Wed, Nov 12, 2014 at 6:12 AM, Finn Thain  wrote:
> Changes since v2:
> - Give more precise addresses in platform resource initializers and

Thanks!

>   move PAGE_SIZE back to ioremap() arguments.

Sorry, I think I didn't make myself clear, and you thus misunderstood.
I meant PAGE_SIZE makes sense when ioremap()ing a small region
at a hardcoded address, where you don't care about the exact size, as it
will be rounded up to PAGE_SIZE anyway.

When ioremap()ing resources, please use the size provided by the resource.

> +static int __init sun3_scsi_probe(struct platform_device *pdev)
> +{
> +   struct Scsi_Host *instance;
> +   int error;
> +   struct resource *irq, *mem;
> +   unsigned char *ioaddr;
> +#ifdef SUN3_SCSI_VME
> +   int i;
> +#endif
> +
> +   if (setup_can_queue > 0)
> +   sun3_scsi_template.can_queue = setup_can_queue;
> +   if (setup_cmd_per_lun > 0)
> +   sun3_scsi_template.cmd_per_lun = setup_cmd_per_lun;
> +   if (setup_sg_tablesize >= 0)
> +   sun3_scsi_template.sg_tablesize = setup_sg_tablesize;
> +   if (setup_hostid >= 0)
> +   sun3_scsi_template.this_id = setup_hostid & 7;
> +
> +#ifdef SUPPORT_TAGS
> +   if (setup_use_tagged_queuing < 0)
> +   setup_use_tagged_queuing = 1;
> +#endif
> +
> +#ifdef SUN3_SCSI_VME
> +   ioaddr = NULL;
> +   for (i = 0; i < 2; i++) {
> +   unsigned char x;
> +
> +   irq = platform_get_resource(pdev, IORESOURCE_IRQ, i);
> +   mem = platform_get_resource(pdev, IORESOURCE_MEM, i);
> +   if (!irq || !mem)
> +   break;
> +
> +   ioaddr = sun3_ioremap(mem->start, PAGE_SIZE,

Here ...

> + SUN3_PAGE_TYPE_VME16);
> +   dregs = (struct sun3_dma_regs *)(ioaddr + 8);
> +
> +   if (sun3_map_test((unsigned long)dregs, &x)) {
> +   unsigned short oldcsr;
> +
> +   oldcsr = dregs->csr;
> +   dregs->csr = 0;
> +   udelay(SUN3_DMA_DELAY);
> +   if (dregs->csr == 0x1400)
> +   break;
> +
> +   dregs->csr = oldcsr;
> +   }
> +
> +   iounmap(ioaddr);
> +   ioaddr = NULL;
> +   }
> +   if (!ioaddr)
> +   return -ENODEV;
> +#else
> +   irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +   if (!irq || !mem)
> +   return -ENODEV;
> +
> +   ioaddr = ioremap(mem->start, PAGE_SIZE);

and here.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html