diff mbox series

mmc: sh_mmcif: correctly report success when obtaining DMA channels

Message ID 20240928094454.3592-2-wsa+renesas@sang-engineering.com
State New
Headers show
Series mmc: sh_mmcif: correctly report success when obtaining DMA channels | expand

Commit Message

Wolfram Sang Sept. 28, 2024, 9:44 a.m. UTC
The debug message could still report success when getting the channels
was OK but configuring them failed. This actually caused a minor detour
when debugging DMA problems, so make sure the success is only reported
when the channels are really ready-to-use.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/sh_mmcif.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Geert Uytterhoeven Sept. 30, 2024, 2:17 p.m. UTC | #1
Hi Wolfram,

On Sat, Sep 28, 2024 at 11:45 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> The debug message could still report success when getting the channels
> was OK but configuring them failed. This actually caused a minor detour
> when debugging DMA problems, so make sure the success is only reported
> when the channels are really ready-to-use.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Thanks for your patch!

> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
> @@ -439,14 +439,15 @@ static void sh_mmcif_request_dma(struct sh_mmcif_host *host)
>                 if (IS_ERR(host->chan_rx))
>                         host->chan_rx = NULL;
>         }
> -       dev_dbg(dev, "%s: got channel TX %p RX %p\n", __func__, host->chan_tx,
> -               host->chan_rx);

This was not a real success indicator, which could indeed confuse
people, but an obfuscated NULL-pointer still prints as NULL, right?

>         if (!host->chan_tx || !host->chan_rx ||
>             sh_mmcif_dma_slave_config(host, host->chan_tx, DMA_MEM_TO_DEV) ||
>             sh_mmcif_dma_slave_config(host, host->chan_rx, DMA_DEV_TO_MEM))
>                 goto error;
>
> +       dev_dbg(dev, "%s: got channel TX %p RX %p\n", __func__, host->chan_tx,
> +               host->chan_rx);

This means we no longer see a debug message in case only one DMA
channel could be requested, thus requiring manual addition of more code
to find out what was really wrong?

> +
>         return;
>
>  error:

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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
Ulf Hansson Oct. 8, 2024, 2:35 p.m. UTC | #2
On Sat, 28 Sept 2024 at 11:44, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> The debug message could still report success when getting the channels
> was OK but configuring them failed. This actually caused a minor detour
> when debugging DMA problems, so make sure the success is only reported
> when the channels are really ready-to-use.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Applied for next, thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/host/sh_mmcif.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> index 08b4312af94e..0e7fa3e9c9fe 100644
> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
> @@ -439,14 +439,15 @@ static void sh_mmcif_request_dma(struct sh_mmcif_host *host)
>                 if (IS_ERR(host->chan_rx))
>                         host->chan_rx = NULL;
>         }
> -       dev_dbg(dev, "%s: got channel TX %p RX %p\n", __func__, host->chan_tx,
> -               host->chan_rx);
>
>         if (!host->chan_tx || !host->chan_rx ||
>             sh_mmcif_dma_slave_config(host, host->chan_tx, DMA_MEM_TO_DEV) ||
>             sh_mmcif_dma_slave_config(host, host->chan_rx, DMA_DEV_TO_MEM))
>                 goto error;
>
> +       dev_dbg(dev, "%s: got channel TX %p RX %p\n", __func__, host->chan_tx,
> +               host->chan_rx);
> +
>         return;
>
>  error:
> --
> 2.45.2
>
diff mbox series

Patch

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 08b4312af94e..0e7fa3e9c9fe 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -439,14 +439,15 @@  static void sh_mmcif_request_dma(struct sh_mmcif_host *host)
 		if (IS_ERR(host->chan_rx))
 			host->chan_rx = NULL;
 	}
-	dev_dbg(dev, "%s: got channel TX %p RX %p\n", __func__, host->chan_tx,
-		host->chan_rx);
 
 	if (!host->chan_tx || !host->chan_rx ||
 	    sh_mmcif_dma_slave_config(host, host->chan_tx, DMA_MEM_TO_DEV) ||
 	    sh_mmcif_dma_slave_config(host, host->chan_rx, DMA_DEV_TO_MEM))
 		goto error;
 
+	dev_dbg(dev, "%s: got channel TX %p RX %p\n", __func__, host->chan_tx,
+		host->chan_rx);
+
 	return;
 
 error: