Re: [PATCH] acornscsi: remove linked command support
On Sat, 2014-05-24 at 15:16 +0200, Paul Bolle wrote: On Sat, 2014-05-24 at 16:13 +0400, James Bottomley wrote: Wait, no, that's not a good idea. We leave obsolete drivers to bitrot. Particularly we try not to touch them unless we have to because there might be a few people still using them and the more we tamper, the greater the risk that something gets broken. Which is also a way to notice whether people still use those obsolete drivers. Really, no. We don't deliberately break old drivers to see if anyone notices. Usually the feedback loop is months to years for the long tail to notice and by that time fixing the problem becomes a real pain if you allow driver churn. We keep old drivers that compile until there's a problem caused usually by something like API changes. On that principle, since there's no real reason to remove the code, (Unless one carries the hope that any check, treewide, for a CONFIG_* macro can be linked to a proper Kconfig symbol.) The check can be fixed. If you look at what Fengguang Wu does, he has a list of expected problems which he diffs. Don't churn the tree to match the checker, make the checker match the tree. it should stay ... until the whole driver bitrots to the extent that we can no-longer compile it. I've run into this depreciation policy before. With aic7xxx_old (which I eventually convinced Fedora to disable, a few relases before it was removed from the tree). With aic94xx (which I also convinced Fedora to disable). I also tried multiple times to shut up advansys' compile time[1]. It seems SCSI might risk not to notice their bitrot, because eventually everybody stops compiling these obsolete drivers, leaving SCSI without feedback on their state. Why would we care? If it compiles that's fine, it's not causing a problem and it might just be useful to somebody. The time obsolete drivers cause problems is tree or subsystem wide API changes. Then we look at the amount of work required and sometimes remove them or do hack fixes. Anyhow, SCSI seems to be the only subsystem that uses this subcategory of not-really-maintained drivers. Other subsystems appear to allow anything to be fixed, even trivialities, which are what I tend to fix, and only stop allowing fixes if the drivers involved are going to be removed anyway. But maybe I just never ran into that category in other subsystems. Try ide ... they have the same policy. Try to understand the reason: we have a long tail of people using obsolete systems who we try not to break. Any change to an unmaintained driver which can't be tested risks that ... and I'm the one who would have to try to sort out the problem when it's noticed, hence the caution. If we allow trivial churn, by the time the breakage is noticed (usually months to years later), the driver will have picked up a ton of changes and finding the problem one becomes really hard. So unmaintained drivers get a default deep freeze maintenance policy. Even for a maintained driver, if the maintainer has little access to the hardware and few users they may also choose a deep freeze maintenance policy for the same reasons above; that's why no changes without maintainer ack. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] iser-target: Add missing target_put_sess_cmd for ImmedateData failure
On 5/23/2014 10:32 PM, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org This patch addresses a bug where an early exception for SCSI WRITE with ImmediateData=Yes was missing the target_put_sess_cmd() call to drop the extra se_cmd-cmd_kref reference obtained during the normal iscsit_setup_scsi_cmd() codepath execution. This bug was manifesting itself during session shutdown within isert_cq_rx_comp_err() where target_wait_for_sess_cmds() would end up waiting indefinately for the last se_cmd-cmd_kref put to occur for the failed SCSI WRITE + ImmediateData descriptors. This fix follows what traditional iscsi-target code already does for the same failure case within iscsit_get_immediate_data(). Reported-by: Sagi Grimberg sa...@dev.mellanox.co.il Cc: Sagi Grimberg sa...@dev.mellanox.co.il Cc: Or Gerlitz ogerl...@mellanox.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org --- drivers/infiniband/ulp/isert/ib_isert.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c index b622783..52c5ab8 100644 --- a/drivers/infiniband/ulp/isert/ib_isert.c +++ b/drivers/infiniband/ulp/isert/ib_isert.c @@ -1209,6 +1209,8 @@ sequence_cmd: if (!rc dump_payload == false unsol_data) iscsit_set_unsoliticed_dataout(cmd); + else if (dump_payload imm_data) + target_put_sess_cmd(conn-sess-se_sess, cmd-se_cmd); return 0; } Thanks Nic! Tested-by: Sagi Grimberg sa...@mellanox.com -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: esp_scsi QTAG in FAS216
Hi Michael,
[sorry for the long delay]
On Mon, Apr 14, 2014 at 10:51 AM, Michael Schmitz schmitz...@gmail.com wrote:
Not sure my patch had ever made it into Geert's m68k-queue - except for the
patch in my previous mail, my zorro_esp.c is still the same as I got from
you in October last year. The project has been on the back burner for too
long ...
I don't think it makes much sense to add it to my queue, as it's purely SCSI
and not critical for current m68k.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] scsi: increase upper limit for max_sectors
This patchset increases the SCSI mid level's limitation for max_sectors upto the block layer's limitation for max_hw_sectors, and enables the scsi_debug driver to request read/write commands with huge transfer length. This also includes a few fixes for the problems which can be caused by huge max_sectors in the block layer. Akinobu Mita (4): block: fix BLKSECTGET ioctl when max_sectors is greater than USHRT_MAX block: fix SG_[GS]ET_RESERVED_SIZE ioctl when max_sectors is huge scsi: increase upper limit for max_sectors scsi_debug: allow huge transfer length for read/write commands block/compat_ioctl.c | 6 -- block/ioctl.c | 5 - block/scsi_ioctl.c| 15 +++ drivers/scsi/scsi_debug.c | 6 +++--- drivers/scsi/sd.c | 5 + drivers/scsi/sg.c | 17 + include/scsi/scsi_host.h | 4 ++-- 7 files changed, 38 insertions(+), 20 deletions(-) Cc: Jens Axboe ax...@kernel.dk Cc: James E.J. Bottomley jbottom...@parallels.com Cc: Douglas Gilbert dgilb...@interlog.com Cc: linux-scsi@vger.kernel.org -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] block: fix BLKSECTGET ioctl when max_sectors is greater than USHRT_MAX
BLKSECTGET ioctl loads the request queue's max_sectors as unsigned
short value to the argument pointer. So if the max_sector is greater
than USHRT_MAX, the upper 16 bits of that is just discarded.
In such case, USHRT_MAX is more preferable than the lower 16 bits of
max_sectors.
Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Jens Axboe ax...@kernel.dk
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: linux-scsi@vger.kernel.org
---
block/compat_ioctl.c | 6 --
block/ioctl.c| 5 -
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/block/compat_ioctl.c b/block/compat_ioctl.c
index fbd5a67..e0393cd 100644
--- a/block/compat_ioctl.c
+++ b/block/compat_ioctl.c
@@ -663,6 +663,7 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd,
unsigned long arg)
fmode_t mode = file-f_mode;
struct backing_dev_info *bdi;
loff_t size;
+ unsigned int max_sectors;
/*
* O_NDELAY can be altered using fcntl(.., F_SETFL, ..), so we have
@@ -718,8 +719,9 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd,
unsigned long arg)
case BLKSSZGET: /* get block device hardware sector size */
return compat_put_int(arg, bdev_logical_block_size(bdev));
case BLKSECTGET:
- return compat_put_ushort(arg,
-
queue_max_sectors(bdev_get_queue(bdev)));
+ max_sectors = min_t(unsigned int, USHRT_MAX,
+ queue_max_sectors(bdev_get_queue(bdev)));
+ return compat_put_ushort(arg, max_sectors);
case BLKROTATIONAL:
return compat_put_ushort(arg,
!blk_queue_nonrot(bdev_get_queue(bdev)));
diff --git a/block/ioctl.c b/block/ioctl.c
index 7d5c3b2..d6cda81 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -278,6 +278,7 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode,
unsigned cmd,
struct backing_dev_info *bdi;
loff_t size;
int ret, n;
+ unsigned int max_sectors;
switch(cmd) {
case BLKFLSBUF:
@@ -375,7 +376,9 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode,
unsigned cmd,
case BLKDISCARDZEROES:
return put_uint(arg, bdev_discard_zeroes_data(bdev));
case BLKSECTGET:
- return put_ushort(arg, queue_max_sectors(bdev_get_queue(bdev)));
+ max_sectors = min_t(unsigned int, USHRT_MAX,
+ queue_max_sectors(bdev_get_queue(bdev)));
+ return put_ushort(arg, max_sectors);
case BLKROTATIONAL:
return put_ushort(arg, !blk_queue_nonrot(bdev_get_queue(bdev)));
case BLKRASET:
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] scsi: increase upper limit for max_sectors
max_sectors in struct Scsi_Host specifies maximum number of sectors
allowed in a single SCSI command. The data type of max_sectors is
unsigned short, so the maximum transfer length per SCSI command is
limited to less than 256MB in 4096-bytes sector size. (0x * 4096)
This commit increases the SCSI mid level's limitation for max_sectors
upto the block layer's limitation for max_hw_sectors by extending the
data type of max_sectors in struct Scsi_Host and scsi_host_template,
so that SCSI lower level drivers can specify more than 0x.
This change requires the scsi disk (sd) driver to handle the requests
whose transfer length is more than 0x with READ_16 or WRITE_16.
Also, this needs to prevent SG_GET_RESERVED_SIZE and SG_SET_RESERVED_SIZE
ioctls for the scsi generic (sg) driver from integer overflow when
converting max_sectors to bytes.
Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Jens Axboe ax...@kernel.dk
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: linux-scsi@vger.kernel.org
---
drivers/scsi/sd.c| 5 +
drivers/scsi/sg.c| 17 +
include/scsi/scsi_host.h | 4 ++--
3 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index efcbcd1..4508115 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1056,7 +1056,7 @@ static int sd_prep_fn(struct request_queue *q, struct
request *rq)
SCpnt-cmnd[29] = (unsigned char) (this_count 16) 0xff;
SCpnt-cmnd[30] = (unsigned char) (this_count 8) 0xff;
SCpnt-cmnd[31] = (unsigned char) this_count 0xff;
- } else if (sdp-use_16_for_rw) {
+ } else if ((this_count 0x) || sdp-use_16_for_rw) {
SCpnt-cmnd[0] += READ_16 - READ_6;
SCpnt-cmnd[1] = protect | ((rq-cmd_flags REQ_FUA) ? 0x8 :
0);
SCpnt-cmnd[2] = sizeof(block) 4 ? (unsigned char) (block
56) 0xff : 0;
@@ -1075,9 +1075,6 @@ static int sd_prep_fn(struct request_queue *q, struct
request *rq)
} else if ((this_count 0xff) || (block 0x1f) ||
scsi_device_protection(SCpnt-device) ||
SCpnt-device-use_10_for_rw) {
- if (this_count 0x)
- this_count = 0x;
-
SCpnt-cmnd[0] += READ_10 - READ_6;
SCpnt-cmnd[1] = protect | ((rq-cmd_flags REQ_FUA) ? 0x8 :
0);
SCpnt-cmnd[2] = (unsigned char) (block 24) 0xff;
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index df5e961..e3404d2 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -806,6 +806,15 @@ static int srp_done(Sg_fd *sfp, Sg_request *srp)
return ret;
}
+static int max_sectors_bytes(struct request_queue *q)
+{
+ unsigned int max_sectors = queue_max_sectors(q);
+
+ max_sectors = min_t(unsigned int, max_sectors, INT_MAX 9);
+
+ return max_sectors 9;
+}
+
static long
sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
{
@@ -945,7 +954,7 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned
long arg)
if (val 0)
return -EINVAL;
val = min_t(int, val,
- queue_max_sectors(sdp-device-request_queue) *
512);
+ max_sectors_bytes(sdp-device-request_queue));
if (val != sfp-reserve.bufflen) {
if (sg_res_in_use(sfp) || sfp-mmap_called)
return -EBUSY;
@@ -955,7 +964,7 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned
long arg)
return 0;
case SG_GET_RESERVED_SIZE:
val = min_t(int, sfp-reserve.bufflen,
- queue_max_sectors(sdp-device-request_queue) *
512);
+ max_sectors_bytes(sdp-device-request_queue));
return put_user(val, ip);
case SG_SET_COMMAND_Q:
result = get_user(val, ip);
@@ -1095,7 +1104,7 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned
long arg)
return -ENODEV;
return scsi_ioctl(sdp-device, cmd_in, p);
case BLKSECTGET:
- return put_user(queue_max_sectors(sdp-device-request_queue) *
512,
+ return put_user(max_sectors_bytes(sdp-device-request_queue),
ip);
case BLKTRACESETUP:
return blk_trace_setup(sdp-device-request_queue,
@@ -2086,7 +2095,7 @@ sg_add_sfp(Sg_device * sdp, int dev)
sg_big_buff = def_reserved_size;
bufflen = min_t(int, sg_big_buff,
- queue_max_sectors(sdp-device-request_queue) * 512);
+ max_sectors_bytes(sdp-device-request_queue));
sg_build_reserve(sfp, bufflen);
SCSI_LOG_TIMEOUT(3, printk(sg_add_sfp: bufflen=%d, k_use_sg=%d\n,
[PATCH 4/4] scsi_debug: allow huge transfer length for read/write commands
This change enables to test read/write commands with huge transfer
length such as 1GB. For example:
# modprobe scsi_debug dev_size_mb=1024 clustering=1 opts=1
# cat /sys/block/$DEV/queue/max_hw_sectors_kb \
/sys/block/$DEV/queue/max_sectors_kb
# fio --name=test --rw=write --bs=1g --size=1g --filename=/dev/$DEV \
--mem=mmaphuge --direct=1
The data type of max_sectors in scsi_host_template has been extended
to unsigned int by the previous change. So we can increase it from
0x to 0x to allow such huge transfer length.
Also, this increases sg_tablesize and max_segment_size, otherwise the
maximum transfer length is limited to 64MB.
(sg_tablesize * max_segment_size = 256 * 256KB)
Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Jens Axboe ax...@kernel.dk
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: linux-scsi@vger.kernel.org
---
drivers/scsi/scsi_debug.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index f3e9cc0..35ce1fa 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -2483,7 +2483,7 @@ static int scsi_debug_slave_configure(struct scsi_device
*sdp)
if (sdp-host-cmd_per_lun)
scsi_adjust_queue_depth(sdp, SDEBUG_TAGGED_QUEUING,
sdp-host-cmd_per_lun);
- blk_queue_max_segment_size(sdp-request_queue, 256 * 1024);
+ blk_queue_max_segment_size(sdp-request_queue, -1U);
if (scsi_debug_no_uld)
sdp-no_uld_attach = 1;
return 0;
@@ -3938,9 +3938,9 @@ static struct scsi_host_template sdebug_driver_template =
{
.bios_param = scsi_debug_biosparam,
.can_queue =SCSI_DEBUG_CANQUEUE,
.this_id = 7,
- .sg_tablesize = 256,
+ .sg_tablesize = SCSI_MAX_SG_CHAIN_SEGMENTS,
.cmd_per_lun = 16,
- .max_sectors = 0x,
+ .max_sectors = -1U,
.use_clustering = DISABLE_CLUSTERING,
.module = THIS_MODULE,
};
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] block: fix SG_[GS]ET_RESERVED_SIZE ioctl when max_sectors is huge
SG_GET_RESERVED_SIZE and SG_SET_RESERVED_SIZE ioctls access a reserved
buffer in bytes as int type. The value needs to be capped at the request
queue's max_sectors. But integer overflow is not correctly handled in
the calculation when converting max_sectors from sectors to bytes.
Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Jens Axboe ax...@kernel.dk
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: linux-scsi@vger.kernel.org
---
block/scsi_ioctl.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 2648797..6174bba 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -82,9 +82,18 @@ static int sg_set_timeout(struct request_queue *q, int
__user *p)
return err;
}
+static int max_sectors_bytes(struct request_queue *q)
+{
+ unsigned int max_sectors = queue_max_sectors(q);
+
+ max_sectors = min_t(unsigned int, max_sectors, INT_MAX 9);
+
+ return max_sectors 9;
+}
+
static int sg_get_reserved_size(struct request_queue *q, int __user *p)
{
- unsigned val = min(q-sg_reserved_size, queue_max_sectors(q) 9);
+ int val = min_t(int, q-sg_reserved_size, max_sectors_bytes(q));
return put_user(val, p);
}
@@ -98,10 +107,8 @@ static int sg_set_reserved_size(struct request_queue *q,
int __user *p)
if (size 0)
return -EINVAL;
- if (size (queue_max_sectors(q) 9))
- size = queue_max_sectors(q) 9;
- q-sg_reserved_size = size;
+ q-sg_reserved_size = min(size, max_sectors_bytes(q));
return 0;
}
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
iSCSI Expected Data Transfer Length for T10-PI
Hey All, Recently, iSER end-to-end T10-PI support maid it mainline. I am wandering about the impact T10-PI should or shouldn't have on iSCSI header field Expected Data Transfer Length. RFC-7143 states: the Expected Data Transfer Length field contains the number of bytes of data involved in this SCSI operation. Since this field relates to *data bytes* I kept T10-PI implicit wrt this field. The iSCSI target calculates the total transfer length (data + protection) from the cdb transfer length field and protect bits. In FC, the fc_dl field was updated to relate to the total number of transfer bytes and includes data and protection bytes. virtio_scsi was added with a header PI section (virtio_scsi_cmd_req_pi). So my question is, should this field be updated to explicitly include T10-PI bytes like the FC equivalent fc_dl? Or should T10-PI bytes be implicit? I want to pin down this one to avoid a situation where the standard is open for interpretations. Thanks, Sagi. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: iSCSI Expected Data Transfer Length for T10-PI
I have some trouble parsing you English/Question. I think the intent of SCSI PI was that wherever the PI exist it should be checked end-to-end and it may be checked in between. A storage client (server) will have the PI appended in the memory when writing and reading and checking it. As the values are standardized storage may also check it both when reading and when writing but it should not change it. If by implicit you mean inclusive I assume iSCSI Expected data transfer length will take whatever is in the cdb. Block transfer devices will likely add the PI length to the pure data length - i.e. inclusive. iSER may allow placing PI in diferent memory areas than the pure data but I think it has already provisions for that. Regards, Julo On May 25, 2014, at 18:30, Sagi Grimberg sa...@dev.mellanox.co.il wrote: Hey All, Recently, iSER end-to-end T10-PI support maid it mainline. I am wandering about the impact T10-PI should or shouldn't have on iSCSI header field Expected Data Transfer Length. RFC-7143 states: the Expected Data Transfer Length field contains the number of bytes of data involved in this SCSI operation. Since this field relates to *data bytes* I kept T10-PI implicit wrt this field. The iSCSI target calculates the total transfer length (data + protection) from the cdb transfer length field and protect bits. In FC, the fc_dl field was updated to relate to the total number of transfer bytes and includes data and protection bytes. virtio_scsi was added with a header PI section (virtio_scsi_cmd_req_pi). So my question is, should this field be updated to explicitly include T10-PI bytes like the FC equivalent fc_dl? Or should T10-PI bytes be implicit? I want to pin down this one to avoid a situation where the standard is open for interpretations. Thanks, Sagi. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: iSCSI Expected Data Transfer Length for T10-PI
On 5/25/2014 10:39 PM, Julian Satran wrote: Hi Julian, I have some trouble parsing you English/Question. I'll try to clarify. I think the intent of SCSI PI was that wherever the PI exist it should be checked end-to-end and it may be checked in between. A storage client (server) will have the PI appended in the memory when writing and reading and checking it. As the values are standardized storage may also check it both when reading and when writing but it should not change it. All true. If by implicit you mean inclusive I assume iSCSI Expected data transfer length will take whatever is in the cdb. Block transfer devices will likely add the PI length to the pure data length - i.e. inclusive. My intention was that there is no explicit indication in the CDB that X additional PI bytes should be transferred, it is implied that each block is appended with 8 bytes of PI. For example: Say we have an iSCSI target exposing a LUN which is formatted with PI. The iSCSI initiator supports PI transfer as well. What do you expect iSCSI.ExpectedDataTransferLength field to be in the case of a single 512B block SCSI READ? 512? 520? My understanding is that you think it should be the number of pure data bytes (i.e. 512). As I mentioned, FC's fc_dl for example, was updated to be the *total* number of bytes (data + PI) in the SCSI operation (i.e. will be 520 in the above example). virtio_scsi header was updated to additionally specify the number of IN PI bytes and the number of OUT PI bytes involved. What I understood from the spec, is that iSCSI.ExpectedDataTransferLength corresponds to the number of *data* bytes involved in this SCSI operation. That's why I kept iSCSI.ExpectedDataTransferLength to be the number of pure data bytes. So I guess I'm basically asking is, should iSCSI header include the number of PI bytes that should be transferred over the wire as well? Hope things are clearer, Sagi. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: esp_scsi QTAG in FAS216
Hi Geert, [sorry for the long delay] Tell me about it :-) I haven't had a good idea how to address this problem so rather kept mum about it. On Mon, Apr 14, 2014 at 10:51 AM, Michael Schmitz schmitz...@gmail.com wrote: Not sure my patch had ever made it into Geert's m68k-queue - except for the patch in my previous mail, my zorro_esp.c is still the same as I got from you in October last year. The project has been on the back burner for too long ... I don't think it makes much sense to add it to my queue, as it's purely SCSI and not critical for current m68k. Fine, Ill try and resolve that with Dave then. Tuomas will need to do the testing (unless someone wants to drop off the necessary hardware with me - unlikely). Thanks, Michael -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND PATCH V7 2/6] scsi: ufs: make undeclared functions static
Make undeclared functions static and declare exported symbols
to suppress warnings from sparse tool.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
Acked-by: Vinayak Holikatti vinholika...@gmail.com
---
drivers/scsi/ufs/ufshcd.c | 4 ++--
drivers/scsi/ufs/ufshcd.h | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 064c9d9..d476cc3 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1148,7 +1148,7 @@ out_unlock:
*
* Returns 0 for success, non-zero in case of failure
*/
-int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
+static int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
enum attr_idn idn, u8 index, u8 selector, u32 *attr_val)
{
struct ufs_query_req *request;
@@ -1459,7 +1459,7 @@ EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
*
* Returns 0 on success, non-zero value on failure
*/
-int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode)
+static int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode)
{
struct uic_command uic_cmd = {0};
struct completion pwr_done;
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 577679a..767ee9e 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -263,6 +263,8 @@ static inline void check_upiu_size(void)
GENERAL_UPIU_REQUEST_SIZE + QUERY_DESC_MAX_SIZE);
}
+extern int ufshcd_suspend(struct ufs_hba *hba, pm_message_t state);
+extern int ufshcd_resume(struct ufs_hba *hba);
extern int ufshcd_runtime_suspend(struct ufs_hba *hba);
extern int ufshcd_runtime_resume(struct ufs_hba *hba);
extern int ufshcd_runtime_idle(struct ufs_hba *hba);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND PATCH V7 3/6] scsi: ufs: Fix broken task management command implementation
Currently, sending Task Management (TM) command to the card might
be broken in some scenarios as listed below:
Problem: If there are more than 8 TM commands the implementation
returns error to the caller.
Fix: Wait for one of the slots to be emptied and send the command.
Problem: Sometimes it is necessary for the caller to know the TM service
response code to determine the task status.
Fix: Propogate the service response to the caller.
Problem: If the TM command times out no proper error recovery is
implemented.
Fix: Clear the command in the controller door-bell register, so that
further commands for the same slot don't fail.
Problem: While preparing the TM command descriptor, the task tag used
should be unique across SCSI/NOP/QUERY/TM commands and not the
task tag of the command which the TM command is trying to manage.
Fix: Use a unique task tag instead of task tag of SCSI command.
Problem: Since the TM command involves H/W communication, abruptly ending
the request on kill interrupt signal might cause h/w malfunction.
Fix: Wait for hardware completion interrupt with TASK_UNINTERRUPTIBLE
set.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
Reviewed-by: Yaniv Gardi yga...@codeaurora.org
Tested-by: Dolev Raviv dra...@codeaurora.org
Acked-by: Vinayak Holikatti vinholika...@gmail.com
---
drivers/scsi/ufs/ufshcd.c | 169 +++---
drivers/scsi/ufs/ufshcd.h | 8 ++-
2 files changed, 122 insertions(+), 55 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index d476cc3..c3acadc 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -55,6 +55,9 @@
/* Query request timeout */
#define QUERY_REQ_TIMEOUT 30 /* msec */
+/* Task management command timeout */
+#define TM_CMD_TIMEOUT 100 /* msecs */
+
/* Expose the flag value from utp_upiu_query.value */
#define MASK_QUERY_UPIU_FLAG_LOC 0xFF
@@ -182,13 +185,35 @@ ufshcd_get_tmr_ocs(struct utp_task_req_desc
*task_req_descp)
/**
* ufshcd_get_tm_free_slot - get a free slot for task management request
* @hba: per adapter instance
+ * @free_slot: pointer to variable with available slot value
*
- * Returns maximum number of task management request slots in case of
- * task management queue full or returns the free slot number
+ * Get a free tag and lock it until ufshcd_put_tm_slot() is called.
+ * Returns 0 if free slot is not available, else return 1 with tag value
+ * in @free_slot.
*/
-static inline int ufshcd_get_tm_free_slot(struct ufs_hba *hba)
+static bool ufshcd_get_tm_free_slot(struct ufs_hba *hba, int *free_slot)
{
- return find_first_zero_bit(hba-outstanding_tasks, hba-nutmrs);
+ int tag;
+ bool ret = false;
+
+ if (!free_slot)
+ goto out;
+
+ do {
+ tag = find_first_zero_bit(hba-tm_slots_in_use, hba-nutmrs);
+ if (tag = hba-nutmrs)
+ goto out;
+ } while (test_and_set_bit_lock(tag, hba-tm_slots_in_use));
+
+ *free_slot = tag;
+ ret = true;
+out:
+ return ret;
+}
+
+static inline void ufshcd_put_tm_slot(struct ufs_hba *hba, int slot)
+{
+ clear_bit_unlock(slot, hba-tm_slots_in_use);
}
/**
@@ -1912,10 +1937,11 @@ static void ufshcd_slave_destroy(struct scsi_device
*sdev)
* ufshcd_task_req_compl - handle task management request completion
* @hba: per adapter instance
* @index: index of the completed request
+ * @resp: task management service response
*
- * Returns SUCCESS/FAILED
+ * Returns non-zero value on error, zero on success
*/
-static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index)
+static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index, u8 *resp)
{
struct utp_task_req_desc *task_req_descp;
struct utp_upiu_task_rsp *task_rsp_upiup;
@@ -1936,19 +1962,15 @@ static int ufshcd_task_req_compl(struct ufs_hba *hba,
u32 index)
task_req_descp[index].task_rsp_upiu;
task_result = be32_to_cpu(task_rsp_upiup-header.dword_1);
task_result = ((task_result MASK_TASK_RESPONSE) 8);
-
- if (task_result != UPIU_TASK_MANAGEMENT_FUNC_COMPL
- task_result != UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED)
- task_result = FAILED;
- else
- task_result = SUCCESS;
+ if (resp)
+ *resp = (u8)task_result;
} else {
- task_result = FAILED;
- dev_err(hba-dev,
- trc: Invalid ocs = %x\n, ocs_value);
+ dev_err(hba-dev, %s: failed, ocs = 0x%x\n,
+ __func__, ocs_value);
}
spin_unlock_irqrestore(hba-host-host_lock, flags);
- return task_result;
+
+ return ocs_value;
}
/**
@@ -2447,7 +2469,7 @@ static void
[RESEND PATCH V7 6/6] scsi: ufs: Improve UFS fatal error handling
Error handling in UFS driver is broken and resets the host controller
for fatal errors without re-initialization. Correct the fatal error
handling sequence according to UFS Host Controller Interface (HCI)
v1.1 specification.
o Processed requests which are completed w/wo error are reported to
SCSI layer and any pending commands that are not started are aborted
in the controller and re-queued into scsi mid-layer queue.
o Upon determining fatal error condition the host controller may hang
forever until a reset is applied. Block SCSI layer for sending new
requests and apply reset in a separate error handling work.
o SCSI is informed about the expected Unit-Attention exception from the
device for the immediate command after a reset so that the SCSI layer
take necessary steps to establish communication with the device.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
Reviewed-by: Yaniv Gardi yga...@codeaurora.org
Tested-by: Dolev Raviv dra...@codeaurora.org
Acked-by: Vinayak Holikatti vinholika...@gmail.com
---
drivers/scsi/ufs/ufshcd.c | 229 --
drivers/scsi/ufs/ufshcd.h | 10 +-
2 files changed, 149 insertions(+), 90 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 5462310..0c28772 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -84,6 +84,14 @@ enum {
UFSHCD_EH_IN_PROGRESS = (1 0),
};
+/* UFSHCD UIC layer error flags */
+enum {
+ UFSHCD_UIC_DL_PA_INIT_ERROR = (1 0), /* Data link layer error */
+ UFSHCD_UIC_NL_ERROR = (1 1), /* Network layer error */
+ UFSHCD_UIC_TL_ERROR = (1 2), /* Transport Layer error */
+ UFSHCD_UIC_DME_ERROR = (1 3), /* DME error */
+};
+
/* Interrupt configuration options */
enum {
UFSHCD_INT_DISABLE,
@@ -100,6 +108,8 @@ enum {
static void ufshcd_tmc_handler(struct ufs_hba *hba);
static void ufshcd_async_scan(void *data, async_cookie_t cookie);
+static int ufshcd_reset_and_restore(struct ufs_hba *hba);
+static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag);
/*
* ufshcd_wait_for_register - wait for register value to change
@@ -1735,9 +1745,6 @@ static int ufshcd_make_hba_operational(struct ufs_hba
*hba)
goto out;
}
- if (hba-ufshcd_state == UFSHCD_STATE_RESET)
- scsi_unblock_requests(hba-host);
-
out:
return err;
}
@@ -1863,66 +1870,6 @@ static int ufshcd_verify_dev_init(struct ufs_hba *hba)
}
/**
- * ufshcd_do_reset - reset the host controller
- * @hba: per adapter instance
- *
- * Returns SUCCESS/FAILED
- */
-static int ufshcd_do_reset(struct ufs_hba *hba)
-{
- struct ufshcd_lrb *lrbp;
- unsigned long flags;
- int tag;
-
- /* block commands from midlayer */
- scsi_block_requests(hba-host);
-
- spin_lock_irqsave(hba-host-host_lock, flags);
- hba-ufshcd_state = UFSHCD_STATE_RESET;
-
- /* send controller to reset state */
- ufshcd_hba_stop(hba);
- spin_unlock_irqrestore(hba-host-host_lock, flags);
-
- /* abort outstanding commands */
- for (tag = 0; tag hba-nutrs; tag++) {
- if (test_bit(tag, hba-outstanding_reqs)) {
- lrbp = hba-lrb[tag];
- if (lrbp-cmd) {
- scsi_dma_unmap(lrbp-cmd);
- lrbp-cmd-result = DID_RESET 16;
- lrbp-cmd-scsi_done(lrbp-cmd);
- lrbp-cmd = NULL;
- clear_bit_unlock(tag, hba-lrb_in_use);
- }
- }
- }
-
- /* complete device management command */
- if (hba-dev_cmd.complete)
- complete(hba-dev_cmd.complete);
-
- /* clear outstanding request/task bit maps */
- hba-outstanding_reqs = 0;
- hba-outstanding_tasks = 0;
-
- /* Host controller enable */
- if (ufshcd_hba_enable(hba)) {
- dev_err(hba-dev,
- Reset: Controller initialization failed\n);
- return FAILED;
- }
-
- if (ufshcd_link_startup(hba)) {
- dev_err(hba-dev,
- Reset: Link start-up failed\n);
- return FAILED;
- }
-
- return SUCCESS;
-}
-
-/**
* ufshcd_slave_alloc - handle initial SCSI device configurations
* @sdev: pointer to SCSI device
*
@@ -1939,6 +1886,9 @@ static int ufshcd_slave_alloc(struct scsi_device *sdev)
sdev-use_10_for_ms = 1;
scsi_set_tag_type(sdev, MSG_SIMPLE_TAG);
+ /* allow SCSI layer to restart the device in case of errors */
+ sdev-allow_restart = 1;
+
/*
* Inform SCSI Midlayer that the LUN queue depth is same as the
* controller queue depth. If a LUN queue depth is less than the
@@ -2134,6 +2084,9 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct
ufshcd_lrb *lrbp)
case
[RESEND PATCH V7 4/6] scsi: ufs: Fix hardware race conditions while aborting a command
There is a possible race condition in the hardware when the abort
command is issued to terminate the ongoing SCSI command as described
below:
- A bit in the door-bell register is set in the controller for a
new SCSI command.
- In some rare situations, before controller get a chance to issue
the command to the device, the software issued an abort command.
- If the device recieves abort command first then it returns success
because the command itself is not present.
- Now if the controller commits the command to device it will be
processed.
- Software thinks that command is aborted and proceed while still
the device is processing it.
- The software, controller and device may go out of sync because of
this race condition.
To avoid this, query task presence in the device before sending abort
task command so that after the abort operation, the command is guaranteed
to be non-existent in both controller and the device.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
Reviewed-by: Yaniv Gardi yga...@codeaurora.org
Tested-by: Dolev Raviv dra...@codeaurora.org
Acked-by: Vinayak Holikatti vinholika...@gmail.com
---
drivers/scsi/ufs/ufshcd.c | 70 +--
1 file changed, 55 insertions(+), 15 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c3acadc..52f66e4 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2695,6 +2695,12 @@ static int ufshcd_host_reset(struct scsi_cmnd *cmd)
* ufshcd_abort - abort a specific command
* @cmd: SCSI command pointer
*
+ * Abort the pending command in device by sending UFS_ABORT_TASK task
management
+ * command, and in host controller by clearing the door-bell register. There
can
+ * be race between controller sending the command to the device while abort is
+ * issued. To avoid that, first issue UFS_QUERY_TASK to check if the command is
+ * really issued and then try to abort it.
+ *
* Returns SUCCESS/FAILED
*/
static int ufshcd_abort(struct scsi_cmnd *cmd)
@@ -2703,7 +2709,8 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
struct ufs_hba *hba;
unsigned long flags;
unsigned int tag;
- int err;
+ int err = 0;
+ int poll_cnt;
u8 resp = 0xF;
struct ufshcd_lrb *lrbp;
@@ -2711,33 +2718,59 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
hba = shost_priv(host);
tag = cmd-request-tag;
- spin_lock_irqsave(host-host_lock, flags);
+ /* If command is already aborted/completed, return SUCCESS */
+ if (!(test_bit(tag, hba-outstanding_reqs)))
+ goto out;
- /* check if command is still pending */
- if (!(test_bit(tag, hba-outstanding_reqs))) {
- err = FAILED;
- spin_unlock_irqrestore(host-host_lock, flags);
+ lrbp = hba-lrb[tag];
+ for (poll_cnt = 100; poll_cnt; poll_cnt--) {
+ err = ufshcd_issue_tm_cmd(hba, lrbp-lun, lrbp-task_tag,
+ UFS_QUERY_TASK, resp);
+ if (!err resp == UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
+ /* cmd pending in the device */
+ break;
+ } else if (!err resp == UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
+ u32 reg;
+
+ /*
+* cmd not pending in the device, check if it is
+* in transition.
+*/
+ reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
+ if (reg (1 tag)) {
+ /* sleep for max. 200us to stabilize */
+ usleep_range(100, 200);
+ continue;
+ }
+ /* command completed already */
+ goto out;
+ } else {
+ if (!err)
+ err = resp; /* service response error */
+ goto out;
+ }
+ }
+
+ if (!poll_cnt) {
+ err = -EBUSY;
goto out;
}
- spin_unlock_irqrestore(host-host_lock, flags);
- lrbp = hba-lrb[tag];
err = ufshcd_issue_tm_cmd(hba, lrbp-lun, lrbp-task_tag,
UFS_ABORT_TASK, resp);
if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
- err = FAILED;
+ if (!err)
+ err = resp; /* service response error */
goto out;
- } else {
- err = SUCCESS;
}
+ err = ufshcd_clear_cmd(hba, tag);
+ if (err)
+ goto out;
+
scsi_dma_unmap(cmd);
spin_lock_irqsave(host-host_lock, flags);
-
- /* clear the respective UTRLCLR register bit */
- ufshcd_utrl_clear(hba, tag);
-
__clear_bit(tag, hba-outstanding_reqs);
[RESEND PATCH V7 5/6] scsi: ufs: Fix device and host reset methods
As of now SCSI initiated error handling is broken because,
the reset APIs don't try to bring back the device initialized and
ready for further transfers.
In case of timeouts, the scsi error handler takes care of handling aborts
and resets. Improve the error handling in such scenario by resetting the
device and host and re-initializing them in proper manner.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
Reviewed-by: Yaniv Gardi yga...@codeaurora.org
Tested-by: Dolev Raviv dra...@codeaurora.org
Acked-by: Vinayak Holikatti vinholika...@gmail.com
---
drivers/scsi/ufs/ufshcd.c | 240 --
drivers/scsi/ufs/ufshcd.h | 2 +
2 files changed, 189 insertions(+), 53 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 52f66e4..5462310 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -74,9 +74,14 @@ enum {
/* UFSHCD states */
enum {
- UFSHCD_STATE_OPERATIONAL,
UFSHCD_STATE_RESET,
UFSHCD_STATE_ERROR,
+ UFSHCD_STATE_OPERATIONAL,
+};
+
+/* UFSHCD error handling flags */
+enum {
+ UFSHCD_EH_IN_PROGRESS = (1 0),
};
/* Interrupt configuration options */
@@ -86,6 +91,16 @@ enum {
UFSHCD_INT_CLEAR,
};
+#define ufshcd_set_eh_in_progress(h) \
+ (h-eh_flags |= UFSHCD_EH_IN_PROGRESS)
+#define ufshcd_eh_in_progress(h) \
+ (h-eh_flags UFSHCD_EH_IN_PROGRESS)
+#define ufshcd_clear_eh_in_progress(h) \
+ (h-eh_flags = ~UFSHCD_EH_IN_PROGRESS)
+
+static void ufshcd_tmc_handler(struct ufs_hba *hba);
+static void ufshcd_async_scan(void *data, async_cookie_t cookie);
+
/*
* ufshcd_wait_for_register - wait for register value to change
* @hba - per-adapter interface
@@ -856,10 +871,25 @@ static int ufshcd_queuecommand(struct Scsi_Host *host,
struct scsi_cmnd *cmd)
tag = cmd-request-tag;
- if (hba-ufshcd_state != UFSHCD_STATE_OPERATIONAL) {
+ spin_lock_irqsave(hba-host-host_lock, flags);
+ switch (hba-ufshcd_state) {
+ case UFSHCD_STATE_OPERATIONAL:
+ break;
+ case UFSHCD_STATE_RESET:
err = SCSI_MLQUEUE_HOST_BUSY;
- goto out;
+ goto out_unlock;
+ case UFSHCD_STATE_ERROR:
+ set_host_byte(cmd, DID_ERROR);
+ cmd-scsi_done(cmd);
+ goto out_unlock;
+ default:
+ dev_WARN_ONCE(hba-dev, 1, %s: invalid state %d\n,
+ __func__, hba-ufshcd_state);
+ set_host_byte(cmd, DID_BAD_TARGET);
+ cmd-scsi_done(cmd);
+ goto out_unlock;
}
+ spin_unlock_irqrestore(hba-host-host_lock, flags);
/* acquire the tag to make sure device cmds don't use it */
if (test_and_set_bit_lock(tag, hba-lrb_in_use)) {
@@ -896,6 +926,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host,
struct scsi_cmnd *cmd)
/* issue command to the controller */
spin_lock_irqsave(hba-host-host_lock, flags);
ufshcd_send_command(hba, tag);
+out_unlock:
spin_unlock_irqrestore(hba-host-host_lock, flags);
out:
return err;
@@ -1707,8 +1738,6 @@ static int ufshcd_make_hba_operational(struct ufs_hba
*hba)
if (hba-ufshcd_state == UFSHCD_STATE_RESET)
scsi_unblock_requests(hba-host);
- hba-ufshcd_state = UFSHCD_STATE_OPERATIONAL;
-
out:
return err;
}
@@ -2455,8 +2484,12 @@ static void ufshcd_err_handler(struct ufs_hba *hba)
}
return;
fatal_eh:
- hba-ufshcd_state = UFSHCD_STATE_ERROR;
- schedule_work(hba-feh_workq);
+ /* handle fatal errors only when link is functional */
+ if (hba-ufshcd_state == UFSHCD_STATE_OPERATIONAL) {
+ /* block commands at driver layer until error is handled */
+ hba-ufshcd_state = UFSHCD_STATE_ERROR;
+ schedule_work(hba-feh_workq);
+ }
}
/**
@@ -2621,12 +2654,13 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba *hba, int
lun_id, int task_id,
}
/**
- * ufshcd_device_reset - reset device and abort all the pending commands
+ * ufshcd_eh_device_reset_handler - device reset handler registered to
+ *scsi layer.
* @cmd: SCSI command pointer
*
* Returns SUCCESS/FAILED
*/
-static int ufshcd_device_reset(struct scsi_cmnd *cmd)
+static int ufshcd_eh_device_reset_handler(struct scsi_cmnd *cmd)
{
struct Scsi_Host *host;
struct ufs_hba *hba;
@@ -2635,6 +2669,7 @@ static int ufshcd_device_reset(struct scsi_cmnd *cmd)
int err;
u8 resp = 0xF;
struct ufshcd_lrb *lrbp;
+ unsigned long flags;
host = cmd-device-host;
hba = shost_priv(host);
@@ -2643,55 +2678,33 @@ static int ufshcd_device_reset(struct scsi_cmnd *cmd)
lrbp = hba-lrb[tag];
err = ufshcd_issue_tm_cmd(hba, lrbp-lun, 0, UFS_LOGICAL_RESET, resp);
if (err || resp !=
[RESEND PATCH V7 0/6] scsi: ufs: TM, fatal-error handling and other fixes
This patch version fixes various static checker warnings along with fixing usage of deprecated flush_work_sync() API that led James drop the patchset from scsi-misc tree. The first patch fixes many issues with current task management handling in UFSHCD driver. Others improve error handling in various scenarios. These patches are rebased on scsi-misc and are ack'ed by UFS maintainer. Changes from v6: - Two new patches to fix static checker warnings - remove usage of deprecated flush_work_sync() API. Changes from v5: - correct typo in [PATCH V5 1/4] scsi: ufs: Fix broken task ... - correct poll wait in [PATCH V5 2/4] scsi: ufs: Fix hardware ... - Included Tested-by and Reviewed-by ack's. Changes from v4: - Addressed comments from Seungwon Jeon on V3 - Updated with proper locking while ufshcd state changes - Retained LUN reset instead of device reset with DME_END_POINT_RESET - Removed error handling decisions dependent on OCS value - Simplified fatal error handling to abort the requests first and then carry out reset. Changes from v3: - Rebased. Changes from v2: - [PATCH V3 1/4]: Make the task management command task tag unique across SCSI/NOP/QUERY request tags. - [PATCH V3 3/4]: While handling device/host reset, wait for pending fatal handler to return if running. Changes from v1: - [PATCH V2 1/4]: Fix a race condition because of overloading outstanding_tasks variable to lock the slots. A new variable tm_slots_in_use will track which slots are in use by the driver. - [PATCH V2 2/4]: Commit text update to clarify the hardware race with more details. - [PATCH V2 3/4]: Minor cleanup and rebase - [PATCH V2 4/4]: Fix a bug - sleeping in atomic context Sujit Reddy Thumma (6): scsi: ufs: fix endianness sparse warnings scsi: ufs: make undeclared functions static scsi: ufs: Fix broken task management command implementation scsi: ufs: Fix hardware race conditions while aborting a command scsi: ufs: Fix device and host reset methods scsi: ufs: Improve UFS fatal error handling drivers/scsi/ufs/ufs.h| 36 +-- drivers/scsi/ufs/ufshcd.c | 722 +++--- drivers/scsi/ufs/ufshcd.h | 22 +- drivers/scsi/ufs/ufshci.h | 32 +- 4 files changed, 545 insertions(+), 267 deletions(-) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND PATCH V7 1/6] scsi: ufs: fix endianness sparse warnings
Fix many warnings with incorrect endian assumptions
which makes the code unportable to new architectures.
The UFS specification defines the byte order as big-endian
for UPIU structure and little-endian for the host controller
transfer/task management descriptors.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
Acked-by: Vinayak Holikatti vinholika...@gmail.com
---
drivers/scsi/ufs/ufs.h| 36 ++--
drivers/scsi/ufs/ufshcd.c | 42 --
drivers/scsi/ufs/ufshci.h | 32
3 files changed, 42 insertions(+), 68 deletions(-)
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 7210500..f42d1ce 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -196,9 +196,9 @@ enum {
* @dword_2: UPIU header DW-2
*/
struct utp_upiu_header {
- u32 dword_0;
- u32 dword_1;
- u32 dword_2;
+ __be32 dword_0;
+ __be32 dword_1;
+ __be32 dword_2;
};
/**
@@ -207,7 +207,7 @@ struct utp_upiu_header {
* @cdb: Command Descriptor Block CDB DW-4 to DW-7
*/
struct utp_upiu_cmd {
- u32 exp_data_transfer_len;
+ __be32 exp_data_transfer_len;
u8 cdb[MAX_CDB_SIZE];
};
@@ -228,10 +228,10 @@ struct utp_upiu_query {
u8 idn;
u8 index;
u8 selector;
- u16 reserved_osf;
- u16 length;
- u32 value;
- u32 reserved[2];
+ __be16 reserved_osf;
+ __be16 length;
+ __be32 value;
+ __be32 reserved[2];
};
/**
@@ -256,9 +256,9 @@ struct utp_upiu_req {
* @sense_data: Sense data field DW-8 to DW-12
*/
struct utp_cmd_rsp {
- u32 residual_transfer_count;
- u32 reserved[4];
- u16 sense_data_len;
+ __be32 residual_transfer_count;
+ __be32 reserved[4];
+ __be16 sense_data_len;
u8 sense_data[18];
};
@@ -286,10 +286,10 @@ struct utp_upiu_rsp {
*/
struct utp_upiu_task_req {
struct utp_upiu_header header;
- u32 input_param1;
- u32 input_param2;
- u32 input_param3;
- u32 reserved[2];
+ __be32 input_param1;
+ __be32 input_param2;
+ __be32 input_param3;
+ __be32 reserved[2];
};
/**
@@ -301,9 +301,9 @@ struct utp_upiu_task_req {
*/
struct utp_upiu_task_rsp {
struct utp_upiu_header header;
- u32 output_param1;
- u32 output_param2;
- u32 reserved[3];
+ __be32 output_param1;
+ __be32 output_param2;
+ __be32 reserved[3];
};
/**
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 04884d6..064c9d9 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -163,7 +163,7 @@ static inline int ufshcd_is_device_present(u32 reg_hcs)
*/
static inline int ufshcd_get_tr_ocs(struct ufshcd_lrb *lrbp)
{
- return lrbp-utr_descriptor_ptr-header.dword_2 MASK_OCS;
+ return le32_to_cpu(lrbp-utr_descriptor_ptr-header.dword_2) MASK_OCS;
}
/**
@@ -176,7 +176,7 @@ static inline int ufshcd_get_tr_ocs(struct ufshcd_lrb *lrbp)
static inline int
ufshcd_get_tmr_ocs(struct utp_task_req_desc *task_req_descp)
{
- return task_req_descp-header.dword_2 MASK_OCS;
+ return le32_to_cpu(task_req_descp-header.dword_2) MASK_OCS;
}
/**
@@ -390,26 +390,6 @@ static inline void ufshcd_copy_sense_data(struct
ufshcd_lrb *lrbp)
}
/**
- * ufshcd_query_to_cpu() - formats the buffer to native cpu endian
- * @response: upiu query response to convert
- */
-static inline void ufshcd_query_to_cpu(struct utp_upiu_query *response)
-{
- response-length = be16_to_cpu(response-length);
- response-value = be32_to_cpu(response-value);
-}
-
-/**
- * ufshcd_query_to_be() - formats the buffer to big endian
- * @request: upiu query request to convert
- */
-static inline void ufshcd_query_to_be(struct utp_upiu_query *request)
-{
- request-length = cpu_to_be16(request-length);
- request-value = cpu_to_be32(request-value);
-}
-
-/**
* ufshcd_copy_query_response() - Copy the Query Response and the data
* descriptor
* @hba: per adapter instance
@@ -425,7 +405,6 @@ void ufshcd_copy_query_response(struct ufs_hba *hba, struct
ufshcd_lrb *lrbp)
UPIU_RSP_CODE_OFFSET;
memcpy(query_res-upiu_res, lrbp-ucd_rsp_ptr-qr, QUERY_OSF_SIZE);
- ufshcd_query_to_cpu(query_res-upiu_res);
/* Get the descriptor */
@@ -749,7 +728,7 @@ static void ufshcd_prepare_utp_query_req_upiu(struct
ufs_hba *hba,
{
struct utp_upiu_req *ucd_req_ptr = lrbp-ucd_req_ptr;
struct ufs_query *query = hba-dev_cmd.query;
- u16 len = query-request.upiu_req.length;
+ u16 len = be16_to_cpu(query-request.upiu_req.length);
u8 *descp = (u8 *)lrbp-ucd_req_ptr + GENERAL_UPIU_REQUEST_SIZE;
/* Query request header */
@@ -766,7 +745,6 @@ static void ufshcd_prepare_utp_query_req_upiu(struct
ufs_hba *hba,
/* Copy the Query Request buffer as
Re: [PATCH V7 1/6] scsi: ufs: fix endianness sparse warnings
On 11/18/2013 2:34 PM, vinayak holikatti wrote:
On Mon, Sep 23, 2013 at 5:44 PM, vinayak holikatti
vinholika...@gmail.com wrote:
On Thu, Sep 19, 2013 at 4:44 PM, Sujit Reddy Thumma
sthu...@codeaurora.org wrote:
Fix many warnings with incorrect endian assumptions
which makes the code unportable to new architectures.
The UFS specification defines the byte order as big-endian
for UPIU structure and little-endian for the host controller
transfer/task management descriptors.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
---
drivers/scsi/ufs/ufs.h| 36 ++--
drivers/scsi/ufs/ufshcd.c | 42 --
drivers/scsi/ufs/ufshci.h | 32
3 files changed, 42 insertions(+), 68 deletions(-)
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 7210500..f42d1ce 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -196,9 +196,9 @@ enum {
* @dword_2: UPIU header DW-2
*/
struct utp_upiu_header {
- u32 dword_0;
- u32 dword_1;
- u32 dword_2;
+ __be32 dword_0;
+ __be32 dword_1;
+ __be32 dword_2;
};
/**
@@ -207,7 +207,7 @@ struct utp_upiu_header {
* @cdb: Command Descriptor Block CDB DW-4 to DW-7
*/
struct utp_upiu_cmd {
- u32 exp_data_transfer_len;
+ __be32 exp_data_transfer_len;
u8 cdb[MAX_CDB_SIZE];
};
@@ -228,10 +228,10 @@ struct utp_upiu_query {
u8 idn;
u8 index;
u8 selector;
- u16 reserved_osf;
- u16 length;
- u32 value;
- u32 reserved[2];
+ __be16 reserved_osf;
+ __be16 length;
+ __be32 value;
+ __be32 reserved[2];
};
/**
@@ -256,9 +256,9 @@ struct utp_upiu_req {
* @sense_data: Sense data field DW-8 to DW-12
*/
struct utp_cmd_rsp {
- u32 residual_transfer_count;
- u32 reserved[4];
- u16 sense_data_len;
+ __be32 residual_transfer_count;
+ __be32 reserved[4];
+ __be16 sense_data_len;
u8 sense_data[18];
};
@@ -286,10 +286,10 @@ struct utp_upiu_rsp {
*/
struct utp_upiu_task_req {
struct utp_upiu_header header;
- u32 input_param1;
- u32 input_param2;
- u32 input_param3;
- u32 reserved[2];
+ __be32 input_param1;
+ __be32 input_param2;
+ __be32 input_param3;
+ __be32 reserved[2];
};
/**
@@ -301,9 +301,9 @@ struct utp_upiu_task_req {
*/
struct utp_upiu_task_rsp {
struct utp_upiu_header header;
- u32 output_param1;
- u32 output_param2;
- u32 reserved[3];
+ __be32 output_param1;
+ __be32 output_param2;
+ __be32 reserved[3];
};
/**
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 04884d6..064c9d9 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -163,7 +163,7 @@ static inline int ufshcd_is_device_present(u32 reg_hcs)
*/
static inline int ufshcd_get_tr_ocs(struct ufshcd_lrb *lrbp)
{
- return lrbp-utr_descriptor_ptr-header.dword_2 MASK_OCS;
+ return le32_to_cpu(lrbp-utr_descriptor_ptr-header.dword_2) MASK_OCS;
}
/**
@@ -176,7 +176,7 @@ static inline int ufshcd_get_tr_ocs(struct ufshcd_lrb *lrbp)
static inline int
ufshcd_get_tmr_ocs(struct utp_task_req_desc *task_req_descp)
{
- return task_req_descp-header.dword_2 MASK_OCS;
+ return le32_to_cpu(task_req_descp-header.dword_2) MASK_OCS;
}
/**
@@ -390,26 +390,6 @@ static inline void ufshcd_copy_sense_data(struct
ufshcd_lrb *lrbp)
}
/**
- * ufshcd_query_to_cpu() - formats the buffer to native cpu endian
- * @response: upiu query response to convert
- */
-static inline void ufshcd_query_to_cpu(struct utp_upiu_query *response)
-{
- response-length = be16_to_cpu(response-length);
- response-value = be32_to_cpu(response-value);
-}
-
-/**
- * ufshcd_query_to_be() - formats the buffer to big endian
- * @request: upiu query request to convert
- */
-static inline void ufshcd_query_to_be(struct utp_upiu_query *request)
-{
- request-length = cpu_to_be16(request-length);
- request-value = cpu_to_be32(request-value);
-}
-
-/**
* ufshcd_copy_query_response() - Copy the Query Response and the data
* descriptor
* @hba: per adapter instance
@@ -425,7 +405,6 @@ void ufshcd_copy_query_response(struct ufs_hba *hba, struct
ufshcd_lrb *lrbp)
UPIU_RSP_CODE_OFFSET;
memcpy(query_res-upiu_res, lrbp-ucd_rsp_ptr-qr, QUERY_OSF_SIZE);
- ufshcd_query_to_cpu(query_res-upiu_res);
/* Get the descriptor */
@@ -749,7 +728,7 @@ static void ufshcd_prepare_utp_query_req_upiu(struct
ufs_hba *hba,
{
struct utp_upiu_req *ucd_req_ptr = lrbp-ucd_req_ptr;
struct ufs_query *query = hba-dev_cmd.query;
- u16 len = query-request.upiu_req.length;
+ u16 len = be16_to_cpu(query-request.upiu_req.length);
u8 *descp = (u8
