| Message ID | 20180404170035.5507-1-wsa@the-dreams.de (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
On 4 April 2018 at 19:00, Wolfram Sang <wsa@the-dreams.de> wrote: > From: Wolfram Sang <wsa+renesas@sang-engineering.com> > > If we detect an incompatible scatterlist, we should fall back to PIO, > too. > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > --- > > I found this while working on the RX DMA issue. I don't see a reason why we > shouldn't fall back in this case as well. But maybe I am missing something. > > Shimoda-san: what do you think? > > drivers/mmc/host/renesas_sdhi_internal_dmac.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c > index 380570a26a09..561e90755a3b 100644 > --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c > +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c > @@ -161,11 +161,7 @@ renesas_sdhi_internal_dmac_start_dma(struct tmio_mmc_host *host, > enum dma_data_direction dir; > int ret; > > - /* This DMAC cannot handle if sg_len is not 1 */ > - WARN_ON(host->sg_len > 1); > - > - /* This DMAC cannot handle if buffer is not 8-bytes alignment */ > - if (!IS_ALIGNED(sg->offset, 8)) > + if (WARN_ON(host->sg_len > 1) || !IS_ALIGNED(sg->offset, 8)) The WARN_ON becomes a bit misleading being a part of the if statement, as it should never happen when the driver has set ->max_segs = 1. For the alignment check, and the moving to PIO, this certainly makes sense to me. > goto force_pio; > > if (data->flags & MMC_DATA_READ) { Kind regards Uffe
> > - /* This DMAC cannot handle if sg_len is not 1 */ > > - WARN_ON(host->sg_len > 1); > > - > > - /* This DMAC cannot handle if buffer is not 8-bytes alignment */ > > - if (!IS_ALIGNED(sg->offset, 8)) > > + if (WARN_ON(host->sg_len > 1) || !IS_ALIGNED(sg->offset, 8)) > > The WARN_ON becomes a bit misleading being a part of the if statement, > as it should never happen when the driver has set ->max_segs = 1. Uh, yeah, right. So, we could simply drop the WARN_ON?
Hi Wolfram-san, Thank you for the patch! > From: Wolfram Sang, Sent: Thursday, April 5, 2018 2:01 AM > > From: Wolfram Sang <wsa+renesas@sang-engineering.com> > > If we detect an incompatible scatterlist, we should fall back to PIO, > too. > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > --- > > I found this while working on the RX DMA issue. I don't see a reason why we > shouldn't fall back in this case as well. But maybe I am missing something. > > Shimoda-san: what do you think? > > drivers/mmc/host/renesas_sdhi_internal_dmac.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c > index 380570a26a09..561e90755a3b 100644 > --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c > +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c > @@ -161,11 +161,7 @@ renesas_sdhi_internal_dmac_start_dma(struct tmio_mmc_host *host, > enum dma_data_direction dir; > int ret; > > - /* This DMAC cannot handle if sg_len is not 1 */ I would like to keep this comment. > - WARN_ON(host->sg_len > 1); > - > - /* This DMAC cannot handle if buffer is not 8-bytes alignment */ > - if (!IS_ALIGNED(sg->offset, 8)) > + if (WARN_ON(host->sg_len > 1) || !IS_ALIGNED(sg->offset, 8)) As Ulf-san said on other thread about this WARN_ON(), I think dropping the WARN_ON() is better. Best regards, Yoshihiro Shimoda
diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c index 380570a26a09..561e90755a3b 100644 --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c @@ -161,11 +161,7 @@ renesas_sdhi_internal_dmac_start_dma(struct tmio_mmc_host *host, enum dma_data_direction dir; int ret; - /* This DMAC cannot handle if sg_len is not 1 */ - WARN_ON(host->sg_len > 1); - - /* This DMAC cannot handle if buffer is not 8-bytes alignment */ - if (!IS_ALIGNED(sg->offset, 8)) + if (WARN_ON(host->sg_len > 1) || !IS_ALIGNED(sg->offset, 8)) goto force_pio; if (data->flags & MMC_DATA_READ) {