[PATCH] Marvell 6440 SAS/SATA driver (rough draft)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
* 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
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
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
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
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
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