[PATCH] Marvell 6440 SAS/SATA driver (rough draft)

2007-09-25 Thread Jeff Garzik

Same disclaimer as with broadsas...  it's better to go ahead and get
this work-in-progress out there for people to comment on.

The chip:  the 6440 has per-HBA rings for TX (delivery) and RX
(completions, events).  You build a DMA command descriptor describing
the SSP, SMP, STP or SATA command, and let it rip.  Wide ports are
supported (search for phy_mask in the code).  Have plenty of control.

SATA is currently unsupported.  The 6440 has some useful internal
buffers SATA register sets, which permit dynamic associations between
SATA targets and SATA register blocks.

Most error handling, and some phy handling code is notably missing.

The 'sas' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/misc-2.6.git sas

contains the following updates:

 drivers/scsi/Kconfig|   20 +
 drivers/scsi/Makefile   |2 +
 drivers/scsi/broadsas.c | 1025 +
 drivers/scsi/mvsas.c| 1064 +++
 4 files changed, 2111 insertions(+), 0 deletions(-)
 create mode 100644 drivers/scsi/broadsas.c
 create mode 100644 drivers/scsi/mvsas.c

Jeff Garzik (3):
  Add Broadcom 8603 SAS/SATA driver (rough draft).
  Add Marvell 88SE6440 SAS/SATA driver (rough draft).
  broadsas, mvsas: fill in sas_port, sas_phy arrays

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 6f2c71e..bb041b8 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -486,6 +486,16 @@ config SCSI_AIC7XXX_OLD
 source drivers/scsi/aic7xxx/Kconfig.aic79xx
 source drivers/scsi/aic94xx/Kconfig
 
+config SCSI_BROADSAS
+   tristate Broadcom 8603 SAS/SATA support
+   depends on PCI
+   select SCSI_SAS_LIBSAS
+   help
+ This driver supports Broadcom SAS/SATA PCI devices.
+
+ To compile this driver as a module, choose M here: the
+ module will be called broadsas.
+
 # All the I2O code and drivers do not seem to be 64bit safe.
 config SCSI_DPT_I2O
tristate Adaptec I2O RAID support 
@@ -961,6 +971,16 @@ config SCSI_IZIP_SLOW_CTR
 
  Generally, saying N is fine.
 
+config SCSI_MVSAS
+   tristate Marvell 88SE6440 SAS/SATA support
+   depends on PCI
+   select SCSI_SAS_LIBSAS
+   help
+ This driver supports Marvell SAS/SATA PCI devices.
+
+ To compiler this driver as a module, choose M here: the module
+ will be called mvsas.
+
 config SCSI_NCR53C406A
tristate NCR53c406a SCSI support
depends on ISA  SCSI
diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
index 86a7ba7..928b6a2 100644
--- a/drivers/scsi/Makefile
+++ b/drivers/scsi/Makefile
@@ -72,6 +72,7 @@ obj-$(CONFIG_SCSI_AIC79XX)+= aic7xxx/
 obj-$(CONFIG_SCSI_AACRAID) += aacraid/
 obj-$(CONFIG_SCSI_AIC7XXX_OLD) += aic7xxx_old.o
 obj-$(CONFIG_SCSI_AIC94XX) += aic94xx/
+obj-$(CONFIG_SCSI_BROADSAS)+= broadsas.o
 obj-$(CONFIG_SCSI_IPS) += ips.o
 obj-$(CONFIG_SCSI_FD_MCS)  += fd_mcs.o
 obj-$(CONFIG_SCSI_FUTURE_DOMAIN)+= fdomain.o
@@ -132,6 +133,7 @@ obj-$(CONFIG_SCSI_IBMVSCSI) += ibmvscsi/
 obj-$(CONFIG_SCSI_IBMVSCSIS)   += ibmvscsi/
 obj-$(CONFIG_SCSI_HPTIOP)  += hptiop.o
 obj-$(CONFIG_SCSI_STEX)+= stex.o
+obj-$(CONFIG_SCSI_MVSAS)   += mvsas.o
 obj-$(CONFIG_PS3_ROM)  += ps3rom.o
 
 obj-$(CONFIG_ARM)  += arm/
diff --git a/drivers/scsi/broadsas.c b/drivers/scsi/broadsas.c
new file mode 100644
index 000..eeba635
--- /dev/null
+++ b/drivers/scsi/broadsas.c

[EDITED OUT, POSTED EARLIER]

diff --git a/drivers/scsi/mvsas.c b/drivers/scsi/mvsas.c
new file mode 100644
index 000..6dc518e
--- /dev/null
+++ b/drivers/scsi/mvsas.c
@@ -0,0 +1,1064 @@
+/*
+   mvsas.c - Marvell 88SE6440 SAS/SATA support
+
+   Copyright 2007 Red Hat, Inc.
+
+   This program is free software; you can redistribute it and/or
+   modify it under the terms of the GNU General Public License as
+   published by the Free Software Foundation; either version 2,
+   or (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty
+   of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+   See the GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public
+   License along with this program; see the file COPYING.  If not,
+   write to the Free Software Foundation, 675 Mass Ave, Cambridge,
+   MA 02139, USA.
+
+   ---
+
+   Random notes:
+   * hardware supports controlling the endian-ness of data
+ structures.  this permits elimination of all the le32_to_cpu()
+ and cpu_to_le32() conversions.
+
+   * hardware supports much more efficient interrupt handling
+ than we current employ.
+
+ */
+
+#include linux/kernel.h
+#include 

question on SDEV_BLOCK vs. SDEV_QUIESCE

2007-09-25 Thread Oliver Neukum
Hi,

what is the difference between these states? What function should
I call to block a device in order to securely flush the caches?

Regards
Oliver
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: question on SDEV_BLOCK vs. SDEV_QUIESCE

2007-09-25 Thread Hannes Reinecke
Oliver Neukum wrote:
 Hi,
 
 what is the difference between these states? What function should
 I call to block a device in order to securely flush the caches?
 
SDEV_BLOCK blocks _any_ commands from the midlayer; eg. if the LLDD
is doing some internal recovery.
SDEV_QUIESCE only blocks 'normal' data commands; others (like control
commands) can still get through.

So for flushing you should use SDEV_QUIESCE.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke   zSeries  Storage
[EMAIL PROTECTED] +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [linux-usb-devel] question on flushing buffers and spinning down disk

2007-09-25 Thread Oliver Neukum
Am Montag 24 September 2007 schrieb Alan Stern:
 On Mon, 24 Sep 2007, Oliver Neukum wrote:
 
   subsystem implements its own form of runtime PM support, then you'll be
   able to use it properly.  But until then, there isn't anything much you
   can do.
  
  Well, we can't simply cut power. Buffers must be flushed and the disk
  spun down. The suspend() method of the storage driver will have to become
  complex.
 
 That's right, if the device is a real disk.  If it's a flash memory
 device then of course these issues don't arise.  Unfortunately the

With respect to buffers how sure are you about that?

 driver isn't able to tell one from the other; the user will have to
 do that for us.

Code for what we need is in sd_suspend(). The only trouble is locking.
We need to make sure no commands that dirty buffers are issued after
flushing the buffers.

Regards
Oliver


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


Re: question on SDEV_BLOCK vs. SDEV_QUIESCE

2007-09-25 Thread Oliver Neukum
Am Dienstag 25 September 2007 schrieb Hannes Reinecke:
 Oliver Neukum wrote:
  Hi,
  
  what is the difference between these states? What function should
  I call to block a device in order to securely flush the caches?
  
 SDEV_BLOCK blocks _any_ commands from the midlayer; eg. if the LLDD
 is doing some internal recovery.
 SDEV_QUIESCE only blocks 'normal' data commands; others (like control
 commands) can still get through.
 
 So for flushing you should use SDEV_QUIESCE.

Thank you, that's very helpful.

Regards
Oliver

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


Re: [PATCH 0/4] more gdth patches for your amusement

2007-09-25 Thread Christoph Hellwig
On Mon, Sep 24, 2007 at 08:25:28PM -0400, Jeff Garzik wrote:
 OK, we've had these competing patch sets floating around for two months
 now.  Christoph and Jeff, can we get agreement on which is going in?
 
 Well, my opinion is
 
 1) When judging by total amount of positive improvement, Christoph's 
 patches are superior -- he has more overall cleanups than I do.
 
 2) When judging by likelihood of inducing breakage, I feel my changes 
 are superior.  My gdth changes tightly adhere to the 
 equivalent-transformation method of shuffing code around, enabling 
 further improvements.  IOW, I resisted the urge to make cleanups and fix 
 insignificant, pre-existing bugs during the transformations.
 
 3) I am utterly unmotivated to merge the two patchsets.  Someone should 
 make an executive decision, pull one patchset, and drop the other.  My 
 coding mood has swung from cleaning up code to writing new SAS drivers :)

Go ahead with your patches as I don't have time working on mine right now.
I'll port them ontop of your patches when I get time for it.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] more gdth patches for your amusement

2007-09-25 Thread Boaz Harrosh
On Tue, Sep 25 2007 at 10:20 +0200, Christoph Hellwig [EMAIL PROTECTED] wrote:
 On Mon, Sep 24, 2007 at 08:25:28PM -0400, Jeff Garzik wrote:
 OK, we've had these competing patch sets floating around for two months
 now.  Christoph and Jeff, can we get agreement on which is going in?
 Well, my opinion is

 1) When judging by total amount of positive improvement, Christoph's 
 patches are superior -- he has more overall cleanups than I do.

 2) When judging by likelihood of inducing breakage, I feel my changes 
 are superior.  My gdth changes tightly adhere to the 
 equivalent-transformation method of shuffing code around, enabling 
 further improvements.  IOW, I resisted the urge to make cleanups and fix 
 insignificant, pre-existing bugs during the transformations.

 3) I am utterly unmotivated to merge the two patchsets.  Someone should 
 make an executive decision, pull one patchset, and drop the other.  My 
 coding mood has swung from cleaning up code to writing new SAS drivers :)
 
 Go ahead with your patches as I don't have time working on mine right now.
 I'll port them ontop of your patches when I get time for it.
 -
 To unsubscribe from this list: send the line unsubscribe linux-scsi in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

I have attempted a merge of both patchsets, and was about to send them
but found that some code must be moved to earlier patches if I want 
bisectability.
So it will take me another day to redo the patches and clean them up.

I have went even farther than Christoph with my patchset to totally 
remove the gdth_ctr_tab array and only use the new gdth_instances LIST_HEAD.
I have done that by simply passing gdth_ha_str pointers around instead of
hanum's.

On top of that I have my own agenda of cleaning the !use_sg code paths and 
getting
rid of scsi_cmnd abuse, so there is also that.

So the patchset certainly takes the likelihood of inducing breakage approach.
Being a merge of code from three people and none of them tested. So they will
have to be exercised on real hardware before inclusion. Is there someone, 
perhaps
in Adaptec, that can try some code.

Boaz

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


Re: [PATCH 0/4] more gdth patches for your amusement

2007-09-25 Thread Matthew Wilcox
On Tue, Sep 25, 2007 at 11:51:13AM +0200, Boaz Harrosh wrote:
 On top of that I have my own agenda of cleaning the !use_sg code paths and 
 getting
 rid of scsi_cmnd abuse, so there is also that.

This seems like a good time to post my own patch that removes the use of
-scsi_done from gdth.  I have a plan to remove the -scsi_done() callback
(drivers will simply call the scsi_done() function directly), and fixing
the half-dozen drivers that override it is part of that.

I haven't looked at Christoph's, Jeff's or your patches yet, so this
patch may be entirely worthless.  My goal with it was not to clean up
the driver (though it does a little), but to get gdth out of the way of
cleaning up scsi_cmnd.

commit 06142e2394d83929b8b25feab70caab47ddfb791
Author: Matthew Wilcox [EMAIL PROTECTED]
Date:   Sat Sep 22 22:57:06 2007 -0400

gdth: Make one abuse of scsi_cmnd less obvious

Rather than having internal commands abuse scsi_done to call
gdth_scsi_done, have all the places that use to call scsi_done directly
call gdth_scsi_done, which now checks whether the command was internal,
and calls scsi_done if not.

Signed-off-by: Matthew Wilcox [EMAIL PROTECTED]

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index b20c188..8a6a5f8 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -475,7 +475,6 @@ static int gdth_ioctl(struct inode *inode, struct file 
*filep,
 static void gdth_flush(int hanum);
 static int gdth_halt(struct notifier_block *nb, ulong event, void *buf);
 static int gdth_queuecommand(Scsi_Cmnd *scp,void (*done)(Scsi_Cmnd *));
-static void gdth_scsi_done(struct scsi_cmnd *scp);
 
 #ifdef DEBUG_GDTH
 static unchar   DebugState = DEBUG_GDTH;
@@ -710,13 +709,14 @@ static void gdth_delay(int milliseconds)
 }
 }
 
-#if LINUX_VERSION_CODE = KERNEL_VERSION(2,6,0)
 static void gdth_scsi_done(struct scsi_cmnd *scp)
 {
-TRACE2((gdth_scsi_done()\n));
+   TRACE2((gdth_scsi_done()\n));
 
-if (scp-request)
-complete((struct completion *)scp-request);
+   if (scp-done == gdth_scsi_done)
+   complete((struct completion *)scp-request);
+   else 
+   scp-scsi_done(scp);
 }
 
 int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd,
@@ -738,8 +738,8 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str 
*gdtcmd, char *cmnd,
 scp-cmd_len = 12;
 memcpy(scp-cmnd, cmnd, 12);
 scp-SCp.this_residual = IOCTL_PRI;   /* priority */
-scp-done = gdth_scsi_done; /* some fn. test this */
-gdth_queuecommand(scp, gdth_scsi_done);
+scp-done = gdth_scsi_done;
+gdth_queuecommand(scp, NULL);
 wait_for_completion(wait);
 
 rval = scp-SCp.Status;
@@ -748,42 +748,6 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str 
*gdtcmd, char *cmnd,
 kfree(scp);
 return rval;
 }
-#else
-static void gdth_scsi_done(Scsi_Cmnd *scp)
-{
-TRACE2((gdth_scsi_done()\n));
-
-scp-request.rq_status = RQ_SCSI_DONE;
-if (scp-request.waiting)
-complete(scp-request.waiting);
-}
-
-int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd,
-   int timeout, u32 *info)
-{
-Scsi_Cmnd *scp = scsi_allocate_device(sdev, 1, FALSE);
-unsigned bufflen = gdtcmd ? sizeof(gdth_cmd_str) : 0;
-DECLARE_COMPLETION_ONSTACK(wait);
-int rval;
-
-if (!scp)
-return -ENOMEM;
-scp-cmd_len = 12;
-scp-use_sg = 0;
-scp-SCp.this_residual = IOCTL_PRI;   /* priority */
-scp-request.rq_status = RQ_SCSI_BUSY;
-scp-request.waiting = wait;
-scsi_do_cmd(scp, cmnd, gdtcmd, bufflen, gdth_scsi_done, timeout*HZ, 1);
-wait_for_completion(wait);
-
-rval = scp-SCp.Status;
-if (info)
-*info = scp-SCp.Message;
-
-scsi_release_command(scp);
-return rval;
-}
-#endif
 
 int gdth_execute(struct Scsi_Host *shost, gdth_cmd_str *gdtcmd, char *cmnd,
  int timeout, u32 *info)
@@ -2505,7 +2469,7 @@ static void gdth_next(int hanum)
 if (!nscp-SCp.have_data_in)
 nscp-SCp.have_data_in++;
 else
-nscp-scsi_done(nscp);
+gdth_scsi_done(nscp);
 }
 } else if (nscp-done == gdth_scsi_done) {
 if (!(cmd_index=gdth_special_cmd(hanum,nscp)))
@@ -2524,7 +2488,7 @@ static void gdth_next(int hanum)
 if (!nscp-SCp.have_data_in)
 nscp-SCp.have_data_in++;
 else
-nscp-scsi_done(nscp);
+gdth_scsi_done(nscp);
 } else {
 switch (nscp-cmnd[0]) {
   case TEST_UNIT_READY:
@@ -2550,9 +2514,9 @@ static void gdth_next(int hanum)
 if (!nscp-SCp.have_data_in)
 nscp-SCp.have_data_in++;
 else
-nscp-scsi_done(nscp);
+gdth_scsi_done(nscp);
 } else if 

Re: [PATCH 0/4] more gdth patches for your amusement

2007-09-25 Thread Boaz Harrosh
On Tue, Sep 25 2007 at 13:56 +0200, Matthew Wilcox [EMAIL PROTECTED] wrote:
 On Tue, Sep 25, 2007 at 11:51:13AM +0200, Boaz Harrosh wrote:
 On top of that I have my own agenda of cleaning the !use_sg code paths and 
 getting
 rid of scsi_cmnd abuse, so there is also that.
 
 This seems like a good time to post my own patch that removes the use of
 -scsi_done from gdth.  I have a plan to remove the -scsi_done() callback
 (drivers will simply call the scsi_done() function directly), and fixing
 the half-dozen drivers that override it is part of that.
 
 I haven't looked at Christoph's, Jeff's or your patches yet, so this
 patch may be entirely worthless.  My goal with it was not to clean up
 the driver (though it does a little), but to get gdth out of the way of
 cleaning up scsi_cmnd.
 
 commit 06142e2394d83929b8b25feab70caab47ddfb791
 Author: Matthew Wilcox [EMAIL PROTECTED]
 Date:   Sat Sep 22 22:57:06 2007 -0400
 
 gdth: Make one abuse of scsi_cmnd less obvious
 
 Rather than having internal commands abuse scsi_done to call
 gdth_scsi_done, have all the places that use to call scsi_done directly
 call gdth_scsi_done, which now checks whether the command was internal,
 and calls scsi_done if not.
 
 Signed-off-by: Matthew Wilcox [EMAIL PROTECTED]
 
 diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
 index b20c188..8a6a5f8 100644
 --- 

Hi Matthew!
This patch looks grate, Thanks. It is very good for the direction
I'm going to. However it does have a smallish conflict with One
of Jeff's patches where he completely removes the 2.4.x support.

If it is OK with you I will add your patch to my patchset with your
Singed-off-by, minus the conflict?
 
Boaz
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] more gdth patches for your amusement

2007-09-25 Thread Matthew Wilcox
On Tue, Sep 25, 2007 at 02:17:44PM +0200, Boaz Harrosh wrote:
 This patch looks grate, Thanks. It is very good for the direction
 I'm going to. However it does have a smallish conflict with One
 of Jeff's patches where he completely removes the 2.4.x support.
 
 If it is OK with you I will add your patch to my patchset with your
 Singed-off-by, minus the conflict?

That's absolutely fine; the fewer patches I have to track, the better.

-- 
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Some plans for scsi_cmnd

2007-09-25 Thread Matthew Wilcox

Christoph grabbed me on IRC and we debated some of my plans for scsi_cmnd;
with his permission I'm summarising the result of that debate for posterity.
There's four main things discussed:

1. I'm going to be writing and posting patches over the next week or so
to kill all the (ab)uses of -scsi_done in drivers.  Once that is done,
I'll post a patch that exports the midlayer's scsi_done and switch all
the drivers to calling that.  After that, I'll post another patch that
changes the prototype *and the semantics* of queuecommand; the midlayer
will no longer take the host_lock for the driver.  I'll just push the
acquisition down into queuecommand, and it'll be up to individual driver
authors to do something sensible with it then.

2. Thanks to a thinko, we also discussed the upper-layer -done.  We think
it should be feasible to move this from the scsi_cmnd to the scsi_device
since sg doesn't use it.

3. We also discussed scsi_pointer.  It's really quite crufty, and it
gets recycled for storing all kinds of things.  The ambitious plan here
is to change the whole way scsi_cmnds are allocated.  Code is clearer
than my description ...

sym2.c:

struct sym2_cmnd {
struct scsi_cmnd cmd;
int Phase;
char *data_in;
}

struct scsi_host_template sym2_template {
.cmnd_size = sizeof(sym2_cmnd);
}

int sym2_queuecommand(struct scsi_cmnd *scp)
{
struct sym2_cmnd *cmnd = container_of(scp, sym2_cmnd, cmd);
}

The midlayer will create a slab pool per host template for allocating
scsi_cmnd.

As I said, it's ambitious.  But it'll let us get rid of scsi_pointer
and host_scribble entirely.

4. We don't understand why sense_buffer is 96 bytes long.  mkp says that
devices are coming which need more than 96 bytes (the standard allows up
to 252).  I propose splitting the 96-byte buffer like so:

-#define SCSI_SENSE_BUFFERSIZE  96
-   unsigned char sense_buffer[SCSI_SENSE_BUFFERSIZE];
+   unsigned char sense_buffer_head[8];
+   unsigned char *sense_buffer_desc;

and then adding:

+int scsi_set_sense_buffer(struct scsi_cmnd *cmd, unsigned char *sense,
+   int sense_len)
+{
+   int len = min(sense[7], sense_len - 8);
+   memcpy(cmd-sense_buffer_head, sense, min(8, sense_len));
+   if (len = 0)
+   return 0;
+   cmd-sense_buffer_desc = kmalloc(len, GFP_ATOMIC);
+   if (!cmd-sense_buffer_desc)
+   return 1;
+   memcpy(cmd-sense_buffer_desc, sense + 8, len);
+   return 0;
+}
+EXPORT_SYMBOL(scsi_set_sense_buffer);

Drivers then do something like:

-   memset(cmd-sense_buffer, 0, sizeof(cmd-sense_buffer))
-   memcpy(cmd-sense_buffer, cp-sns_bbuf,
- min(sizeof(cmd-sense_buffer),
- (size_t)SYM_SNS_BBUF_LEN));
+   scsi_set_sense_buffer(cmd, cp-sns_bbuf,
+   SYM_SNS_BBUF_LEN);

or even ...

-   /* Copy Sense Data into sense buffer. */
-   memset(cp-sense_buffer, 0, sizeof(cp-sense_buffer));
-
if (!(scsi_status  SS_SENSE_LEN_VALID))
break;
 
-   if (sense_len = sizeof(cp-sense_buffer))
-   sense_len = sizeof(cp-sense_buffer);
-
-   CMD_ACTUAL_SNSLEN(cp) = sense_len;
-   sp-request_sense_length = sense_len;
-   sp-request_sense_ptr = cp-sense_buffer;
-
-   if (sp-request_sense_length  32)
-   sense_len = 32;
-
-   memcpy(cp-sense_buffer, sense_data, sense_len);
-
-   sp-request_sense_ptr += sense_len;
-   sp-request_sense_length -= sense_len;
-   if (sp-request_sense_length != 0)
-   ha-status_srb = sp;
+   scsi_set_sense_buffer(cp, sense_data, sense_len);

If any of this seems unwelcome, please say so.  It's going to be a lot of
churn (and part 4 may well take six months or more), so I'd like people
to voice objections now rather than later.

-- 
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Some plans for scsi_cmnd

2007-09-25 Thread Christoph Hellwig
On Tue, Sep 25, 2007 at 07:37:33AM -0600, Matthew Wilcox wrote:
 2. Thanks to a thinko, we also discussed the upper-layer -done.  We think
 it should be feasible to move this from the scsi_cmnd to the scsi_device
 since sg doesn't use it.

I suspect putting it into the scsi_driver would be even better.

 3. We also discussed scsi_pointer.  It's really quite crufty, and it
 gets recycled for storing all kinds of things.  The ambitious plan here
 is to change the whole way scsi_cmnds are allocated.  Code is clearer
 than my description ...
 
 sym2.c:
 
 struct sym2_cmnd {
   struct scsi_cmnd cmd;
   int Phase;
   char *data_in;
 }
 
 struct scsi_host_template sym2_template {
   .cmnd_size = sizeof(sym2_cmnd);
 }

I'd prefer to add alloc_mnd and destroy_cmnd methods as per struct
inode.  That also allows drivers to do things like dma_pool allocations
ahead of time and not worry about needing to do this kind of allocations
from softirq context which is at least theoretically deadlockable
without emergency pool schemes.

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


RE: Patch added to scsi-pending-2.6: [SCSI] ips: Update version information

2007-09-25 Thread Salyzyn, Mark
ACK

Sincerely -- Mark Salyzyn

 -Original Message-
 From: James Bottomley [mailto:[EMAIL PROTECTED] 
 Sent: Sunday, September 23, 2007 10:45 AM
 To: AACRAID; Bernhard Walle; James Bottomley
 Subject: Patch added to scsi-pending-2.6: [SCSI] ips: Update 
 version information
 
 Your commit:
 
 [SCSI] ips: Update version information
 
 This patch just makes the version number in ips.c and ips.h
consistent. It
 seems that this has been forgotten in
a60768e2d43eb30a1adb8a119aeac35dc0d03ef6.
 
 It also removes code duplication, each number is now only once in
the code to
 avoid similar errors in the future.
 
 Signed-off-by: Bernhard Walle [EMAIL PROTECTED]
 Cc: Mark Salyzyn [EMAIL PROTECTED]
 Signed-off-by: James Bottomley [EMAIL PROTECTED]
 
 has been added to the pending SCSI tree
 You can find it here:
 

http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-pending-2.6.git;a=co
mmit;h=7c3f6ca91cd906476274db6b9ecc536eb66fb908
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Some plans for scsi_cmnd

2007-09-25 Thread Matthew Wilcox
On Tue, Sep 25, 2007 at 02:47:50PM +0100, Christoph Hellwig wrote:
 On Tue, Sep 25, 2007 at 07:37:33AM -0600, Matthew Wilcox wrote:
  2. Thanks to a thinko, we also discussed the upper-layer -done.  We think
  it should be feasible to move this from the scsi_cmnd to the scsi_device
  since sg doesn't use it.
 
 I suspect putting it into the scsi_driver would be even better.

That could work, but I don't think we have a pointer from the
scsi_device (or scsi_cmnd) to the scsi_driver.

 I'd prefer to add alloc_mnd and destroy_cmnd methods as per struct
 inode.  That also allows drivers to do things like dma_pool allocations
 ahead of time and not worry about needing to do this kind of allocations
 from softirq context which is at least theoretically deadlockable
 without emergency pool schemes.

OK, we clearly weren't quite envisaging the same thing for this part.
Good job I put some pseudo-code down.  I don't have any objection to
this scheme.

-- 
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [linux-usb-devel] question on flushing buffers and spinning down disk

2007-09-25 Thread Alan Stern
On Mon, 24 Sep 2007, Oliver Neukum wrote:

 Am Montag 24 September 2007 schrieb Alan Stern:
  On Mon, 24 Sep 2007, Oliver Neukum wrote:
  
subsystem implements its own form of runtime PM support, then you'll be
able to use it properly.  But until then, there isn't anything much you
can do.
   
   Well, we can't simply cut power. Buffers must be flushed and the disk
   spun down. The suspend() method of the storage driver will have to become
   complex.
  
  That's right, if the device is a real disk.  If it's a flash memory
  device then of course these issues don't arise.  Unfortunately the
  driver isn't able to tell one from the other; the user will have to
  do that for us.
 
 So those devices which draw most power we don't suspend :-(
 Furthermore for many devices it will take real user intervention to
 determine what is in the enclosure. This isn't very good. I think
 we can do better.

If by we you mean the Linux kernel in general, then I agree.  If by
we you mean the USB developers, then I disagree.  Help is needed from
the SCSI people.

Alan Stern

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


Re: Some plans for scsi_cmnd

2007-09-25 Thread Boaz Harrosh
On Tue, Sep 25 2007 at 15:37 +0200, Matthew Wilcox [EMAIL PROTECTED] wrote:
 Christoph grabbed me on IRC and we debated some of my plans for scsi_cmnd;
 with his permission I'm summarising the result of that debate for posterity.
 There's four main things discussed:
 
 1. I'm going to be writing and posting patches over the next week or so
 to kill all the (ab)uses of -scsi_done in drivers.  Once that is done,
 I'll post a patch that exports the midlayer's scsi_done and switch all
 the drivers to calling that.  After that, I'll post another patch that
 changes the prototype *and the semantics* of queuecommand; the midlayer
 will no longer take the host_lock for the driver.  I'll just push the
 acquisition down into queuecommand, and it'll be up to individual driver
 authors to do something sensible with it then.
 
 2. Thanks to a thinko, we also discussed the upper-layer -done.  We think
 it should be feasible to move this from the scsi_cmnd to the scsi_device
 since sg doesn't use it.
 
 3. We also discussed scsi_pointer.  It's really quite crufty, and it
 gets recycled for storing all kinds of things.  The ambitious plan here
 is to change the whole way scsi_cmnds are allocated.  Code is clearer
 than my description ...
 
 sym2.c:
 
 struct sym2_cmnd {
   struct scsi_cmnd cmd;
   int Phase;
   char *data_in;
 }
 
 struct scsi_host_template sym2_template {
   .cmnd_size = sizeof(sym2_cmnd);
 }
 
 int sym2_queuecommand(struct scsi_cmnd *scp)
 {
   struct sym2_cmnd *cmnd = container_of(scp, sym2_cmnd, cmd);
 }
 
 The midlayer will create a slab pool per host template for allocating
 scsi_cmnd.
 
I have in my Q a small variation on this principle where I wanted
to allocate bigger commands for bidi-able hosts like iscsi_tcp. So
not to pay the extra allocation per command. Above will do just fine.

 As I said, it's ambitious.  But it'll let us get rid of scsi_pointer
 and host_scribble entirely.
 
This all is an excellent idea and you will find that in the patchset to
gdth, I have made the work of one driver a bit easier for you.

I suggest gradual work, where both Scp and host_scribble are intact
but the .cmnd_size and mechanics are available with few major drivers
moved to that. Than one kernel after that deprecating both, while 
converting lots of drivers, and 1-2 kernels after that remove them when
all drivers are converted. Don't sit on a large patchset with lots of
drivers and submit them all at once.

 4. We don't understand why sense_buffer is 96 bytes long.  mkp says that
 devices are coming which need more than 96 bytes (the standard allows up
 to 252).  I propose splitting the 96-byte buffer like so:
 
 -#define SCSI_SENSE_BUFFERSIZE  96
 -   unsigned char sense_buffer[SCSI_SENSE_BUFFERSIZE];
 +   unsigned char sense_buffer_head[8];
 +   unsigned char *sense_buffer_desc;
 
 and then adding:
 
 +int scsi_set_sense_buffer(struct scsi_cmnd *cmd, unsigned char *sense,
 +   int sense_len)
 +{
 +   int len = min(sense[7], sense_len - 8);
 +   memcpy(cmd-sense_buffer_head, sense, min(8, sense_len));
 +   if (len = 0)
 +   return 0;
 +   cmd-sense_buffer_desc = kmalloc(len, GFP_ATOMIC);
 +   if (!cmd-sense_buffer_desc)
 +   return 1;
 +   memcpy(cmd-sense_buffer_desc, sense + 8, len);
 +   return 0;
 +}
 +EXPORT_SYMBOL(scsi_set_sense_buffer);
 
 Drivers then do something like:
 
 -   memset(cmd-sense_buffer, 0, 
 sizeof(cmd-sense_buffer))
 -   memcpy(cmd-sense_buffer, cp-sns_bbuf,
 - min(sizeof(cmd-sense_buffer),
 - (size_t)SYM_SNS_BBUF_LEN));
 +   scsi_set_sense_buffer(cmd, cp-sns_bbuf,
 +   SYM_SNS_BBUF_LEN);
 
 or even ...
 
 -   /* Copy Sense Data into sense buffer. */
 -   memset(cp-sense_buffer, 0, sizeof(cp-sense_buffer));
 -
 if (!(scsi_status  SS_SENSE_LEN_VALID))
 break;
  
 -   if (sense_len = sizeof(cp-sense_buffer))
 -   sense_len = sizeof(cp-sense_buffer);
 -
 -   CMD_ACTUAL_SNSLEN(cp) = sense_len;
 -   sp-request_sense_length = sense_len;
 -   sp-request_sense_ptr = cp-sense_buffer;
 -
 -   if (sp-request_sense_length  32)
 -   sense_len = 32;
 -
 -   memcpy(cp-sense_buffer, sense_data, sense_len);
 -
 -   sp-request_sense_ptr += sense_len;
 -   sp-request_sense_length -= sense_len;
 -   if (sp-request_sense_length != 0)
 -   ha-status_srb = sp;
 +   scsi_set_sense_buffer(cp, sense_data, sense_len);
 
Please review my patches to scsi_error and the REQUEST_SENSE paths
(James are they not going to be accepted into 2.6.24-rcx?)
I have introduced 

Re: [linux-usb-devel] question on flushing buffers and spinning down disk

2007-09-25 Thread Alan Stern
On Tue, 25 Sep 2007, Oliver Neukum wrote:

 Am Montag 24 September 2007 schrieb Alan Stern:
  On Mon, 24 Sep 2007, Oliver Neukum wrote:
  
subsystem implements its own form of runtime PM support, then you'll be
able to use it properly.  But until then, there isn't anything much you
can do.
   
   Well, we can't simply cut power. Buffers must be flushed and the disk
   spun down. The suspend() method of the storage driver will have to become
   complex.
  
  That's right, if the device is a real disk.  If it's a flash memory
  device then of course these issues don't arise.  Unfortunately the
 
 With respect to buffers how sure are you about that?

Well, I'm not intimately familiar with the design of these chips.  But 
as long as the device remains powered, internally-buffered data 
shouldn't be lost.  It should be written out to flash storage when the 
device is resumed.

Are you worried about what might happen if the device is unplugged 
while it is still suspended?  Then yes, there is a risk of data loss.  
But that's also true if the device isn't suspended -- unplugging it 
without flushing its buffers could lose some data.

  driver isn't able to tell one from the other; the user will have to
  do that for us.
 
 Code for what we need is in sd_suspend(). The only trouble is locking.
 We need to make sure no commands that dirty buffers are issued after
 flushing the buffers.

No -- _we_ don't need to do that.  The sd driver needs to, by making 
sure that once the buffers have been flushed, any new commands will 
cause sd_resume() to be called.

That's what I meant when I said that sd has to support autoresume.

In addition, sd_suspend() and sd_resume() have to pass information up
the stack.  sd_suspend() has to let the SCSI core know that there is
one less active device on the bus, and when there are no active devices
left then the SCSI core has to call the LLD (in this case, usb-storage)  
to tell it the host adapter can be suspended.

Likewise, before sd_resume() runs it has to tell the SCSI core that the
bus needs to be active, and if the bus is suspended then the SCSI core
has to tell the LLD that the host adapter must be resumed.

The same reasoning applies to sr and any other high-level SCSI drivers
that might get written.  sg and SG_IO might present problems, since
they effectively are additional interfaces to the same devices already
managed by sd and sr -- they could be treated more or less the way we
treat access through usbfs.

Getting all this to work will be tricky, because sd is a block device
driven by requests issued through the block layer in atomic context,
whereas suspend and resume require a process context.  Probably the 
SCSI core would have to get involved in managing the synchronization.

Alan Stern

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


Re: [linux-usb-devel] question on flushing buffers and spinning down disk

2007-09-25 Thread Oliver Neukum
Am Dienstag 25 September 2007 schrieb Alan Stern:
  Furthermore for many devices it will take real user intervention to
  determine what is in the enclosure. This isn't very good. I think
  we can do better.
 
 If by we you mean the Linux kernel in general, then I agree.  If by
 we you mean the USB developers, then I disagree.  Help is needed from
 the SCSI people.

I am working on it. It seems to me that all necessary functions exist.

Regards
Oliver

PS: Whoever designed klists is suffering from a case of overengineeringosis.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Some plans for scsi_cmnd

2007-09-25 Thread Matthew Wilcox
On Tue, Sep 25, 2007 at 04:51:06PM +0200, Boaz Harrosh wrote:
  As I said, it's ambitious.  But it'll let us get rid of scsi_pointer
  and host_scribble entirely.
  
 This all is an excellent idea and you will find that in the patchset to
 gdth, I have made the work of one driver a bit easier for you.

;-)

 I suggest gradual work, where both Scp and host_scribble are intact
 but the .cmnd_size and mechanics are available with few major drivers
 moved to that. Than one kernel after that deprecating both, while 
 converting lots of drivers, and 1-2 kernels after that remove them when
 all drivers are converted. Don't sit on a large patchset with lots of
 drivers and submit them all at once.

Yup, incremental work is always good.

 Please review my patches to scsi_error and the REQUEST_SENSE paths
 (James are they not going to be accepted into 2.6.24-rcx?)
 I have introduced an abstract way to convert a command to point to
 it's sense buffer, So drivers can now transfer data to the sense buffer
 the way they do to regular IO, throw an sg_list.

Brilliant.  I'd only just started looking at the sense buffer issue, so
now I can stop entirely ;-)

 RFC patches early and set up a git tree, if possible. I will help in any way 
 I can also with drivers.

I was planning on just throwing patches at this list and seeing what
sticks.  You'll see a few from me today ... looks like item 2 on the
original mail (getting rid of -done) may be easier than we thought, and
lead to some really nice cleanups.  Of course, gdth is still the
sticking point for that ;-(

-- 
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] pluto: Don't abuse -done for internal commands

2007-09-25 Thread Matthew Wilcox
From: Matthew Wilcox [EMAIL PROTECTED]

We can simply call the internal done function directly

Signed-off-by: Matthew Wilcox [EMAIL PROTECTED]
---
 drivers/scsi/pluto.c |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/pluto.c b/drivers/scsi/pluto.c
index e221389..0363c1c 100644
--- a/drivers/scsi/pluto.c
+++ b/drivers/scsi/pluto.c
@@ -160,7 +160,6 @@ int __init pluto_detect(struct scsi_host_template *tpnt)

SCpnt-request-cmd_flags = ~REQ_STARTED;

-   SCpnt-done = pluto_detect_done;
SCpnt-request_bufflen = 256;
SCpnt-request_buffer = fcs[i].inquiry;
PLD((set up %d %08lx\n, i, (long)SCpnt))
@@ -195,7 +194,7 @@ int __init pluto_detect(struct scsi_host_template *tpnt)
SCpnt = (fcs[i].cmd);

/* Let FC mid-level free allocated resources */
-   SCpnt-done (SCpnt);
+   pluto_detect_scsi_done(SCpnt);

if (!SCpnt-result) {
struct pluto_inquiry *inq;
-- 
1.5.3.1

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


Remove -done from scsi_cmnd

2007-09-25 Thread Matthew Wilcox

This series of four patches gets rid of -done from scsi_cmnd.  It applies
on top of the patch I sent earlier today to improve gdth's abuse of
scsi_done.  Sorry, Boaz.  I'd like to thank Christoph for talking me
through some of the bits of the block layer I didn't grok.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Fix mistaken uses of -done

2007-09-25 Thread Matthew Wilcox
From: Matthew Wilcox [EMAIL PROTECTED]

All these drivers meant to call -scsi_done() but got confused.

Signed-off-by: Matthew Wilcox [EMAIL PROTECTED]
---
 drivers/scsi/NCR5380.c   |8 
 drivers/scsi/NCR53C9x.c  |2 +-
 drivers/scsi/atari_NCR5380.c |4 ++--
 drivers/scsi/sun3_NCR5380.c  |4 ++--
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index f8e449a..5b27966 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -2133,7 +2133,7 @@ static void NCR5380_information_transfer(struct Scsi_Host 
*instance) {
sink = 1;
do_abort(instance);
cmd-result = DID_ERROR  16;
-   cmd-done(cmd);
+   cmd-scsi_done(cmd);
return;
 #endif
/* 
@@ -2196,7 +2196,7 @@ static void NCR5380_information_transfer(struct Scsi_Host 
*instance) {
sink = 1;
do_abort(instance);
cmd-result = DID_ERROR  16;
-   cmd-done(cmd);
+   cmd-scsi_done(cmd);
/* XXX - need to source or sink 
data here, as appropriate */
} else
cmd-SCp.this_residual -= 
transfersize - len;
@@ -2740,7 +2740,7 @@ static int NCR5380_abort(Scsi_Cmnd * cmd) {
tmp-host_scribble = NULL;
tmp-result = DID_ABORT  16;
dprintk(NDEBUG_ABORT, (scsi%d : abort removed command 
from issue queue.\n, instance-host_no));
-   tmp-done(tmp);
+   tmp-scsi_done(tmp);
return SUCCESS;
}
 #if (NDEBUG   NDEBUG_ABORT)
@@ -2805,7 +2805,7 @@ static int NCR5380_abort(Scsi_Cmnd * cmd) {
*prev = (Scsi_Cmnd *) 
tmp-host_scribble;
tmp-host_scribble = NULL;
tmp-result = DID_ABORT  16;
-   tmp-done(tmp);
+   tmp-scsi_done(tmp);
return SUCCESS;
}
}
diff --git a/drivers/scsi/NCR53C9x.c b/drivers/scsi/NCR53C9x.c
index 79b4df1..96e8e29 100644
--- a/drivers/scsi/NCR53C9x.c
+++ b/drivers/scsi/NCR53C9x.c
@@ -1385,7 +1385,7 @@ int esp_abort(Scsi_Cmnd *SCptr)
this-host_scribble = NULL;
esp_release_dmabufs(esp, this);
this-result = DID_ABORT  16;
-   this-done(this);
+   this-scsi_done(this);
if(don)
esp-dma_ints_on(esp);
return SUCCESS;
diff --git a/drivers/scsi/atari_NCR5380.c b/drivers/scsi/atari_NCR5380.c
index 03dbe60..743df4c 100644
--- a/drivers/scsi/atari_NCR5380.c
+++ b/drivers/scsi/atari_NCR5380.c
@@ -2041,7 +2041,7 @@ static void NCR5380_information_transfer(struct Scsi_Host 
*instance)
sink = 1;
do_abort(instance);
cmd-result = DID_ERROR  16;
-   cmd-done(cmd);
+   cmd-scsi_done(cmd);
return;
 #endif
case PHASE_DATAIN:
@@ -2100,7 +2100,7 @@ static void NCR5380_information_transfer(struct Scsi_Host 
*instance)
sink = 1;
do_abort(instance);
cmd-result = DID_ERROR  16;
-   cmd-done(cmd);
+   cmd-scsi_done(cmd);
/* XXX - need to source or sink 
data here, as appropriate */
} else {
 #ifdef REAL_DMA
diff --git a/drivers/scsi/sun3_NCR5380.c b/drivers/scsi/sun3_NCR5380.c
index 98e3fe1..3769537 100644
--- a/drivers/scsi/sun3_NCR5380.c
+++ b/drivers/scsi/sun3_NCR5380.c
@@ -2055,7 +2055,7 @@ static void NCR5380_information_transfer (struct 
Scsi_Host *instance)
sink = 1;
do_abort(instance);
cmd-result = DID_ERROR   16;
-   cmd-done(cmd);
+   cmd-scsi_done(cmd);
return;
 #endif
case PHASE_DATAIN:
@@ -2115,7 +2115,7 @@ static void 

[PATCH] gdth: Stop abusing -done for internal commands

2007-09-25 Thread Matthew Wilcox
From: Matthew Wilcox [EMAIL PROTECTED]

The -done member was being used to mark commands as being internal.
I decided to put a magic number in -underflow instead.  I believe this
to be safe as no current user of -underflow has any of the bottom 9
bits set.

Signed-off-by: Matthew Wilcox [EMAIL PROTECTED]
---
 drivers/scsi/gdth.c  |   19 +++
 drivers/scsi/gdth_proc.c |4 ++--
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index 8a6a5f8..42a200f 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -691,6 +691,9 @@ static const struct file_operations gdth_fops = {
 .release = gdth_close,
 };
 
+#define GDTH_MAGIC 0xc2e7c389  /* I got it from /dev/urandom */
+#define IS_GDTH_INTERNAL_CMD(scp)  (scp-underflow == GDTH_MAGIC)
+
 #include gdth_proc.h
 #include gdth_proc.c
 
@@ -713,7 +716,7 @@ static void gdth_scsi_done(struct scsi_cmnd *scp)
 {
TRACE2((gdth_scsi_done()\n));
 
-   if (scp-done == gdth_scsi_done)
+   if (IS_GDTH_INTERNAL_CMD(scp))
complete((struct completion *)scp-request);
else 
scp-scsi_done(scp);
@@ -738,7 +741,7 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str 
*gdtcmd, char *cmnd,
 scp-cmd_len = 12;
 memcpy(scp-cmnd, cmnd, 12);
 scp-SCp.this_residual = IOCTL_PRI;   /* priority */
-scp-done = gdth_scsi_done;
+scp-underflow = GDTH_MAGIC;
 gdth_queuecommand(scp, NULL);
 wait_for_completion(wait);
 
@@ -2319,7 +2322,7 @@ static void gdth_putq(int hanum,Scsi_Cmnd *scp,unchar 
priority)
 ha = HADATA(gdth_ctr_tab[hanum]);
 spin_lock_irqsave(ha-smp_lock, flags);
 
-if (scp-done != gdth_scsi_done) {
+if (!IS_GDTH_INTERNAL_CMD(scp)) {
 scp-SCp.this_residual = (int)priority;
 b = virt_ctr ? NUMDATA(scp-device-host)-busnum:scp-device-channel;
 t = scp-device-id;
@@ -2382,7 +2385,7 @@ static void gdth_next(int hanum)
 for (nscp = pscp = ha-req_first; nscp; nscp = (Scsi_Cmnd *)nscp-SCp.ptr) 
{
 if (nscp != pscp  nscp != (Scsi_Cmnd *)pscp-SCp.ptr)
 pscp = (Scsi_Cmnd *)pscp-SCp.ptr;
-if (nscp-done != gdth_scsi_done) {
+if (!IS_GDTH_INTERNAL_CMD(nscp)) {
 b = virt_ctr ?
 NUMDATA(nscp-device-host)-busnum : nscp-device-channel;
 t = nscp-device-id;
@@ -2408,7 +2411,7 @@ static void gdth_next(int hanum)
 firsttime = FALSE;
 }
 
-if (nscp-done != gdth_scsi_done) {
+if (!IS_GDTH_INTERNAL_CMD(nscp)) {
 if (nscp-SCp.phase == -1) {
 nscp-SCp.phase = CACHESERVICE;   /* default: cache svc. 
*/ 
 if (nscp-cmnd[0] == TEST_UNIT_READY) {
@@ -2471,7 +2474,7 @@ static void gdth_next(int hanum)
 else
 gdth_scsi_done(nscp);
 }
-} else if (nscp-done == gdth_scsi_done) {
+} else if (IS_GDTH_INTERNAL_CMD(nscp)) {
 if (!(cmd_index=gdth_special_cmd(hanum,nscp)))
 this_cmd = FALSE;
 next_cmd = FALSE;
@@ -3812,7 +3815,7 @@ static int gdth_sync_event(int hanum,int service,unchar 
index,Scsi_Cmnd *scp)
 scp-sense_buffer[2] = NOT_READY;
 scp-result = (DID_OK  16) | (CHECK_CONDITION  1);
 }
-if (scp-done != gdth_scsi_done) {
+if (!IS_GDTH_INTERNAL_CMD(scp)) {
 ha-dvr.size = sizeof(ha-dvr.eu.sync);
 ha-dvr.eu.sync.ionode  = hanum;
 ha-dvr.eu.sync.service = service;
@@ -4910,7 +4913,7 @@ static int gdth_queuecommand(struct scsi_cmnd *scp,
 #endif
 
 priority = DEFAULT_PRI;
-if (scp-done == gdth_scsi_done)
+if (IS_GDTH_INTERNAL_CMD(scp))
 priority = scp-SCp.this_residual;
 else
 gdth_update_timeout(hanum, scp, scp-timeout_per_command * 6);
diff --git a/drivers/scsi/gdth_proc.c b/drivers/scsi/gdth_proc.c
index 32982eb..1f8d9a5 100644
--- a/drivers/scsi/gdth_proc.c
+++ b/drivers/scsi/gdth_proc.c
@@ -805,7 +805,7 @@ static void gdth_stop_timeout(int hanum, int busnum, int id)
 spin_lock_irqsave(ha-smp_lock, flags);
 
 for (scp = ha-req_first; scp; scp = (Scsi_Cmnd *)scp-SCp.ptr) {
-if (scp-done != gdth_scsi_done) {
+if (!IS_GDTH_INTERNAL_CMD(scp)) {
 b = virt_ctr ?
 NUMDATA(scp-device-host)-busnum : scp-device-channel;
 t = scp-device-id;
@@ -829,7 +829,7 @@ static void gdth_start_timeout(int hanum, int busnum, int 
id)
 spin_lock_irqsave(ha-smp_lock, flags);
 
 for (scp = ha-req_first; scp; scp = (Scsi_Cmnd *)scp-SCp.ptr) {
-if (scp-done != gdth_scsi_done) {
+if (!IS_GDTH_INTERNAL_CMD(scp)) {
 b = virt_ctr ?
 NUMDATA(scp-device-host)-busnum : scp-device-channel;
 t = scp-device-id;
-- 
1.5.3.1

-
To unsubscribe from this list: send the line 

[PATCH] Get rid of scsi_cmnd-done

2007-09-25 Thread Matthew Wilcox
From: Matthew Wilcox [EMAIL PROTECTED]

The ULD -done callback moves into the scsi_driver.  By moving the call
to scsi_io_completion() from scsi_blk_pc_done() to scsi_finish_command(),
we can eliminate the latter entirely.  By returning 'good_bytes' from
the -done callback (rather than invoking scsi_io_completion()), we can
stop exporting scsi_io_completion().

Also move the prototypes from sd.h to sd.c as they're all internal anyway.
Rename sd_rw_intr to sd_done and rw_intr to sr_done.

Inspired-by: Christoph Hellwig [EMAIL PROTECTED]
Signed-off-by: Matthew Wilcox [EMAIL PROTECTED]
---
 drivers/scsi/scsi.c|   20 +---
 drivers/scsi/scsi_error.c  |1 -
 drivers/scsi/scsi_lib.c|   14 --
 drivers/scsi/scsi_priv.h   |1 +
 drivers/scsi/sd.c  |   28 ++--
 drivers/scsi/sr.c  |   21 ++---
 include/scsi/scsi_cmnd.h   |2 --
 include/scsi/scsi_driver.h |1 +
 include/scsi/sd.h  |   13 -
 9 files changed, 43 insertions(+), 58 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index a5de1a8..60799ff 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -59,6 +59,7 @@
 #include scsi/scsi_cmnd.h
 #include scsi/scsi_dbg.h
 #include scsi/scsi_device.h
+#include scsi/scsi_driver.h
 #include scsi/scsi_eh.h
 #include scsi/scsi_host.h
 #include scsi/scsi_tcq.h
@@ -367,9 +368,8 @@ void scsi_log_send(struct scsi_cmnd *cmd)
scsi_print_command(cmd);
if (level  3) {
printk(KERN_INFO buffer = 0x%p, bufflen = %d,
-   done = 0x%p, queuecommand 0x%p\n,
+   queuecommand 0x%p\n,
scsi_sglist(cmd), scsi_bufflen(cmd),
-   cmd-done,
cmd-device-host-hostt-queuecommand);
 
}
@@ -658,6 +658,12 @@ void __scsi_done(struct scsi_cmnd *cmd)
blk_complete_request(rq);
 }
 
+/* Move this to a header if it becomes more generally useful */
+static struct scsi_driver *scsi_cmd_to_driver(struct scsi_cmnd *cmd)
+{
+   return *(struct scsi_driver **)cmd-request-rq_disk-private_data;
+}
+
 /*
  * Function:scsi_finish_command
  *
@@ -669,6 +675,8 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
 {
struct scsi_device *sdev = cmd-device;
struct Scsi_Host *shost = sdev-host;
+   struct scsi_driver *drv;
+   unsigned int good_bytes;
 
scsi_device_unbusy(sdev);
 
@@ -694,7 +702,13 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
Notifying upper driver of completion 
(result %x)\n, cmd-result));
 
-   cmd-done(cmd);
+   good_bytes = cmd-request_bufflen;
+if (cmd-request-cmd_type != REQ_TYPE_BLOCK_PC) {
+   drv = scsi_cmd_to_driver(cmd);
+   if (drv-done)
+   good_bytes = drv-done(cmd);
+   }
+   scsi_io_completion(cmd, good_bytes);
 }
 EXPORT_SYMBOL(scsi_finish_command);
 
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index c05d020..10f6acc 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1671,7 +1671,6 @@ scsi_reset_provider(struct scsi_device *dev, int flag)
memset(scmd-cmnd, '\0', sizeof(scmd-cmnd));
 
scmd-scsi_done = scsi_reset_provider_done_command;
-   scmd-done  = NULL;
scmd-request_buffer= NULL;
scmd-request_bufflen   = 0;
 
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 94d82cb..cb8a577 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -982,7 +982,6 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int 
good_bytes)
}
scsi_end_request(cmd, 0, this_count, !result);
 }
-EXPORT_SYMBOL(scsi_io_completion);
 
 /*
  * Function:scsi_init_io()
@@ -1063,18 +1062,6 @@ static struct scsi_cmnd *scsi_get_cmd_from_req(struct 
scsi_device *sdev,
return cmd;
 }
 
-static void scsi_blk_pc_done(struct scsi_cmnd *cmd)
-{
-   BUG_ON(!blk_pc_request(cmd-request));
-   /*
-* This will complete the whole command with uptodate=1 so
-* as far as the block layer is concerned the command completed
-* successfully. Since this is a REQ_BLOCK_PC command the
-* caller should check the request's errors value
-*/
-   scsi_io_completion(cmd, cmd-request_bufflen);
-}
-
 int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req)
 {
struct scsi_cmnd *cmd;
@@ -1124,7 +,6 @@ int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, 
struct request *req)
cmd-transfersize = req-data_len;
cmd-allowed = req-retries;
cmd-timeout_per_command = req-timeout;
-   cmd-done = 

Re: [PATCH] Get rid of scsi_cmnd-done

2007-09-25 Thread Boaz Harrosh
On Tue, Sep 25 2007 at 18:42 +0200, Matthew Wilcox [EMAIL PROTECTED] wrote:
 From: Matthew Wilcox [EMAIL PROTECTED]
 
 The ULD -done callback moves into the scsi_driver.  By moving the call
 to scsi_io_completion() from scsi_blk_pc_done() to scsi_finish_command(),
 we can eliminate the latter entirely.  By returning 'good_bytes' from
 the -done callback (rather than invoking scsi_io_completion()), we can
 stop exporting scsi_io_completion().
 
 Also move the prototypes from sd.h to sd.c as they're all internal anyway.
 Rename sd_rw_intr to sd_done and rw_intr to sr_done.
 
 Inspired-by: Christoph Hellwig [EMAIL PROTECTED]
 Signed-off-by: Matthew Wilcox [EMAIL PROTECTED]
 ---
  drivers/scsi/scsi.c|   20 +---
  drivers/scsi/scsi_error.c  |1 -
  drivers/scsi/scsi_lib.c|   14 --
  drivers/scsi/scsi_priv.h   |1 +
  drivers/scsi/sd.c  |   28 ++--
  drivers/scsi/sr.c  |   21 ++---
  include/scsi/scsi_cmnd.h   |2 --
  include/scsi/scsi_driver.h |1 +
  include/scsi/sd.h  |   13 -
  9 files changed, 43 insertions(+), 58 deletions(-)
 
 diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
 index a5de1a8..60799ff 100644
 --- a/drivers/scsi/scsi.c
 +++ b/drivers/scsi/scsi.c
 @@ -59,6 +59,7 @@
  #include scsi/scsi_cmnd.h
  #include scsi/scsi_dbg.h
  #include scsi/scsi_device.h
 +#include scsi/scsi_driver.h
  #include scsi/scsi_eh.h
  #include scsi/scsi_host.h
  #include scsi/scsi_tcq.h
 @@ -367,9 +368,8 @@ void scsi_log_send(struct scsi_cmnd *cmd)
   scsi_print_command(cmd);
   if (level  3) {
   printk(KERN_INFO buffer = 0x%p, bufflen = %d,
 - done = 0x%p, queuecommand 0x%p\n,
 + queuecommand 0x%p\n,
   scsi_sglist(cmd), scsi_bufflen(cmd),
 - cmd-done,
   cmd-device-host-hostt-queuecommand);
  
   }
 @@ -658,6 +658,12 @@ void __scsi_done(struct scsi_cmnd *cmd)
   blk_complete_request(rq);
  }
  
 +/* Move this to a header if it becomes more generally useful */
 +static struct scsi_driver *scsi_cmd_to_driver(struct scsi_cmnd *cmd)
 +{
 + return *(struct scsi_driver **)cmd-request-rq_disk-private_data;
 +}
 +
  /*
   * Function:scsi_finish_command
   *
 @@ -669,6 +675,8 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
  {
   struct scsi_device *sdev = cmd-device;
   struct Scsi_Host *shost = sdev-host;
 + struct scsi_driver *drv;
 + unsigned int good_bytes;
  
   scsi_device_unbusy(sdev);
  
 @@ -694,7 +702,13 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
   Notifying upper driver of completion 
   (result %x)\n, cmd-result));
  
 - cmd-done(cmd);
 + good_bytes = cmd-request_bufflen;
Please use scsi_bufflen(cmd) this file was already converted to
accessors.

 +if (cmd-request-cmd_type != REQ_TYPE_BLOCK_PC) {
 + drv = scsi_cmd_to_driver(cmd);
 + if (drv-done)
 + good_bytes = drv-done(cmd);
 + }
 + scsi_io_completion(cmd, good_bytes);
  }
  EXPORT_SYMBOL(scsi_finish_command);
  
 diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
 index c05d020..10f6acc 100644
 --- a/drivers/scsi/scsi_error.c
 +++ b/drivers/scsi/scsi_error.c
 @@ -1671,7 +1671,6 @@ scsi_reset_provider(struct scsi_device *dev, int flag)
   memset(scmd-cmnd, '\0', sizeof(scmd-cmnd));
  
   scmd-scsi_done = scsi_reset_provider_done_command;
 - scmd-done  = NULL;
   scmd-request_buffer= NULL;
   scmd-request_bufflen   = 0;
  
 diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
 index 94d82cb..cb8a577 100644
 --- a/drivers/scsi/scsi_lib.c
 +++ b/drivers/scsi/scsi_lib.c
 @@ -982,7 +982,6 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned 
 int good_bytes)
   }
   scsi_end_request(cmd, 0, this_count, !result);
  }
 -EXPORT_SYMBOL(scsi_io_completion);
  
  /*
   * Function:scsi_init_io()
 @@ -1063,18 +1062,6 @@ static struct scsi_cmnd *scsi_get_cmd_from_req(struct 
 scsi_device *sdev,
   return cmd;
  }
  
 -static void scsi_blk_pc_done(struct scsi_cmnd *cmd)
 -{
 - BUG_ON(!blk_pc_request(cmd-request));
 - /*
 -  * This will complete the whole command with uptodate=1 so
 -  * as far as the block layer is concerned the command completed
 -  * successfully. Since this is a REQ_BLOCK_PC command the
 -  * caller should check the request's errors value
 -  */
 - scsi_io_completion(cmd, cmd-request_bufflen);
 -}
 -
  int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req)
  {
   struct scsi_cmnd *cmd;
 @@ -1124,7 +,6 @@ int 

Re: [PATCH] Get rid of scsi_cmnd-done

2007-09-25 Thread Matthew Wilcox
On Tue, Sep 25, 2007 at 07:05:09PM +0200, Boaz Harrosh wrote:
  @@ -694,7 +702,13 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
  Notifying upper driver of completion 
  (result %x)\n, cmd-result));
   
  -   cmd-done(cmd);
  +   good_bytes = cmd-request_bufflen;
 Please use scsi_bufflen(cmd) this file was already converted to
 accessors.

Ah; I was basing this on linus + scsi-misc.  I guess scsi_lib hadn't
been converted because I just took the code from there.

 Thanks otherwise looks very good

Thank you for the review!

-- 
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] gdth: Stop abusing -done for internal commands

2007-09-25 Thread Boaz Harrosh
On Tue, Sep 25 2007 at 18:42 +0200, Matthew Wilcox [EMAIL PROTECTED] wrote:
 From: Matthew Wilcox [EMAIL PROTECTED]
 
 The -done member was being used to mark commands as being internal.
 I decided to put a magic number in -underflow instead.  I believe this
 to be safe as no current user of -underflow has any of the bottom 9
 bits set.
 
 Signed-off-by: Matthew Wilcox [EMAIL PROTECTED]
 ---
  drivers/scsi/gdth.c  |   19 +++
  drivers/scsi/gdth_proc.c |4 ++--
  2 files changed, 13 insertions(+), 10 deletions(-)
 
 diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
 index 8a6a5f8..42a200f 100644
 --- a/drivers/scsi/gdth.c
 +++ b/drivers/scsi/gdth.c
 @@ -691,6 +691,9 @@ static const struct file_operations gdth_fops = {
  .release = gdth_close,
  };
  
 +#define GDTH_MAGIC   0xc2e7c389  /* I got it from /dev/urandom */
 +#define IS_GDTH_INTERNAL_CMD(scp)(scp-underflow == GDTH_MAGIC)
 +
  #include gdth_proc.h
  #include gdth_proc.c
  
 @@ -713,7 +716,7 @@ static void gdth_scsi_done(struct scsi_cmnd *scp)
  {
   TRACE2((gdth_scsi_done()\n));
  
 - if (scp-done == gdth_scsi_done)
 + if (IS_GDTH_INTERNAL_CMD(scp))
   complete((struct completion *)scp-request);
   else 
   scp-scsi_done(scp);
 @@ -738,7 +741,7 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str 
 *gdtcmd, char *cmnd,
  scp-cmd_len = 12;
  memcpy(scp-cmnd, cmnd, 12);
  scp-SCp.this_residual = IOCTL_PRI;   /* priority */
 -scp-done = gdth_scsi_done;
 +scp-underflow = GDTH_MAGIC;
  gdth_queuecommand(scp, NULL);
  wait_for_completion(wait);
  
 @@ -2319,7 +2322,7 @@ static void gdth_putq(int hanum,Scsi_Cmnd *scp,unchar 
 priority)
  ha = HADATA(gdth_ctr_tab[hanum]);
  spin_lock_irqsave(ha-smp_lock, flags);
  
 -if (scp-done != gdth_scsi_done) {
 +if (!IS_GDTH_INTERNAL_CMD(scp)) {
  scp-SCp.this_residual = (int)priority;
  b = virt_ctr ? 
 NUMDATA(scp-device-host)-busnum:scp-device-channel;
  t = scp-device-id;
 @@ -2382,7 +2385,7 @@ static void gdth_next(int hanum)
  for (nscp = pscp = ha-req_first; nscp; nscp = (Scsi_Cmnd 
 *)nscp-SCp.ptr) {
  if (nscp != pscp  nscp != (Scsi_Cmnd *)pscp-SCp.ptr)
  pscp = (Scsi_Cmnd *)pscp-SCp.ptr;
 -if (nscp-done != gdth_scsi_done) {
 +if (!IS_GDTH_INTERNAL_CMD(nscp)) {
  b = virt_ctr ?
  NUMDATA(nscp-device-host)-busnum : nscp-device-channel;
  t = nscp-device-id;
 @@ -2408,7 +2411,7 @@ static void gdth_next(int hanum)
  firsttime = FALSE;
  }
  
 -if (nscp-done != gdth_scsi_done) {
 +if (!IS_GDTH_INTERNAL_CMD(nscp)) {
  if (nscp-SCp.phase == -1) {
  nscp-SCp.phase = CACHESERVICE;   /* default: cache svc. 
 */ 
  if (nscp-cmnd[0] == TEST_UNIT_READY) {
 @@ -2471,7 +2474,7 @@ static void gdth_next(int hanum)
  else
  gdth_scsi_done(nscp);
  }
 -} else if (nscp-done == gdth_scsi_done) {
 +} else if (IS_GDTH_INTERNAL_CMD(nscp)) {
  if (!(cmd_index=gdth_special_cmd(hanum,nscp)))
  this_cmd = FALSE;
  next_cmd = FALSE;
 @@ -3812,7 +3815,7 @@ static int gdth_sync_event(int hanum,int service,unchar 
 index,Scsi_Cmnd *scp)
  scp-sense_buffer[2] = NOT_READY;
  scp-result = (DID_OK  16) | (CHECK_CONDITION  1);
  }
 -if (scp-done != gdth_scsi_done) {
 +if (!IS_GDTH_INTERNAL_CMD(scp)) {
  ha-dvr.size = sizeof(ha-dvr.eu.sync);
  ha-dvr.eu.sync.ionode  = hanum;
  ha-dvr.eu.sync.service = service;
 @@ -4910,7 +4913,7 @@ static int gdth_queuecommand(struct scsi_cmnd *scp,
  #endif
  
  priority = DEFAULT_PRI;
 -if (scp-done == gdth_scsi_done)
 +if (IS_GDTH_INTERNAL_CMD(scp))
  priority = scp-SCp.this_residual;
  else
  gdth_update_timeout(hanum, scp, scp-timeout_per_command * 6);
 diff --git a/drivers/scsi/gdth_proc.c b/drivers/scsi/gdth_proc.c
 index 32982eb..1f8d9a5 100644
 --- a/drivers/scsi/gdth_proc.c
 +++ b/drivers/scsi/gdth_proc.c
 @@ -805,7 +805,7 @@ static void gdth_stop_timeout(int hanum, int busnum, int 
 id)
  spin_lock_irqsave(ha-smp_lock, flags);
  
  for (scp = ha-req_first; scp; scp = (Scsi_Cmnd *)scp-SCp.ptr) {
 -if (scp-done != gdth_scsi_done) {
 +if (!IS_GDTH_INTERNAL_CMD(scp)) {
  b = virt_ctr ?
  NUMDATA(scp-device-host)-busnum : scp-device-channel;
  t = scp-device-id;
 @@ -829,7 +829,7 @@ static void gdth_start_timeout(int hanum, int busnum, int 
 id)
  spin_lock_irqsave(ha-smp_lock, flags);
  
  for (scp = ha-req_first; scp; scp = (Scsi_Cmnd *)scp-SCp.ptr) {
 -if (scp-done != gdth_scsi_done) {
 +if (!IS_GDTH_INTERNAL_CMD(scp)) {

RE: [PATCH 6/9] mpt fusion: error recovery improvements, andsynchronizing internal commands

2007-09-25 Thread James Bottomley
On Mon, 2007-09-24 at 19:26 -0600, Moore, Eric wrote:
 On Saturday, September 22, 2007 10:23 AM,  James Bottomley wrote:
 
  
  OK, I thought I'd wait here for the breakout, but in the meantime I
  tried to compile the first five patches, but they don't:
  
CC [M]  drivers/message/fusion/mptscsih.o
  drivers/message/fusion/mptscsih.c: In function 'mptscsih_qcmd':
  drivers/message/fusion/mptscsih.c:1357: error: 'MPT_SCSI_HOST' has no
  member named 'resetPending'
  drivers/message/fusion/mptscsih.c: In function 'mptscsih_TMHandler':
  drivers/message/fusion/mptscsih.c:1554: error: 'MPT_ADAPTER' has no
  member named 'diagLock'
  ...
 
 Its not going to compile if you didn't pull down all 6 patchs associated
 to error recovery improvements.   Those six patchs were divided by
 sources.  Meaning patch 4 was mptbase.[h,c] changes, then patch5 was
 mptctl.[c,h] changes, and so on.  So the changes assoicated for
 mptscsich.c are in patch 8.   Since you didn't apply the 8th patch,
 offcourse your going to get the errrors in mptscish.c.All patchs 4-9
 need to go togeather, as mentioned in the first patch 0.

If that's the case, and they can't be split up into logically separate
sub patches (each of which individually compiles), then they'll have to
go as a single patch.

  
  The series can't be applied in this form, because if git bisect steps
  into the middle of this, the compile will break (and hence the
  bisection) and a lot of people will say a lot of nasty things.
 
  
  A patch series really needs to be one patch per logical change (with
  other peoples' patches broken out) in a form that is separately
  compilable for each patch.  Additionally, with a separate 
  change log and
  summary (i.e. not four patches all saying mpt fusion: error recovery
  improvements, and synchronizing internal commands).
 
 
 I understand, and thats been going on for the past couple months,
 starting with Sathya's patchs early August.   The changes associated
 with the rewrite of internal commands, and errror recovery is rather
 difficult to seperate due to many depandancies and new structure
 changes, and my concerned risk of what small steps could introduce more
 issues, so IMO, the least risk was jump starting you to the customer and
 factory tested code. 
  
  I'll back all of this out; can you resend the series conforming to the
  above request?
  
 
 The first three patchs did conform that, which are:
 
 1) adding usage of shost_priv and removing all the typecasting
 2) Fixing sparse warnings
 3) locking down ScsiLookup

OK ... I'll look at applying those.

 Youve rejected the error recovery patchs, which is fair enough.  

Just the separation ... the actual patch looks OK.

James


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


Re: [PATCH] gdth: Stop abusing -done for internal commands

2007-09-25 Thread Matthew Wilcox
On Tue, Sep 25, 2007 at 07:30:29PM +0200, Boaz Harrosh wrote:
 OK This is on top of the first patch that I will take care of,
 right? (http://www.spinics.net/lists/linux-scsi/msg19470.html)

That's right.

 Here 2 it will not merge with my patchset. So I'll take it off your hands
 and rebase it to mine.

Wonderful.  Again, I don't care about this driver, just want it to not
break (more than it's already broken).

 In my patchset I have introduced a gdth_cmnd_priv. That is associated
 with every command and is put on cmnd-host_scribble, I than moved lots
 of abused Scp and other members to that new structure. I also put a 
 simple boolean that states that these are Internal commands. So what
 is left of this patch is the:
 -scp-done = gdth_scsi_done;
 I will add that one line removal to your previous patch.

Excellent.  Makes perfect sense.

  Ah; I was basing this on linus + scsi-misc.  I guess scsi_lib hadn't
  been converted because I just took the code from there.
 Yes scsi_lib was not converted, but scsi.c was.

OK.  I've converted that in my tree now, and fixed a small whitespace
error I introduced.  I'll wait for more comments before posting a
replacement patch.

-- 
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 4/9] mpt fusion: error recovery improvements, andsynchronizing internal commands

2007-09-25 Thread James Bottomley
On Mon, 2007-09-24 at 19:26 -0600, Moore, Eric wrote:
 On Saturday, September 22, 2007 9:39 AM,  James Bottomley wrote:
  
  Well, I'll put this in this time.  However, it contains a 
  whole slew of
  pointless changes like this:
  
  
   -   mdelay (10);
   +   udelay (1);
  
  and
  
   -   mdelay(1);
   +   udelay(1000);
  
  Which is going to excite the janitors into a frenzy of replace udelay
  with mdelay patches, which I can well do without ... please don't do
  this type of change unless there's some actual reason for it.
  
 
 I recall the reason for this change.  I found that medlay called during
 interrupt context didn't work well, whereas udelay did.  Oringally when
 mpt_fault_reset_work was added, this code was called using timers, which
 as you know, is called as part of softirq bottom half handler.  Since
 then, we converted mpt_fault_reset_work to being called using user
 context work task, so really its a non-issue I guess.

That shouldn't have happened ... if you look (include/linux/delay.h)
mdelay is implemented in terms of udelay, so the behaviour should be the
same.

There is a technical difference, though.  Because udelay is busy waiting
in a calculated processor loop, it can overflow for large values (and
large is defined to be anything  1000) mdelay() is careful to call
udelay multiple times to avoid the potential overflow.

James



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


RE: [PATCH 6/9] mpt fusion: error recovery improvements, andsynchronizing internal commands

2007-09-25 Thread James Bottomley
On Mon, 2007-09-24 at 19:35 -0600, Moore, Eric wrote:
 On Saturday, September 22, 2007 10:02 AM, James Bottomley  wrote: 
 
  What fixes?
  
  The object of a change log is to preserve the history of a particular
  change (as per the Developer Certificate of Origin).  This 
  means if you
  get a patch from someone, you should also collect their 
  signoff with it,
  then you pass it on to me complete with your signoff added (and a note
  if you had to modify it to fit it into the patch series or otherwise
  alter it).
  
  James
  
  
  
 
 James, Mike sent me an email back on June 13, contain 11 suggested
 changes associated with his test efforts with my new code having rewrote
 internal command error handling, and adding new polling function for
 controller fault handling.   The changes he quoted associated to the
 ones in mptfc seem vague.  I have the email I could forward, but at that
 time, It probably doesnt matter.

I just need a description that says what the patch is doing.  Fixes
provided by SGI isn't descriptive enough.

James


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


RE: [PATCH 6/9] mpt fusion: error recovery improvements,andsynchronizing internal commands

2007-09-25 Thread Moore, Eric
On Tuesday, September 25, 2007 11:32 AM,  James Bottomley  wrote:
 
  Youve rejected the error recovery patchs, which is fair enough.  
 
 Just the separation ... the actual patch looks OK.
 

I'll will continue working to separate the error recovery
improvements: into smaller feature add, but will take some time.   I
want some constructive feedback, and too big of patch is deterring some
people from looking at it.

Thanks,
Eric 
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/9] mpt fusion: error recovery improvements,andsynchronizing internal commands

2007-09-25 Thread Jeff Garzik

Moore, Eric wrote:

On Tuesday, September 25, 2007 11:32 AM,  James Bottomley  wrote:
 
Youve rejected the error recovery patchs, which is fair enough.  

Just the separation ... the actual patch looks OK.



I'll will continue working to separate the error recovery
improvements: into smaller feature add, but will take some time.   I
want some constructive feedback, and too big of patch is deterring some
people from looking at it.


I think it's fair to break it up, as long as its clearly noted that the 
patch is for review only.


Jeff



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


Re: [PATCH 6/9] mpt fusion: error recovery improvements,andsynchronizing internal commands

2007-09-25 Thread Michael Reed

Jeff Garzik wrote:
 Moore, Eric wrote:
 On Tuesday, September 25, 2007 11:32 AM,  James Bottomley  wrote:
  
 Youve rejected the error recovery patchs, which is fair enough.  
 Just the separation ... the actual patch looks OK.


 I'll will continue working to separate the error recovery
 improvements: into smaller feature add, but will take some time.   I
 want some constructive feedback, and too big of patch is deterring some
 people from looking at it.
 
 I think it's fair to break it up, as long as its clearly noted that the
 patch is for review only.

Any chance the changes, properly documented, could be accepted as
one big patch?  Breaking this code into smaller patches, when it's
really intended as a driver update, could prove problematic.

Mike


 
 Jeff
 
 
 
 -
 To unsubscribe from this list: send the line unsubscribe linux-scsi in
 the body of a message to [EMAIL PROTECTED]
 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 [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/9] mpt fusion: error recovery improvements,andsynchronizing internal commands

2007-09-25 Thread James Bottomley
On Tue, 2007-09-25 at 17:38 -0500, Michael Reed wrote:
 Jeff Garzik wrote:
  Moore, Eric wrote:
  On Tuesday, September 25, 2007 11:32 AM,  James Bottomley  wrote:
   
  Youve rejected the error recovery patchs, which is fair enough.  
  Just the separation ... the actual patch looks OK.
 
 
  I'll will continue working to separate the error recovery
  improvements: into smaller feature add, but will take some time.   I
  want some constructive feedback, and too big of patch is deterring some
  people from looking at it.
  
  I think it's fair to break it up, as long as its clearly noted that the
  patch is for review only.
 
 Any chance the changes, properly documented, could be accepted as
 one big patch?  Breaking this code into smaller patches, when it's
 really intended as a driver update, could prove problematic.

Yes, that's OK ... separate patches with signoffs are not absolute
requirements as long as the description and the heritage is in the
change log (although if you can do it, I prefer the separate signoffs
and distinct patches).

James


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


Re: [PATCH 6/9] mpt fusion: error recovery improvements,andsynchronizing internal commands

2007-09-25 Thread Jeff Garzik

Michael Reed wrote:

Jeff Garzik wrote:

Moore, Eric wrote:

On Tuesday, September 25, 2007 11:32 AM,  James Bottomley  wrote:
 
Youve rejected the error recovery patchs, which is fair enough.  

Just the separation ... the actual patch looks OK.


I'll will continue working to separate the error recovery
improvements: into smaller feature add, but will take some time.   I
want some constructive feedback, and too big of patch is deterring some
people from looking at it.

I think it's fair to break it up, as long as its clearly noted that the
patch is for review only.


Any chance the changes, properly documented, could be accepted as
one big patch?  Breaking this code into smaller patches, when it's
really intended as a driver update, could prove problematic.


I'll let James provide an authoritative answer, but in general, that's a 
bad way to do Linux engineering.  The worst case would be appearing 
every month or two, posted a single, huge driver update patch.  That 
set of changes is bi-sectable, but it does not fully collect the changes 
within the driver update.


Some consolidation of changes is just plain natural, so you strike a 
balance.  Nothing but big driver-update patches?  Bad.  Four bazillion 
individual patches, each for a single whitespace change, cleanup, etc.? 
 Also unwise.  The true balance is somewhere in the middle.


The more important or intrusive the change, within the overall driver 
update, the more important it is to separate out that logical change.


Jeff



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


queued patches for SCSI for 2.6.24

2007-09-25 Thread James Bottomley
Andrew asked that I provide a status report of pending updates.  The
list is attached below.  It's pretty much driver updates and minor bug
fixes.  The main functionality changes are Kay's sysfs updates and the
shift of the ULD attachement towards the block prep function.

James

---


Adrian Bunk (2):
  make scsi_decode_sense_buffer and scsi_decode_sense_extras static
  scsi_error.c should #include scsi_transport_api.h

Alan Cox (3):
  dtc: Fix typo
  eata_pio: Clean up proc handling, bracketing and use cpu_relax()
  dtc: clean up indent damage and add printk levels

Andrew Morton (3):
  ips: warning fix
  aacraid: rename check_reset
  bsg: declare structures for the non BSG case

Andrew Vasquez (15):
  qla2xxx: Update version number to 8.02.00-k4.
  qla2xxx: Limit iIDMA speed adjustments.
  qla2xxx: Rework MSI-X handlers.
  qla2xxx: Clear options-flags while staging firmware-execution.
  qla2xxx: Sparse cleanups in qla_mid.c
  qla2xxx: Cleanup several 'sparse' warnings.
  qla2xxx: Use shost_priv().
  qla2xxx: Remove unused member (list) from srb_t structure.
  qla2xxx: Use the correct pointer-address during NVRAM writes.
  qla2xxx: Set correct attribute count during FDMI RPA.
  qla2xxx: Query additional RISC registers during ISP25XX firmware dump.
  qla2xxx: Correct staging of RISC while attempting to pause.
  qla2xxx: Query additional RISC information during a pause.
  qla2xxx: Add flash burst-read/write support.
  qla2xxx: Collapse and simplify ISP2XXX firmware dump routines.

Bartlomiej Zolnierkiewicz (1):
  MAINTAINERS: mark ide-scsi as Orphan

Bernhard Walle (1):
  ips: Update version information

Boaz Harrosh (2):
  ide-scsi.: convert to data accessors and !use_sg cleanup
  microtek: use data accessors and !use_sg cleanup

Christof Schmitt (5):
  zfcp: Enable debug feature before setting adapter online
  scsi_transport_fc: Introduce disable_target_scan flag
  zfcp: Remove braces for only one statement
  zfcp: Remove unnecessary assignment
  zfcp: correct indentation for nested if-else

David Miller (1):
  esp: fix instance numbering.

David Woodhouse (1):
  Fix ibmvscsi client for multiplatform iSeries+pSeries kernel

Eric Moore (10):
  MAINTAINERS : mpt fusion mailing list change
  mpt fusion: bump version to 3.04.06
  mpt fusion: Kconfig cleanup
  mpt fusion: removing Dell copyright
  mpt fusion: removing references to hd-ioc
  mpt fusion: rename vdev to vdevice
  mpt fusion: adding/removing white space
  mpt fusion: standardize printks and debug info
  mpt fusion: Add support for ATTO 4LD: Rebranded LSI 53C1030
  Addition to pci_ids.h for ATTO Technology, Inc.

FUJITA Tomonori (17):
  srp_transport: convert to use supported_mode attribute
  fc_transport: add target driver support
  add supported_mode and active_mode attributes to the host
  tgt: fix can_queue bug
  fc4: convert to use the data buffer accessors
  sg: increase sglist_len of the sg_scatter_hold structure
  ps3rom: convert to use the data buffer accessors
  scsi_transport_srp: remove tgt dependencies
  tgt: convert ibmvstgt to use transport tsk_mgmt_response callback
  tgt: move tsk_mgmt_response callback to transport class
  tgt: convert libsrp and ibmvstgt to use srp_transport
  srp_transport: add target driver support
  tgt: add I_T nexus support
  transport_srp: add rport roles attribute
  ib_srp: convert to use the srp transport class
  ibmvscsi: convert to use the srp transport class
  add srp transport class

Gabriel C (1):
  NCR5380: fix NCR53C400_PSEUDO_DMA is not defined

Gilbert Wu (1):
  aic94xx: Add new PCI ID for ASC58300

Heiko Carstens (3):
  zfcp: avoid if (whatever) ; constructs.
  zfcp: allocate gid_pn_data objects from gid_pn_cache.
  zfcp: fix memory leak.

HighPoint Linux Team (1):
  hptiop: adding new firmware interface and more PCI device IDs

James Bottomley (3):
  sg: use idr to replace static arrays
  move ULD attachment into the prep function
  arcmsr: fix compile problems

Jan Engelhardt (1):
  mpt fusion: Use menuconfig objects

Jeff Garzik (2):
  arcmsr: irq handler fixes, cleanups, micro-opts
  arcmsr: Fix hardware wait loops

Jesper Juhl (3):
  mpt fusion: fix two potential mem leaks
  NCR_D700, lpfc: Clean up duplicate includes
  lpfc: fix potential overflow of hbqs array

Joe Carnuccio (1):
  qla2xxx: Allow region-based flash-part accesses.

Kay Sievers (1):
  switch sdev sysfs attributes to default attributes

Mariusz Kozlowski (3):
  mpt fusion: remove redundant memset
  mpt fusion: mostly kmalloc + memset conversion to kzalloc
  kmalloc + memset conversion to kzalloc

Masatake YAMATO (1):
  Fix signness of parameters in scsi module

Matthew Wilcox (49):
  ips: Close 

Re: queued patches for SCSI for 2.6.24

2007-09-25 Thread David Miller
From: James Bottomley [EMAIL PROTECTED]
Date: Tue, 25 Sep 2007 20:00:02 -0500

 David Miller (1):
   esp: fix instance numbering.

I'd like to request that this one goes into 2.6.23 as
it is a bug fix and the bug confuses users.

Thanks.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: queued patches for SCSI for 2.6.24

2007-09-25 Thread Boaz Harrosh
On Wed, Sep 26 2007 at 3:00 +0200, James Bottomley [EMAIL PROTECTED] wrote:
 Andrew asked that I provide a status report of pending updates.  The
 list is attached below.  It's pretty much driver updates and minor bug
 fixes.  The main functionality changes are Kay's sysfs updates and the
 shift of the ULD attachement towards the block prep function.
 
 James
 
 ---
 
 
 Adrian Bunk (2):
   make scsi_decode_sense_buffer and scsi_decode_sense_extras static
   scsi_error.c should #include scsi_transport_api.h
 
 Alan Cox (3):
   dtc: Fix typo
   eata_pio: Clean up proc handling, bracketing and use cpu_relax()
   dtc: clean up indent damage and add printk levels
 
 Andrew Morton (3):
   ips: warning fix
   aacraid: rename check_reset
   bsg: declare structures for the non BSG case
 
 Andrew Vasquez (15):
   qla2xxx: Update version number to 8.02.00-k4.
   qla2xxx: Limit iIDMA speed adjustments.
   qla2xxx: Rework MSI-X handlers.
   qla2xxx: Clear options-flags while staging firmware-execution.
   qla2xxx: Sparse cleanups in qla_mid.c
   qla2xxx: Cleanup several 'sparse' warnings.
   qla2xxx: Use shost_priv().
   qla2xxx: Remove unused member (list) from srb_t structure.
   qla2xxx: Use the correct pointer-address during NVRAM writes.
   qla2xxx: Set correct attribute count during FDMI RPA.
   qla2xxx: Query additional RISC registers during ISP25XX firmware dump.
   qla2xxx: Correct staging of RISC while attempting to pause.
   qla2xxx: Query additional RISC information during a pause.
   qla2xxx: Add flash burst-read/write support.
   qla2xxx: Collapse and simplify ISP2XXX firmware dump routines.
 
 Bartlomiej Zolnierkiewicz (1):
   MAINTAINERS: mark ide-scsi as Orphan
 
 Bernhard Walle (1):
   ips: Update version information
 
 Boaz Harrosh (2):
   ide-scsi.: convert to data accessors and !use_sg cleanup
   microtek: use data accessors and !use_sg cleanup
 
 Christof Schmitt (5):
   zfcp: Enable debug feature before setting adapter online
   scsi_transport_fc: Introduce disable_target_scan flag
   zfcp: Remove braces for only one statement
   zfcp: Remove unnecessary assignment
   zfcp: correct indentation for nested if-else
 
 David Miller (1):
   esp: fix instance numbering.
 
 David Woodhouse (1):
   Fix ibmvscsi client for multiplatform iSeries+pSeries kernel
 
 Eric Moore (10):
   MAINTAINERS : mpt fusion mailing list change
   mpt fusion: bump version to 3.04.06
   mpt fusion: Kconfig cleanup
   mpt fusion: removing Dell copyright
   mpt fusion: removing references to hd-ioc
   mpt fusion: rename vdev to vdevice
   mpt fusion: adding/removing white space
   mpt fusion: standardize printks and debug info
   mpt fusion: Add support for ATTO 4LD: Rebranded LSI 53C1030
   Addition to pci_ids.h for ATTO Technology, Inc.
 
 FUJITA Tomonori (17):
   srp_transport: convert to use supported_mode attribute
   fc_transport: add target driver support
   add supported_mode and active_mode attributes to the host
   tgt: fix can_queue bug
   fc4: convert to use the data buffer accessors
   sg: increase sglist_len of the sg_scatter_hold structure
   ps3rom: convert to use the data buffer accessors
   scsi_transport_srp: remove tgt dependencies
   tgt: convert ibmvstgt to use transport tsk_mgmt_response callback
   tgt: move tsk_mgmt_response callback to transport class
   tgt: convert libsrp and ibmvstgt to use srp_transport
   srp_transport: add target driver support
   tgt: add I_T nexus support
   transport_srp: add rport roles attribute
   ib_srp: convert to use the srp transport class
   ibmvscsi: convert to use the srp transport class
   add srp transport class
 
 Gabriel C (1):
   NCR5380: fix NCR53C400_PSEUDO_DMA is not defined
 
 Gilbert Wu (1):
   aic94xx: Add new PCI ID for ASC58300
 
 Heiko Carstens (3):
   zfcp: avoid if (whatever) ; constructs.
   zfcp: allocate gid_pn_data objects from gid_pn_cache.
   zfcp: fix memory leak.
 
 HighPoint Linux Team (1):
   hptiop: adding new firmware interface and more PCI device IDs
 
 James Bottomley (3):
   sg: use idr to replace static arrays
   move ULD attachment into the prep function
   arcmsr: fix compile problems
 
 Jan Engelhardt (1):
   mpt fusion: Use menuconfig objects
 
 Jeff Garzik (2):
   arcmsr: irq handler fixes, cleanups, micro-opts
   arcmsr: Fix hardware wait loops
 
 Jesper Juhl (3):
   mpt fusion: fix two potential mem leaks
   NCR_D700, lpfc: Clean up duplicate includes
   lpfc: fix potential overflow of hbqs array
 
 Joe Carnuccio (1):
   qla2xxx: Allow region-based flash-part accesses.
 
 Kay Sievers (1):
   switch sdev sysfs attributes to default attributes
 
 Mariusz Kozlowski (3):
   mpt fusion: remove redundant memset
   mpt 

Re: queued patches for SCSI for 2.6.24

2007-09-25 Thread James Bottomley
On Wed, 2007-09-26 at 10:28 +0900, FUJITA Tomonori wrote:
 On Tue, 25 Sep 2007 20:00:02 -0500
 James Bottomley [EMAIL PROTECTED] wrote:
 
  Andrew asked that I provide a status report of pending updates.  The
  list is attached below.  It's pretty much driver updates and minor bug
  fixes.  The main functionality changes are Kay's sysfs updates and the
  shift of the ULD attachement towards the block prep function.
 
 Can we make new 'supporrted_mode' and 'active_mode' attributes look
 better?
 
 http://marc.info/?l=linux-scsim=118991196128245w=2

Sure, but Jeff's suggestion was a good one to avoid me having to change
hundreds of files.  Could you roll it up and resubmit?

James


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


Re: [GIT PATCH] SCSI bug fix for 2.6.23-rc8

2007-09-25 Thread James Bottomley
On Tue, 2007-09-25 at 12:55 -0500, James Bottomley wrote:
 This is, unfortunately, not a recent regression but it's only been
 recently diagnosed.  Apparently the SCSI Parallel transport class domain
 validation cable width detection wasn't working, leading to cases where
 controllers with damaged cables would end up hanging the system (the
 reported one was an aic79xx controller, but the potential is there for
 all SPI based systems).  This bug would *only* affect systems whose
 cable integrity or connectors were compromised, so it isn't life
 threatening to every SCSI Parallel installation, but the consequence of
 running into it is a system hang.
 
 The fix is available here:
 
 master.kernel.org:/pub/scm/linux/kernel/git/jejb/scsi-rc-fixes-2.6.git
 
 The short changelog is:
 
 James Bottomley (1):
   scsi_transport_spi: fix domain validation failure from incorrect
 width setting
 
 And the diffstat:
 
  scsi_transport_spi.c |   28 ++--
  1 file changed, 22 insertions(+), 6 deletions(-)

We've had another late arriving bug fix:

commit ff4abd6cfacf0bb23a077f615d3a5cd17359db1b
Author: David Miller [EMAIL PROTECTED]
Date:   Fri Aug 24 22:25:58 2007 -0700

[SCSI] esp: fix instance numbering.

Because the -unique_id is set too late, the ESP scsi host
instance numbers in the kernel log during probing are
wrong.

Bug reported by Meelis Roos.

Signed-off-by: David S. Miller [EMAIL PROTECTED]
Signed-off-by: James Bottomley [EMAIL PROTECTED]

Which I've added.  The diffstat is now:

 esp_scsi.c   |3 ++-
 scsi_transport_spi.c |   28 ++--
 2 files changed, 24 insertions(+), 7 deletions(-)

James


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


Re: queued patches for SCSI for 2.6.24

2007-09-25 Thread FUJITA Tomonori
On Tue, 25 Sep 2007 20:42:35 -0500
James Bottomley [EMAIL PROTECTED] wrote:

 On Wed, 2007-09-26 at 10:28 +0900, FUJITA Tomonori wrote:
  On Tue, 25 Sep 2007 20:00:02 -0500
  James Bottomley [EMAIL PROTECTED] wrote:
  
   Andrew asked that I provide a status report of pending updates.  The
   list is attached below.  It's pretty much driver updates and minor bug
   fixes.  The main functionality changes are Kay's sysfs updates and the
   shift of the ULD attachement towards the block prep function.
  
  Can we make new 'supporrted_mode' and 'active_mode' attributes look
  better?
  
  http://marc.info/?l=linux-scsim=118991196128245w=2
 
 Sure, but Jeff's suggestion was a good one to avoid me having to change
 hundreds of files.  Could you roll it up and resubmit?

This can be cleanly applied to scsi-misc.

-
From: FUJITA Tomonori [EMAIL PROTECTED]
Subject: [PATCH] set supported_mode to MODE_INITIATOR by default

This sets supported_mode to MODE_INITIATOR if a lld doesn't specify
supported_mode in scsi_host_template.

Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED]
---
 drivers/scsi/hosts.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index adc9559..694015d 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -342,6 +342,10 @@ struct Scsi_Host *scsi_host_alloc(struct 
scsi_host_template *sht, int privsize)
shost-unchecked_isa_dma = sht-unchecked_isa_dma;
shost-use_clustering = sht-use_clustering;
shost-ordered_tag = sht-ordered_tag;
+
+   if (!sht-supported_mode)
+   sht-supported_mode = MODE_INITIATOR;
+
shost-active_mode = sht-supported_mode;
 
if (sht-max_host_blocked)
-- 
1.5.2.4

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


Re: queued patches for SCSI for 2.6.24

2007-09-25 Thread Jeff Garzik

FUJITA Tomonori wrote:

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index adc9559..694015d 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -342,6 +342,10 @@ struct Scsi_Host *scsi_host_alloc(struct 
scsi_host_template *sht, int privsize)
shost-unchecked_isa_dma = sht-unchecked_isa_dma;
shost-use_clustering = sht-use_clustering;
shost-ordered_tag = sht-ordered_tag;
+
+   if (!sht-supported_mode)
+   sht-supported_mode = MODE_INITIATOR;
+
shost-active_mode = sht-supported_mode;



I almost hesitate to speak up, after making the original suggestion, but:

Are there any const-ness worries for scsi_host_template, or plans for 
the future?  I do not see any other examples of the host template 
members getting modified.


Perhaps this value should instead be mirrored in scsi_host, like many 
others?


Jeff


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


Re: queued patches for SCSI for 2.6.24

2007-09-25 Thread Matthew Wilcox
On Tue, Sep 25, 2007 at 10:37:33PM -0400, Jeff Garzik wrote:
 Are there any const-ness worries for scsi_host_template, or plans for 
 the future?  I do not see any other examples of the host template 
 members getting modified.

Goodness, Jeff, you haven't looked too hard.  There's dozens of examples
I've come across trawling the horrible unmaintained drivers.  I'd love
to see scsi_host_template become const, but it's not happening any time
soon, and we can address this little piece when the time comes.

-- 
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: queued patches for SCSI for 2.6.24

2007-09-25 Thread Jeff Garzik

Matthew Wilcox wrote:

On Tue, Sep 25, 2007 at 10:37:33PM -0400, Jeff Garzik wrote:
Are there any const-ness worries for scsi_host_template, or plans for 
the future?  I do not see any other examples of the host template 
members getting modified.


Goodness, Jeff, you haven't looked too hard.  There's dozens of examples
I've come across trawling the horrible unmaintained drivers.  I'd love
to see scsi_host_template become const, but it's not happening any time
soon, and we can address this little piece when the time comes.


Well, sure, the driver is the owner of that memory.

We're talking about common code.

If everybody agrees SHT is R/W in the core, Fujita-san's patch is fine.

Jeff



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


Re: queued patches for SCSI for 2.6.24

2007-09-25 Thread Matthew Wilcox
On Tue, Sep 25, 2007 at 11:34:00PM -0400, Jeff Garzik wrote:
 Well, sure, the driver is the owner of that memory.
 
 We're talking about common code.
 
 If everybody agrees SHT is R/W in the core, Fujita-san's patch is fine.

Oh ... harder to find, but scsi_module.c does that, as does
scsi_register/scsi_unregister.  OK, those are legacy, but they still
exist today.

-- 
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: queued patches for SCSI for 2.6.24

2007-09-25 Thread FUJITA Tomonori
On Tue, 25 Sep 2007 22:37:33 -0400
Jeff Garzik [EMAIL PROTECTED] wrote:

 FUJITA Tomonori wrote:
  diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
  index adc9559..694015d 100644
  --- a/drivers/scsi/hosts.c
  +++ b/drivers/scsi/hosts.c
  @@ -342,6 +342,10 @@ struct Scsi_Host *scsi_host_alloc(struct 
  scsi_host_template *sht, int privsize)
  shost-unchecked_isa_dma = sht-unchecked_isa_dma;
  shost-use_clustering = sht-use_clustering;
  shost-ordered_tag = sht-ordered_tag;
  +
  +   if (!sht-supported_mode)
  +   sht-supported_mode = MODE_INITIATOR;
  +
  shost-active_mode = sht-supported_mode;
 
 
 I almost hesitate to speak up, after making the original suggestion, but:
 
 Are there any const-ness worries for scsi_host_template, or plans for 
 the future?  I do not see any other examples of the host template 
 members getting modified.

Yeah, that's why I said it's hacky in the previous
discussion. Changing scsi_host_template behind llds is not nice, I
think.


 Perhaps this value should instead be mirrored in scsi_host, like
 many others?

supported_mode should be static like 'name'. I'm not sure about having
supported_mode in scsi_host. All the scsi_hosts of one driver always
use the same supported_mode value unlike active_mode.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: queued patches for SCSI for 2.6.24

2007-09-25 Thread James Bottomley
On Tue, 2007-09-25 at 23:34 -0400, Jeff Garzik wrote:
 Matthew Wilcox wrote:
  On Tue, Sep 25, 2007 at 10:37:33PM -0400, Jeff Garzik wrote:
  Are there any const-ness worries for scsi_host_template, or plans for 
  the future?  I do not see any other examples of the host template 
  members getting modified.
  
  Goodness, Jeff, you haven't looked too hard.  There's dozens of examples
  I've come across trawling the horrible unmaintained drivers.  I'd love
  to see scsi_host_template become const, but it's not happening any time
  soon, and we can address this little piece when the time comes.
 
 Well, sure, the driver is the owner of that memory.
 
 We're talking about common code.
 
 If everybody agrees SHT is R/W in the core, Fujita-san's patch is fine.

Well, I don't like mucking with the template either.

This whole mess is generated basically because the zero default of the
template should be treated as initiator.  How about this, which makes
that manifest?

James

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index adc9559..7e26440 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -342,7 +342,11 @@ struct Scsi_Host *scsi_host_alloc(struct 
scsi_host_template *sht, int privsize)
shost-unchecked_isa_dma = sht-unchecked_isa_dma;
shost-use_clustering = sht-use_clustering;
shost-ordered_tag = sht-ordered_tag;
-   shost-active_mode = sht-supported_mode;
+   if (sht-supported_mode == MODE_UNKNOWN)
+   /* means we didn't set it ... default to INITIATOR */
+   shost-active_mode = MODE_INITIATOR;
+   else
+   shost-active_mode = sht-supported_mode;
 
if (sht-max_host_blocked)
shost-max_host_blocked = sht-max_host_blocked;
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 0088c4d..4965e9e 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -209,11 +209,13 @@ show_shost_mode(unsigned int mode, char *buf)
 static ssize_t show_shost_supported_mode(struct class_device *class_dev, char 
*buf)
 {
struct Scsi_Host *shost = class_to_shost(class_dev);
+   unsigned int supported_mode = shost-hostt-supported_mode;
 
-   if (shost-hostt-supported_mode == MODE_UNKNOWN)
-   return snprintf(buf, 20, unknown\n);
-   else
-   return show_shost_mode(shost-hostt-supported_mode, buf);
+   if (supported_mode == MODE_UNKNOWN)
+   /* by default this should be initiator */
+   supported_mode = MODE_INITIATOR;
+
+   return show_shost_mode(shost-hostt-supported_mode, buf);
 }
 
 static CLASS_DEVICE_ATTR(supported_mode, S_IRUGO | S_IWUSR, 
show_shost_supported_mode, NULL);


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


Re: queued patches for SCSI for 2.6.24

2007-09-25 Thread Jeff Garzik

James Bottomley wrote:

This whole mess is generated basically because the zero default of the
template should be treated as initiator.  How about this, which makes
that manifest?

James

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index adc9559..7e26440 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -342,7 +342,11 @@ struct Scsi_Host *scsi_host_alloc(struct 
scsi_host_template *sht, int privsize)
shost-unchecked_isa_dma = sht-unchecked_isa_dma;
shost-use_clustering = sht-use_clustering;
shost-ordered_tag = sht-ordered_tag;
-   shost-active_mode = sht-supported_mode;
+   if (sht-supported_mode == MODE_UNKNOWN)
+   /* means we didn't set it ... default to INITIATOR */
+   shost-active_mode = MODE_INITIATOR;
+   else
+   shost-active_mode = sht-supported_mode;
 
 	if (sht-max_host_blocked)

shost-max_host_blocked = sht-max_host_blocked;
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 0088c4d..4965e9e 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -209,11 +209,13 @@ show_shost_mode(unsigned int mode, char *buf)
 static ssize_t show_shost_supported_mode(struct class_device *class_dev, char 
*buf)
 {
struct Scsi_Host *shost = class_to_shost(class_dev);
+   unsigned int supported_mode = shost-hostt-supported_mode;
 
-	if (shost-hostt-supported_mode == MODE_UNKNOWN)

-   return snprintf(buf, 20, unknown\n);
-   else
-   return show_shost_mode(shost-hostt-supported_mode, buf);
+   if (supported_mode == MODE_UNKNOWN)
+   /* by default this should be initiator */
+   supported_mode = MODE_INITIATOR;
+
+   return show_shost_mode(shost-hostt-supported_mode, buf);
 }
 
 static CLASS_DEVICE_ATTR(supported_mode, S_IRUGO | S_IWUSR, show_shost_supported_mode, NULL);



ACK

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


Re: queued patches for SCSI for 2.6.24

2007-09-25 Thread FUJITA Tomonori
On Tue, 25 Sep 2007 22:45:53 -0500
James Bottomley [EMAIL PROTECTED] wrote:

 On Tue, 2007-09-25 at 23:34 -0400, Jeff Garzik wrote:
  Matthew Wilcox wrote:
   On Tue, Sep 25, 2007 at 10:37:33PM -0400, Jeff Garzik wrote:
   Are there any const-ness worries for scsi_host_template, or plans for 
   the future?  I do not see any other examples of the host template 
   members getting modified.
   
   Goodness, Jeff, you haven't looked too hard.  There's dozens of examples
   I've come across trawling the horrible unmaintained drivers.  I'd love
   to see scsi_host_template become const, but it's not happening any time
   soon, and we can address this little piece when the time comes.
  
  Well, sure, the driver is the owner of that memory.
  
  We're talking about common code.
  
  If everybody agrees SHT is R/W in the core, Fujita-san's patch is fine.
 
 Well, I don't like mucking with the template either.
 
 This whole mess is generated basically because the zero default of the
 template should be treated as initiator.  How about this, which makes
 that manifest?

But how can we handle dual-mode drivers?

luce:/sys/class/scsi_host/host0$ cat supported_mode
Initiator, Target


The values are not enumerated. They are like FC_PORT_ROLE.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: queued patches for SCSI for 2.6.24

2007-09-25 Thread James Bottomley
On Wed, 2007-09-26 at 12:55 +0900, FUJITA Tomonori wrote:
 On Tue, 25 Sep 2007 22:45:53 -0500
 James Bottomley [EMAIL PROTECTED] wrote:
 
  On Tue, 2007-09-25 at 23:34 -0400, Jeff Garzik wrote:
   Matthew Wilcox wrote:
On Tue, Sep 25, 2007 at 10:37:33PM -0400, Jeff Garzik wrote:
Are there any const-ness worries for scsi_host_template, or plans for 
the future?  I do not see any other examples of the host template 
members getting modified.

Goodness, Jeff, you haven't looked too hard.  There's dozens of examples
I've come across trawling the horrible unmaintained drivers.  I'd love
to see scsi_host_template become const, but it's not happening any time
soon, and we can address this little piece when the time comes.
   
   Well, sure, the driver is the owner of that memory.
   
   We're talking about common code.
   
   If everybody agrees SHT is R/W in the core, Fujita-san's patch is fine.
  
  Well, I don't like mucking with the template either.
  
  This whole mess is generated basically because the zero default of the
  template should be treated as initiator.  How about this, which makes
  that manifest?
 
 But how can we handle dual-mode drivers?
 
 luce:/sys/class/scsi_host/host0$ cat supported_mode
 Initiator, Target
 
 
 The values are not enumerated. They are like FC_PORT_ROLE.

Any driver that does other than the default INITIATOR has to set it in
the template.  The code only defaults zero (which is what the templates
get if its unset) to MODE_INITIATOR.

James


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


Re: queued patches for SCSI for 2.6.24

2007-09-25 Thread FUJITA Tomonori
On Tue, 25 Sep 2007 23:01:53 -0500
James Bottomley [EMAIL PROTECTED] wrote:

 On Wed, 2007-09-26 at 12:55 +0900, FUJITA Tomonori wrote:
  On Tue, 25 Sep 2007 22:45:53 -0500
  James Bottomley [EMAIL PROTECTED] wrote:
  
   On Tue, 2007-09-25 at 23:34 -0400, Jeff Garzik wrote:
Matthew Wilcox wrote:
 On Tue, Sep 25, 2007 at 10:37:33PM -0400, Jeff Garzik wrote:
 Are there any const-ness worries for scsi_host_template, or plans 
 for 
 the future?  I do not see any other examples of the host template 
 members getting modified.
 
 Goodness, Jeff, you haven't looked too hard.  There's dozens of 
 examples
 I've come across trawling the horrible unmaintained drivers.  I'd love
 to see scsi_host_template become const, but it's not happening any 
 time
 soon, and we can address this little piece when the time comes.

Well, sure, the driver is the owner of that memory.

We're talking about common code.

If everybody agrees SHT is R/W in the core, Fujita-san's patch is fine.
   
   Well, I don't like mucking with the template either.
   
   This whole mess is generated basically because the zero default of the
   template should be treated as initiator.  How about this, which makes
   that manifest?
  
  But how can we handle dual-mode drivers?
  
  luce:/sys/class/scsi_host/host0$ cat supported_mode
  Initiator, Target
  
  
  The values are not enumerated. They are like FC_PORT_ROLE.
 
 Any driver that does other than the default INITIATOR has to set it in
 the template.  The code only defaults zero (which is what the templates
 get if its unset) to MODE_INITIATOR.

Oh yeah, the patch is fine.

I just wanted to say that supported_mode/active_mode are designed not
to be enumerated and we can't just set INITIATOR to zero and TARGET to
non-zero.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/8] gdth: Split out EISA and ISA register into separate functions

2007-09-25 Thread Jeff Garzik

An equivalent-transformation change.  No functional changes beyond
those necessary to call the new, split-out functions.

Signed-off-by: Jeff Garzik [EMAIL PROTECTED]
---

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index b20c188..f330d34 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -4277,6 +4277,274 @@ int __init option_setup(char *str)
 return 1;
 }
 
+static int __init gdth_start_isa(struct scsi_host_template *shtp,
+ulong32 isa_bios)
+{
+   struct Scsi_Host *shp;
+   gdth_ha_str *ha;
+   int i, hanum;
+   dma_addr_t scratch_dma_handle = 0;
+
+   shp = scsi_register(shtp, sizeof(gdth_ext_str));
+   if (shp == NULL)
+   return 1;   /* continue looping */
+
+   ha = HADATA(shp);
+   if (!gdth_init_isa(isa_bios, ha)) {
+   scsi_unregister(shp);
+   return 1;   /* continue looping */
+   }
+
+#ifdef __ia64__
+   return 0;   /* end loop: success */
+#else
+   /* controller found and initialized */
+   printk(Configuring GDT-ISA HA at BIOS 0x%05X IRQ %u DRQ %u\n,
+  isa_bios, ha-irq, ha-drq);
+
+   if (request_irq(ha-irq, gdth_interrupt, IRQF_DISABLED, gdth, ha)) {
+   printk(GDT-ISA: Unable to allocate IRQ\n);
+   scsi_unregister(shp);
+   return 1;   /* continue looping */
+   }
+   if (request_dma(ha-drq, gdth)) {
+   printk(GDT-ISA: Unable to allocate DMA channel\n);
+   free_irq(ha-irq, ha);
+   scsi_unregister(shp);
+   return 1;   /* continue looping */
+   }
+
+   set_dma_mode(ha-drq, DMA_MODE_CASCADE);
+   enable_dma(ha-drq);
+   shp-unchecked_isa_dma = 1;
+   shp-irq = ha-irq;
+   shp-dma_channel = ha-drq;
+   hanum = gdth_ctr_count; 
+   gdth_ctr_tab[gdth_ctr_count++] = shp;
+   gdth_ctr_vtab[gdth_ctr_vcount++] = shp;
+
+   NUMDATA(shp)-hanum = (ushort)hanum;
+   NUMDATA(shp)-busnum= 0;
+
+   ha-pccb = CMDDATA(shp);
+   ha-ccb_phys = 0L;
+   ha-pdev = NULL;
+   ha-pscratch = pci_alloc_consistent(ha-pdev, GDTH_SCRATCH, 
+   scratch_dma_handle);
+   ha-scratch_phys = scratch_dma_handle;
+   ha-pmsg = pci_alloc_consistent(ha-pdev, sizeof(gdth_msg_str), 
+   scratch_dma_handle);
+   ha-msg_phys = scratch_dma_handle;
+
+#ifdef INT_COAL
+   ha-coal_stat = (gdth_coal_status *)
+   pci_alloc_consistent(ha-pdev, sizeof(gdth_coal_status) *
+MAXOFFSETS, scratch_dma_handle);
+   ha-coal_stat_phys = scratch_dma_handle;
+#endif
+
+   ha-scratch_busy = FALSE;
+   ha-req_first = NULL;
+   ha-tid_cnt = MAX_HDRIVES;
+   if (max_ids  0  max_ids  ha-tid_cnt)
+   ha-tid_cnt = max_ids;
+   for (i=0; iGDTH_MAXCMDS; ++i)
+   ha-cmd_tab[i].cmnd = UNUSED_CMND;
+   ha-scan_mode = rescan ? 0x10 : 0;
+
+   if (ha-pscratch == NULL || ha-pmsg == NULL || 
+   !gdth_search_drives(hanum)) {
+   printk(GDT-ISA: Error during device scan\n);
+   --gdth_ctr_count;
+   --gdth_ctr_vcount;
+
+#ifdef INT_COAL
+   if (ha-coal_stat)
+   pci_free_consistent(ha-pdev, sizeof(gdth_coal_status) *
+   MAXOFFSETS, ha-coal_stat,
+   ha-coal_stat_phys);
+#endif
+
+   if (ha-pscratch)
+   pci_free_consistent(ha-pdev, GDTH_SCRATCH, 
+   ha-pscratch, ha-scratch_phys);
+   if (ha-pmsg)
+   pci_free_consistent(ha-pdev, sizeof(gdth_msg_str), 
+   ha-pmsg, ha-msg_phys);
+
+   free_irq(ha-irq,ha);
+   scsi_unregister(shp);
+   return 1;   /* continue looping */
+   }
+
+   if (hdr_channel  0 || hdr_channel  ha-bus_cnt)
+   hdr_channel = ha-bus_cnt;
+   ha-virt_bus = hdr_channel;
+
+#if LINUX_VERSION_CODE = KERNEL_VERSION(2,4,20)  \
+LINUX_VERSION_CODE  KERNEL_VERSION(2,6,0)
+   shp-highmem_io  = 0;
+#endif
+   if (ha-cache_feat  ha-raw_feat  ha-screen_feat  GDT_64BIT) 
+   shp-max_cmd_len = 16;
+
+   shp-max_id  = ha-tid_cnt;
+   shp-max_lun = MAXLUN;
+   shp-max_channel = virt_ctr ? 0 : ha-bus_cnt;
+   if (virt_ctr) {
+   unchar b;
+
+   virt_ctr = 1;
+   /* register addit. SCSI channels as virtual controllers */
+   for (b = 1; b  ha-bus_cnt + 1; ++b) {
+   shp = scsi_register(shtp,sizeof(gdth_num_str));
+   shp-unchecked_isa_dma = 1;
+   shp-irq = ha-irq;
+   shp-dma_channel = ha-drq;
+ 

[PATCH 2/8] gdth: Split out PCI register into separate function

2007-09-25 Thread Jeff Garzik

An equivalent-transformation change.  No functional changes beyond those
necessary to call the new function.

Signed-off-by: Jeff Garzik [EMAIL PROTECTED]
---

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index f330d34..840bdf6 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -4545,16 +4545,167 @@ LINUX_VERSION_CODE  KERNEL_VERSION(2,6,0)
return 1;   /* continue looping */
 }
 
+static int __init gdth_start_pci(struct scsi_host_template *shtp,
+gdth_pci_str *pcistr, int ctr)
+{
+   struct Scsi_Host *shp;
+   dma_addr_t scratch_dma_handle = 0;
+   gdth_ha_str *ha;
+   int i, hanum, err;
+
+   if (gdth_ctr_count = MAXHA)
+   return 0;   /* end loop: success */
+   shp = scsi_register(shtp,sizeof(gdth_ext_str));
+   if (shp == NULL)
+   return 1;   /* continue looping */
+
+   ha = HADATA(shp);
+   if (!gdth_init_pci(pcistr[ctr],ha)) {
+   scsi_unregister(shp);
+   return 1;   /* continue looping */
+   }
+
+   /* controller found and initialized */
+   printk(Configuring GDT-PCI HA at %d/%d IRQ %u\n,
+  pcistr[ctr].pdev-bus-number,
+  PCI_SLOT(pcistr[ctr].pdev-devfn), ha-irq);
+
+   if (request_irq(ha-irq, gdth_interrupt,
+   IRQF_DISABLED | IRQF_SHARED, gdth, ha)) {
+   printk(GDT-PCI: Unable to allocate IRQ\n);
+   scsi_unregister(shp);
+   return 1;   /* continue looping */
+   }
+
+   shp-unchecked_isa_dma = 0;
+   shp-irq = ha-irq;
+   shp-dma_channel = 0xff;
+   hanum = gdth_ctr_count;
+   gdth_ctr_tab[gdth_ctr_count++] = shp;
+   gdth_ctr_vtab[gdth_ctr_vcount++] = shp;
+
+   NUMDATA(shp)-hanum = (ushort)hanum;
+   NUMDATA(shp)-busnum= 0;
+
+   ha-pccb = CMDDATA(shp);
+   ha-ccb_phys = 0L;
+
+   ha-pscratch = pci_alloc_consistent(ha-pdev, GDTH_SCRATCH, 
+   scratch_dma_handle);
+   ha-scratch_phys = scratch_dma_handle;
+   ha-pmsg = pci_alloc_consistent(ha-pdev, sizeof(gdth_msg_str), 
+   scratch_dma_handle);
+   ha-msg_phys = scratch_dma_handle;
+
+#ifdef INT_COAL
+   ha-coal_stat = (gdth_coal_status *)
+   pci_alloc_consistent(ha-pdev, sizeof(gdth_coal_status) *
+   MAXOFFSETS, scratch_dma_handle);
+   ha-coal_stat_phys = scratch_dma_handle;
+#endif
+
+   ha-scratch_busy = FALSE;
+   ha-req_first = NULL;
+   ha-tid_cnt = pcistr[ctr].pdev-device = 0x200 ? MAXID : MAX_HDRIVES;
+   if (max_ids  0  max_ids  ha-tid_cnt)
+   ha-tid_cnt = max_ids;
+   for (i=0; iGDTH_MAXCMDS; ++i)
+   ha-cmd_tab[i].cmnd = UNUSED_CMND;
+   ha-scan_mode = rescan ? 0x10 : 0;
+
+   err = FALSE;
+   if (ha-pscratch == NULL || ha-pmsg == NULL || 
+   !gdth_search_drives(hanum)) {
+   err = TRUE;
+   } else {
+   if (hdr_channel  0 || hdr_channel  ha-bus_cnt)
+   hdr_channel = ha-bus_cnt;
+   ha-virt_bus = hdr_channel;
+
+#if LINUX_VERSION_CODE  KERNEL_VERSION(2,6,0)
+   scsi_set_pci_device(shp, pcistr[ctr].pdev);
+#endif
+
+   if (!(ha-cache_feat  ha-raw_feat  ha-screen_feat 
+ GDT_64BIT) ||
+   /* 64-bit DMA only supported from FW = x.43 */
+   (!ha-dma64_support)) {
+   if (pci_set_dma_mask(pcistr[ctr].pdev, DMA_32BIT_MASK)){
+   printk(KERN_WARNING GDT-PCI %d: Unable to 
+  set 32-bit DMA\n, hanum);
+   err = TRUE;
+   }
+   } else {
+   shp-max_cmd_len = 16;
+   if (!pci_set_dma_mask(pcistr[ctr].pdev,DMA_64BIT_MASK)){
+   printk(GDT-PCI %d: 64-bit DMA enabled\n,
+  hanum);
+   } else if (pci_set_dma_mask(pcistr[ctr].pdev,
+   DMA_32BIT_MASK)) {
+   printk(KERN_WARNING GDT-PCI %d: Unable to set 
+  64/32-bit DMA\n, hanum);
+   err = TRUE;
+   }
+   }
+   }
+
+   if (err) {
+   printk(GDT-PCI %d: Error during device scan\n, hanum);
+   --gdth_ctr_count;
+   --gdth_ctr_vcount;
+
+#ifdef INT_COAL
+   if (ha-coal_stat)
+   pci_free_consistent(ha-pdev, sizeof(gdth_coal_status) *
+   MAXOFFSETS, ha-coal_stat,
+   ha-coal_stat_phys);
+#endif
+
+   if (ha-pscratch)
+   

[PATCH 3/8] gdth: Remove 2.4.x support, in-kernel changelog

2007-09-25 Thread Jeff Garzik

* Remove in-source changelog.  It's archived permanently in git and
  various kernel archives, and changelogs should exist purely in git.

* Remove 2.4.x kernel support.  It is an active obstacle to
  modernizing this driver, at this point.  This includes killing
  gdth_kcompat.h which is 100% redundant in modern kernels.

Signed-off-by: Jeff Garzik [EMAIL PROTECTED]
---

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index 840bdf6..a6b5717 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -27,280 +27,8 @@
  * along with this kernel; if not, write to the Free Software   *
  * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.*
  *  *
- * Linux kernel 2.4.x, 2.6.x supported  *
+ * Linux kernel 2.6.x supported
*
  *  *
- * $Log: gdth.c,v $
- * Revision 1.74  2006/04/10 13:44:47  achim
- * Community changes for 2.6.x
- * Kernel 2.2.x no longer supported
- * scsi_request interface removed, thanks to Christoph Hellwig
- *
- * Revision 1.73  2004/03/31 13:33:03  achim
- * Special command 0xfd implemented to detect 64-bit DMA support
- *
- * Revision 1.72  2004/03/17 08:56:04  achim
- * 64-bit DMA only enabled if FW = x.43
- *
- * Revision 1.71  2004/03/05 15:51:29  achim
- * Screen service: separate message buffer, bugfixes
- *
- * Revision 1.70  2004/02/27 12:19:07  achim
- * Bugfix: Reset bit in config (0xfe) call removed
- *
- * Revision 1.69  2004/02/20 09:50:24  achim
- * Compatibility changes for kernels  2.4.20
- * Bugfix screen service command size
- * pci_set_dma_mask() error handling added
- *
- * Revision 1.68  2004/02/19 15:46:54  achim
- * 64-bit DMA bugfixes
- * Drive size bugfix for drives  1TB
- *
- * Revision 1.67  2004/01/14 13:11:57  achim
- * Tool access over /proc no longer supported
- * Bugfixes IOCTLs
- *
- * Revision 1.66  2003/12/19 15:04:06  achim
- * Bugfixes support for drives  2TB
- *
- * Revision 1.65  2003/12/15 11:21:56  achim
- * 64-bit DMA support added
- * Support for drives  2 TB implemented
- * Kernels 2.2.x, 2.4.x, 2.6.x supported
- *
- * Revision 1.64  2003/09/17 08:30:26  achim
- * EISA/ISA controller scan disabled
- * Command line switch probe_eisa_isa added
- *
- * Revision 1.63  2003/07/12 14:01:00  Daniele Bellucci [EMAIL PROTECTED]
- * Minor cleanups in gdth_ioctl.
- *
- * Revision 1.62  2003/02/27 15:01:59  achim
- * Dynamic DMA mapping implemented
- * New (character device) IOCTL interface added
- * Other controller related changes made
- *
- * Revision 1.61  2002/11/08 13:09:52  boji
- * Added support for XSCALE based RAID Controllers
- * Fixed SCREENSERVICE initialization in SMP cases
- * Added checks for gdth_polling before GDTH_HA_LOCK
- *
- * Revision 1.60  2002/02/05 09:35:22  achim
- * MODULE_LICENSE only if kernel = 2.4.11
- *
- * Revision 1.59  2002/01/30 09:46:33  achim
- * Small changes
- *
- * Revision 1.58  2002/01/29 15:30:02  achim
- * Set default value of shared_access to Y
- * New status S_CACHE_RESERV for clustering added
- *
- * Revision 1.57  2001/08/21 11:16:35  achim
- * Bugfix free_irq()
- *
- * Revision 1.56  2001/08/09 11:19:39  achim
- * Scsi_Host_Template changes
- *
- * Revision 1.55  2001/08/09 10:11:28  achim
- * Command HOST_UNFREEZE_IO before cache service init.
- *
- * Revision 1.54  2001/07/20 13:48:12  achim
- * Expand: gdth_analyse_hdrive() removed
- *
- * Revision 1.53  2001/07/17 09:52:49  achim
- * Small OEM related change
- *
- * Revision 1.52  2001/06/19 15:06:20  achim
- * New host command GDT_UNFREEZE_IO added
- *
- * Revision 1.51  2001/05/22 06:42:37  achim
- * PCI: Subdevice ID added
- *
- * Revision 1.50  2001/05/17 13:42:16  achim
- * Support for Intel Storage RAID Controllers added
- *
- * Revision 1.50  2001/05/17 12:12:34  achim
- * Support for Intel Storage RAID Controllers added
- *
- * Revision 1.49  2001/03/15 15:07:17  achim
- * New __setup interface for boot command line options added
- *
- * Revision 1.48  2001/02/06 12:36:28  achim
- * Bugfix Cluster protocol
- *
- * Revision 1.47  2001/01/10 14:42:06  achim
- * New switch shared_access added
- *
- * Revision 1.46  2001/01/09 08:11:35  achim
- * gdth_command() removed
- * meaning of Scsi_Pointer members changed
- *
- * Revision 1.45  2000/11/16 12:02:24  achim
- * Changes for kernel 2.4
- *
- * Revision 1.44  2000/10/11 08:44:10  achim
- * Clustering changes: New flag media_changed added
- *
- * Revision 1.43  2000/09/20 12:59:01  achim
- * DPMEM remap functions for all PCI controller types implemented
- * Small changes for ia64 platform
- *
- * Revision 1.42  2000/07/20 09:04:50  achim
- * Small changes for kernel 2.4
- *
- * Revision 1.41  2000/07/04 14:11:11  achim
- * gdth_analyse_hdrive() added to rescan drives after online expansion
- *
- * Revision 1.40  2000/06/27 11:24:16  achim
- * 

[PATCH 4/8] gdth: Isolate driver-global variables using helpers

2007-09-25 Thread Jeff Garzik

Sanitizes access to some driver-global variables, in preparation for
making them dynamic rather than statically-sized.

This equivalent-transform style change blissfully ignores the fact that
gdth_ctr_vtab[] is never used (only assigned), and that the driver
itself seems to get a bit confused between gdth_ctr_count and
gdth_ctr_vcount.  The whole virtual-controller stuff needs reviewing
for edge cases.

Signed-off-by: Jeff Garzik [EMAIL PROTECTED]
---

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index a6b5717..b9d1f69 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -394,6 +394,44 @@ static const struct file_operations gdth_fops = {
 .release = gdth_close,
 };
 
+static struct Scsi_Host *gdth_find_shp(int ha_idx)
+{
+   return gdth_ctr_tab[ha_idx];
+}
+
+static gdth_ha_str *gdth_find_ha(int ha_idx)
+{
+   struct Scsi_Host *shp = gdth_find_shp(ha_idx);
+   if (!shp)
+   return NULL;
+   return HADATA(shp);
+}
+
+static void gdth_push_vshp(struct Scsi_Host *shp)
+{
+   gdth_ctr_vtab[gdth_ctr_vcount++] = shp;
+}
+
+static int gdth_push_shp(struct Scsi_Host *shp)
+{
+   int idx;
+
+   idx = gdth_ctr_count;
+   gdth_ctr_count++;
+
+   gdth_ctr_tab[idx] = shp;
+
+   gdth_push_vshp(shp);
+
+   return idx;
+}
+
+static void gdth_pop_shp(void)
+{
+   gdth_ctr_count--;
+   gdth_ctr_vcount--;
+}
+
 #include gdth_proc.h
 #include gdth_proc.c
 
@@ -1220,7 +1258,7 @@ static void __init gdth_enable_int(int hanum)
 gdt6m_dpram_str __iomem *dp6m_ptr;
 
 TRACE((gdth_enable_int() hanum %d\n,hanum));
-ha = HADATA(gdth_ctr_tab[hanum]);
+ha = gdth_find_ha(hanum);
 spin_lock_irqsave(ha-smp_lock, flags);
 
 if (ha-type == GDT_EISA) {
@@ -1260,7 +1298,7 @@ static int gdth_get_status(unchar *pIStatus,int irq)
 
 *pIStatus = 0;
 for (i=0; igdth_ctr_count; ++i) {
-ha = HADATA(gdth_ctr_tab[i]);
+ha = gdth_find_ha(i);
 if (ha-irq != (unchar)irq) /* check IRQ */
 continue;
 if (ha-type == GDT_EISA)
@@ -1291,7 +1329,7 @@ static int gdth_test_busy(int hanum)
 
 TRACE((gdth_test_busy() hanum %d\n,hanum));
 
-ha = HADATA(gdth_ctr_tab[hanum]);
+ha = gdth_find_ha(hanum);
 if (ha-type == GDT_EISA)
 gdtsema0 = (int)inb(ha-bmic + SEMA0REG);
 else if (ha-type == GDT_ISA)
@@ -1315,7 +1353,7 @@ static int gdth_get_cmd_index(int hanum)
 
 TRACE((gdth_get_cmd_index() hanum %d\n,hanum));
 
-ha = HADATA(gdth_ctr_tab[hanum]);
+ha = gdth_find_ha(hanum);
 for (i=0; iGDTH_MAXCMDS; ++i) {
 if (ha-cmd_tab[i].cmnd == UNUSED_CMND) {
 ha-cmd_tab[i].cmnd = ha-pccb-RequestBuffer;
@@ -1334,7 +1372,7 @@ static void gdth_set_sema0(int hanum)
 
 TRACE((gdth_set_sema0() hanum %d\n,hanum));
 
-ha = HADATA(gdth_ctr_tab[hanum]);
+ha = gdth_find_ha(hanum);
 if (ha-type == GDT_EISA) {
 outb(1, ha-bmic + SEMA0REG);
 } else if (ha-type == GDT_ISA) {
@@ -1361,7 +1399,7 @@ static void gdth_copy_command(int hanum)
 
 TRACE((gdth_copy_command() hanum %d\n,hanum));
 
-ha = HADATA(gdth_ctr_tab[hanum]);
+ha = gdth_find_ha(hanum);
 cp_count = ha-cmd_len;
 dp_offset= ha-cmd_offs_dpmem;
 cmd_no   = ha-cmd_cnt;
@@ -1415,7 +1453,7 @@ static void gdth_release_event(int hanum)
 register gdth_ha_str *ha;
 
 TRACE((gdth_release_event() hanum %d\n,hanum));
-ha = HADATA(gdth_ctr_tab[hanum]);
+ha = gdth_find_ha(hanum);
 
 #ifdef GDTH_STATISTICS
 {
@@ -1457,7 +1495,7 @@ static int gdth_wait(int hanum,int index,ulong32 time)
 
 TRACE((gdth_wait() hanum %d index %d time %d\n,hanum,index,time));
 
-ha = HADATA(gdth_ctr_tab[hanum]);
+ha = gdth_find_ha(hanum);
 if (index == 0)
 return 1;   /* no wait required */
 
@@ -1488,7 +1526,7 @@ static int gdth_internal_cmd(int hanum,unchar 
service,ushort opcode,ulong32 p1,
 
 TRACE2((gdth_internal_cmd() service %d opcode %d\n,service,opcode));
 
-ha = HADATA(gdth_ctr_tab[hanum]);
+ha = gdth_find_ha(hanum);
 cmd_ptr = ha-pccb;
 memset((char*)cmd_ptr,0,sizeof(gdth_cmd_str));
 
@@ -1581,7 +1619,7 @@ static int __init gdth_search_drives(int hanum)
 #endif 

 TRACE((gdth_search_drives() hanum %d\n,hanum));
-ha = HADATA(gdth_ctr_tab[hanum]);
+ha = gdth_find_ha(hanum);
 ok = 0;
 
 /* initialize controller services, at first: screen service */
@@ -1938,7 +1976,7 @@ static int gdth_analyse_hdrive(int hanum,ushort hdrive)
 TRACE((gdth_analyse_hdrive() hanum %d drive %d\n,hanum,hdrive));
 if (hdrive = MAX_HDRIVES)
 return 0;
-ha = HADATA(gdth_ctr_tab[hanum]);
+ha = gdth_find_ha(hanum);
 
 if (!gdth_internal_cmd(hanum,CACHESERVICE,GDT_INFO,hdrive,0,0)) 
 return 0;
@@ -2005,7 +2043,7 @@ static void gdth_putq(int hanum,Scsi_Cmnd *scp,unchar 
priority)
 unchar b, t;
 
 TRACE((gdth_putq() priority 

[PATCH 6/8] gdth: Move probe-time error handling code to end of each function

2007-09-25 Thread Jeff Garzik

Use standard laddered-goto error handling approach in three probe-time
functions.

Signed-off-by: Jeff Garzik [EMAIL PROTECTED]
---

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index d7ef159..89ac155 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -3964,13 +3964,11 @@ static int __init gdth_start_isa(struct 
scsi_host_template *shtp,
 
shp = scsi_register(shtp, sizeof(gdth_ext_str));
if (shp == NULL)
-   return 1;   /* continue looping */
+   goto err_out;
 
ha = HADATA(shp);
-   if (!gdth_init_isa(isa_bios, ha)) {
-   scsi_unregister(shp);
-   return 1;   /* continue looping */
-   }
+   if (!gdth_init_isa(isa_bios, ha))
+   goto err_out_shp;
 
 #ifdef __ia64__
return 0;   /* end loop: success */
@@ -3981,14 +3979,11 @@ static int __init gdth_start_isa(struct 
scsi_host_template *shtp,
 
if (request_irq(ha-irq, gdth_interrupt, IRQF_DISABLED, gdth, ha)) {
printk(GDT-ISA: Unable to allocate IRQ\n);
-   scsi_unregister(shp);
-   return 1;   /* continue looping */
+   goto err_out_shp;
}
if (request_dma(ha-drq, gdth)) {
printk(GDT-ISA: Unable to allocate DMA channel\n);
-   free_irq(ha-irq, ha);
-   scsi_unregister(shp);
-   return 1;   /* continue looping */
+   goto err_out_irq;
}
 
set_dma_mode(ha-drq, DMA_MODE_CASCADE);
@@ -4030,25 +4025,7 @@ static int __init gdth_start_isa(struct 
scsi_host_template *shtp,
if (ha-pscratch == NULL || ha-pmsg == NULL || 
!gdth_search_drives(hanum)) {
printk(GDT-ISA: Error during device scan\n);
-   gdth_pop_shp();
-
-#ifdef INT_COAL
-   if (ha-coal_stat)
-   pci_free_consistent(ha-pdev, sizeof(gdth_coal_status) *
-   MAXOFFSETS, ha-coal_stat,
-   ha-coal_stat_phys);
-#endif
-
-   if (ha-pscratch)
-   pci_free_consistent(ha-pdev, GDTH_SCRATCH, 
-   ha-pscratch, ha-scratch_phys);
-   if (ha-pmsg)
-   pci_free_consistent(ha-pdev, sizeof(gdth_msg_str), 
-   ha-pmsg, ha-msg_phys);
-
-   free_irq(ha-irq,ha);
-   scsi_unregister(shp);
-   return 1;   /* continue looping */
+   goto err_out_ha;
}
 
if (hdr_channel  0 || hdr_channel  ha-bus_cnt)
@@ -4079,9 +4056,32 @@ static int __init gdth_start_isa(struct 
scsi_host_template *shtp,
 
spin_lock_init(ha-smp_lock);
gdth_enable_int(hanum);
-#endif /* !__ia64__ */
 
return 1;   /* continue looping */
+
+err_out_ha:
+   gdth_pop_shp();
+
+#ifdef INT_COAL
+   if (ha-coal_stat)
+   pci_free_consistent(ha-pdev, sizeof(gdth_coal_status) *
+   MAXOFFSETS, ha-coal_stat,
+   ha-coal_stat_phys);
+#endif
+
+   if (ha-pscratch)
+   pci_free_consistent(ha-pdev, GDTH_SCRATCH, 
+   ha-pscratch, ha-scratch_phys);
+   if (ha-pmsg)
+   pci_free_consistent(ha-pdev, sizeof(gdth_msg_str), 
+   ha-pmsg, ha-msg_phys);
+err_out_irq:
+   free_irq(ha-irq, ha);
+#endif /* !__ia64__ */
+err_out_shp:
+   scsi_unregister(shp);
+err_out:
+   return 1;   /* continue looping */
 }
 
 static int __init gdth_start_eisa(struct scsi_host_template *shtp,
@@ -4094,13 +4094,11 @@ static int __init gdth_start_eisa(struct 
scsi_host_template *shtp,
 
shp = scsi_register(shtp,sizeof(gdth_ext_str));
if (shp == NULL)
-   return 1;   /* continue looping */
+   goto err_out;
 
ha = HADATA(shp);
-   if (!gdth_init_eisa(eisa_slot,ha)) {
-   scsi_unregister(shp);
-   return 1;   /* continue looping */
-   }
+   if (!gdth_init_eisa(eisa_slot,ha))
+   goto err_out_shp;
 
/* controller found and initialized */
printk(Configuring GDT-EISA HA at Slot %d IRQ %u\n,
@@ -4108,8 +4106,7 @@ static int __init gdth_start_eisa(struct 
scsi_host_template *shtp,
 
if (request_irq(ha-irq,gdth_interrupt,IRQF_DISABLED,gdth,ha)) {
printk(GDT-EISA: Unable to allocate IRQ\n);
-   scsi_unregister(shp);
-   return 1;   /* continue looping */
+   goto err_out_shp;
}
 
shp-unchecked_isa_dma = 0;
@@ -4155,26 +4152,7 @@ static int __init gdth_start_eisa(struct 
scsi_host_template *shtp,
if (ha-pscratch == NULL || ha-pmsg == NULL || 
!gdth_search_drives(hanum)) {
printk(GDT-EISA: Error 

[PATCH 8/8] gdth: convert to modern SCSI host alloc/scan

2007-09-25 Thread Jeff Garzik

Signed-off-by: Jeff Garzik [EMAIL PROTECTED]
---

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index f382664..3f3ef4b 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -3960,7 +3960,7 @@ static int __init gdth_register_virt(struct 
scsi_host_template *shtp,
 {
struct Scsi_Host *shp;
unchar b;
-   int done = 0;
+   int rc, done = 0;
 
if (!virt_ctr)
return 0;
@@ -3969,7 +3969,7 @@ static int __init gdth_register_virt(struct 
scsi_host_template *shtp,
 
/* register addit. SCSI channels as virtual controllers */
for (b = 1; b  ha-bus_cnt + 1; ++b) {
-   shp = scsi_register(shtp, sizeof(gdth_num_str));
+   shp = scsi_host_alloc(shtp, sizeof(gdth_num_str));
 
if (isa_dma) {
shp-unchecked_isa_dma = 1;
@@ -3981,12 +3981,17 @@ static int __init gdth_register_virt(struct 
scsi_host_template *shtp,
 
shp-irq = ha-irq;
 
-   gdth_push_vshp(shp);
+   rc = scsi_add_host(shp, dev);
+   if (rc)
+   scsi_host_put(shp);
+   else {
+   gdth_push_vshp(shp);
 
-   NUMDATA(shp)-hanum = (ushort)hanum;
-   NUMDATA(shp)-busnum = b;
+   NUMDATA(shp)-hanum = (ushort)hanum;
+   NUMDATA(shp)-busnum = b;
 
-   done++;
+   done++;
+   }
}
 
return done ? 0 : -ENODEV;
@@ -3997,10 +4002,10 @@ static int __init gdth_start_isa(struct 
scsi_host_template *shtp,
 {
struct Scsi_Host *shp;
gdth_ha_str *ha;
-   int i, hanum;
+   int i, hanum, rc;
dma_addr_t scratch_dma_handle = 0;
 
-   shp = scsi_register(shtp, sizeof(gdth_ext_str));
+   shp = scsi_host_alloc(shtp, sizeof(gdth_ext_str));
if (shp == NULL)
goto err_out;
 
@@ -4009,6 +4014,8 @@ static int __init gdth_start_isa(struct 
scsi_host_template *shtp,
goto err_out_shp;
 
 #ifdef __ia64__
+   if (scsi_add_host(shp, NULL))
+   scsi_host_put(shp);
return 0;   /* end loop: success */
 #else
/* controller found and initialized */
@@ -4077,13 +4084,26 @@ static int __init gdth_start_isa(struct 
scsi_host_template *shtp,
shp-max_lun = MAXLUN;
shp-max_channel = virt_ctr ? 0 : ha-bus_cnt;
 
-   gdth_register_virt(shtp, ha, hanum, NULL, true);
-
spin_lock_init(ha-smp_lock);
+
+   rc = scsi_add_host(shp, NULL);
+   if (rc) {
+   printk(GDT-ISA: Error adding host\n);
+   goto err_out_ha;
+   }
+
+   rc = gdth_register_virt(shtp, ha, hanum, NULL, true);
+   if (rc) {
+   printk(GDT-ISA: Error adding virt controllers\n);
+   goto err_out_host;
+   }
+
gdth_enable_int(hanum);
 
return 1;   /* continue looping */
 
+err_out_host:
+   scsi_remove_host(shp);
 err_out_ha:
gdth_pop_shp();
 
@@ -4104,7 +4124,7 @@ err_out_irq:
free_irq(ha-irq, ha);
 #endif /* !__ia64__ */
 err_out_shp:
-   scsi_unregister(shp);
+   scsi_host_put(shp);
 err_out:
return 1;   /* continue looping */
 }
@@ -4114,10 +4134,10 @@ static int __init gdth_start_eisa(struct 
scsi_host_template *shtp,
 {
struct Scsi_Host *shp;
gdth_ha_str *ha;
-   int i, hanum;
+   int i, hanum, rc;
dma_addr_t scratch_dma_handle = 0;
 
-   shp = scsi_register(shtp,sizeof(gdth_ext_str));
+   shp = scsi_host_alloc(shtp, sizeof(gdth_ext_str));
if (shp == NULL)
goto err_out;
 
@@ -4191,13 +4211,26 @@ static int __init gdth_start_eisa(struct 
scsi_host_template *shtp,
shp-max_lun = MAXLUN;
shp-max_channel = virt_ctr ? 0 : ha-bus_cnt;
 
-   gdth_register_virt(shtp, ha, hanum, NULL, false);
-
spin_lock_init(ha-smp_lock);
+
+   rc = scsi_add_host(shp, NULL);
+   if (rc) {
+   printk(GDT-EISA: Error adding host\n);
+   goto err_out_ha;
+   }
+
+   rc = gdth_register_virt(shtp, ha, hanum, NULL, false);
+   if (rc) {
+   printk(GDT-EISA: Error adding virt controllers\n);
+   goto err_out_host;
+   }
+
gdth_enable_int(hanum);
 
return 1;   /* continue looping */
 
+err_out_host:
+   scsi_remove_host(shp);
 err_out_ha:
gdth_pop_shp();
 
@@ -4218,7 +4251,7 @@ err_out_ha:
sizeof(gdth_cmd_str),PCI_DMA_BIDIRECTIONAL);
free_irq(ha-irq, ha);
 err_out_shp:
-   scsi_unregister(shp);
+   scsi_host_put(shp);
 err_out:
return 1;   /* continue looping */
 }
@@ -4229,11 +4262,11 @@ static int __init gdth_start_pci(struct 
scsi_host_template *shtp,
struct Scsi_Host *shp;
dma_addr_t scratch_dma_handle = 0;
gdth_ha_str *ha;
-   int i, hanum;
+   

[PATCH 5/8] gdth: kill gdth_{read,write}[bwl] wrappers

2007-09-25 Thread Jeff Garzik

They are direct equivalents to {read,write}[bwl].

Signed-off-by: Jeff Garzik [EMAIL PROTECTED]
---

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index b9d1f69..d7ef159 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -294,13 +294,6 @@ static struct timer_list gdth_timer;
 
 #define BUS_L2P(a,b)((b)(a)-virt_bus ? (b-1):(b))
 
-#define gdth_readb(addr)readb(addr)
-#define gdth_readw(addr)readw(addr)
-#define gdth_readl(addr)readl(addr)
-#define gdth_writeb(b,addr) writeb((b),(addr))
-#define gdth_writew(b,addr) writew((b),(addr))
-#define gdth_writel(b,addr) writel((b),(addr))
-
 static unchar   gdth_drq_tab[4] = {5,6,7,7};/* DRQ table */
 static unchar   gdth_irq_tab[6] = {0,10,11,12,14,0};/* IRQ table */
 static unchar   gdth_polling;   /* polling if TRUE */
@@ -544,7 +537,7 @@ static int __init gdth_search_isa(ulong32 bios_adr)
 
 TRACE((gdth_search_isa() bios adr. %x\n,bios_adr));
 if ((addr = ioremap(bios_adr+BIOS_ID_OFFS, sizeof(ulong32))) != NULL) {
-id = gdth_readl(addr);
+id = readl(addr);
 iounmap(addr);
 if (id == GDT2_ID)  /* GDT2000 */
 return 1;
@@ -778,22 +771,22 @@ static int __init gdth_init_isa(ulong32 
bios_adr,gdth_ha_str *ha)
 return 0;
 }
 dp2_ptr = ha-brd;
-gdth_writeb(1, dp2_ptr-io.memlock); /* switch off write protection */
+writeb(1, dp2_ptr-io.memlock); /* switch off write protection */
 /* reset interface area */
 memset_io(dp2_ptr-u, 0, sizeof(dp2_ptr-u));
-if (gdth_readl(dp2_ptr-u) != 0) {
+if (readl(dp2_ptr-u) != 0) {
 printk(GDT-ISA: Initialization error (DPMEM write error)\n);
 iounmap(ha-brd);
 return 0;
 }
 
 /* disable board interrupts, read DRQ and IRQ */
-gdth_writeb(0xff, dp2_ptr-io.irqdel);
-gdth_writeb(0x00, dp2_ptr-io.irqen);
-gdth_writeb(0x00, dp2_ptr-u.ic.S_Status);
-gdth_writeb(0x00, dp2_ptr-u.ic.Cmd_Index);
+writeb(0xff, dp2_ptr-io.irqdel);
+writeb(0x00, dp2_ptr-io.irqen);
+writeb(0x00, dp2_ptr-u.ic.S_Status);
+writeb(0x00, dp2_ptr-u.ic.Cmd_Index);
 
-irq_drq = gdth_readb(dp2_ptr-io.rq);
+irq_drq = readb(dp2_ptr-io.rq);
 for (i=0; i3; ++i) {
 if ((irq_drq  1)==0)
 break;
@@ -801,7 +794,7 @@ static int __init gdth_init_isa(ulong32 
bios_adr,gdth_ha_str *ha)
 }
 ha-drq = gdth_drq_tab[i];
 
-irq_drq = gdth_readb(dp2_ptr-io.rq)  3;
+irq_drq = readb(dp2_ptr-io.rq)  3;
 for (i=1; i5; ++i) {
 if ((irq_drq  1)==0)
 break;
@@ -810,12 +803,12 @@ static int __init gdth_init_isa(ulong32 
bios_adr,gdth_ha_str *ha)
 ha-irq = gdth_irq_tab[i];
 
 /* deinitialize services */
-gdth_writel(bios_adr, dp2_ptr-u.ic.S_Info[0]);
-gdth_writeb(0xff, dp2_ptr-u.ic.S_Cmd_Indx);
-gdth_writeb(0, dp2_ptr-io.event);
+writel(bios_adr, dp2_ptr-u.ic.S_Info[0]);
+writeb(0xff, dp2_ptr-u.ic.S_Cmd_Indx);
+writeb(0, dp2_ptr-io.event);
 retries = INIT_RETRIES;
 gdth_delay(20);
-while (gdth_readb(dp2_ptr-u.ic.S_Status) != 0xff) {
+while (readb(dp2_ptr-u.ic.S_Status) != 0xff) {
 if (--retries == 0) {
 printk(GDT-ISA: Initialization error (DEINIT failed)\n);
 iounmap(ha-brd);
@@ -823,9 +816,9 @@ static int __init gdth_init_isa(ulong32 
bios_adr,gdth_ha_str *ha)
 }
 gdth_delay(1);
 }
-prot_ver = (unchar)gdth_readl(dp2_ptr-u.ic.S_Info[0]);
-gdth_writeb(0, dp2_ptr-u.ic.Status);
-gdth_writeb(0xff, dp2_ptr-io.irqdel);
+prot_ver = (unchar)readl(dp2_ptr-u.ic.S_Info[0]);
+writeb(0, dp2_ptr-u.ic.Status);
+writeb(0xff, dp2_ptr-io.irqdel);
 if (prot_ver != PROTOCOL_VERSION) {
 printk(GDT-ISA: Illegal protocol version\n);
 iounmap(ha-brd);
@@ -839,15 +832,15 @@ static int __init gdth_init_isa(ulong32 
bios_adr,gdth_ha_str *ha)
 ha-brd_phys = bios_adr  4;
 
 /* special request to controller BIOS */
-gdth_writel(0x00, dp2_ptr-u.ic.S_Info[0]);
-gdth_writel(0x00, dp2_ptr-u.ic.S_Info[1]);
-gdth_writel(0x01, dp2_ptr-u.ic.S_Info[2]);
-gdth_writel(0x00, dp2_ptr-u.ic.S_Info[3]);
-gdth_writeb(0xfe, dp2_ptr-u.ic.S_Cmd_Indx);
-gdth_writeb(0, dp2_ptr-io.event);
+writel(0x00, dp2_ptr-u.ic.S_Info[0]);
+writel(0x00, dp2_ptr-u.ic.S_Info[1]);
+writel(0x01, dp2_ptr-u.ic.S_Info[2]);
+writel(0x00, dp2_ptr-u.ic.S_Info[3]);
+writeb(0xfe, dp2_ptr-u.ic.S_Cmd_Indx);
+writeb(0, dp2_ptr-io.event);
 retries = INIT_RETRIES;
 gdth_delay(20);
-while (gdth_readb(dp2_ptr-u.ic.S_Status) != 0xfe) {
+while (readb(dp2_ptr-u.ic.S_Status) != 0xfe) {
 if (--retries == 0) {
 printk(GDT-ISA: Initialization error\n);
 iounmap(ha-brd);
@@ -855,8 +848,8 @@ static int __init gdth_init_isa(ulong32 
bios_adr,gdth_ha_str *ha)
 }
 gdth_delay(1);
 }
-

Re: queued patches for SCSI for 2.6.24

2007-09-25 Thread FUJITA Tomonori
On Tue, 25 Sep 2007 22:45:53 -0500
James Bottomley [EMAIL PROTECTED] wrote:

 On Tue, 2007-09-25 at 23:34 -0400, Jeff Garzik wrote:
  Matthew Wilcox wrote:
   On Tue, Sep 25, 2007 at 10:37:33PM -0400, Jeff Garzik wrote:
   Are there any const-ness worries for scsi_host_template, or plans for 
   the future?  I do not see any other examples of the host template 
   members getting modified.
   
   Goodness, Jeff, you haven't looked too hard.  There's dozens of examples
   I've come across trawling the horrible unmaintained drivers.  I'd love
   to see scsi_host_template become const, but it's not happening any time
   soon, and we can address this little piece when the time comes.
  
  Well, sure, the driver is the owner of that memory.
  
  We're talking about common code.
  
  If everybody agrees SHT is R/W in the core, Fujita-san's patch is fine.
 
 Well, I don't like mucking with the template either.
 
 This whole mess is generated basically because the zero default of the
 template should be treated as initiator.  How about this, which makes
 that manifest?
 
 James
 
 diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
 index adc9559..7e26440 100644
 --- a/drivers/scsi/hosts.c
 +++ b/drivers/scsi/hosts.c
 @@ -342,7 +342,11 @@ struct Scsi_Host *scsi_host_alloc(struct 
 scsi_host_template *sht, int privsize)
   shost-unchecked_isa_dma = sht-unchecked_isa_dma;
   shost-use_clustering = sht-use_clustering;
   shost-ordered_tag = sht-ordered_tag;
 - shost-active_mode = sht-supported_mode;
 + if (sht-supported_mode == MODE_UNKNOWN)
 + /* means we didn't set it ... default to INITIATOR */
 + shost-active_mode = MODE_INITIATOR;
 + else
 + shost-active_mode = sht-supported_mode;
  
   if (sht-max_host_blocked)
   shost-max_host_blocked = sht-max_host_blocked;
 diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
 index 0088c4d..4965e9e 100644
 --- a/drivers/scsi/scsi_sysfs.c
 +++ b/drivers/scsi/scsi_sysfs.c
 @@ -209,11 +209,13 @@ show_shost_mode(unsigned int mode, char *buf)
  static ssize_t show_shost_supported_mode(struct class_device *class_dev, 
 char *buf)
  {
   struct Scsi_Host *shost = class_to_shost(class_dev);
 + unsigned int supported_mode = shost-hostt-supported_mode;
  
 - if (shost-hostt-supported_mode == MODE_UNKNOWN)
 - return snprintf(buf, 20, unknown\n);
 - else
 - return show_shost_mode(shost-hostt-supported_mode, buf);
 + if (supported_mode == MODE_UNKNOWN)
 + /* by default this should be initiator */
 + supported_mode = MODE_INITIATOR;
 +
 + return show_shost_mode(shost-hostt-supported_mode, buf);

should be:

return show_shost_mode(supported_mode, buf);

  
  static CLASS_DEVICE_ATTR(supported_mode, S_IRUGO | S_IWUSR, 
 show_shost_supported_mode, NULL);
 
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html