| Message ID | 20231002131749.2977952-2-kory.maincent@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix support of dw-edma HDMA NATIVE IP in remote setup | expand |
Hi Cai, 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> > --- > > See the following thread mail that talk about this issue: > https://lore.kernel.org/lkml/20230607095832.6d6b1a73@kmaincent-XPS-13-7390/ Cai, do you have anything to add in regards to this change and to the discussion available in the thread above? If no, I guess the solution provided in this patch is the best we can currently come up with. -Serge(y) > > 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 > + * 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 >
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 >
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 + * 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)