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

[Sascha Hauer <sha@pengutronix.de>]

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