Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S942434AbcJ0SHo (ORCPT ); Thu, 27 Oct 2016 14:07:44 -0400 Received: from up.free-electrons.com ([163.172.77.33]:46025 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S933792AbcJ0SHl (ORCPT ); Thu, 27 Oct 2016 14:07:41 -0400 Date: Thu, 27 Oct 2016 19:51:54 +0200 From: Maxime Ripard To: Jean-Francois Moine Cc: =?iso-8859-1?Q?Myl=E8ne?= Josserand , vinod.koul@intel.com, wens@csie.org, mturquette@baylibre.com, sboyd@codeaurora.org, lgirdwood@gmail.com, broonie@kernel.org, perex@perex.cz, tiwai@suse.com, lee.jones@linaro.org, mark.rutland@arm.com, robh+dt@kernel.org, thomas.petazzoni@free-electrons.com, devicetree@vger.kernel.org, alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, linux-sunxi@googlegroups.com, alexandre.belloni@free-electrons.com, dmaengine@vger.kernel.org, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 01/14] dma: sun6i-dma: Add burst case of 4 Message-ID: <20161027175154.v4kuhyvm3r5n6tdo@lukather> References: <3a2404510b5f8b16ae3bb193982d70f158700b2b.1475571575.git.mylene.josserand@free-electrons.com> <20161004124011.d7f5754a082d5f17d5185fc4@free.fr> <20161004165553.GS5228@lukather> <20161023183107.5b75b6aad62853378713299f@free.fr> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="ytmjuq4pyc3h5ao5" Content-Disposition: inline In-Reply-To: <20161023183107.5b75b6aad62853378713299f@free.fr> User-Agent: Mutt/1.6.2-neo (2016-08-21) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6006 Lines: 162 --ytmjuq4pyc3h5ao5 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Oct 23, 2016 at 06:31:07PM +0200, Jean-Francois Moine wrote: > On Tue, 4 Oct 2016 18:55:54 +0200 > Maxime Ripard wrote: >=20 > > On Tue, Oct 04, 2016 at 12:40:11PM +0200, Jean-Francois Moine wrote: > > > On Tue, 4 Oct 2016 11:46:14 +0200 > > > Myl=E8ne Josserand wrote: > > >=20 > > > > Add the case of a burst of 4 which is handled by the SoC. > > > >=20 > > > > Signed-off-by: Myl=E8ne Josserand > > > > --- > > > > drivers/dma/sun6i-dma.c | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > >=20 > > > > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c > > > > index 8346199..0485204 100644 > > > > --- a/drivers/dma/sun6i-dma.c > > > > +++ b/drivers/dma/sun6i-dma.c > > > > @@ -240,6 +240,8 @@ static inline s8 convert_burst(u32 maxburst) > > > > switch (maxburst) { > > > > case 1: > > > > return 0; > > > > + case 4: > > > > + return 1; > > > > case 8: > > > > return 2; > > > > default: > > > > --=20 > > > > 2.9.3 > > >=20 > > > This patch has already been rejected by Maxime in the threads > > > http://www.spinics.net/lists/dmaengine/msg08610.html > > > and > > > http://www.spinics.net/lists/dmaengine/msg08719.html > > >=20 > > > I hope you will find the way he wants for this maxburst to be added. > >=20 > > I was talking about something along these lines (not tested): >=20 > I wonder why you don't submit this yourself. I thought you were the one who cared. You asked for what I had in mind, here it is. > > -------8<--------- > > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c > > index 83461994e418..573ac4608293 100644 > > --- a/drivers/dma/sun6i-dma.c > > +++ b/drivers/dma/sun6i-dma.c > > @@ -240,6 +240,8 @@ static inline s8 convert_burst(u32 maxburst) > > switch (maxburst) { > > case 1: > > return 0; > > + case 4: > > + return 1; > > case 8: > > return 2; > > default: > > @@ -1110,11 +1112,19 @@ static int sun6i_dma_probe(struct platform_devi= ce *pdev) > > sdc->slave.dst_addr_widths =3D BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | > > BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | > > BIT(DMA_SLAVE_BUSWIDTH_4_BYTES); > > + sdc->slave.dst_bursts =3D BIT(1) | BIT(8); > > + sdc->slave.src_bursts =3D BIT(1) | BIT(8); > > sdc->slave.directions =3D BIT(DMA_DEV_TO_MEM) | > > BIT(DMA_MEM_TO_DEV); > > sdc->slave.residue_granularity =3D DMA_RESIDUE_GRANULARITY_BURST; > > sdc->slave.dev =3D &pdev->dev; > > =20 > > + if (of_device_is_compatible(pdev->dev.of_node, > > + "allwinner,sun8i-h3-dma")) { > > + sdc->slave.dst_bursts |=3D BIT(4); > > + sdc->slave.src_bursts |=3D BIT(4); > > + } > > + > > sdc->pchans =3D devm_kcalloc(&pdev->dev, sdc->cfg->nr_max_channels, > > sizeof(struct sun6i_pchan), GFP_KERNEL); > > if (!sdc->pchans) > > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h > > index cc535a478bae..f7bbec24bb58 100644 > > --- a/include/linux/dmaengine.h > > +++ b/include/linux/dmaengine.h > > @@ -673,6 +673,8 @@ struct dma_filter { > > * each type of direction, the dma controller should fill (1 << > > * ) and same should be checked by controller as well > > * @max_burst: max burst capability per-transfer > > + * @dst_bursts: bitfield of the available burst sizes for the destinat= ion > > + * @src_bursts: bitfield of the available burst sizes for the source >=20 > You did not define dst_bursts nor src_bursts. >=20 > > * @residue_granularity: granularity of the transfer residue reported > > * by tx_status > > * @device_alloc_chan_resources: allocate resources and return the > > @@ -800,6 +802,14 @@ struct dma_device { > > static inline int dmaengine_slave_config(struct dma_chan *chan, > > struct dma_slave_config *config) > > { > > + if (config->src_maxburst && config->device->src_bursts && > > + !(BIT(config->src_maxburst) & config->device->src_bursts)) >=20 > The maxburst may be as big as 4Kibytes, then, I am not sure that this > code will work! >=20 > > + return -EINVAL; > > + > > + if (config->dst_maxburst && config->device->dst_bursts && > > + !(BIT(config->dst_maxburst) & config->device->dst_bursts)) > > + return -EINVAL; > > + > > if (chan->device->device_config) > > return chan->device->device_config(chan, config); > > -------8<------------=20 >=20 > Yes, I know that the burst size is always a power of 2. > The best way to check it would be to change the {src,dts}_maxburst to a > bitmap of the possible bursts as 0x0d for 1,4 and 8 bytes. But this > asks for a lot of changes... To be honest, I'm not really a big fan of those tree wide changes without any way to maintain compatibility. It ends up in a single, huge patch if we want to maintain bisectability that just isn't reviewable. Maxime --=20 Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com --ytmjuq4pyc3h5ao5 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBCAAGBQJYEj66AAoJEBx+YmzsjxAg1p0P/RHAvXASog8JmNwxGhGfZtgZ ykeNoQg+PyUNQ13u49vDv/UUkBwQa/bjBRtvM6/YVlosuto/xQCay4DLIte4kjDA qXaQndsHGevPWRJZQgv2Sl/QQD3+lfKHK/gfMHNrgUZMcVEibXWraFMHdCGpRKzR IGTbkyZv1xS5M5Z2E57lWHGsbABaK6vm0eK+eDmaBMFuUpn85vrE5LZLTb1QKnaq HHU90XTlfnrExxPPKXoO2SyW5CZlgrG7Qs28Td2KHPlsQ6hm9LAFwOcX3SJObHMf BLqqnNdzUtHFpMUfa6SqBaf2LzyIj7S3TuTYFxpRn5kMYm9YTeAFKh5O+8c5kUI9 mZ6aPp6ui3WYJ4A5RmfgiVA5BTeBzAAaCYq/Pp5oU5lEYFRDAZLnPAgWqs/5daRr nE4IEXEGBM+chIW6+7eJjUlgjZa2mnTHWA4IfnYWZxE3ehg80CnfIIIjwxRKqLno sk8EVaYaMOLP+RCu8FvyVmE1VhZ75+QigM1won54EnBL8oJsBeV00tc16eH7m9hN 60rEDhCsHMw62TeTTmxB2irtnRNNWHKWpErIVhk0MafYEwRkPzuRk1zAyWG0dtFf hKE6B7R9CB75o/hpylMRdzWBt0Ecw/YST18jGPLQsA3yU5GEFPD8p8McvoMIq+qO JWmFmknZCja+j+Fisjjy =U4fb -----END PGP SIGNATURE----- --ytmjuq4pyc3h5ao5--