[PATCH 1/2] usb: storage: refactor usb 32-bit capacity read

[PATCH 1/2] usb: storage: refactor usb 32-bit capacity read

[Ahmad Fatoum]

usb_stor_read_capacity uses SCSI Read Capacity 10, which assumes
capacity never exceeds 32-bit, which equals 2TiB max capacity at
512 byte sector size.

In preparation for porting support for SCSI Read Capacity 16 from
Linux, move over all Read Capacity 10 related code into a single
function. Some more refactoring is done to make the function look
more like the Linux implementation. This also makes it easier to spot
the differences in retry/timeout handling, which we might want to
adopt in future.

No functional change intended.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/storage/usb.c | 62 +++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 29 deletions(-)

diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index c264dd4b71e2..122af659820f 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -157,31 +157,49 @@ static int usb_stor_test_unit_ready(struct us_blk_dev *usb_blkdev)
 	return 0;
 }
 
-static int usb_stor_read_capacity(struct us_blk_dev *usb_blkdev,
-				  u32 *last_lba, u32 *block_length)
+static int read_capacity_10(struct us_blk_dev *usb_blkdev)
 {
 	struct device_d *dev = &usb_blkdev->us->pusb_dev->dev;
+	unsigned char cmd[16];
 	const u32 datalen = 8;
-	u32 *data = xzalloc(datalen);
-	u8 cmd[10];
+	__be32 *data = xzalloc(datalen);
 	int ret;
+	sector_t lba;
+	unsigned sector_size;
 
 	memset(cmd, 0, sizeof(cmd));
 	cmd[0] = SCSI_RD_CAPAC;
 
 	ret = usb_stor_transport(usb_blkdev, cmd, sizeof(cmd), data, datalen,
 				 3, USB_STOR_NO_REQUEST_SENSE);
-	if (ret < 0)
-		goto exit;
 
-	dev_dbg(dev, "Read Capacity returns: 0x%x, 0x%x\n",
-		data[0], data[1]);
-	*last_lba = be32_to_cpu(data[0]);
-	*block_length = be32_to_cpu(data[1]);
+	if (ret < 0) {
+		dev_dbg(dev, "Cannot read device capacity\n");
+		return ret;
+	}
 
-exit:
-	free(data);
-	return ret;
+	sector_size = be32_to_cpu(data[1]);
+	lba = be32_to_cpu(data[0]);
+
+	dev_dbg(dev, "LBA (10) = 0x%llx w/ sector size = %u\n",
+		lba, sector_size);
+
+
+	if (lba == U32_MAX) {
+		lba = U32_MAX - 1;
+		dev_warn(dev,
+			 "Limiting device size due to 32 bit constraints\n");
+		/* To support LBA >= U32_MAX, a READ CAPACITY (16) should be issued instead */
+	}
+
+
+	if (sector_size != SECTOR_SIZE)
+		dev_warn(dev, "Support only %d bytes sectors\n", SECTOR_SIZE);
+
+	usb_blkdev->blk.num_blocks = lba + 1;
+	usb_blkdev->blk.blockbits = SECTOR_SHIFT;
+
+	return SECTOR_SIZE;
 }
 
 static int usb_stor_io_10(struct us_blk_dev *usb_blkdev, u8 opcode,
@@ -275,7 +293,6 @@ static int usb_stor_init_blkdev(struct us_blk_dev *pblk_dev)
 {
 	struct us_data *us = pblk_dev->us;
 	struct device_d *dev = &us->pusb_dev->dev;
-	u32 last_lba = 0, block_length = 0;
 	int result;
 
 	/* get device info */
@@ -299,23 +316,10 @@ static int usb_stor_init_blkdev(struct us_blk_dev *pblk_dev)
 	/* read capacity */
 	dev_dbg(dev, "Reading capacity\n");
 
-	result = usb_stor_read_capacity(pblk_dev, &last_lba, &block_length);
-	if (result < 0) {
-		dev_dbg(dev, "Cannot read device capacity\n");
+	result = read_capacity_10(pblk_dev);
+	if (result < 0)
 		return result;
-	}
-
-	if (last_lba == U32_MAX) {
-		last_lba = U32_MAX - 1;
-		dev_warn(dev,
-			 "Limiting device size due to 32 bit constraints\n");
-		/* To support LBA >= U32_MAX, a READ CAPACITY (16) should be issued here */
-	}
 
-	pblk_dev->blk.num_blocks = last_lba + 1;
-	if (block_length != SECTOR_SIZE)
-		pr_warn("Support only %d bytes sectors\n", SECTOR_SIZE);
-	pblk_dev->blk.blockbits = SECTOR_SHIFT;
 	dev_dbg(dev, "Capacity = 0x%llx, blockshift = 0x%x\n",
 		 pblk_dev->blk.num_blocks, pblk_dev->blk.blockbits);
 
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] usb: storage: add support for drivers larger than 2TiB

[Ahmad Fatoum]

Consumer USB disks usually have emulated 512 byte sectors at the
USB/SCSI level, which means SCSI Read/Write/Capacity 10 can only
handle up to 2TiB USB disks. Add support for the optional 16 byte
command variants to handle disks larger than that.

Disks smaller than 2 TiB should not be affected.

Tested with 2 different 4TiB disks as well as one 2TiB disk.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/storage/usb.c | 94 +++++++++++++++++++++++++++++++++------
 include/scsi.h            |  5 +++
 2 files changed, 85 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 122af659820f..b417640186a2 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -157,6 +157,47 @@ static int usb_stor_test_unit_ready(struct us_blk_dev *usb_blkdev)
 	return 0;
 }
 
+static int read_capacity_16(struct us_blk_dev *usb_blkdev)
+{
+	struct device_d *dev = &usb_blkdev->us->pusb_dev->dev;
+	unsigned char cmd[16];
+	const u8 datalen = 32;
+	u8 *data = xzalloc(datalen);
+	int ret;
+	sector_t lba;
+	unsigned sector_size;
+
+	memset(cmd, 0, 16);
+	cmd[0] = SERVICE_ACTION_IN_16;
+	cmd[1] = SAI_READ_CAPACITY_16;
+	cmd[13] = datalen;
+
+	ret = usb_stor_transport(usb_blkdev, cmd, sizeof(cmd), data, datalen,
+				 3, USB_STOR_NO_REQUEST_SENSE);
+
+	if (ret < 0) {
+		dev_warn(dev, "Read Capacity(16) failed\n");
+		return ret;
+	}
+
+	/* Note this is logical, not physical sector size */
+	sector_size = be32_to_cpup((u32 *)&data[8]);
+	lba = be64_to_cpup((u64 *)&data[0]);
+
+	dev_dbg(dev, "LBA (16) = 0x%llx w/ sector size = %u\n",
+		lba, sector_size);
+
+	if ((data[12] & 1) == 1) {
+		dev_warn(dev, "Protection unsupported\n");
+		return -ENOTSUPP;
+	}
+
+	usb_blkdev->blk.blockbits = SECTOR_SHIFT;
+	usb_blkdev->blk.num_blocks = lba + 1;
+
+	return sector_size;
+}
+
 static int read_capacity_10(struct us_blk_dev *usb_blkdev)
 {
 	struct device_d *dev = &usb_blkdev->us->pusb_dev->dev;
@@ -174,7 +215,7 @@ static int read_capacity_10(struct us_blk_dev *usb_blkdev)
 				 3, USB_STOR_NO_REQUEST_SENSE);
 
 	if (ret < 0) {
-		dev_dbg(dev, "Cannot read device capacity\n");
+		dev_warn(dev, "Read Capacity(10) failed\n");
 		return ret;
 	}
 
@@ -184,15 +225,6 @@ static int read_capacity_10(struct us_blk_dev *usb_blkdev)
 	dev_dbg(dev, "LBA (10) = 0x%llx w/ sector size = %u\n",
 		lba, sector_size);
 
-
-	if (lba == U32_MAX) {
-		lba = U32_MAX - 1;
-		dev_warn(dev,
-			 "Limiting device size due to 32 bit constraints\n");
-		/* To support LBA >= U32_MAX, a READ CAPACITY (16) should be issued instead */
-	}
-
-
 	if (sector_size != SECTOR_SIZE)
 		dev_warn(dev, "Support only %d bytes sectors\n", SECTOR_SIZE);
 
@@ -202,6 +234,20 @@ static int read_capacity_10(struct us_blk_dev *usb_blkdev)
 	return SECTOR_SIZE;
 }
 
+static int usb_stor_io_16(struct us_blk_dev *usb_blkdev, u8 opcode,
+			  sector_t start, u8 *data, u16 blocks)
+{
+	u8 cmd[16];
+
+	memset(cmd, 0, sizeof(cmd));
+	cmd[0] = opcode;
+	put_unaligned_be64(start, &cmd[2]);
+	put_unaligned_be32(blocks, &cmd[10]);
+
+	return usb_stor_transport(usb_blkdev, cmd, sizeof(cmd), data,
+				  blocks * SECTOR_SIZE, 10, 0);
+}
+
 static int usb_stor_io_10(struct us_blk_dev *usb_blkdev, u8 opcode,
 			  sector_t start, u8 *data, u16 blocks)
 {
@@ -232,6 +278,7 @@ static int usb_stor_blk_io(struct block_device *disk_dev,
 						   blk);
 	struct us_data *us = pblk_dev->us;
 	struct device_d *dev = &us->pusb_dev->dev;
+	int result;
 
 	/* ensure unit ready */
 	dev_dbg(dev, "Testing for unit ready\n");
@@ -248,13 +295,24 @@ static int usb_stor_blk_io(struct block_device *disk_dev,
 	while (sector_count > 0) {
 		u16 n = min_t(blkcnt_t, sector_count, US_MAX_IO_BLK);
 
-		if (usb_stor_io_10(pblk_dev,
-				   read ? SCSI_READ10 : SCSI_WRITE10,
-				   sector_start,
-				   buffer, n)) {
+		if (disk_dev->num_blocks > 0xffffffff) {
+			result = usb_stor_io_16(pblk_dev,
+						read ? SCSI_READ16 : SCSI_WRITE16,
+						sector_start,
+						buffer, n);
+		} else {
+
+			result = usb_stor_io_10(pblk_dev,
+						read ? SCSI_READ10 : SCSI_WRITE10,
+						sector_start,
+						buffer, n);
+		}
+
+		if (result) {
 			dev_dbg(dev, "I/O error at sector %llu\n", sector_start);
 			break;
 		}
+
 		sector_start += n;
 		sector_count -= n;
 		buffer += n * SECTOR_SIZE;
@@ -320,6 +378,14 @@ static int usb_stor_init_blkdev(struct us_blk_dev *pblk_dev)
 	if (result < 0)
 		return result;
 
+	if (pblk_dev->blk.num_blocks > 0xffffffff) {
+		result = read_capacity_16(pblk_dev);
+		if (result < 0) {
+			dev_notice(dev, "Using 0xffffffff as device size\n");
+			pblk_dev->blk.num_blocks = 1 + (sector_t) 0xffffffff;
+		}
+	}
+
 	dev_dbg(dev, "Capacity = 0x%llx, blockshift = 0x%x\n",
 		 pblk_dev->blk.num_blocks, pblk_dev->blk.blockbits);
 
diff --git a/include/scsi.h b/include/scsi.h
index e2397489ead9..f84513b813e9 100644
--- a/include/scsi.h
+++ b/include/scsi.h
@@ -109,6 +109,7 @@
 #define SCSI_MED_REMOVL	0x1E		/* Prevent/Allow medium Removal (O) */
 #define SCSI_READ6	0x08		/* Read 6-byte (MANDATORY) */
 #define SCSI_READ10	0x28		/* Read 10-byte (MANDATORY) */
+#define SCSI_READ16	0x88		/* Read 16-byte (O) */
 #define SCSI_RD_CAPAC	0x25		/* Read Capacity (MANDATORY) */
 #define SCSI_RD_DEFECT	0x37		/* Read Defect Data (O) */
 #define SCSI_READ_LONG	0x3E		/* Read Long (O) */
@@ -128,10 +129,14 @@
 #define SCSI_VERIFY	0x2F		/* Verify (O) */
 #define SCSI_WRITE6	0x0A		/* Write 6-Byte (MANDATORY) */
 #define SCSI_WRITE10	0x2A		/* Write 10-Byte (MANDATORY) */
+#define SCSI_WRITE16	0x8A		/* Write 16-Byte (O) */
 #define SCSI_WRT_VERIFY	0x2E		/* Write and Verify (O) */
 #define SCSI_WRITE_LONG	0x3F		/* Write Long (O) */
 #define SCSI_WRITE_SAME	0x41		/* Write Same (O) */
 
+#define SERVICE_ACTION_IN_16	0x9e
+/* values for service action in */
+#define	SAI_READ_CAPACITY_16	0x10
 
 /****************************************************************************
  * decleration of functions which have to reside in the LowLevel Part Driver
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] usb: storage: refactor usb 32-bit capacity read

[Sascha Hauer]

On Thu, Feb 25, 2021 at 11:46:17AM +0100, Ahmad Fatoum wrote:
> usb_stor_read_capacity uses SCSI Read Capacity 10, which assumes
> capacity never exceeds 32-bit, which equals 2TiB max capacity at
> 512 byte sector size.
> 
> In preparation for porting support for SCSI Read Capacity 16 from
> Linux, move over all Read Capacity 10 related code into a single
> function. Some more refactoring is done to make the function look
> more like the Linux implementation. This also makes it easier to spot
> the differences in retry/timeout handling, which we might want to
> adopt in future.
> 
> No functional change intended.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/usb/storage/usb.c | 62 +++++++++++++++++++++------------------
>  1 file changed, 33 insertions(+), 29 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
> index c264dd4b71e2..122af659820f 100644
> --- a/drivers/usb/storage/usb.c
> +++ b/drivers/usb/storage/usb.c
> @@ -157,31 +157,49 @@ static int usb_stor_test_unit_ready(struct us_blk_dev *usb_blkdev)
>  	return 0;
>  }
>  
> -static int usb_stor_read_capacity(struct us_blk_dev *usb_blkdev,
> -				  u32 *last_lba, u32 *block_length)
> +static int read_capacity_10(struct us_blk_dev *usb_blkdev)
>  {
>  	struct device_d *dev = &usb_blkdev->us->pusb_dev->dev;
> +	unsigned char cmd[16];
>  	const u32 datalen = 8;
> -	u32 *data = xzalloc(datalen);
> -	u8 cmd[10];
> +	__be32 *data = xzalloc(datalen);
>  	int ret;
> +	sector_t lba;
> +	unsigned sector_size;
>  
>  	memset(cmd, 0, sizeof(cmd));
>  	cmd[0] = SCSI_RD_CAPAC;
>  
>  	ret = usb_stor_transport(usb_blkdev, cmd, sizeof(cmd), data, datalen,
>  				 3, USB_STOR_NO_REQUEST_SENSE);
> -	if (ret < 0)
> -		goto exit;
>  
> -	dev_dbg(dev, "Read Capacity returns: 0x%x, 0x%x\n",
> -		data[0], data[1]);
> -	*last_lba = be32_to_cpu(data[0]);
> -	*block_length = be32_to_cpu(data[1]);
> +	if (ret < 0) {
> +		dev_dbg(dev, "Cannot read device capacity\n");
> +		return ret;
> +	}
>  
> -exit:
> -	free(data);
> -	return ret;
> +	sector_size = be32_to_cpu(data[1]);
> +	lba = be32_to_cpu(data[0]);
> +
> +	dev_dbg(dev, "LBA (10) = 0x%llx w/ sector size = %u\n",
> +		lba, sector_size);
> +
> +
> +	if (lba == U32_MAX) {
> +		lba = U32_MAX - 1;
> +		dev_warn(dev,
> +			 "Limiting device size due to 32 bit constraints\n");
> +		/* To support LBA >= U32_MAX, a READ CAPACITY (16) should be issued instead */
> +	}
> +
> +
> +	if (sector_size != SECTOR_SIZE)
> +		dev_warn(dev, "Support only %d bytes sectors\n", SECTOR_SIZE);
> +
> +	usb_blkdev->blk.num_blocks = lba + 1;
> +	usb_blkdev->blk.blockbits = SECTOR_SHIFT;
> +
> +	return SECTOR_SIZE;
>  }
>  
>  static int usb_stor_io_10(struct us_blk_dev *usb_blkdev, u8 opcode,
> @@ -275,7 +293,6 @@ static int usb_stor_init_blkdev(struct us_blk_dev *pblk_dev)
>  {
>  	struct us_data *us = pblk_dev->us;
>  	struct device_d *dev = &us->pusb_dev->dev;
> -	u32 last_lba = 0, block_length = 0;
>  	int result;
>  
>  	/* get device info */
> @@ -299,23 +316,10 @@ static int usb_stor_init_blkdev(struct us_blk_dev *pblk_dev)
>  	/* read capacity */
>  	dev_dbg(dev, "Reading capacity\n");
>  
> -	result = usb_stor_read_capacity(pblk_dev, &last_lba, &block_length);
> -	if (result < 0) {
> -		dev_dbg(dev, "Cannot read device capacity\n");
> +	result = read_capacity_10(pblk_dev);
> +	if (result < 0)
>  		return result;
> -	}
> -
> -	if (last_lba == U32_MAX) {
> -		last_lba = U32_MAX - 1;
> -		dev_warn(dev,
> -			 "Limiting device size due to 32 bit constraints\n");
> -		/* To support LBA >= U32_MAX, a READ CAPACITY (16) should be issued here */
> -	}
>  
> -	pblk_dev->blk.num_blocks = last_lba + 1;
> -	if (block_length != SECTOR_SIZE)
> -		pr_warn("Support only %d bytes sectors\n", SECTOR_SIZE);
> -	pblk_dev->blk.blockbits = SECTOR_SHIFT;
>  	dev_dbg(dev, "Capacity = 0x%llx, blockshift = 0x%x\n",
>  		 pblk_dev->blk.num_blocks, pblk_dev->blk.blockbits);
>  
> -- 
> 2.29.2
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox