From: Mattijs Korpershoek <mkorpershoek@baylibre.com>
To: Caleb Connolly <caleb.connolly@linaro.org>,
Marek Vasut <marex@denx.de>, Tom Rini <trini@konsulko.com>,
Lukasz Majewski <lukma@denx.de>,
Caleb Connolly <caleb.connolly@linaro.org>,
Neil Armstrong <neil.armstrong@linaro.org>,
Sumit Garg <sumit.garg@linaro.org>
Cc: u-boot@lists.denx.de
Subject: Re: [PATCH 4/5] usb: gadget: UMS: support multiple sector sizes
Date: Thu, 01 Feb 2024 14:49:40 +0100 [thread overview]
Message-ID: <878r44mgzv.fsf@baylibre.com> (raw)
In-Reply-To: <20240131-b4-qcom-usb-v1-4-6438b2a2285e@linaro.org>
Hi Caleb,
Thank you for the patch.
On mer., janv. 31, 2024 at 14:57, Caleb Connolly <caleb.connolly@linaro.org> wrote:
> UFS storage often uses a 4096-byte sector size, add support for dynamic
> sector sizes based loosely on the Linux implementation.
>
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> ---
> cmd/usb_mass_storage.c | 4 --
> drivers/usb/gadget/f_mass_storage.c | 101 ++++++++++++++++++++----------------
> drivers/usb/gadget/storage_common.c | 12 +++--
> include/usb_mass_storage.h | 1 -
> 4 files changed, 65 insertions(+), 53 deletions(-)
>
> diff --git a/cmd/usb_mass_storage.c b/cmd/usb_mass_storage.c
> index a8ddeb494628..751701fe73af 100644
> --- a/cmd/usb_mass_storage.c
> +++ b/cmd/usb_mass_storage.c
> @@ -88,10 +88,6 @@ static int ums_init(const char *devtype, const char *devnums_part_str)
> if (!strchr(devnum_part_str, ':'))
> partnum = 0;
>
> - /* f_mass_storage.c assumes SECTOR_SIZE sectors */
> - if (block_dev->blksz != SECTOR_SIZE)
> - goto cleanup;
> -
> ums_new = realloc(ums, (ums_count + 1) * sizeof(*ums));
> if (!ums_new)
> goto cleanup;
> diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
> index c725aed3f626..d880928044f4 100644
> --- a/drivers/usb/gadget/f_mass_storage.c
> +++ b/drivers/usb/gadget/f_mass_storage.c
> @@ -724,12 +724,13 @@ static int do_read(struct fsg_common *common)
> curlun->sense_data = SS_LOGICAL_BLOCK_ADDRESS_OUT_OF_RANGE;
> return -EINVAL;
> }
> - file_offset = ((loff_t) lba) << 9;
> + file_offset = ((loff_t)lba) << curlun->blkbits;
>
> /* Carry out the file reads */
> amount_left = common->data_size_from_cmnd;
> - if (unlikely(amount_left == 0))
> + if (unlikely(amount_left == 0)) {
> return -EIO; /* No default reply */
> + }
>
> for (;;) {
>
> @@ -768,13 +769,13 @@ static int do_read(struct fsg_common *common)
>
> /* Perform the read */
> rc = ums[common->lun].read_sector(&ums[common->lun],
> - file_offset / SECTOR_SIZE,
> - amount / SECTOR_SIZE,
> + file_offset / curlun->blksize,
> + amount / curlun->blksize,
> (char __user *)bh->buf);
> if (!rc)
> return -EIO;
>
> - nread = rc * SECTOR_SIZE;
> + nread = rc * curlun->blksize;
>
> VLDBG(curlun, "file read %u @ %llu -> %d\n", amount,
> (unsigned long long) file_offset,
> @@ -787,7 +788,7 @@ static int do_read(struct fsg_common *common)
> } else if (nread < amount) {
> LDBG(curlun, "partial file read: %d/%u\n",
> (int) nread, amount);
> - nread -= (nread & 511); /* Round down to a block */
> + nread -= (nread & (curlun->blksize - 1)); /* Round down to a block */
> }
> file_offset += nread;
> amount_left -= nread;
> @@ -861,7 +862,7 @@ static int do_write(struct fsg_common *common)
>
> /* Carry out the file writes */
> get_some_more = 1;
> - file_offset = usb_offset = ((loff_t) lba) << 9;
> + file_offset = usb_offset = ((loff_t)lba) << curlun->blkbits;
> amount_left_to_req = common->data_size_from_cmnd;
> amount_left_to_write = common->data_size_from_cmnd;
>
> @@ -893,7 +894,7 @@ static int do_write(struct fsg_common *common)
> curlun->info_valid = 1;
> continue;
> }
> - amount -= (amount & 511);
> + amount -= (amount & (curlun->blksize - 1));
> if (amount == 0) {
>
> /* Why were we were asked to transfer a
> @@ -942,12 +943,12 @@ static int do_write(struct fsg_common *common)
>
> /* Perform the write */
> rc = ums[common->lun].write_sector(&ums[common->lun],
> - file_offset / SECTOR_SIZE,
> - amount / SECTOR_SIZE,
> + file_offset / curlun->blksize,
> + amount / curlun->blksize,
> (char __user *)bh->buf);
> if (!rc)
> return -EIO;
> - nwritten = rc * SECTOR_SIZE;
> + nwritten = rc * curlun->blksize;
>
> VLDBG(curlun, "file write %u @ %llu -> %d\n", amount,
> (unsigned long long) file_offset,
> @@ -960,7 +961,7 @@ static int do_write(struct fsg_common *common)
> } else if (nwritten < amount) {
> LDBG(curlun, "partial file write: %d/%u\n",
> (int) nwritten, amount);
> - nwritten -= (nwritten & 511);
> + nwritten -= (nwritten & (curlun->blksize - 1));
> /* Round down to a block */
> }
> file_offset += nwritten;
> @@ -1034,8 +1035,8 @@ static int do_verify(struct fsg_common *common)
> return -EIO; /* No default reply */
>
> /* Prepare to carry out the file verify */
> - amount_left = verification_length << 9;
> - file_offset = ((loff_t) lba) << 9;
> + amount_left = verification_length << curlun->blkbits;
> + file_offset = ((loff_t) lba) << curlun->blkbits;
>
> /* Write out all the dirty buffers before invalidating them */
>
> @@ -1058,12 +1059,12 @@ static int do_verify(struct fsg_common *common)
>
> /* Perform the read */
> rc = ums[common->lun].read_sector(&ums[common->lun],
> - file_offset / SECTOR_SIZE,
> - amount / SECTOR_SIZE,
> + file_offset / curlun->blksize,
> + amount / curlun->blksize,
> (char __user *)bh->buf);
> if (!rc)
> return -EIO;
> - nread = rc * SECTOR_SIZE;
> + nread = rc * curlun->blksize;
>
> VLDBG(curlun, "file read %u @ %llu -> %d\n", amount,
> (unsigned long long) file_offset,
> @@ -1075,7 +1076,7 @@ static int do_verify(struct fsg_common *common)
> } else if (nread < amount) {
> LDBG(curlun, "partial file verify: %d/%u\n",
> (int) nread, amount);
> - nread -= (nread & 511); /* Round down to a sector */
> + nread -= (nread & (curlun->blksize - 1)); /* Round down to a sector */
> }
> if (nread == 0) {
> curlun->sense_data = SS_UNRECOVERED_READ_ERROR;
> @@ -1183,7 +1184,7 @@ static int do_read_capacity(struct fsg_common *common, struct fsg_buffhd *bh)
>
> put_unaligned_be32(curlun->num_sectors - 1, &buf[0]);
> /* Max logical block */
> - put_unaligned_be32(512, &buf[4]); /* Block length */
> + put_unaligned_be32(curlun->blksize, &buf[4]); /* Block length */
> return 8;
> }
>
> @@ -1370,7 +1371,7 @@ static int do_read_format_capacities(struct fsg_common *common,
>
> put_unaligned_be32(curlun->num_sectors, &buf[0]);
> /* Number of blocks */
> - put_unaligned_be32(512, &buf[4]); /* Block length */
> + put_unaligned_be32(curlun->blksize, &buf[4]); /* Block length */
> buf[4] = 0x02; /* Current capacity */
> return 12;
> }
> @@ -1781,6 +1782,16 @@ static int check_command(struct fsg_common *common, int cmnd_size,
> return 0;
> }
>
> +/* wrapper of check_command for data size in blocks handling */
> +static int check_command_size_in_blocks(struct fsg_common *common,
> + int cmnd_size, enum data_direction data_dir,
> + unsigned int mask, int needs_medium, const char *name)
> +{
> + common->data_size_from_cmnd <<= common->luns[common->lun].blkbits;
> + return check_command(common, cmnd_size, data_dir,
> + mask, needs_medium, name);
> +}
> +
>
> static int do_scsi_command(struct fsg_common *common)
> {
> @@ -1865,30 +1876,30 @@ static int do_scsi_command(struct fsg_common *common)
>
> case SC_READ_6:
> i = common->cmnd[4];
> - common->data_size_from_cmnd = (i == 0 ? 256 : i) << 9;
> - reply = check_command(common, 6, DATA_DIR_TO_HOST,
> - (7<<1) | (1<<4), 1,
> - "READ(6)");
> + common->data_size_from_cmnd = (i == 0 ? 256 : i);
> + reply = check_command_size_in_blocks(common, 6, DATA_DIR_TO_HOST,
> + (7<<1) | (1<<4), 1,
> + "READ(6)");
> if (reply == 0)
> reply = do_read(common);
> break;
>
> case SC_READ_10:
> common->data_size_from_cmnd =
> - get_unaligned_be16(&common->cmnd[7]) << 9;
> - reply = check_command(common, 10, DATA_DIR_TO_HOST,
> - (1<<1) | (0xf<<2) | (3<<7), 1,
> - "READ(10)");
> + get_unaligned_be16(&common->cmnd[7]);
> + reply = check_command_size_in_blocks(common, 10, DATA_DIR_TO_HOST,
> + (1<<1) | (0xf<<2) | (3<<7), 1,
> + "READ(10)");
> if (reply == 0)
> reply = do_read(common);
> break;
>
> case SC_READ_12:
> common->data_size_from_cmnd =
> - get_unaligned_be32(&common->cmnd[6]) << 9;
> - reply = check_command(common, 12, DATA_DIR_TO_HOST,
> - (1<<1) | (0xf<<2) | (0xf<<6), 1,
> - "READ(12)");
> + get_unaligned_be32(&common->cmnd[6]);
> + reply = check_command_size_in_blocks(common, 12, DATA_DIR_TO_HOST,
> + (1<<1) | (0xf<<2) | (0xf<<6), 1,
> + "READ(12)");
> if (reply == 0)
> reply = do_read(common);
> break;
> @@ -1983,30 +1994,30 @@ static int do_scsi_command(struct fsg_common *common)
>
> case SC_WRITE_6:
> i = common->cmnd[4];
> - common->data_size_from_cmnd = (i == 0 ? 256 : i) << 9;
> - reply = check_command(common, 6, DATA_DIR_FROM_HOST,
> - (7<<1) | (1<<4), 1,
> - "WRITE(6)");
> + common->data_size_from_cmnd = (i == 0 ? 256 : i);
> + reply = check_command_size_in_blocks(common, 6, DATA_DIR_FROM_HOST,
> + (7<<1) | (1<<4), 1,
> + "WRITE(6)");
> if (reply == 0)
> reply = do_write(common);
> break;
>
> case SC_WRITE_10:
> common->data_size_from_cmnd =
> - get_unaligned_be16(&common->cmnd[7]) << 9;
> - reply = check_command(common, 10, DATA_DIR_FROM_HOST,
> - (1<<1) | (0xf<<2) | (3<<7), 1,
> - "WRITE(10)");
> + get_unaligned_be16(&common->cmnd[7]);
> + reply = check_command_size_in_blocks(common, 10, DATA_DIR_FROM_HOST,
> + (1<<1) | (0xf<<2) | (3<<7), 1,
> + "WRITE(10)");
> if (reply == 0)
> reply = do_write(common);
> break;
>
> case SC_WRITE_12:
> common->data_size_from_cmnd =
> - get_unaligned_be32(&common->cmnd[6]) << 9;
> - reply = check_command(common, 12, DATA_DIR_FROM_HOST,
> - (1<<1) | (0xf<<2) | (0xf<<6), 1,
> - "WRITE(12)");
> + get_unaligned_be32(&common->cmnd[6]);
> + reply = check_command_size_in_blocks(common, 12, DATA_DIR_FROM_HOST,
> + (1<<1) | (0xf<<2) | (0xf<<6), 1,
> + "WRITE(12)");
> if (reply == 0)
> reply = do_write(common);
> break;
> @@ -2497,7 +2508,7 @@ static struct fsg_common *fsg_common_init(struct fsg_common *common,
> for (i = 0; i < nluns; i++) {
> common->luns[i].removable = 1;
>
> - rc = fsg_lun_open(&common->luns[i], ums[i].num_sectors, "");
> + rc = fsg_lun_open(&common->luns[i], ums[i].num_sectors, ums->block_dev.blksz, "");
> if (rc)
> goto error_luns;
> }
> diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
> index 5674e8fe4940..97dc6b6f729c 100644
> --- a/drivers/usb/gadget/storage_common.c
> +++ b/drivers/usb/gadget/storage_common.c
> @@ -269,6 +269,7 @@ struct device_attribute { int i; };
> #define ETOOSMALL 525
>
> #include <log.h>
> +#include <linux/log2.h>
> #include <usb_mass_storage.h>
> #include <dm/device_compat.h>
>
> @@ -290,6 +291,8 @@ struct fsg_lun {
> u32 sense_data;
> u32 sense_data_info;
> u32 unit_attention_data;
> + unsigned int blkbits;
> + unsigned int blksize; /* logical block size of bound block device */
>
> struct device dev;
> };
> @@ -566,7 +569,7 @@ static struct usb_gadget_strings fsg_stringtab = {
> */
>
> static int fsg_lun_open(struct fsg_lun *curlun, unsigned int num_sectors,
> - const char *filename)
> + unsigned int sector_size, const char *filename)
> {
> int ro;
>
> @@ -574,9 +577,12 @@ static int fsg_lun_open(struct fsg_lun *curlun, unsigned int num_sectors,
> ro = curlun->initially_ro;
>
> curlun->ro = ro;
> - curlun->file_length = num_sectors << 9;
> + curlun->file_length = num_sectors * sector_size;
> curlun->num_sectors = num_sectors;
> - debug("open backing file: %s\n", filename);
> + curlun->blksize = sector_size;
> + curlun->blkbits = order_base_2(sector_size >> 9) + 9;
> + debug("blksize: %u\n", sector_size);
> + debug("open backing file: '%s'\n", filename);
>
> return 0;
> }
> diff --git a/include/usb_mass_storage.h b/include/usb_mass_storage.h
> index 83ab93b530d7..6d83d93cad7f 100644
> --- a/include/usb_mass_storage.h
> +++ b/include/usb_mass_storage.h
> @@ -7,7 +7,6 @@
> #ifndef __USB_MASS_STORAGE_H__
> #define __USB_MASS_STORAGE_H__
>
> -#define SECTOR_SIZE 0x200
> #include <part.h>
> #include <linux/usb/composite.h>
>
>
> --
> 2.43.0
next prev parent reply other threads:[~2024-02-01 13:49 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-31 14:57 [PATCH 0/5] Qualcomm DWC3 USB support Caleb Connolly
2024-01-31 14:57 ` [PATCH 1/5] usb: dwc3-generic: implement Qualcomm wrapper Caleb Connolly
2024-02-01 9:34 ` Mattijs Korpershoek
2024-02-01 14:24 ` Caleb Connolly
2024-02-02 9:16 ` Mattijs Korpershoek
2024-02-06 20:36 ` Marek Vasut
2024-03-13 18:22 ` Caleb Connolly
2024-03-21 5:34 ` Marek Vasut
2024-03-21 9:25 ` Mattijs Korpershoek
2024-03-21 11:34 ` Caleb Connolly
2024-03-21 14:14 ` Mattijs Korpershoek
2024-01-31 14:57 ` [PATCH 2/5] usb: dwc3: select DM_USB_GADGET Caleb Connolly
2024-02-01 13:25 ` Mattijs Korpershoek
2024-01-31 14:57 ` [PATCH 3/5] usb: gadget: CDC ACM: call usb_gadget_initialize Caleb Connolly
2024-02-01 13:37 ` Mattijs Korpershoek
2024-01-31 14:57 ` [PATCH 4/5] usb: gadget: UMS: support multiple sector sizes Caleb Connolly
2024-02-01 13:49 ` Mattijs Korpershoek [this message]
2024-01-31 14:57 ` [PATCH 5/5] iommu: qcom-smmu: fix debugging Caleb Connolly
2024-02-01 13:51 ` Mattijs Korpershoek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=878r44mgzv.fsf@baylibre.com \
--to=mkorpershoek@baylibre.com \
--cc=caleb.connolly@linaro.org \
--cc=lukma@denx.de \
--cc=marex@denx.de \
--cc=neil.armstrong@linaro.org \
--cc=sumit.garg@linaro.org \
--cc=trini@konsulko.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.