| Message ID | 20201218075312.67338-1-lebed.dmitry@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: meson-gx: check for scatterlist size alignment in block mode | expand |
Am 18.12.2020 um 08:53 schrieb Dmitry Lebed: > Enable SGDMA support for SD_IO_RW_EXTENDED and add proper check > for scatterlist size alignment in block mode. > > According to documentation, in SDIO block mode meson-gx DMA could > only handle buffers with sizes that are multiples of SDIO block size. > > Some SDIO drivers like brcmfmac use scatterlist API, but do not enforce > proper scatterlist buffer size alignemnt, this looks like a root cause > of non-working CMD53. > It's been too long ago that I worked on this to provide real feedback. Just one comment: Your commit description sounds like there's a problem in drivers like brcmfmac. Wouldn't it be better then to first fix these drivers? > Some minor style fixes. > > Signed-off-by: Dmitry Lebed <lebed.dmitry@gmail.com> > --- > drivers/mmc/host/meson-gx-mmc.c | 37 ++++++++++++++++++++------------- > 1 file changed, 22 insertions(+), 15 deletions(-) > > diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c > index 13f6a2c0ed04..eb6c02bc4a02 100644 > --- a/drivers/mmc/host/meson-gx-mmc.c > +++ b/drivers/mmc/host/meson-gx-mmc.c > @@ -227,7 +227,6 @@ static void meson_mmc_get_transfer_mode(struct mmc_host *mmc, > struct mmc_data *data = mrq->data; > struct scatterlist *sg; > int i; > - bool use_desc_chain_mode = true; > > /* > * When Controller DMA cannot directly access DDR memory, disable > @@ -237,25 +236,33 @@ static void meson_mmc_get_transfer_mode(struct mmc_host *mmc, > if (host->dram_access_quirk) > return; > > - /* > - * Broken SDIO with AP6255-based WiFi on Khadas VIM Pro has been > - * reported. For some strange reason this occurs in descriptor > - * chain mode only. So let's fall back to bounce buffer mode > - * for command SD_IO_RW_EXTENDED. > - */ > - if (mrq->cmd->opcode == SD_IO_RW_EXTENDED) > - return; > + if (data->blocks > 1) { > + /* > + * In block mode DMA descriptor format, "length" field indicates > + * number of blocks and there is no way to pass DMA size that > + * is not multiple of SDIO block size, making it impossible to > + * tie more than one memory buffer with single SDIO block. > + * Block mode sg buffer size should be aligned with SDIO block > + * size, otherwise chain mode could not be used. > + */ > + for_each_sg(data->sg, sg, data->sg_len, i) { > + if (sg->length % data->blksz) { > + WARN_ONCE(1, "unaligned sg len %u blksize %u\n", > + sg->length, data->blksz); > + return; > + } > + } > + } > > - for_each_sg(data->sg, sg, data->sg_len, i) > + for_each_sg(data->sg, sg, data->sg_len, i) { > /* check for 8 byte alignment */ > - if (sg->offset & 7) { > + if (sg->offset % 8) { > WARN_ONCE(1, "unaligned scatterlist buffer\n"); > - use_desc_chain_mode = false; > - break; > + return; > } > + } > > - if (use_desc_chain_mode) > - data->host_cookie |= SD_EMMC_DESC_CHAIN_MODE; > + data->host_cookie |= SD_EMMC_DESC_CHAIN_MODE; > } > > static inline bool meson_mmc_desc_chain_mode(const struct mmc_data *data) >
On Fri 18 Dec 2020 at 10:08, Heiner Kallweit <hkallweit1@gmail.com> wrote: > Am 18.12.2020 um 08:53 schrieb Dmitry Lebed: >> Enable SGDMA support for SD_IO_RW_EXTENDED and add proper check >> for scatterlist size alignment in block mode. >> >> According to documentation, in SDIO block mode meson-gx DMA could >> only handle buffers with sizes that are multiples of SDIO block size. >> >> Some SDIO drivers like brcmfmac use scatterlist API, but do not enforce >> proper scatterlist buffer size alignemnt, this looks like a root cause >> of non-working CMD53. >> > It's been too long ago that I worked on this to provide real feedback. > Just one comment: Your commit description sounds like there's a problem > in drivers like brcmfmac. Wouldn't it be better then to first fix these > drivers? While this may be true, I think the proposed change is better than the device/command specific work around we had so far. > >> Some minor style fixes. >> >> Signed-off-by: Dmitry Lebed <lebed.dmitry@gmail.com> Thx. Reviewed-by: Jerome Brunet <jbrunet@baylibre.com> >> --- >> drivers/mmc/host/meson-gx-mmc.c | 37 ++++++++++++++++++++------------- >> 1 file changed, 22 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c >> index 13f6a2c0ed04..eb6c02bc4a02 100644 >> --- a/drivers/mmc/host/meson-gx-mmc.c >> +++ b/drivers/mmc/host/meson-gx-mmc.c >> @@ -227,7 +227,6 @@ static void meson_mmc_get_transfer_mode(struct mmc_host *mmc, >> struct mmc_data *data = mrq->data; >> struct scatterlist *sg; >> int i; >> - bool use_desc_chain_mode = true; >> >> /* >> * When Controller DMA cannot directly access DDR memory, disable >> @@ -237,25 +236,33 @@ static void meson_mmc_get_transfer_mode(struct mmc_host *mmc, >> if (host->dram_access_quirk) >> return; >> >> - /* >> - * Broken SDIO with AP6255-based WiFi on Khadas VIM Pro has been >> - * reported. For some strange reason this occurs in descriptor >> - * chain mode only. So let's fall back to bounce buffer mode >> - * for command SD_IO_RW_EXTENDED. >> - */ >> - if (mrq->cmd->opcode == SD_IO_RW_EXTENDED) >> - return; >> + if (data->blocks > 1) { >> + /* >> + * In block mode DMA descriptor format, "length" field indicates >> + * number of blocks and there is no way to pass DMA size that >> + * is not multiple of SDIO block size, making it impossible to >> + * tie more than one memory buffer with single SDIO block. >> + * Block mode sg buffer size should be aligned with SDIO block >> + * size, otherwise chain mode could not be used. >> + */ >> + for_each_sg(data->sg, sg, data->sg_len, i) { >> + if (sg->length % data->blksz) { >> + WARN_ONCE(1, "unaligned sg len %u blksize %u\n", >> + sg->length, data->blksz); >> + return; >> + } >> + } >> + } >> >> - for_each_sg(data->sg, sg, data->sg_len, i) >> + for_each_sg(data->sg, sg, data->sg_len, i) { >> /* check for 8 byte alignment */ >> - if (sg->offset & 7) { >> + if (sg->offset % 8) { >> WARN_ONCE(1, "unaligned scatterlist buffer\n"); >> - use_desc_chain_mode = false; >> - break; >> + return; >> } >> + } >> >> - if (use_desc_chain_mode) >> - data->host_cookie |= SD_EMMC_DESC_CHAIN_MODE; >> + data->host_cookie |= SD_EMMC_DESC_CHAIN_MODE; >> } >> >> static inline bool meson_mmc_desc_chain_mode(const struct mmc_data *data) >> > > > _______________________________________________ > linux-amlogic mailing list > linux-amlogic@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-amlogic
On Fri, 18 Dec 2020 at 08:54, Dmitry Lebed <lebed.dmitry@gmail.com> wrote: > > Enable SGDMA support for SD_IO_RW_EXTENDED and add proper check > for scatterlist size alignment in block mode. > > According to documentation, in SDIO block mode meson-gx DMA could > only handle buffers with sizes that are multiples of SDIO block size. > > Some SDIO drivers like brcmfmac use scatterlist API, but do not enforce > proper scatterlist buffer size alignemnt, this looks like a root cause > of non-working CMD53. > > Some minor style fixes. > > Signed-off-by: Dmitry Lebed <lebed.dmitry@gmail.com> Applied for next, thanks! Kind regards Uffe > --- > drivers/mmc/host/meson-gx-mmc.c | 37 ++++++++++++++++++++------------- > 1 file changed, 22 insertions(+), 15 deletions(-) > > diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c > index 13f6a2c0ed04..eb6c02bc4a02 100644 > --- a/drivers/mmc/host/meson-gx-mmc.c > +++ b/drivers/mmc/host/meson-gx-mmc.c > @@ -227,7 +227,6 @@ static void meson_mmc_get_transfer_mode(struct mmc_host *mmc, > struct mmc_data *data = mrq->data; > struct scatterlist *sg; > int i; > - bool use_desc_chain_mode = true; > > /* > * When Controller DMA cannot directly access DDR memory, disable > @@ -237,25 +236,33 @@ static void meson_mmc_get_transfer_mode(struct mmc_host *mmc, > if (host->dram_access_quirk) > return; > > - /* > - * Broken SDIO with AP6255-based WiFi on Khadas VIM Pro has been > - * reported. For some strange reason this occurs in descriptor > - * chain mode only. So let's fall back to bounce buffer mode > - * for command SD_IO_RW_EXTENDED. > - */ > - if (mrq->cmd->opcode == SD_IO_RW_EXTENDED) > - return; > + if (data->blocks > 1) { > + /* > + * In block mode DMA descriptor format, "length" field indicates > + * number of blocks and there is no way to pass DMA size that > + * is not multiple of SDIO block size, making it impossible to > + * tie more than one memory buffer with single SDIO block. > + * Block mode sg buffer size should be aligned with SDIO block > + * size, otherwise chain mode could not be used. > + */ > + for_each_sg(data->sg, sg, data->sg_len, i) { > + if (sg->length % data->blksz) { > + WARN_ONCE(1, "unaligned sg len %u blksize %u\n", > + sg->length, data->blksz); > + return; > + } > + } > + } > > - for_each_sg(data->sg, sg, data->sg_len, i) > + for_each_sg(data->sg, sg, data->sg_len, i) { > /* check for 8 byte alignment */ > - if (sg->offset & 7) { > + if (sg->offset % 8) { > WARN_ONCE(1, "unaligned scatterlist buffer\n"); > - use_desc_chain_mode = false; > - break; > + return; > } > + } > > - if (use_desc_chain_mode) > - data->host_cookie |= SD_EMMC_DESC_CHAIN_MODE; > + data->host_cookie |= SD_EMMC_DESC_CHAIN_MODE; > } > > static inline bool meson_mmc_desc_chain_mode(const struct mmc_data *data) > -- > 2.24.3 (Apple Git-128) >
diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c index 13f6a2c0ed04..eb6c02bc4a02 100644 --- a/drivers/mmc/host/meson-gx-mmc.c +++ b/drivers/mmc/host/meson-gx-mmc.c @@ -227,7 +227,6 @@ static void meson_mmc_get_transfer_mode(struct mmc_host *mmc, struct mmc_data *data = mrq->data; struct scatterlist *sg; int i; - bool use_desc_chain_mode = true; /* * When Controller DMA cannot directly access DDR memory, disable @@ -237,25 +236,33 @@ static void meson_mmc_get_transfer_mode(struct mmc_host *mmc, if (host->dram_access_quirk) return; - /* - * Broken SDIO with AP6255-based WiFi on Khadas VIM Pro has been - * reported. For some strange reason this occurs in descriptor - * chain mode only. So let's fall back to bounce buffer mode - * for command SD_IO_RW_EXTENDED. - */ - if (mrq->cmd->opcode == SD_IO_RW_EXTENDED) - return; + if (data->blocks > 1) { + /* + * In block mode DMA descriptor format, "length" field indicates + * number of blocks and there is no way to pass DMA size that + * is not multiple of SDIO block size, making it impossible to + * tie more than one memory buffer with single SDIO block. + * Block mode sg buffer size should be aligned with SDIO block + * size, otherwise chain mode could not be used. + */ + for_each_sg(data->sg, sg, data->sg_len, i) { + if (sg->length % data->blksz) { + WARN_ONCE(1, "unaligned sg len %u blksize %u\n", + sg->length, data->blksz); + return; + } + } + } - for_each_sg(data->sg, sg, data->sg_len, i) + for_each_sg(data->sg, sg, data->sg_len, i) { /* check for 8 byte alignment */ - if (sg->offset & 7) { + if (sg->offset % 8) { WARN_ONCE(1, "unaligned scatterlist buffer\n"); - use_desc_chain_mode = false; - break; + return; } + } - if (use_desc_chain_mode) - data->host_cookie |= SD_EMMC_DESC_CHAIN_MODE; + data->host_cookie |= SD_EMMC_DESC_CHAIN_MODE; } static inline bool meson_mmc_desc_chain_mode(const struct mmc_data *data)
Enable SGDMA support for SD_IO_RW_EXTENDED and add proper check for scatterlist size alignment in block mode. According to documentation, in SDIO block mode meson-gx DMA could only handle buffers with sizes that are multiples of SDIO block size. Some SDIO drivers like brcmfmac use scatterlist API, but do not enforce proper scatterlist buffer size alignemnt, this looks like a root cause of non-working CMD53. Some minor style fixes. Signed-off-by: Dmitry Lebed <lebed.dmitry@gmail.com> --- drivers/mmc/host/meson-gx-mmc.c | 37 ++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 15 deletions(-)