RE: [PATCH 1/2] mmc: renesas_sdhi_internal_dmac: Fix missing unmap in error patch
Hi Geert-san,
> From: Geert Uytterhoeven, Sent: Thursday, June 28, 2018 9:50 PM
>
> Hi Shimoda-san,
>
> On Thu, Jun 28, 2018 at 1:53 PM Yoshihiro Shimoda
> wrote:
> > This patch fixes an issue that lacks the dma_unmap_sg() calling in
> > the error patch of renesas_sdhi_internal_dmac_start_dma().
>
> Nice catch!
> Thanks for your patch!
>
> > Fixes: 0cbc94daa554 ("mmc: renesas_sdhi_internal_dmac: limit DMA RX for old
> > SoCs")
> > Cc: # v4.17+
> > Signed-off-by: Yoshihiro Shimoda
>
> Reviewed-by: Geert Uytterhoeven
Thank you for your review!
> > --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> > +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> > @@ -173,8 +173,11 @@
> > if (data->flags & MMC_DATA_READ) {
> > dtran_mode |= DTRAN_MODE_CH_NUM_CH1;
> > if (test_bit(SDHI_INTERNAL_DMAC_ONE_RX_ONLY, _flags)
> > &&
> > - test_and_set_bit(SDHI_INTERNAL_DMAC_RX_IN_USE,
> > _flags))
> > + test_and_set_bit(SDHI_INTERNAL_DMAC_RX_IN_USE,
> > _flags)) {
> > + dma_unmap_sg(>pdev->dev, sg, host->sg_len,
> > +mmc_get_dma_dir(data));
>
> Given there is already a call to dma_unmap_sg() a few lines earlier , you
> may want to introduce a new label before force_pio, and move the call to
> dma_unmap_sg() there.
Thank you for the comment. So, I'll submit v2 patch.
Best regards,
Yoshihiro Shimoda
Re: [PATCH 1/2] mmc: renesas_sdhi_internal_dmac: Fix missing unmap in error patch
Hi Shimoda-san,
On Thu, Jun 28, 2018 at 1:53 PM Yoshihiro Shimoda
wrote:
> This patch fixes an issue that lacks the dma_unmap_sg() calling in
> the error patch of renesas_sdhi_internal_dmac_start_dma().
Nice catch!
Thanks for your patch!
> Fixes: 0cbc94daa554 ("mmc: renesas_sdhi_internal_dmac: limit DMA RX for old
> SoCs")
> Cc: # v4.17+
> Signed-off-by: Yoshihiro Shimoda
Reviewed-by: Geert Uytterhoeven
> --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> @@ -173,8 +173,11 @@
> if (data->flags & MMC_DATA_READ) {
> dtran_mode |= DTRAN_MODE_CH_NUM_CH1;
> if (test_bit(SDHI_INTERNAL_DMAC_ONE_RX_ONLY, _flags) &&
> - test_and_set_bit(SDHI_INTERNAL_DMAC_RX_IN_USE,
> _flags))
> + test_and_set_bit(SDHI_INTERNAL_DMAC_RX_IN_USE,
> _flags)) {
> + dma_unmap_sg(>pdev->dev, sg, host->sg_len,
> +mmc_get_dma_dir(data));
Given there is already a call to dma_unmap_sg() a few lines earlier , you
may want to introduce a new label before force_pio, and move the call to
dma_unmap_sg() there.
> goto force_pio;
> + }
> } else {
> dtran_mode |= DTRAN_MODE_CH_NUM_CH0;
> }
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
