RE: [PATCH v4 2/4] drivers: dma: sh: Add DMAC driver for RZ/G2L SoC

From: Biju Das <biju.das.jz@bp.renesas.com>
To: Vinod Koul <vkoul@kernel.org>
Cc: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	Chris Paterson <Chris.Paterson2@renesas.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	"dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>,
	Chris Brandt <Chris.Brandt@renesas.com>,
	"linux-renesas-soc@vger.kernel.org" 
	<linux-renesas-soc@vger.kernel.org>
Subject: RE: [PATCH v4 2/4] drivers: dma: sh: Add DMAC driver for RZ/G2L SoC
Date: Wed, 28 Jul 2021 11:58:21 +0000	[thread overview]
Message-ID: <OS0PR01MB592222F26C8684E8D88DA55486EA9@OS0PR01MB5922.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <YQE542kiEGcYCCFO@matsya>

Hi Vinod,

Thanks for the feedback.

> Subject: Re: [PATCH v4 2/4] drivers: dma: sh: Add DMAC driver for RZ/G2L
> SoC
> 
> Hi Biju,
> 
> On 28-07-21, 07:00, Biju Das wrote:
> > >
> > > Sorry I dont like passing numbers like this :(
> > >
> > > Can you explain what is meant by each of the above values and looks
> > > like some (if not all) can be derived (slave config as well as
> > > transaction
> > > properties)
> >
> >
> > 0x11228 (Tx)
> > 0x11220 (Rx)
> >
> > BIT 22:- TM :- Transfer Mode
> 
> What are the values, here it seems 0

Yes, that is correct single bit. 0 means single transfer mode, 1 block transfer mode.

> 
> > Bits 16->19 :- DDS(Destination Data Size) --> 0x0001 (16 bits) Bits
> > 12->15 :- SDS(Source Data size)--> 0x0001 (16 bits)
> 
> use src_addr_width/dst_addr_width ..?

We support 128,256,512 and 1024 bits as well. I will extend enum dma_slave_buswidth to support this in another patch.
Is it ok?

> 
> > Bit  11     :- Reserved
> > Bits 8->10 :- Ack mode  --> 0x010 (Bus cycle mode)
> 
> What does this mean?

DMAACK output mode is coming from HW manual, A big table of around 230 entries for on chip request with dedicated values for the above bits.

0x000 -- Initial value
0x001 -- 001 (LEVEL Mode) (001 for MTU,PWM,CAN etccc
0x01x -- Bus cycle mode (010 for OSTM,I2C, SSIF)
0x1xx -- DMAACK not to output(SCIF)

> 
> > Bit 7 :-  Reserved
> > Bit 6:- LVL -->  Level -->0 (DMA request based on edge of thesignal)
> > Bit 5:- HIEN -->  High Enable --> 1 (Detects a DMA request on rising
> > edge of the signal) Bit 4:- LOEN --> Low Enable -->0 (Does not DMA
> > request on falling edge of the signal) Bit 3:- REQD --> Request
> > Direction ->1 (DMAREQ is Destination)
> 
> how and what decides these values
> It is now hardcoded in the client driver, 

It is SoC specific, coming from HW manual. Each on chip peripheral has it's own values.
Even source address/Destination address of the on chip module is also part of that table.

can we do that in dma driver
> instead? While deriving most of the values?

If we add this in DMA driver, it won't be generic. We need to prepare a big LUT(based on MID +RID) for all the peripherals 
If SSI then use a value from LUT, SCIF then another value like that.

So please let me know how do we want to proceed here?

Regards,
Biju

> 
> --
> ~Vinod