Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 7 Jun 2023 14:40:14 +0200
From:   =?UTF-8?B?S8O2cnk=?= Maincent <kory.maincent@bootlin.com>
To:     Serge Semin <fancer.lancer@gmail.com>
Cc:     Cai Huoqing <cai.huoqing@linux.dev>, vkoul@kernel.org,
        Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        Manivannan Sadhasivam <mani@kernel.org>,
        Gustavo Pimentel <gustavo.pimentel@synopsys.com>,
        Jingoo Han <jingoohan1@gmail.com>,
        Lorenzo Pieralisi <lpieralisi@kernel.org>,
        Krzysztof =?UTF-8?B?V2lsY3p5xYRza2k=?= <kw@linux.com>,
        Rob Herring <robh@kernel.org>,
        Bjorn Helgaas <bhelgaas@google.com>,
        linux-kernel@vger.kernel.org, dmaengine@vger.kernel.org,
        linux-pci@vger.kernel.org,
        Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Subject: Re: [PATCH v11 3/4] dmaengine: dw-edma: Add support for native HDMA
Message-ID: <20230607144014.6356a197@kmaincent-XPS-13-7390>
In-Reply-To: <20230607114950.zml4l2rs77cbeesy@mobilestation>
References: <20230520050854.73160-1-cai.huoqing@linux.dev>
        <20230520050854.73160-4-cai.huoqing@linux.dev>
        <20230607095832.6d6b1a73@kmaincent-XPS-13-7390>
        <20230607114950.zml4l2rs77cbeesy@mobilestation>
Organization: bootlin
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, 7 Jun 2023 14:49:50 +0300
Serge Semin <fancer.lancer@gmail.com> wrote:

> Hi K=C3=B6ry
> 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 callba=
ck.
> > 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? =20
>=20
> 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.
>=20
> 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.=20
As there is no way to know this, the dw_hdma_v0_core_ch_count will simply
return HDMA_V0_MAX_NR_CH.
Cai how does it works on your side? does the ch_en register already enabled=
 by
the implementation?