Re: [PATCH v2 1/5] dmaengine: dw-edma: Fix the ch_count hdma callback

Manivannan Sadhasivam
2023-10-10T14:56:44Z

On Mon, Oct 02, 2023 at 03:17:45PM +0200, Köry Maincent wrote:
> From: Kory Maincent <kory.maincent@bootlin.com>
>
> The current check of ch_en enabled to know the maximum number of available
> hardware channels is wrong as it check the number of ch_en register set
> but all of them are unset at probe. This register is set at the
> dw_hdma_v0_core_start function which is run lately before a DMA transfer.
>
> The HDMA IP have no way to know the number of hardware channels available
> like the eDMA IP, then let set it to maximum channels and let the platform
> set the right number of channels.
>
> Fixes: e74c39573d35 ("dmaengine: dw-edma: Add support for native HDMA")
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

One nitpick below. With that,

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> ---
>
> See the following thread mail that talk about this issue:
> https://lore.kernel.org/lkml/20230607095832.6d6b1a73@kmaincent-XPS-13-7390/
>
> Changes in v2:
> - Add comment
> ---
> drivers/dma/dw-edma/dw-hdma-v0-core.c | 17 +++++------------
> 1 file changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> index 00b735a0202a..3e78d4fd3955 100644
> --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> @@ -65,18 +65,11 @@ static void dw_hdma_v0_core_off(struct dw_edma *dw)
>
> static u16 dw_hdma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> {
> - u32 num_ch = 0;
> - int id;
> -
> - for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
> - if (GET_CH_32(dw, id, dir, ch_en) & BIT(0))
> - num_ch++;
> - }
> -
> - if (num_ch > HDMA_V0_MAX_NR_CH)
> - num_ch = HDMA_V0_MAX_NR_CH;
> -
> - return (u16)num_ch;
> + /* The HDMA IP have no way to know the number of hardware channels

Please use below style for multi line comment:

/*
* ...
*/

- Mani

> + * available, we set it to maximum channels and let the platform
> + * set the right number of channels.
> + */
> + return HDMA_V0_MAX_NR_CH;
> }
>
> static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)
> --
> 2.25.1
>

--
மணிவண்ணன் சதாசிவம்