[{"id":1760866,"web_url":"http://patchwork.ozlabs.org/comment/1760866/","msgid":"<CAEKpxB=_CbbBem6EhA32P80esRhfCFoRPEv_rKKQauhzAdw_4Q@mail.gmail.com>","list_archive_url":null,"date":"2017-08-31T11:44:05","subject":"Re: [PATCH 3/3] dmaengine: sun6i: Add support for Allwinner A64","submitter":{"id":65868,"url":"http://patchwork.ozlabs.org/api/people/65868/","name":"Code Kipper","email":"codekipper@gmail.com"},"content":"On 31 August 2017 at 01:36, Stefan Brüns <stefan.bruens@rwth-aachen.de> wrote:\n> The A64 SoC has the same dma engine as the H3 (sun8i), with a\n> reduced amount of physical channels. Add the proper config data\n> and compatible string to support it.\n>\n> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>\n> ---\n>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 4 ++++\n>  drivers/dma/sun6i-dma.c                       | 7 +++++++\n>  2 files changed, 11 insertions(+)\n>\n> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi\n> index f96287d3043a..b86019238b77 100644\n> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi\n> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi\n> @@ -494,6 +494,8 @@\n>                         interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>;\n>                         clocks = <&ccu CLK_BUS_SPI0>, <&ccu CLK_SPI0>;\n>                         clock-names = \"ahb\", \"mod\";\n> +                       dmas = <&dma 23>, <&dma 23>;\n> +                       dma-names = \"rx\", \"tx\";\nHi Stefan,\nthe dtsi parts should be in a separate patch\n\n>                         pinctrl-names = \"default\";\n>                         pinctrl-0 = <&spi0_pins>;\n>                         resets = <&ccu RST_BUS_SPI0>;\n> @@ -509,6 +511,8 @@\n>                         interrupts = <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>;\n>                         clocks = <&ccu CLK_BUS_SPI1>, <&ccu CLK_SPI1>;\n>                         clock-names = \"ahb\", \"mod\";\n> +                       dmas = <&dma 24>, <&dma 24>;\n> +                       dma-names = \"rx\", \"tx\";\n>                         pinctrl-names = \"default\";\n>                         pinctrl-0 = <&spi1_pins>;\n>                         resets = <&ccu RST_BUS_SPI1>;\n> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c\n> index 5f4eee4513e5..6a17c5d63582 100644\n> --- a/drivers/dma/sun6i-dma.c\n> +++ b/drivers/dma/sun6i-dma.c\n> @@ -1068,6 +1068,12 @@ static struct sun6i_dma_config sun8i_h3_dma_cfg = {\n>         .nr_max_vchans   = 34,\n>         .dmac_variant    = DMAC_VARIANT_H3,\n>  };\n> +\n> +static struct sun6i_dma_config sun50i_a64_dma_cfg = {\n> +       .nr_max_channels = 8,\n> +       .nr_max_requests = 27,\n> +       .nr_max_vchans   = 38,\n> +       .dmac_variant    = DMAC_VARIANT_H3,\n>  };\n>\n>  static const struct of_device_id sun6i_dma_match[] = {\n> @@ -1075,6 +1081,7 @@ static const struct of_device_id sun6i_dma_match[] = {\n>         { .compatible = \"allwinner,sun8i-a23-dma\", .data = &sun8i_a23_dma_cfg },\n>         { .compatible = \"allwinner,sun8i-a83t-dma\", .data = &sun8i_a83t_dma_cfg },\n>         { .compatible = \"allwinner,sun8i-h3-dma\", .data = &sun8i_h3_dma_cfg },\n> +       { .compatible = \"allwinner,sun50i-a64-dma\", .data = &sun50i_a64_dma_cfg },\nThis all looks fine...it's similar to my patch here\nhttps://github.com/codekipper/linux-sunxi/commit/8c54d9852dfad6ceb478c579a1213f38fb12fa80\nwhich I've been too lazy to post. I think the binding documentation\nshould go with this patch and this should also be the 1st patch in the\nseries, followed by the dtsi changes.\nBR,\nCK\n>         { /* sentinel */ }\n>  };\n>  MODULE_DEVICE_TABLE(of, sun6i_dma_match);\n> --\n> 2.14.1\n>\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"r/Z+09X5\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xjgX96mCpz9sQl\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tThu, 31 Aug 2017 21:44:41 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751382AbdHaLoL (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tThu, 31 Aug 2017 07:44:11 -0400","from mail-io0-f193.google.com ([209.85.223.193]:33082 \"EHLO\n\tmail-io0-f193.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751074AbdHaLoH (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Thu, 31 Aug 2017 07:44:07 -0400","by mail-io0-f193.google.com with SMTP id f99so1141408ioi.0;\n\tThu, 31 Aug 2017 04:44:06 -0700 (PDT)","by 10.79.6.216 with HTTP; Thu, 31 Aug 2017 04:44:05 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=JabKohxhV/4qDkYsNGA6sjJrUAX5x5mRK33VluCEZg0=;\n\tb=r/Z+09X58nIobAAne1HOu0yoayiLI3bEMDsvTDAq5dCf0asLzVAKugC9q3SUBrj0/S\n\tWqPWU3j+klG6l+eORsq4jUwrt0983eET/6qnq8L9GxGTJcenwKsNPgh7VMnrdlJh5pJJ\n\tuUcPumxgnLeVBgwy7oh/ZJjtb/zwlQebYCohMveW/r93AcvZNJaHsjoNicm64iiQHL42\n\twRxXKRc9LKADxfBxaM1aoqMKIhPiiYdEuTQxUIovhtnRuDy/t1aGLCrEqzUBErzWefiD\n\tX5rqJHTepwFsRptEf0kp5m1GCsmblSdjy5VwROG5wY4Dp7qifmNr+hDhUpwKwi/qWZWJ\n\tvy8A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=JabKohxhV/4qDkYsNGA6sjJrUAX5x5mRK33VluCEZg0=;\n\tb=JaZBGebqYMzaRgtr1w8bKSizD52RoZK2wV2iyc1I3M+wXTO10vEU8e06KFrN/fQIDC\n\twymla2ODeD7b/4YbnQwF2aKAQdNvhGHAR0L3cj4JwAKgnhCxbl+FDKClwS2h6xtCI2yd\n\tj6Do1zNB8OTlBLwhJuW/CsG2lo2DeWw4oYYm3OXn/6Pdw9yEnXaGv24WYvWVqBz8o1wX\n\tmfQAI0UshKPK/4TU1imLrKfuHSl3HD0WXb9ZVZ8QhipBb7WP7aTkI5DlhnatVO7MlaHF\n\tUuHYuvNNv5nNuyrL1U32mOKRaWXlzF5YRGQVsArqIgyJP5HLkKCVF1oi8rMzXPcSTjBc\n\tfAXA==","X-Gm-Message-State":"AHYfb5jD/bKPqQGHepvFJp39YfBHcVXA3krC6IT1/VuMKv0Fd+xRMBnd\n\t0FdUF+3aX3NmCtBh/acvJjucKIi31g==","X-Google-Smtp-Source":"ADKCNb5ghw/FQn+97aWkEDgXtoqF9d+O+oDX900XMq05waxyfIHEDcXHTA8Z4QsV5YxjLYBsnR+oy7b/o7tv/wxYoxM=","X-Received":"by 10.36.144.195 with SMTP id x186mr451309itd.5.1504179846224;\n\tThu, 31 Aug 2017 04:44:06 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170830233609.13855-4-stefan.bruens@rwth-aachen.de>","References":"<20170830233609.13855-1-stefan.bruens@rwth-aachen.de>\n\t<20170830233609.13855-4-stefan.bruens@rwth-aachen.de>","From":"Code Kipper <codekipper@gmail.com>","Date":"Thu, 31 Aug 2017 13:44:05 +0200","Message-ID":"<CAEKpxB=_CbbBem6EhA32P80esRhfCFoRPEv_rKKQauhzAdw_4Q@mail.gmail.com>","Subject":"Re: [PATCH 3/3] dmaengine: sun6i: Add support for Allwinner A64","To":"=?utf-8?q?Stefan_Br=C3=BCns?= <stefan.bruens@rwth-aachen.de>","Cc":"linux-sunxi <linux-sunxi@googlegroups.com>,\n\tdevicetree <devicetree@vger.kernel.org>,\n\tdmaengine@vger.kernel.org, Vinod Koul <vinod.koul@intel.com>,\n\tlinux-arm-kernel <linux-arm-kernel@lists.infradead.org>,\n\tlinux-kernel <linux-kernel@vger.kernel.org>,\n\tMaxime Ripard <maxime.ripard@free-electrons.com>,\n\tChen-Yu Tsai <wens@csie.org>, Rob Herring <robh+dt@kernel.org>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1761079,"web_url":"http://patchwork.ozlabs.org/comment/1761079/","msgid":"<20170831145135.c6a2wubcf6xu34tz@flea.lan>","list_archive_url":null,"date":"2017-08-31T14:51:35","subject":"Re: [PATCH 1/3] dmaengine: sun6i: Correct DMA support on H3","submitter":{"id":12916,"url":"http://patchwork.ozlabs.org/api/people/12916/","name":"Maxime Ripard","email":"maxime.ripard@free-electrons.com"},"content":"Hi,\n\nOn Thu, Aug 31, 2017 at 01:36:07AM +0200, Stefan Brüns wrote:\n> +/* Between SoC generations, there are some significant differences:\n> + * - A23 added a clock gate register\n> + * - the H3 burst length field has a different offset\n> + */\n\nThis is not the proper comment style.\n\n> +enum dmac_variant {\n> +\tDMAC_VARIANT_A31,\n> +\tDMAC_VARIANT_A23,\n> +\tDMAC_VARIANT_H3,\n> +};\n> +\n\nAnd this is redundant with what we already have in our structures.\n\n>  /*\n>   * Hardware channels / ports representation\n>   *\n> @@ -101,6 +116,7 @@ struct sun6i_dma_config {\n>  \tu32 nr_max_channels;\n>  \tu32 nr_max_requests;\n>  \tu32 nr_max_vchans;\n> +\tenum dmac_variant dmac_variant;\n>  };\n>  \n>  /*\n> @@ -240,8 +256,12 @@ static inline s8 convert_burst(u32 maxburst)\n>  \tswitch (maxburst) {\n>  \tcase 1:\n>  \t\treturn 0;\n> +\tcase 4:\n> +\t\treturn 1;\n>  \tcase 8:\n>  \t\treturn 2;\n> +\tcase 16:\n> +\t\treturn 3;\n>  \tdefault:\n>  \t\treturn -EINVAL;\n>  \t}\n> @@ -249,11 +269,7 @@ static inline s8 convert_burst(u32 maxburst)\n>  \n>  static inline s8 convert_buswidth(enum dma_slave_buswidth addr_width)\n>  {\n> -\tif ((addr_width < DMA_SLAVE_BUSWIDTH_1_BYTE) ||\n> -\t    (addr_width > DMA_SLAVE_BUSWIDTH_4_BYTES))\n> -\t\treturn -EINVAL;\n> -\n> -\treturn addr_width >> 1;\n> +\treturn ilog2(addr_width);\n>  }\n\nThis isn't really the same operation. There should be some explanation\nabout why it's the right thing to do.\n\n>  static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)\n> @@ -499,45 +515,58 @@ static int set_config(struct sun6i_dma_dev *sdev,\n>  \t\t\tenum dma_transfer_direction direction,\n>  \t\t\tu32 *p_cfg)\n>  {\n> +\tenum dma_slave_buswidth src_addr_width, dst_addr_width;\n> +\tu32 src_maxburst, dst_maxburst, supported_burst_length;\n>  \ts8 src_width, dst_width, src_burst, dst_burst;\n>  \n> +\tsrc_addr_width = sconfig->src_addr_width;\n> +\tdst_addr_width = sconfig->dst_addr_width;\n> +\tsrc_maxburst = sconfig->src_maxburst;\n> +\tdst_maxburst = sconfig->dst_maxburst;\n> +\n> +\tif (sdev->cfg->dmac_variant == DMAC_VARIANT_H3)\n> +\t\tsupported_burst_length = BIT(1) | BIT(4) | BIT(8) | BIT(16);\n> +\telse\n> +\t\tsupported_burst_length = BIT(1) | BIT(8);\n\nThis could be stored in the structure for example.\n\n>  \tswitch (direction) {\n>  \tcase DMA_MEM_TO_DEV:\n> -\t\tsrc_burst = convert_burst(sconfig->src_maxburst ?\n> -\t\t\t\t\tsconfig->src_maxburst : 8);\n> -\t\tsrc_width = convert_buswidth(sconfig->src_addr_width !=\n> -\t\t\t\t\t\tDMA_SLAVE_BUSWIDTH_UNDEFINED ?\n> -\t\t\t\tsconfig->src_addr_width :\n> -\t\t\t\tDMA_SLAVE_BUSWIDTH_4_BYTES);\n> -\t\tdst_burst = convert_burst(sconfig->dst_maxburst);\n> -\t\tdst_width = convert_buswidth(sconfig->dst_addr_width);\n> +\t\tif (src_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED)\n> +\t\t\tsrc_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;\n> +\t\tsrc_maxburst = src_maxburst ? src_maxburst : 8;\n>  \t\tbreak;\n>  \tcase DMA_DEV_TO_MEM:\n> -\t\tsrc_burst = convert_burst(sconfig->src_maxburst);\n> -\t\tsrc_width = convert_buswidth(sconfig->src_addr_width);\n> -\t\tdst_burst = convert_burst(sconfig->dst_maxburst ?\n> -\t\t\t\t\tsconfig->dst_maxburst : 8);\n> -\t\tdst_width = convert_buswidth(sconfig->dst_addr_width !=\n> -\t\t\t\t\t\tDMA_SLAVE_BUSWIDTH_UNDEFINED ?\n> -\t\t\t\tsconfig->dst_addr_width :\n> -\t\t\t\tDMA_SLAVE_BUSWIDTH_4_BYTES);\n> +\t\tif (dst_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED)\n> +\t\t\tdst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;\n> +\t\tdst_maxburst = dst_maxburst ? dst_maxburst : 8;\n>  \t\tbreak;\n>  \tdefault:\n>  \t\treturn -EINVAL;\n>  \t}\n>  \n> -\tif (src_burst < 0)\n> -\t\treturn src_burst;\n> -\tif (src_width < 0)\n> -\t\treturn src_width;\n> -\tif (dst_burst < 0)\n> -\t\treturn dst_burst;\n> -\tif (dst_width < 0)\n> -\t\treturn dst_width;\n> -\n> -\t*p_cfg = DMA_CHAN_CFG_SRC_BURST(src_burst) |\n> -\t\tDMA_CHAN_CFG_SRC_WIDTH(src_width) |\n> -\t\tDMA_CHAN_CFG_DST_BURST(dst_burst) |\n> +\tif (!(BIT(src_addr_width) & sdev->slave.src_addr_widths))\n> +\t\treturn -EINVAL;\n> +\tif (!(BIT(dst_addr_width) & sdev->slave.dst_addr_widths))\n> +\t\treturn -EINVAL;\n> +\tif (!(BIT(src_maxburst) & supported_burst_length))\n> +\t\treturn -EINVAL;\n> +\tif (!(BIT(dst_maxburst) & supported_burst_length))\n> +\t\treturn -EINVAL;\n> +\n> +\tsrc_width = convert_buswidth(src_addr_width);\n> +\tdst_width = convert_buswidth(dst_addr_width);\n> +\tdst_burst = convert_burst(dst_maxburst);\n> +\tsrc_burst = convert_burst(src_maxburst);\n\nI'm not sure what you're trying to do here. Could you split your patch\nby logical change, this doesn't seem related to just supporting the\nH3, but a heavier refactoring.\n\nMaxime","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xjlhL0YWmz9s75\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tFri,  1 Sep 2017 00:52:02 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751456AbdHaOvr (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tThu, 31 Aug 2017 10:51:47 -0400","from mail.free-electrons.com ([62.4.15.54]:53868 \"EHLO\n\tmail.free-electrons.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751440AbdHaOvr (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Thu, 31 Aug 2017 10:51:47 -0400","by mail.free-electrons.com (Postfix, from userid 110)\n\tid 8316A208B8; Thu, 31 Aug 2017 16:51:44 +0200 (CEST)","from localhost (LStLambert-657-1-97-87.w90-63.abo.wanadoo.fr\n\t[90.63.216.87])\n\tby mail.free-electrons.com (Postfix) with ESMTPSA id 5020020882;\n\tThu, 31 Aug 2017 16:51:34 +0200 (CEST)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on\n\tmail.free-electrons.com","X-Spam-Level":"","X-Spam-Status":"No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT\n\tshortcircuit=ham autolearn=disabled version=3.4.0","Date":"Thu, 31 Aug 2017 16:51:35 +0200","From":"Maxime Ripard <maxime.ripard@free-electrons.com>","To":"Stefan =?iso-8859-1?q?Br=FCns?= <stefan.bruens@rwth-aachen.de>","Cc":"linux-sunxi@googlegroups.com, devicetree@vger.kernel.org,\n\tdmaengine@vger.kernel.org, Vinod Koul <vinod.koul@intel.com>,\n\tlinux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,\n\tChen-Yu Tsai <wens@csie.org>, Rob Herring <robh+dt@kernel.org>","Subject":"Re: [PATCH 1/3] dmaengine: sun6i: Correct DMA support on H3","Message-ID":"<20170831145135.c6a2wubcf6xu34tz@flea.lan>","References":"<20170830233609.13855-1-stefan.bruens@rwth-aachen.de>\n\t<20170830233609.13855-2-stefan.bruens@rwth-aachen.de>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha1;\n\tprotocol=\"application/pgp-signature\"; boundary=\"6ccoewc44gprs6ks\"","Content-Disposition":"inline","In-Reply-To":"<20170830233609.13855-2-stefan.bruens@rwth-aachen.de>","User-Agent":"NeoMutt/20170714 (1.8.3)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1761083,"web_url":"http://patchwork.ozlabs.org/comment/1761083/","msgid":"<20170831145246.p4k3uakbn67na3pv@flea.lan>","list_archive_url":null,"date":"2017-08-31T14:52:46","subject":"Re: [PATCH 3/3] dmaengine: sun6i: Add support for Allwinner A64","submitter":{"id":12916,"url":"http://patchwork.ozlabs.org/api/people/12916/","name":"Maxime Ripard","email":"maxime.ripard@free-electrons.com"},"content":"On Thu, Aug 31, 2017 at 01:36:09AM +0200, Stefan Brüns wrote:\n> The A64 SoC has the same dma engine as the H3 (sun8i), with a\n> reduced amount of physical channels. Add the proper config data\n> and compatible string to support it.\n> \n> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>\n> ---\n>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 4 ++++\n>  drivers/dma/sun6i-dma.c                       | 7 +++++++\n>  2 files changed, 11 insertions(+)\n\nWith what device did you test this? As far as I know, the SPI driver\ndoesn't use DMA at all.\n\nMaxime","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xjljS0z1fz9s7c\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tFri,  1 Sep 2017 00:53:00 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751742AbdHaOw6 (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tThu, 31 Aug 2017 10:52:58 -0400","from mail.free-electrons.com ([62.4.15.54]:53938 \"EHLO\n\tmail.free-electrons.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751463AbdHaOw5 (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Thu, 31 Aug 2017 10:52:57 -0400","by mail.free-electrons.com (Postfix, from userid 110)\n\tid 6ADC8209F6; Thu, 31 Aug 2017 16:52:55 +0200 (CEST)","from localhost (LStLambert-657-1-97-87.w90-63.abo.wanadoo.fr\n\t[90.63.216.87])\n\tby mail.free-electrons.com (Postfix) with ESMTPSA id 3DBB820872;\n\tThu, 31 Aug 2017 16:52:45 +0200 (CEST)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on\n\tmail.free-electrons.com","X-Spam-Level":"","X-Spam-Status":"No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT,\n\tURIBL_BLOCKED shortcircuit=ham autolearn=disabled version=3.4.0","Date":"Thu, 31 Aug 2017 16:52:46 +0200","From":"Maxime Ripard <maxime.ripard@free-electrons.com>","To":"Stefan =?iso-8859-1?q?Br=FCns?= <stefan.bruens@rwth-aachen.de>","Cc":"linux-sunxi@googlegroups.com, devicetree@vger.kernel.org,\n\tdmaengine@vger.kernel.org, Vinod Koul <vinod.koul@intel.com>,\n\tlinux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,\n\tChen-Yu Tsai <wens@csie.org>, Rob Herring <robh+dt@kernel.org>","Subject":"Re: [PATCH 3/3] dmaengine: sun6i: Add support for Allwinner A64","Message-ID":"<20170831145246.p4k3uakbn67na3pv@flea.lan>","References":"<20170830233609.13855-1-stefan.bruens@rwth-aachen.de>\n\t<20170830233609.13855-4-stefan.bruens@rwth-aachen.de>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha1;\n\tprotocol=\"application/pgp-signature\"; boundary=\"q6ijb5qpgbsb6jtp\"","Content-Disposition":"inline","In-Reply-To":"<20170830233609.13855-4-stefan.bruens@rwth-aachen.de>","User-Agent":"NeoMutt/20170714 (1.8.3)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1761175,"web_url":"http://patchwork.ozlabs.org/comment/1761175/","msgid":"<CAEKpxBkKAVV0wnQq7YD8MpPM31DGindVgHDstTc082HiUSSd=g@mail.gmail.com>","list_archive_url":null,"date":"2017-08-31T16:35:43","subject":"Re: [linux-sunxi] Re: [PATCH 3/3] dmaengine: sun6i: Add support for\n\tAllwinner A64","submitter":{"id":65868,"url":"http://patchwork.ozlabs.org/api/people/65868/","name":"Code Kipper","email":"codekipper@gmail.com"},"content":"On 31 August 2017 at 16:52, Maxime Ripard\n<maxime.ripard@free-electrons.com> wrote:\n> On Thu, Aug 31, 2017 at 01:36:09AM +0200, Stefan Brüns wrote:\n>> The A64 SoC has the same dma engine as the H3 (sun8i), with a\n>> reduced amount of physical channels. Add the proper config data\n>> and compatible string to support it.\n>>\n>> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>\n>> ---\n>>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 4 ++++\n>>  drivers/dma/sun6i-dma.c                       | 7 +++++++\n>>  2 files changed, 11 insertions(+)\n>\n> With what device did you test this? As far as I know, the SPI driver\n> doesn't use DMA at all.\n>\nHi Maxime,\nI've tested this on spdif and i2s on the Pine64.\nBR,\nCK\n> Maxime\n>\n> --\n> Maxime Ripard, Free Electrons\n> Embedded Linux and Kernel engineering\n> http://free-electrons.com\n>\n> --\n> You received this message because you are subscribed to the Google Groups \"linux-sunxi\" group.\n> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.\n> For more options, visit https://groups.google.com/d/optout.\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"KGKYmKGy\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xjp030LtCz9sPm\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tFri,  1 Sep 2017 02:35:46 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751823AbdHaQfp (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tThu, 31 Aug 2017 12:35:45 -0400","from mail-io0-f196.google.com ([209.85.223.196]:34430 \"EHLO\n\tmail-io0-f196.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751667AbdHaQfo (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Thu, 31 Aug 2017 12:35:44 -0400","by mail-io0-f196.google.com with SMTP id k22so188987iod.1;\n\tThu, 31 Aug 2017 09:35:43 -0700 (PDT)","by 10.79.6.216 with HTTP; Thu, 31 Aug 2017 09:35:43 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=6Ru057Q/XytgNF+Hmr62a1eZpuV4ziSOuw43t4ZtBHQ=;\n\tb=KGKYmKGyIOmmRlx+GGKIo9G4q6J3O1oRWmi9c+vbeDbb9FjZjzNFeYQJYYqiIm7/gN\n\tm+ZnfP2tehGU6DoLp8zewrLz8iybxKb2KsIMQSbwPBn4KoiPzF3zyONAmbmg/vCbefcR\n\ttWHZnz+NwTHZx9ARJmbVZaJPdIFqPvzmpX57k9S1lZTyBwjB1XVfvksqHB9J4mCw0jmh\n\tnXTjgSWYTc9yWhVHFoq0Um2XjEjZF/Q69bGgWVcnfFyaNGaOWOEFvbV4zc+9RY0cO6+s\n\tJ0WIdxJoPxrQ/Dd0c60Xc68jPXxTQYhUxCs/Swui1gSXfWJEbIoFARzpAxchoifedoHr\n\t/asA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=6Ru057Q/XytgNF+Hmr62a1eZpuV4ziSOuw43t4ZtBHQ=;\n\tb=TgaeRZVZqBxhcyhlpR0z+l82OMdfnWd11WI4gGgon7mZ7uQn+5XNfyOHgEO5VzWpuB\n\tvNaHXQtB0yE6GtYw1n/ttMEhprLT7KiMf4MrLRBtAnMRMawkds2YUT4dqjLp3UdJWtEB\n\t0raSi/82eCcUG+Rw7ZlFiZvmTrpQHYJ45rterLh9iom4VdWPwt/XgotU5JSjn9a5vQNv\n\tKDo0iqqEgVsMSFIUmh9aDlsFeHIf5tKKl3Xxqz9d43PVFCf5AyFgC7UJ3/RkcjIUMenU\n\t7f7+2N1jpm2qLq/qOSO0uVZjCAxpN8xNl6C7ilPjtb0wiD9yNBrfWEi02VYeM+gxSZm2\n\t3kxA==","X-Gm-Message-State":"AHPjjUhH4Lhc+RUdPu6QWKehpy/Z9ULWsiVfV+Aw/9KEqhrdpw6HNIrL\n\ts5cEmuoj6uTjE/Q1+pdJal1lVGyi/kzG","X-Google-Smtp-Source":"ADKCNb4ia0haJ1MzcZyKsHCCMzaF+d8tUGa7r9eSWPKIiZQktwkG17BPVyQsATk4B+ZcfpqT5FiRoXvcf6GutS0dOYs=","X-Received":"by 10.107.32.83 with SMTP id g80mr5373698iog.357.1504197343599; \n\tThu, 31 Aug 2017 09:35:43 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170831145246.p4k3uakbn67na3pv@flea.lan>","References":"<20170830233609.13855-1-stefan.bruens@rwth-aachen.de>\n\t<20170830233609.13855-4-stefan.bruens@rwth-aachen.de>\n\t<20170831145246.p4k3uakbn67na3pv@flea.lan>","From":"Code Kipper <codekipper@gmail.com>","Date":"Thu, 31 Aug 2017 18:35:43 +0200","Message-ID":"<CAEKpxBkKAVV0wnQq7YD8MpPM31DGindVgHDstTc082HiUSSd=g@mail.gmail.com>","Subject":"Re: [linux-sunxi] Re: [PATCH 3/3] dmaengine: sun6i: Add support for\n\tAllwinner A64","To":"Maxime Ripard <maxime.ripard@free-electrons.com>","Cc":"=?utf-8?q?Stefan_Br=C3=BCns?= <stefan.bruens@rwth-aachen.de>, linux-sunxi\n\t<linux-sunxi@googlegroups.com>, \n\tdevicetree <devicetree@vger.kernel.org>, dmaengine@vger.kernel.org,\n\tVinod Koul <vinod.koul@intel.com>, linux-arm-kernel\n\t<linux-arm-kernel@lists.infradead.org>, linux-kernel\n\t<linux-kernel@vger.kernel.org>, Chen-Yu Tsai <wens@csie.org>,\n\tRob Herring <robh+dt@kernel.org>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1761446,"web_url":"http://patchwork.ozlabs.org/comment/1761446/","msgid":"<a649f98d4ac44abc873c5110ce9363e5@rwthex-w2-b.rwth-ad.de>","list_archive_url":null,"date":"2017-09-01T03:04:54","subject":"Re: [PATCH 1/3] dmaengine: sun6i: Correct DMA support on H3","submitter":{"id":67055,"url":"http://patchwork.ozlabs.org/api/people/67055/","name":"Stefan Brüns","email":"stefan.bruens@rwth-aachen.de"},"content":"On Donnerstag, 31. August 2017 16:51:35 CEST Maxime Ripard wrote:\n> Hi,\n> \n> On Thu, Aug 31, 2017 at 01:36:07AM +0200, Stefan Brüns wrote:\n> > +/* Between SoC generations, there are some significant differences:\n> > + * - A23 added a clock gate register\n> > + * - the H3 burst length field has a different offset\n> > + */\n> \n> This is not the proper comment style.\n> \n> > +enum dmac_variant {\n> > +\tDMAC_VARIANT_A31,\n> > +\tDMAC_VARIANT_A23,\n> > +\tDMAC_VARIANT_H3,\n> > +};\n> > +\n> \n> And this is redundant with what we already have in our structures.\n\nActually, its not. For H3, there are currently at least 3 register compatible \nSoCs: H5 is identical, R40 has 16 dma channels, A64 has 8 channels. So if the \ncurrent config structure is kept, we need 3 different compatible strings. Same \nfor the A23, which is register compatible to e.g. A83t and V3s, but with \ndifferent numbers of DMA channels.\n\nSo either you decorate the code with a cascade of\n\nif ((of_is_compatible(..A23..) || of_is_compatible(..A83T..) || ...) {\n} else if ((of_is_compatible(..H3..) || of_is_compatible(..A64..) || ...) {\n} else { /* A31 */\n}\n\nin a number of places, or you do it just once.\n\n> \n> >  /*\n> >  \n> >   * Hardware channels / ports representation\n> >   *\n> > \n> > @@ -101,6 +116,7 @@ struct sun6i_dma_config {\n> > \n> >  \tu32 nr_max_channels;\n> >  \tu32 nr_max_requests;\n> >  \tu32 nr_max_vchans;\n> > \n> > +\tenum dmac_variant dmac_variant;\n> > \n> >  };\n> >  \n> >  /*\n> > \n> > @@ -240,8 +256,12 @@ static inline s8 convert_burst(u32 maxburst)\n> > \n> >  \tswitch (maxburst) {\n> >  \t\n> >  \tcase 1:\n> >  \t\treturn 0;\n> > \n> > +\tcase 4:\n> > +\t\treturn 1;\n> > \n> >  \tcase 8:\n> >  \t\treturn 2;\n> > \n> > +\tcase 16:\n> > +\t\treturn 3;\n> > \n> >  \tdefault:\n> >  \t\treturn -EINVAL;\n> >  \t\n> >  \t}\n> > \n> > @@ -249,11 +269,7 @@ static inline s8 convert_burst(u32 maxburst)\n> > \n> >  static inline s8 convert_buswidth(enum dma_slave_buswidth addr_width)\n> >  {\n> > \n> > -\tif ((addr_width < DMA_SLAVE_BUSWIDTH_1_BYTE) ||\n> > -\t    (addr_width > DMA_SLAVE_BUSWIDTH_4_BYTES))\n> > -\t\treturn -EINVAL;\n> > -\n> > -\treturn addr_width >> 1;\n> > +\treturn ilog2(addr_width);\n> > \n> >  }\n> \n> This isn't really the same operation. There should be some explanation\n> about why it's the right thing to do.\n\nFor 1, 2 and 4 it is the same. The correct register value for 8 bytes, \nsupported by H3, H5, A64 and R40, is 3.\n\n> \n> >  static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)\n> > \n> > @@ -499,45 +515,58 @@ static int set_config(struct sun6i_dma_dev *sdev,\n> > \n> >  \t\t\tenum dma_transfer_direction direction,\n> >  \t\t\tu32 *p_cfg)\n> >  \n> >  {\n> > \n> > +\tenum dma_slave_buswidth src_addr_width, dst_addr_width;\n> > +\tu32 src_maxburst, dst_maxburst, supported_burst_length;\n> > \n> >  \ts8 src_width, dst_width, src_burst, dst_burst;\n> > \n> > +\tsrc_addr_width = sconfig->src_addr_width;\n> > +\tdst_addr_width = sconfig->dst_addr_width;\n> > +\tsrc_maxburst = sconfig->src_maxburst;\n> > +\tdst_maxburst = sconfig->dst_maxburst;\n> > +\n> > +\tif (sdev->cfg->dmac_variant == DMAC_VARIANT_H3)\n> > +\t\tsupported_burst_length = BIT(1) | BIT(4) | BIT(8) | BIT(16);\n> > +\telse\n> > +\t\tsupported_burst_length = BIT(1) | BIT(8);\n> \n> This could be stored in the structure for example.\n\nWhich one? sun6i_dma_dev? sun6i_dma_config?\n \n> >  \tswitch (direction) {\n> > \n> >  \tcase DMA_MEM_TO_DEV:\n> > -\t\tsrc_burst = convert_burst(sconfig->src_maxburst ?\n> > -\t\t\t\t\tsconfig->src_maxburst : 8);\n> > -\t\tsrc_width = convert_buswidth(sconfig->src_addr_width !=\n> > -\t\t\t\t\t\tDMA_SLAVE_BUSWIDTH_UNDEFINED ?\n> > -\t\t\t\tsconfig->src_addr_width :\n> > -\t\t\t\tDMA_SLAVE_BUSWIDTH_4_BYTES);\n> > -\t\tdst_burst = convert_burst(sconfig->dst_maxburst);\n> > -\t\tdst_width = convert_buswidth(sconfig->dst_addr_width);\n> > +\t\tif (src_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED)\n> > +\t\t\tsrc_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;\n> > +\t\tsrc_maxburst = src_maxburst ? src_maxburst : 8;\n> > \n> >  \t\tbreak;\n> >  \t\n> >  \tcase DMA_DEV_TO_MEM:\n> > -\t\tsrc_burst = convert_burst(sconfig->src_maxburst);\n> > -\t\tsrc_width = convert_buswidth(sconfig->src_addr_width);\n> > -\t\tdst_burst = convert_burst(sconfig->dst_maxburst ?\n> > -\t\t\t\t\tsconfig->dst_maxburst : 8);\n> > -\t\tdst_width = convert_buswidth(sconfig->dst_addr_width !=\n> > -\t\t\t\t\t\tDMA_SLAVE_BUSWIDTH_UNDEFINED ?\n> > -\t\t\t\tsconfig->dst_addr_width :\n> > -\t\t\t\tDMA_SLAVE_BUSWIDTH_4_BYTES);\n> > +\t\tif (dst_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED)\n> > +\t\t\tdst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;\n> > +\t\tdst_maxburst = dst_maxburst ? dst_maxburst : 8;\n> > \n> >  \t\tbreak;\n> >  \t\n> >  \tdefault:\n> >  \t\treturn -EINVAL;\n> >  \t\n> >  \t}\n> > \n> > -\tif (src_burst < 0)\n> > -\t\treturn src_burst;\n> > -\tif (src_width < 0)\n> > -\t\treturn src_width;\n> > -\tif (dst_burst < 0)\n> > -\t\treturn dst_burst;\n> > -\tif (dst_width < 0)\n> > -\t\treturn dst_width;\n> > -\n> > -\t*p_cfg = DMA_CHAN_CFG_SRC_BURST(src_burst) |\n> > -\t\tDMA_CHAN_CFG_SRC_WIDTH(src_width) |\n> > -\t\tDMA_CHAN_CFG_DST_BURST(dst_burst) |\n> > +\tif (!(BIT(src_addr_width) & sdev->slave.src_addr_widths))\n> > +\t\treturn -EINVAL;\n> > +\tif (!(BIT(dst_addr_width) & sdev->slave.dst_addr_widths))\n> > +\t\treturn -EINVAL;\n> > +\tif (!(BIT(src_maxburst) & supported_burst_length))\n> > +\t\treturn -EINVAL;\n> > +\tif (!(BIT(dst_maxburst) & supported_burst_length))\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tsrc_width = convert_buswidth(src_addr_width);\n> > +\tdst_width = convert_buswidth(dst_addr_width);\n> > +\tdst_burst = convert_burst(dst_maxburst);\n> > +\tsrc_burst = convert_burst(src_maxburst);\n> \n> I'm not sure what you're trying to do here. Could you split your patch\n> by logical change, this doesn't seem related to just supporting the\n> H3, but a heavier refactoring.\n\nUntangling 3 independent steps - handling of DMA_SLAVE_BUSWIDTH_UNDEFINED, \nrange checking, and conversion to register value. As the valid ranges depend \non the controller, the code is much easier to read if the range check is done \nfirst, and then the conversion.\n\nKind regards,\n\nStefan","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xk3y65wzMz9s83\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tFri,  1 Sep 2017 13:05:02 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751325AbdIADFB convert rfc822-to-8bit (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tThu, 31 Aug 2017 23:05:01 -0400","from mail-out-2.itc.rwth-aachen.de ([134.130.5.47]:2880 \"EHLO\n\tmail-out-2.itc.rwth-aachen.de\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751243AbdIADFA (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Thu, 31 Aug 2017 23:05:00 -0400","from rwthex-w2-b.rwth-ad.de ([134.130.26.159])\n\tby mail-in-2.itc.rwth-aachen.de with ESMTP; 01 Sep 2017 05:04:58 +0200","from pebbles.site (78.49.70.254) by rwthex-w2-b.rwth-ad.de\n\t(2002:8682:1a9f::8682:1a9f) with Microsoft SMTP Server (TLS) id\n\t15.0.1320.4; Fri, 1 Sep 2017 05:04:55 +0200"],"X-IronPort-AV":"E=Sophos;i=\"5.41,456,1498514400\"; d=\"scan'208\";a=\"11235768\"","From":"Stefan Bruens <stefan.bruens@rwth-aachen.de>","To":"Maxime Ripard <maxime.ripard@free-electrons.com>","CC":"Stefan =?iso-8859-1?q?Br=FCns?= <stefan.bruens@rwth-aachen.de>,\n\tlinux-sunxi <linux-sunxi@googlegroups.com>, \n\t<devicetree@vger.kernel.org>, <dmaengine@vger.kernel.org>, \n\t<vinod.koul@intel.com>, <linux-arm-kernel@lists.infradead.org>, \n\t<linux-kernel@vger.kernel.org>, Chen-Yu Tsai <wens@csie.org>, \n\tRob Herring <robh+dt@kernel.org>","Subject":"Re: [PATCH 1/3] dmaengine: sun6i: Correct DMA support on H3","Date":"Fri, 1 Sep 2017 05:04:54 +0200","In-Reply-To":"<20170831145135.c6a2wubcf6xu34tz@flea.lan>","References":"<20170830233609.13855-1-stefan.bruens@rwth-aachen.de>\n\t<20170830233609.13855-2-stefan.bruens@rwth-aachen.de>\n\t<20170831145135.c6a2wubcf6xu34tz@flea.lan>","MIME-Version":"1.0","Content-Transfer-Encoding":"8BIT","Content-Type":"text/plain; charset=\"iso-8859-1\"","X-Originating-IP":"[78.49.70.254]","X-ClientProxiedBy":"rwthex-s1-a.rwth-ad.de (2002:8682:1a98::8682:1a98) To\n\trwthex-w2-b.rwth-ad.de (2002:8682:1a9f::8682:1a9f)","Message-ID":"<a649f98d4ac44abc873c5110ce9363e5@rwthex-w2-b.rwth-ad.de>","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1761688,"web_url":"http://patchwork.ozlabs.org/comment/1761688/","msgid":"<20170901133549.cr2ivmfr5mnrdujg@flea>","list_archive_url":null,"date":"2017-09-01T13:35:49","subject":"Re: [PATCH 1/3] dmaengine: sun6i: Correct DMA support on H3","submitter":{"id":12916,"url":"http://patchwork.ozlabs.org/api/people/12916/","name":"Maxime Ripard","email":"maxime.ripard@free-electrons.com"},"content":"On Fri, Sep 01, 2017 at 05:04:54AM +0200, Stefan Bruens wrote:\n> On Donnerstag, 31. August 2017 16:51:35 CEST Maxime Ripard wrote:\n> > Hi,\n> > \n> > On Thu, Aug 31, 2017 at 01:36:07AM +0200, Stefan Brüns wrote:\n> > > +/* Between SoC generations, there are some significant differences:\n> > > + * - A23 added a clock gate register\n> > > + * - the H3 burst length field has a different offset\n> > > + */\n> > \n> > This is not the proper comment style.\n> > \n> > > +enum dmac_variant {\n> > > +\tDMAC_VARIANT_A31,\n> > > +\tDMAC_VARIANT_A23,\n> > > +\tDMAC_VARIANT_H3,\n> > > +};\n> > > +\n> > \n> > And this is redundant with what we already have in our structures.\n> \n> Actually, its not. For H3, there are currently at least 3 register compatible \n> SoCs: H5 is identical, R40 has 16 dma channels, A64 has 8 channels. So if the \n> current config structure is kept, we need 3 different compatible strings. Same \n> for the A23, which is register compatible to e.g. A83t and V3s, but with \n> different numbers of DMA channels.\n> \n> So either you decorate the code with a cascade of\n> \n> if ((of_is_compatible(..A23..) || of_is_compatible(..A83T..) || ...) {\n> } else if ((of_is_compatible(..H3..) || of_is_compatible(..A64..) || ...) {\n> } else { /* A31 */\n> }\n> \n> in a number of places, or you do it just once.\n\nThat's not how you retrieve the structures. They are already\nassociated to the compatible, and you need to do a single lookup to\nget them. So that's nowhere near what you're suggesting. You can have\na look at the of_match_device in the probe function.\n\n> \n> > \n> > >  /*\n> > >  \n> > >   * Hardware channels / ports representation\n> > >   *\n> > > \n> > > @@ -101,6 +116,7 @@ struct sun6i_dma_config {\n> > > \n> > >  \tu32 nr_max_channels;\n> > >  \tu32 nr_max_requests;\n> > >  \tu32 nr_max_vchans;\n> > > \n> > > +\tenum dmac_variant dmac_variant;\n> > > \n> > >  };\n> > >  \n> > >  /*\n> > > \n> > > @@ -240,8 +256,12 @@ static inline s8 convert_burst(u32 maxburst)\n> > > \n> > >  \tswitch (maxburst) {\n> > >  \t\n> > >  \tcase 1:\n> > >  \t\treturn 0;\n> > > \n> > > +\tcase 4:\n> > > +\t\treturn 1;\n> > > \n> > >  \tcase 8:\n> > >  \t\treturn 2;\n> > > \n> > > +\tcase 16:\n> > > +\t\treturn 3;\n> > > \n> > >  \tdefault:\n> > >  \t\treturn -EINVAL;\n> > >  \t\n> > >  \t}\n> > > \n> > > @@ -249,11 +269,7 @@ static inline s8 convert_burst(u32 maxburst)\n> > > \n> > >  static inline s8 convert_buswidth(enum dma_slave_buswidth addr_width)\n> > >  {\n> > > \n> > > -\tif ((addr_width < DMA_SLAVE_BUSWIDTH_1_BYTE) ||\n> > > -\t    (addr_width > DMA_SLAVE_BUSWIDTH_4_BYTES))\n> > > -\t\treturn -EINVAL;\n> > > -\n> > > -\treturn addr_width >> 1;\n> > > +\treturn ilog2(addr_width);\n> > > \n> > >  }\n> > \n> > This isn't really the same operation. There should be some explanation\n> > about why it's the right thing to do.\n> \n> For 1, 2 and 4 it is the same. The correct register value for 8 bytes, \n> supported by H3, H5, A64 and R40, is 3.\n\nThat should be in a separate patch, with that in the commit log.\n\n> > >  static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)\n> > > \n> > > @@ -499,45 +515,58 @@ static int set_config(struct sun6i_dma_dev *sdev,\n> > > \n> > >  \t\t\tenum dma_transfer_direction direction,\n> > >  \t\t\tu32 *p_cfg)\n> > >  \n> > >  {\n> > > \n> > > +\tenum dma_slave_buswidth src_addr_width, dst_addr_width;\n> > > +\tu32 src_maxburst, dst_maxburst, supported_burst_length;\n> > > \n> > >  \ts8 src_width, dst_width, src_burst, dst_burst;\n> > > \n> > > +\tsrc_addr_width = sconfig->src_addr_width;\n> > > +\tdst_addr_width = sconfig->dst_addr_width;\n> > > +\tsrc_maxburst = sconfig->src_maxburst;\n> > > +\tdst_maxburst = sconfig->dst_maxburst;\n> > > +\n> > > +\tif (sdev->cfg->dmac_variant == DMAC_VARIANT_H3)\n> > > +\t\tsupported_burst_length = BIT(1) | BIT(4) | BIT(8) | BIT(16);\n> > > +\telse\n> > > +\t\tsupported_burst_length = BIT(1) | BIT(8);\n> > \n> > This could be stored in the structure for example.\n> \n> Which one? sun6i_dma_dev? sun6i_dma_config?\n\nThe one associated with the compatible, so sun6i_dma_config.\n\n>  \n> > >  \tswitch (direction) {\n> > > \n> > >  \tcase DMA_MEM_TO_DEV:\n> > > -\t\tsrc_burst = convert_burst(sconfig->src_maxburst ?\n> > > -\t\t\t\t\tsconfig->src_maxburst : 8);\n> > > -\t\tsrc_width = convert_buswidth(sconfig->src_addr_width !=\n> > > -\t\t\t\t\t\tDMA_SLAVE_BUSWIDTH_UNDEFINED ?\n> > > -\t\t\t\tsconfig->src_addr_width :\n> > > -\t\t\t\tDMA_SLAVE_BUSWIDTH_4_BYTES);\n> > > -\t\tdst_burst = convert_burst(sconfig->dst_maxburst);\n> > > -\t\tdst_width = convert_buswidth(sconfig->dst_addr_width);\n> > > +\t\tif (src_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED)\n> > > +\t\t\tsrc_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;\n> > > +\t\tsrc_maxburst = src_maxburst ? src_maxburst : 8;\n> > > \n> > >  \t\tbreak;\n> > >  \t\n> > >  \tcase DMA_DEV_TO_MEM:\n> > > -\t\tsrc_burst = convert_burst(sconfig->src_maxburst);\n> > > -\t\tsrc_width = convert_buswidth(sconfig->src_addr_width);\n> > > -\t\tdst_burst = convert_burst(sconfig->dst_maxburst ?\n> > > -\t\t\t\t\tsconfig->dst_maxburst : 8);\n> > > -\t\tdst_width = convert_buswidth(sconfig->dst_addr_width !=\n> > > -\t\t\t\t\t\tDMA_SLAVE_BUSWIDTH_UNDEFINED ?\n> > > -\t\t\t\tsconfig->dst_addr_width :\n> > > -\t\t\t\tDMA_SLAVE_BUSWIDTH_4_BYTES);\n> > > +\t\tif (dst_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED)\n> > > +\t\t\tdst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;\n> > > +\t\tdst_maxburst = dst_maxburst ? dst_maxburst : 8;\n> > > \n> > >  \t\tbreak;\n> > >  \t\n> > >  \tdefault:\n> > >  \t\treturn -EINVAL;\n> > >  \t\n> > >  \t}\n> > > \n> > > -\tif (src_burst < 0)\n> > > -\t\treturn src_burst;\n> > > -\tif (src_width < 0)\n> > > -\t\treturn src_width;\n> > > -\tif (dst_burst < 0)\n> > > -\t\treturn dst_burst;\n> > > -\tif (dst_width < 0)\n> > > -\t\treturn dst_width;\n> > > -\n> > > -\t*p_cfg = DMA_CHAN_CFG_SRC_BURST(src_burst) |\n> > > -\t\tDMA_CHAN_CFG_SRC_WIDTH(src_width) |\n> > > -\t\tDMA_CHAN_CFG_DST_BURST(dst_burst) |\n> > > +\tif (!(BIT(src_addr_width) & sdev->slave.src_addr_widths))\n> > > +\t\treturn -EINVAL;\n> > > +\tif (!(BIT(dst_addr_width) & sdev->slave.dst_addr_widths))\n> > > +\t\treturn -EINVAL;\n> > > +\tif (!(BIT(src_maxburst) & supported_burst_length))\n> > > +\t\treturn -EINVAL;\n> > > +\tif (!(BIT(dst_maxburst) & supported_burst_length))\n> > > +\t\treturn -EINVAL;\n> > > +\n> > > +\tsrc_width = convert_buswidth(src_addr_width);\n> > > +\tdst_width = convert_buswidth(dst_addr_width);\n> > > +\tdst_burst = convert_burst(dst_maxburst);\n> > > +\tsrc_burst = convert_burst(src_maxburst);\n> > \n> > I'm not sure what you're trying to do here. Could you split your patch\n> > by logical change, this doesn't seem related to just supporting the\n> > H3, but a heavier refactoring.\n> \n> Untangling 3 independent steps - handling of DMA_SLAVE_BUSWIDTH_UNDEFINED, \n> range checking, and conversion to register value. As the valid ranges depend \n> on the controller, the code is much easier to read if the range check is done \n> first, and then the conversion.\n\nPlease make separate patches as well for the splitting up of each of\nthose steps.\n\nThanks!\nMaxime","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xkKyR4fGsz9s2G\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tFri,  1 Sep 2017 23:36:15 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752074AbdIANf5 (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tFri, 1 Sep 2017 09:35:57 -0400","from mail.free-electrons.com ([62.4.15.54]:53206 \"EHLO\n\tmail.free-electrons.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752039AbdIANfv (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Fri, 1 Sep 2017 09:35:51 -0400","by mail.free-electrons.com (Postfix, from userid 110)\n\tid 439692098E; Fri,  1 Sep 2017 15:35:49 +0200 (CEST)","from localhost (LStLambert-657-1-97-87.w90-63.abo.wanadoo.fr\n\t[90.63.216.87])\n\tby mail.free-electrons.com (Postfix) with ESMTPSA id 1202B20989;\n\tFri,  1 Sep 2017 15:35:49 +0200 (CEST)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on\n\tmail.free-electrons.com","X-Spam-Level":"","X-Spam-Status":"No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT,\n\tURIBL_BLOCKED shortcircuit=ham autolearn=disabled version=3.4.0","Date":"Fri, 1 Sep 2017 15:35:49 +0200","From":"Maxime Ripard <maxime.ripard@free-electrons.com>","To":"Stefan Bruens <stefan.bruens@rwth-aachen.de>","Cc":"linux-sunxi <linux-sunxi@googlegroups.com>,\n\tdevicetree@vger.kernel.org, dmaengine@vger.kernel.org,\n\tvinod.koul@intel.com, linux-arm-kernel@lists.infradead.org,\n\tlinux-kernel@vger.kernel.org, Chen-Yu Tsai <wens@csie.org>,\n\tRob Herring <robh+dt@kernel.org>","Subject":"Re: [PATCH 1/3] dmaengine: sun6i: Correct DMA support on H3","Message-ID":"<20170901133549.cr2ivmfr5mnrdujg@flea>","References":"<20170830233609.13855-1-stefan.bruens@rwth-aachen.de>\n\t<20170830233609.13855-2-stefan.bruens@rwth-aachen.de>\n\t<20170831145135.c6a2wubcf6xu34tz@flea.lan>\n\t<a649f98d4ac44abc873c5110ce9363e5@rwthex-w2-b.rwth-ad.de>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha1;\n\tprotocol=\"application/pgp-signature\"; boundary=\"ezvmhlantquspm52\"","Content-Disposition":"inline","In-Reply-To":"<a649f98d4ac44abc873c5110ce9363e5@rwthex-w2-b.rwth-ad.de>","User-Agent":"NeoMutt/20170714 (1.8.3)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1761743,"web_url":"http://patchwork.ozlabs.org/comment/1761743/","msgid":"<5722405.lpx0YHbn2k@sbruens-linux>","list_archive_url":null,"date":"2017-09-01T14:42:47","subject":"Re: [PATCH 1/3] dmaengine: sun6i: Correct DMA support on H3","submitter":{"id":67055,"url":"http://patchwork.ozlabs.org/api/people/67055/","name":"Stefan Brüns","email":"stefan.bruens@rwth-aachen.de"},"content":"On Freitag, 1. September 2017 15:35:49 CEST Maxime Ripard wrote:\n> On Fri, Sep 01, 2017 at 05:04:54AM +0200, Stefan Bruens wrote:\n> > On Donnerstag, 31. August 2017 16:51:35 CEST Maxime Ripard wrote:\n> > > Hi,\n> > > \n> > > On Thu, Aug 31, 2017 at 01:36:07AM +0200, Stefan Brüns wrote:\n> > > > +/* Between SoC generations, there are some significant differences:\n> > > > + * - A23 added a clock gate register\n> > > > + * - the H3 burst length field has a different offset\n> > > > + */\n> > > \n> > > This is not the proper comment style.\n> > > \n> > > > +enum dmac_variant {\n> > > > +\tDMAC_VARIANT_A31,\n> > > > +\tDMAC_VARIANT_A23,\n> > > > +\tDMAC_VARIANT_H3,\n> > > > +};\n> > > > +\n> > > \n> > > And this is redundant with what we already have in our structures.\n> > \n> > Actually, its not. For H3, there are currently at least 3 register\n> > compatible SoCs: H5 is identical, R40 has 16 dma channels, A64 has 8\n> > channels. So if the current config structure is kept, we need 3 different\n> > compatible strings. Same for the A23, which is register compatible to\n> > e.g. A83t and V3s, but with different numbers of DMA channels.\n> > \n> > So either you decorate the code with a cascade of\n> > \n> > if ((of_is_compatible(..A23..) || of_is_compatible(..A83T..) || ...) {\n> > } else if ((of_is_compatible(..H3..) || of_is_compatible(..A64..) || ...)\n> > {\n> > } else { /* A31 */\n> > }\n> > \n> > in a number of places, or you do it just once.\n> \n> That's not how you retrieve the structures. They are already\n> associated to the compatible, and you need to do a single lookup to\n> get them. So that's nowhere near what you're suggesting. You can have\n> a look at the of_match_device in the probe function.\n> \n\n\nPlease have a look at the current implementation of how the clock autogating \nin the probe function is done - it matches with the compatible string.\n\nOf course we can replace this with a match between sdev->config and the \nvarious sun6i_dma_config instances, but we would still have to do 3 matches \nfor the A23 register configuration (A23 || A83T || V3s) and 3 matches for the \nH3 register configuration (H3 || R40 || A64). There are currently *7* \ndifferent configs (V3s, R40 and A64 taken into account), but only 3 different \nregister variants.\n\nThis is the same rationale as the \"gate_needed\" boolean property proposed by \nIcenowy Zheng in the \"Allwinner V3s DMA support\" patch series. Obviously we \ndon't need a boolean, but a ternary option to cater for the gate_needed \ndifferences - \"NO_GATE\", \"A23_STYLE_GATE\", \"H3_STYLE_GATE\".\n\nKind regards,\n\nStefan\n\n\n\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xkMRK0m7fz9sPk\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tSat,  2 Sep 2017 00:42:53 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752028AbdIAOmv convert rfc822-to-8bit (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tFri, 1 Sep 2017 10:42:51 -0400","from mail-out-2.itc.rwth-aachen.de ([134.130.5.47]:8211 \"EHLO\n\tmail-out-2.itc.rwth-aachen.de\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751995AbdIAOmu (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Fri, 1 Sep 2017 10:42:50 -0400","from rwthex-s2-b.rwth-ad.de ([134.130.26.155])\n\tby mail-in-2.itc.rwth-aachen.de with ESMTP; 01 Sep 2017 16:42:48 +0200","from rwthex-w2-b.rwth-ad.de (2002:8682:1a9f::8682:1a9f) by\n\trwthex-s2-b.rwth-ad.de (2002:8682:1a9b::8682:1a9b) with Microsoft\n\tSMTP Server (TLS) id 15.0.1320.4; Fri, 1 Sep 2017 16:42:48 +0200","from rwthex-w2-b.rwth-ad.de ([fe80::200c:2ee4:85cf:8127]) by\n\trwthex-w2-b.rwth-ad.de ([fe80::200c:2ee4:85cf:8127%21]) with mapi id\n\t15.00.1320.000; Fri, 1 Sep 2017 16:42:47 +0200"],"X-IronPort-AV":"E=Sophos;i=\"5.41,458,1498514400\"; d=\"scan'208\";a=\"11402815\"","From":"=?iso-8859-1?q?Br=FCns=2C_Stefan?= <Stefan.Bruens@rwth-aachen.de>","To":"Maxime Ripard <maxime.ripard@free-electrons.com>","CC":"linux-sunxi <linux-sunxi@googlegroups.com>,\n\t\"devicetree@vger.kernel.org\" <devicetree@vger.kernel.org>,\n\t\"dmaengine@vger.kernel.org\" <dmaengine@vger.kernel.org>,\n\t\"vinod.koul@intel.com\" <vinod.koul@intel.com>,\n\t\"linux-arm-kernel@lists.infradead.org\" \n\t<linux-arm-kernel@lists.infradead.org>,\n\t\"linux-kernel@vger.kernel.org\" <linux-kernel@vger.kernel.org>,\n\tChen-Yu Tsai <wens@csie.org>, Rob Herring <robh+dt@kernel.org>","Subject":"Re: [PATCH 1/3] dmaengine: sun6i: Correct DMA support on H3","Thread-Topic":"[PATCH 1/3] dmaengine: sun6i: Correct DMA support on H3","Thread-Index":"AQHTIymuFy/HxNm7v0eY+51WgUeuOKKf+TyA","Date":"Fri, 1 Sep 2017 14:42:47 +0000","Message-ID":"<5722405.lpx0YHbn2k@sbruens-linux>","References":"<20170830233609.13855-1-stefan.bruens@rwth-aachen.de>\n\t<a649f98d4ac44abc873c5110ce9363e5@rwthex-w2-b.rwth-ad.de>\n\t<20170901133549.cr2ivmfr5mnrdujg@flea>","In-Reply-To":"<20170901133549.cr2ivmfr5mnrdujg@flea>","Accept-Language":"en-US, de-DE","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","x-ms-exchange-messagesentrepresentingtype":"1","x-ms-exchange-transport-fromentityheader":"Hosted","x-originating-ip":"[78.35.13.203]","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-ID":"<A6C847B31318F84DA1549A1C38771119@rwth-ad.de>","Content-Transfer-Encoding":"8BIT","MIME-Version":"1.0","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1762452,"web_url":"http://patchwork.ozlabs.org/comment/1762452/","msgid":"<20170904065027.7splxlgxhpu5nj56@flea>","list_archive_url":null,"date":"2017-09-04T06:50:27","subject":"Re: [PATCH 1/3] dmaengine: sun6i: Correct DMA support on H3","submitter":{"id":12916,"url":"http://patchwork.ozlabs.org/api/people/12916/","name":"Maxime Ripard","email":"maxime.ripard@free-electrons.com"},"content":"On Fri, Sep 01, 2017 at 02:42:47PM +0000, Brüns, Stefan wrote:\n> On Freitag, 1. September 2017 15:35:49 CEST Maxime Ripard wrote:\n> > On Fri, Sep 01, 2017 at 05:04:54AM +0200, Stefan Bruens wrote:\n> > > On Donnerstag, 31. August 2017 16:51:35 CEST Maxime Ripard wrote:\n> > > > Hi,\n> > > > \n> > > > On Thu, Aug 31, 2017 at 01:36:07AM +0200, Stefan Brüns wrote:\n> > > > > +/* Between SoC generations, there are some significant differences:\n> > > > > + * - A23 added a clock gate register\n> > > > > + * - the H3 burst length field has a different offset\n> > > > > + */\n> > > > \n> > > > This is not the proper comment style.\n> > > > \n> > > > > +enum dmac_variant {\n> > > > > +\tDMAC_VARIANT_A31,\n> > > > > +\tDMAC_VARIANT_A23,\n> > > > > +\tDMAC_VARIANT_H3,\n> > > > > +};\n> > > > > +\n> > > > \n> > > > And this is redundant with what we already have in our structures.\n> > > \n> > > Actually, its not. For H3, there are currently at least 3 register\n> > > compatible SoCs: H5 is identical, R40 has 16 dma channels, A64 has 8\n> > > channels. So if the current config structure is kept, we need 3 different\n> > > compatible strings. Same for the A23, which is register compatible to\n> > > e.g. A83t and V3s, but with different numbers of DMA channels.\n> > > \n> > > So either you decorate the code with a cascade of\n> > > \n> > > if ((of_is_compatible(..A23..) || of_is_compatible(..A83T..) || ...) {\n> > > } else if ((of_is_compatible(..H3..) || of_is_compatible(..A64..) || ...)\n> > > {\n> > > } else { /* A31 */\n> > > }\n> > > \n> > > in a number of places, or you do it just once.\n> > \n> > That's not how you retrieve the structures. They are already\n> > associated to the compatible, and you need to do a single lookup to\n> > get them. So that's nowhere near what you're suggesting. You can have\n> > a look at the of_match_device in the probe function.\n> \n> Please have a look at the current implementation of how the clock autogating \n> in the probe function is done - it matches with the compatible string.\n\nYeah, and we should get rid of that as well.\n\n> Of course we can replace this with a match between sdev->config and the \n> various sun6i_dma_config instances, but we would still have to do 3 matches \n> for the A23 register configuration (A23 || A83T || V3s) and 3 matches for the \n> H3 register configuration (H3 || R40 || A64). There are currently *7* \n> different configs (V3s, R40 and A64 taken into account), but only 3 different \n> register variants.\n> \n> This is the same rationale as the \"gate_needed\" boolean property proposed by \n> Icenowy Zheng in the \"Allwinner V3s DMA support\" patch series. Obviously we \n> don't need a boolean, but a ternary option to cater for the gate_needed \n> differences - \"NO_GATE\", \"A23_STYLE_GATE\", \"H3_STYLE_GATE\".\n\nOr we can just have an extra field in sun6i_dma_config that would set\nthe gate register offset? If it's non-zero, use whatever you have set\nthere, and then you just have to care about two cases, no matter how\nmany offsets we'll have in the wild.\n\nMaxime","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xm0q64FGpz9s7h\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tMon,  4 Sep 2017 16:50:42 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752064AbdIDGuk (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tMon, 4 Sep 2017 02:50:40 -0400","from mail.free-electrons.com ([62.4.15.54]:33176 \"EHLO\n\tmail.free-electrons.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751620AbdIDGuk (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Mon, 4 Sep 2017 02:50:40 -0400","by mail.free-electrons.com (Postfix, from userid 110)\n\tid 2C9F221EC4; Mon,  4 Sep 2017 08:50:37 +0200 (CEST)","from localhost (LStLambert-657-1-97-87.w90-63.abo.wanadoo.fr\n\t[90.63.216.87])\n\tby mail.free-electrons.com (Postfix) with ESMTPSA id EFE8621EA0;\n\tMon,  4 Sep 2017 08:50:26 +0200 (CEST)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on\n\tmail.free-electrons.com","X-Spam-Level":"","X-Spam-Status":"No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT,\n\tURIBL_BLOCKED shortcircuit=ham autolearn=disabled version=3.4.0","Date":"Mon, 4 Sep 2017 08:50:27 +0200","From":"Maxime Ripard <maxime.ripard@free-electrons.com>","To":"=?iso-8859-1?q?Br=FCns=2C?= Stefan <Stefan.Bruens@rwth-aachen.de>","Cc":"linux-sunxi <linux-sunxi@googlegroups.com>,\n\t\"devicetree@vger.kernel.org\" <devicetree@vger.kernel.org>,\n\t\"dmaengine@vger.kernel.org\" <dmaengine@vger.kernel.org>,\n\t\"vinod.koul@intel.com\" <vinod.koul@intel.com>,\n\t\"linux-arm-kernel@lists.infradead.org\" \n\t<linux-arm-kernel@lists.infradead.org>,\n\t\"linux-kernel@vger.kernel.org\" <linux-kernel@vger.kernel.org>,\n\tChen-Yu Tsai <wens@csie.org>, Rob Herring <robh+dt@kernel.org>","Subject":"Re: [PATCH 1/3] dmaengine: sun6i: Correct DMA support on H3","Message-ID":"<20170904065027.7splxlgxhpu5nj56@flea>","References":"<20170830233609.13855-1-stefan.bruens@rwth-aachen.de>\n\t<a649f98d4ac44abc873c5110ce9363e5@rwthex-w2-b.rwth-ad.de>\n\t<20170901133549.cr2ivmfr5mnrdujg@flea>\n\t<5722405.lpx0YHbn2k@sbruens-linux>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha1;\n\tprotocol=\"application/pgp-signature\"; boundary=\"lybdai6y7ovcphjs\"","Content-Disposition":"inline","In-Reply-To":"<5722405.lpx0YHbn2k@sbruens-linux>","User-Agent":"NeoMutt/20170714 (1.8.3)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}}]