Re: [PATCH v2 2/6] dt-bindings: dma: dw: Add max burst transaction length property

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Serge Semin <fancer.lancer@gmail.com>
Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>,
	Vinod Koul <vkoul@kernel.org>, Viresh Kumar <vireshk@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Paul Burton <paulburton@kernel.org>,
	Ralf Baechle <ralf@linux-mips.org>, Arnd Bergmann <arnd@arndb.de>,
	Dan Williams <dan.j.williams@intel.com>,
	linux-mips@vger.kernel.org, dmaengine@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/6] dt-bindings: dma: dw: Add max burst transaction length property
Date: Tue, 12 May 2020 12:08:04 +0300	[thread overview]
Message-ID: <20200512090804.GR185537@smile.fi.intel.com> (raw)
In-Reply-To: <20200511213531.wnywlljiulvndx6s@mobilestation>

On Tue, May 12, 2020 at 12:35:31AM +0300, Serge Semin wrote:
> On Tue, May 12, 2020 at 12:01:38AM +0300, Andy Shevchenko wrote:
> > On Mon, May 11, 2020 at 11:05:28PM +0300, Serge Semin wrote:
> > > On Fri, May 08, 2020 at 02:12:42PM +0300, Andy Shevchenko wrote:
> > > > On Fri, May 08, 2020 at 01:53:00PM +0300, Serge Semin wrote:
> > > > > This array property is used to indicate the maximum burst transaction
> > > > > length supported by each DMA channel.
> > > > 
> > > > > +  snps,max-burst-len:
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > > +    description: |
> > > > > +      Maximum length of burst transactions supported by hardware.
> > > > > +      It's an array property with one cell per channel in units of
> > > > > +      CTLx register SRC_TR_WIDTH/DST_TR_WIDTH (data-width) field.
> > > > > +    items:
> > > > > +      maxItems: 8
> > > > > +      items:
> > > > 
> > > > > +        enum: [4, 8, 16, 32, 64, 128, 256]
> > > > 
> > > > Isn't 1 allowed?
> > > 
> > > Burst length of 1 unit is supported, but in accordance with Data Book the MAX
> > > burst length is limited to be equal to a value from the set I submitted. So the
> > > max value can be either 4, or 8, or 16 and so on.
> > 
> > Hmm... It seems you mistakenly took here DMAH_CHx_MAX_MULT_SIZE pre-silicon
> > configuration parameter instead of runtime as described in Table 26:
> > CTLx.SRC_MSIZE and DEST_MSIZE Decoding.
> 
> No. You misunderstood what I meant. We shouldn't use a runtime parameters values
> here. Why would we?

Because what we describe in the DTS is what user may do to the hardware. In
some cases user might want to limit this to 1, how to achieve that?

Rob, is there any clarification that schema describes only synthesized values?
Or i.o.w. shall we allow user to setup whatever hardware supports at run time?

> Property "snps,max-burst-len" matches DMAH_CHx_MAX_MULT_SIZE
> config parameter.

Why? User should have a possibility to ask whatever hardware supports at run time.

> See a comment to the "SRC_MSIZE" and "DEST_MSIZE" fields of the
> registers. You'll find out that their maximum value is determined by the
> DMAH_CHx_MAX_MULT_SIZE parameter, which must belong to the set [4, 8, 16, 32, 64,
> 128, 256]. So no matter how you synthesize the DW DMAC block you'll have at least
> 4x max burst length supported.

That's true.

-- 
With Best Regards,
Andy Shevchenko