Received-SPF: pass (google.com: domain of linux-kernel+bounces-122505-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <821da3f70fcd326860a995514791b228e3f3f7b7.camel@maquefel.me>
Subject: Re: [PATCH v9 09/38] dma: cirrus: Convert to DT for Cirrus EP93xx
From: Nikita Shubin <nikita.shubin@maquefel.me>
To: Vinod Koul <vkoul@kernel.org>
Cc: Alexander Sverdlin <alexander.sverdlin@gmail.com>, 
	dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org, Arnd Bergmann
	 <arnd@arndb.de>
Date: Thu, 28 Mar 2024 10:24:25 +0300
In-Reply-To: <ZgTytMtgvqcHlEsO@matsya>
References: <20240326-ep93xx-v9-0-156e2ae5dfc8@maquefel.me>
	 <20240326-ep93xx-v9-9-156e2ae5dfc8@maquefel.me> <ZgTytMtgvqcHlEsO@matsya>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 
Precedence: bulk
MIME-Version: 1.0

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>
> >=20
> > +enum ep93xx_dma_type {
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0M2P_DMA,
>=20
> Is this missing P2M?
>=20
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0M2M_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.


> > =C2=A0struct ep93xx_dma_engine;
> > =C2=A0static int ep93xx_dma_slave_config_write(struct dma_chan *chan,
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0 enum
> > dma_transfer_direction dir,
> > @@ -129,11 +136,17 @@ struct ep93xx_dma_desc {
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct list_head=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0node;
> > =C2=A0};
> > =C2=A0
> > +struct ep93xx_dma_chan_cfg {
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0u8=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0port;
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0enum dma_transfer_direction=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0dir;
>=20
> Why is direction stored here, it should be derived from the prep_xxx
> call, that has direction as an argument
>=20
>=20

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.

May be i am mistaking somewhere.