Received-SPF: pass (google.com: domain of linux-kernel+bounces-122503-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Thu, 28 Mar 2024 12:58:36 +0530
From: Vinod Koul <vkoul@kernel.org>
To: Nikita Shubin <nikita.shubin@maquefel.me>
Cc: Alexander Sverdlin <alexander.sverdlin@gmail.com>,
	dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
	Arnd Bergmann <arnd@arndb.de>
Subject: Re: [PATCH v9 09/38] dma: cirrus: Convert to DT for Cirrus EP93xx
Message-ID: <ZgUcJByXpLJfG10T@matsya>
References: <20240326-ep93xx-v9-0-156e2ae5dfc8@maquefel.me>
 <20240326-ep93xx-v9-9-156e2ae5dfc8@maquefel.me>
 <ZgTytMtgvqcHlEsO@matsya>
 <821da3f70fcd326860a995514791b228e3f3f7b7.camel@maquefel.me>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <821da3f70fcd326860a995514791b228e3f3f7b7.camel@maquefel.me>

On 28-03-24, 10:24, Nikita Shubin wrote:
> Hello Vinod!
> 
> Thank you for looking into this.
> 
> On Thu, 2024-03-28 at 10:01 +0530, Vinod Koul wrote:
> > On 26-03-24, 12:18, Nikita Shubin via B4 Relay wrote:
> > > From: Nikita Shubin <nikita.shubin@maquefel.me>
> > > 
> > > +enum ep93xx_dma_type {
> > > +???????M2P_DMA,
> > 
> > Is this missing P2M?
> > 
> > > +???????M2M_DMA,
> > > +};
> > > +
> 
> These are internal types used only to distinguish M2P/P2M and M2M
> capable controllers in "of_device_id ep93xx_dma_of_ids[]".
> 
> So M2P_DMA is M2P/P2M, a can rename M2P_DMA to M2P_P2M_DMA to avoid
> confusion.
> 
> 
> > > ?struct ep93xx_dma_engine;
> > > ?static int ep93xx_dma_slave_config_write(struct dma_chan *chan,
> > > ???????????????????????????????????????? enum
> > > dma_transfer_direction dir,
> > > @@ -129,11 +136,17 @@ struct ep93xx_dma_desc {
> > > ????????struct list_head????????????????node;
> > > ?};
> > > ?
> > > +struct ep93xx_dma_chan_cfg {
> > > +???????u8??????????????????????????????port;
> > > +???????enum dma_transfer_direction?????dir;
> > 
> > Why is direction stored here, it should be derived from the prep_xxx
> > call, that has direction as an argument
> > 
> > 
> 
> M2P/P2M channels aren't unidirectional.
> 
> Citing "EP9xx User Guide":
> 
> "Ten fully independent, programmable DMA controller internal M2P/P2M
> channels (5 Tx and 5 Rx)."
> 
> We need to return correct channel based on Device Tree provided data,
> because we need direction in device_alloc_chan_resources() for hardware
> setup before prepping.

Okay it sounds okay in that case...

> 
> May be i am mistaking somewhere.

-- 
~Vinod