| Message ID | 20090213144739.GL23889@oksana.dev.rtsoft.ru (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Kumar Gala |
| Headers | show |
On Fri, 13 Feb 2009 17:47:39 +0300 Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > @@ -831,7 +832,12 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data) > sdhci_set_transfer_irqs(host); > > /* We do not handle DMA boundaries, so set it to max (512 KiB) */ > - sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, data->blksz), SDHCI_BLOCK_SIZE); > + if (host->quirks & SDHCI_QUIRK_MAX_BLK_SZ_4096) > + blksz = data->blksz; > + else > + blksz = SDHCI_MAKE_BLKSZ(7, data->blksz); > + > + sdhci_writew(host, blksz, SDHCI_BLOCK_SIZE); > sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT); > } > Hmm.. I seem to have overlooked this part previously. I guess they've basically stripped out the DMA boundary stuff and used the bits for other things? At this point I'm leaning more towards simply not supporting their extended block size. After all, is it ever used? Rgds
On Sat, Feb 21, 2009 at 04:58:44PM +0100, Pierre Ossman wrote: > On Fri, 13 Feb 2009 17:47:39 +0300 > Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > > > @@ -831,7 +832,12 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data) > > sdhci_set_transfer_irqs(host); > > > > /* We do not handle DMA boundaries, so set it to max (512 KiB) */ > > - sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, data->blksz), SDHCI_BLOCK_SIZE); > > + if (host->quirks & SDHCI_QUIRK_MAX_BLK_SZ_4096) > > + blksz = data->blksz; > > + else > > + blksz = SDHCI_MAKE_BLKSZ(7, data->blksz); > > + > > + sdhci_writew(host, blksz, SDHCI_BLOCK_SIZE); > > sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT); > > } > > > > Hmm.. I seem to have overlooked this part previously. I guess they've > basically stripped out the DMA boundary stuff and used the bits for > other things? Yes, the last two "DMA boundary" bits are reserved, and the first one is re-used for blksz of 4096 bytes. > At this point I'm leaning more towards simply not supporting their > extended block size. Eh. But well, OK. We can always persuade you later. :-) I'll get rid of this particular patch, and put some BLOCK_SIZE magic into the writew accessor (to clean the DMA bits) instead. Though, I'll prepare another patch to force blksz to 2048, since eSDHC specifies "3" in the blksz capability bitfield, and that causes SDHCI core to fall back to the 512 byte blocks. > After all, is it ever used? Not sure, maybe `dd bs=' can use it? A bit lazy to check this right now, but from the quick tests, enabling/disabling "blksz of 4096 bytes" doesn't cause any performance change. At least with the ordinary SD cards.
On Wed, 4 Mar 2009 20:47:44 +0300 Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > > I'll get rid of this particular patch, and put some BLOCK_SIZE > magic into the writew accessor (to clean the DMA bits) instead. > > Though, I'll prepare another patch to force blksz to 2048, since > eSDHC specifies "3" in the blksz capability bitfield, and that > causes SDHCI core to fall back to the 512 byte blocks. > Ok. > > After all, is it ever used? > > Not sure, maybe `dd bs=' can use it? A bit lazy to check this > right now, but from the quick tests, enabling/disabling "blksz > of 4096 bytes" doesn't cause any performance change. At least > with the ordinary SD cards. > Memory cards will not use this (at least not with the current standards), as the block layer thinks in 512 byte blocks. Also, the sector size propagates to user space in a way that causes filesystems to behave differently, making cards incompatible with all other operating systems (i.e. if we don't use 512 byte blocks). So the only scenario where this might be used is SDIO, and I'm not sure such big blocks are a win there either because of the overhead of changing block size. Rgds
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index b308dbf..b341a9a 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -682,6 +682,7 @@ static void sdhci_set_transfer_irqs(struct sdhci_host *host) static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data) { + u16 blksz; u8 count; u8 ctrl; int ret; @@ -831,7 +832,12 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data) sdhci_set_transfer_irqs(host); /* We do not handle DMA boundaries, so set it to max (512 KiB) */ - sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, data->blksz), SDHCI_BLOCK_SIZE); + if (host->quirks & SDHCI_QUIRK_MAX_BLK_SZ_4096) + blksz = data->blksz; + else + blksz = SDHCI_MAKE_BLKSZ(7, data->blksz); + + sdhci_writew(host, blksz, SDHCI_BLOCK_SIZE); sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT); } @@ -1840,13 +1846,19 @@ int sdhci_add_host(struct sdhci_host *host) * Maximum block size. This varies from controller to controller and * is specified in the capabilities register. */ - mmc->max_blk_size = (caps & SDHCI_MAX_BLOCK_MASK) >> SDHCI_MAX_BLOCK_SHIFT; - if (mmc->max_blk_size >= 3) { - printk(KERN_WARNING "%s: Invalid maximum block size, " - "assuming 512 bytes\n", mmc_hostname(mmc)); - mmc->max_blk_size = 512; - } else - mmc->max_blk_size = 512 << mmc->max_blk_size; + if (host->quirks & SDHCI_QUIRK_MAX_BLK_SZ_4096) { + mmc->max_blk_size = 3; + } else { + mmc->max_blk_size = (caps & SDHCI_MAX_BLOCK_MASK) >> + SDHCI_MAX_BLOCK_SHIFT; + if (mmc->max_blk_size >= 3) { + printk(KERN_WARNING "%s: Invalid maximum block size, " + "assuming 512 bytes\n", mmc_hostname(mmc)); + mmc->max_blk_size = 0; + } + } + + mmc->max_blk_size = 512 << mmc->max_blk_size; /* * Maximum block count. diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index 5c5a950..c8628b4 100644 --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@ -231,6 +231,8 @@ struct sdhci_host { #define SDHCI_QUIRK_PIO_NEEDS_DELAY (1<<20) /* Controller losing signal/interrupt enable states after reset */ #define SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET (1<<21) +/* Controller supports nonstandard maximum block length of 4096 bytes */ +#define SDHCI_QUIRK_MAX_BLK_SZ_4096 (1<<22) int irq; /* Device IRQ */ void __iomem * ioaddr; /* Mapped address */
FSL eSDHC controllers can support maximum block size up to 4096 bytes. The MBL (Maximum Block Length) field in the capabilities register extended by one bit, and bits 13:15 in the block size register reserved. Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> --- drivers/mmc/host/sdhci.c | 28 ++++++++++++++++++++-------- drivers/mmc/host/sdhci.h | 2 ++ 2 files changed, 22 insertions(+), 8 deletions(-)