Re: [PATCH v11 3/4] dmaengine: dw-edma: Add support for native HDMA

[Serge Semin]

On Wed, Jun 07, 2023 at 02:40:14PM +0200, Köry Maincent wrote:
> On Wed, 7 Jun 2023 14:49:50 +0300
> Serge Semin <fancer.lancer@xxxxxxxxx> wrote:
> 
> > Hi Köry
> > First of all. What is akida platform you are referring to? I failed to
> > find any mention in the mainline kernel repo.
> 
> Yes, sorry akida is the project prefix I am currently working on.
> It is simply a prefix for the symbol export to be different than mainline,
> don't take it into account.
> 
> > > channels by doing the minimum between ll_wr_cnt and the ch_count callback.
> > > The hdma ch_count callback is counting the number of channels enabled by
> > > reading the number of ch_en registers set. At probe time there is no
> > > channels registers that has been set as it is done later in the
> > > dw_hdma_v0_core_start function. Then the dw_hdma_v0_core_ch_count will
> > > always return 0 at probe time and the number of channels will be set to 0
> > > which is not what we want. Could I miss something?  
> > 
> > Based on the HDMA patches content you are right. The channels must be
> > pre-enabled in order to have the dw_hdma_v0_core_ch_count() procedure
> > to work correctly otherwise you'll indeed end up with an empty list of
> > channels. I don't have any device with the HDMA engine embedded so I
> > couldn't have possibly tracked that peculiarity on review. Anyway
> > AFAICS Cai just implemented a method which seemed to work for his
> > hardware setup.
> 
> Alright, on my side I have a board using this FPGA implementation and it
> indeed doesn't work as is.
> 
> > If you think it doesn't work correctly or it isn't portable enough
> > then you are free to provide your own implementation of the method and
> > submit a patch. I hope Cai will be willing to test it out to make sure
> > that it works correctly for you and his platforms.
> > 
> > As for me if I were on your place I would have implemented a loop
> > which would walk over all possible HDMA channels (HDMA_V0_MAX_NR_CH)
> > and counted all channels which could be enabled. If the ch_en CSR is
> > writable (that is the channel could be enabled) then it shall be
> > considered as existent. Of course before that I would have made sure
> > that the non-existent channels had non-writable ch_en CSR.
> 

> This could be a nice idea but it doesn't work, non-existent channels seems to
> be writable. The datasheet of the HDMA IP doesn't have any register to find out
> the maximum existent channel. 
> As there is no way to know this, the dw_hdma_v0_core_ch_count will simply
> return HDMA_V0_MAX_NR_CH.

What about some other CSRs (llp, etc)? Are they all writable even if
the respective channel doesn't exist?

If it isn't possible to auto-detect a number of channels then indeed
we need to come up with a strategy to rely on the platform settings
instead. Your suggestion seems to be the best choice seeing there is a
commit 16b90dd94d3f ("dmaengine: dw-edma: Improve number of channels
check") which converts the rely-on-platform-settings pattern to the
current rely-on-min-between-plat-and-hw-capability.

Cai?

-Serge(y)

> Cai how does it works on your side? does the ch_en register already enabled by
> the implementation?