Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756135AbcLSPlg (ORCPT ); Mon, 19 Dec 2016 10:41:36 -0500 Received: from mail-sn1nam01on0060.outbound.protection.outlook.com ([104.47.32.60]:4426 "EHLO NAM01-SN1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751652AbcLSPle (ORCPT ); Mon, 19 Dec 2016 10:41:34 -0500 Authentication-Results: spf=pass (sender IP is 149.199.60.100) smtp.mailfrom=xilinx.com; vger.kernel.org; dkim=none (message not signed) header.d=none;vger.kernel.org; dmarc=bestguesspass action=none header.from=xilinx.com; X-IncomingTopHeaderMarker: OriginalChecksum:;UpperCasedChecksum:;SizeAsReceived:2775;Count:27 From: Appana Durga Kedareswara Rao To: Laurent Pinchart CC: "dan.j.williams@intel.com" , "vinod.koul@intel.com" , "michal.simek@xilinx.com" , Soren Brinkmann , "moritz.fischer@ettus.com" , "luis@debethencourt.com" , "Jose.Abreu@synopsys.com" , "dmaengine@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" Subject: RE: [PATCH 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma Thread-Topic: [PATCH 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma Thread-Index: AQHSVuWKeAv0STYsN0iupc5V13K6T6EKNZYAgAMbnnA= Date: Mon, 19 Dec 2016 15:41:25 +0000 Message-ID: References: <1481814682-31780-1-git-send-email-appanad@xilinx.com> <1481814682-31780-3-git-send-email-appanad@xilinx.com> <30884836.ckISXSrEvA@avalon> In-Reply-To: <30884836.ckISXSrEvA@avalon> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [172.23.229.139] Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 X-RCIS-Action: ALLOW X-TM-AS-Product-Ver: IMSS-7.1.0.1224-8.0.0.1202-22770.006 X-TM-AS-User-Approved-Sender: Yes;Yes X-IncomingHeaderCount: 27 X-EOPAttributedMessage: 0 X-MS-Office365-Filtering-HT: Tenant X-Forefront-Antispam-Report: CIP:149.199.60.100;IPV:NLI;CTRY:US;EFV:NLI;SFV:NSPM;SFS:(10009020)(6009001)(7916002)(39840400002)(39450400003)(39860400002)(39410400002)(39850400002)(2980300002)(438002)(51914003)(189002)(199003)(7736002)(63266004)(97756001)(46406003)(8746002)(4326007)(33656002)(305945005)(8676002)(81156014)(23726003)(3846002)(6116002)(356003)(102836003)(8936002)(92566002)(81166006)(47776003)(2920100001)(2900100001)(5660300001)(7696004)(106116001)(189998001)(2906002)(38730400001)(55846006)(50466002)(2950100002)(626004)(229853002)(110136003)(106466001)(6916009)(50986999)(76176999)(5250100002)(54356999)(107986001)(5001870100001);DIR:OUT;SFP:1101;SCL:1;SRVR:DM5PR02MB2779;H:xsj-pvapsmtpgw02;FPR:;SPF:Pass;PTR:xapps1.xilinx.com,unknown-60-100.xilinx.com;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;CY1NAM02FT008;1:LhNE5Jk1BDjgLKxpjIBranIiNwMZ4Kq7lsWyztJR0r6wZG6bTKcyO+1Rg36FdTAALXDwp4wLp7ihgBl+kJ00E+sbISOiDGczb6ppH+RKE/qwohKZmsypT9dAubL5t9++Kk1ggPVRHwsdH8Kqngi5+2+T5zJ3CvOKTeYfw5sAdykQLHSmwxvoxAcTjcL3tflDtTq9IkF6FSrfsn92mD/6a1NXhTgbpF6KcCxXGxefzlNW67ex6WlEBHjZ9x3jaHlnq+TvVSoifDQUv7QHMvF6hPBJaAanH2D0we3UuL5TT8A0kP4qt57hP9CBjp29M6u9qlXkW1XFtzHVaIoiKR+D7GvwDXZRVNn2CqFNA3HfNuajhfP81ZbPxnFihzZ3zV/Nwo7bd6VJS0AP56BMiZp2TykQcjrM0E3Ffhql30Y5PCuIgKjEHobupRPVBWG0Zw211nqOiT8i5P/izoRrCK6Jn8pHWpztE6z8lw27XZSuG2ZzZ7qglxT+Lnix2dlAxi335sYEjromvLGLgI5jUMJqqaGDZKaddyh4pjL93uAcYhgz8+itUhQIBn6ND8all80F5B1yUK28ztkkpcqBiwO0KHPTeGbw6owvzUMaLIaAa05W+b3MJGbGyUF4HoDvZwHqwGGgg1B1N0lP904MWVQadw== X-MS-Office365-Filtering-Correlation-Id: bf537979-be51-445d-b559-08d4282580aa X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(8251501002);SRVR:DM5PR02MB2779; X-Microsoft-Exchange-Diagnostics: 1;DM5PR02MB2779;3:0JaZnc2xleQVgGhX10bY501J9zbMeUkVphU47C8ui3D6/EKRxSYRHpgvlm8dc2+C0eZfFyln+il8njAo3CeEqja7iaTDZeeJSdAYg7uRwPjdQvEkEwWvK8YmqW0v6gi0JtTvidi4JZqNc7zFAvKN5uIy3hsBN8tuKj92Lcgq6Q8kcAMlKnIPo2XuCat7Y03ms+DQuqIhWQna6qhKM8Q5+f4yx3uyj6u2ajMeF6/nn50hcQtpmOjMdRx47BqAXQnH2/lsop3smR65zcmuFm7lVRGbbSwjRWcO4bA2E0OHD0WDDWv3uCyolL73LJdR62TpYrj4fsKL5ytFQrBVNlOmV+brc6DnVTPNuPhZc8wXlNP6RRDHzS8W8DpdwLk4MQ+w/zgnfy1o1KCVzUfeu6YmDw== X-Microsoft-Exchange-Diagnostics: 1;DM5PR02MB2779;25:IRCl5VLuM2O2hC9MZUTpTyJmbuUmJT15/YCkbwnsAG88LdRHD3J7MOBlvAZHqJ42Tm3Mqp/UdYgSzNSh2v+9kVn0JZidC6rehZGJLqGT9erfFLRJN9yCgQpQHAiLa1qBIJf5Htt4/vNBvgdcMAHICNx2cma35R5Q8kqluDdRt14SIPn/NUGhD4oqXkvbuymde8w0j3TgWqili6pYHiu4LyBnh6I4AsXDd6kprkIp4GPB+oodZ7yg6nWF+pWype18/g1g9eVMKbuzbH4WduUDlOlWiSdoEi69y7PhtOtG1yFKyXS7TaBqbqHJYZ0YD4SpuzDo3CeZpV+nVOdTXhYNoI8hjKOFpU3sP1el02V7776TF4Gg5QxV9YixiJvNbr8WQZmSkYgCT1mRSb9r1601l3K7a6WrwVPqXjkpCVqHN+lFZJEXVQyEHpMrpO/g810sHlnOpKVd4AJuvW4PewJu+gTBbafGxM4ryHUcjhwZkbxmm2PUuqhP74AjW6sy22BxtgixYBcKdyw7/baw8XY5dujDq50BnIgvDprjCt7ncjDkpS8ApktYmwfHuv4lmzOdmIzLST4yPoNn/WFxUIVpoMRVHlubPZKKzAjbE/np3r3cUimhN4zI61S+UPrhX2+eF5dyWc2X0Vd9UzcRejwf2yeV03DP3jgDhRiBht60BcLWQ9N+f2msI5s8Yr4BpGhpCdW85uqgnWYTZhCgA0W+9fzlc2ipDcJsd0ylvQGemGz03DchOJmITnnRnypDsHT1SU9HDxIhFUSmhCJTJORXXVvW8BThYb9QS5II/1oGTd/vI2QfkwoP91kYB2CaYeTV X-Microsoft-Exchange-Diagnostics: 1;DM5PR02MB2779;31:P9i5dPDPQMG5129O5KthJl127lW0QuOzgTxLnalsNdH3FyocmMEI2p5zA1NUFV4OcLCahhKcD90tbGzoCL3cgBjz9I7Ho7nVfV1fI2EufalLV9ThXvRGA1UfQuG/RlWeC6/DcJ7HvgUuyk25ot0Qkb/6wcbOZQFSP6ttvxeNfSMde82mKl3FBx2kRE9L2aL83FusMCM0P2xESbbRIgiAG1FSHRE5EwxUBSqa8oHkZkO0tWL5YxOMCYQkAxpZvEOBCDrNW7EHIVA/Gas9fVj/hQ==;20:6chEpy1dFPFW4jEef01E2wucpXCCPDbULByJStbpuqJFqxK9B66vZkdlHajYwBHCkb6tOOTa1048nBAuWxQJy/DK0WT2EmoBF9v0E78cyiCp/b8bJpOauTKsxQHVNBSY6RdNDPBzCDSXEhcOmvylK5qy1O+dyENydA2tPjnphrNLSs5GXao7ctQTBDdhvYRyKzyfAxywLrebUYtN1yv1k1PgQ14vK0dBaGxGNDWxL9Ya/yDYeGiP4eKLKaVU9wOEJdIGQYU1Nzxvmty6s26L7Io4xUEoCd/jZXLn+0RnNGzN9qWAVKKJwS4/LdyWomTSGvr2Ioe9HsIRstyLA6Cvbk5Rjvvsf7m76AclVeDyn4xzziUXrPm8tcqAd+i6gy2Xi5a5739dQFRKgHDbGbdOSa45WhAwfBQluf9rhvbQAfi538T/LnrR8hVQzs+zfetQuPL1+vkVW1pSXIkgvoFprASakqLnTbIHRGQZbt34+zvH+CVuMMn+QBRlfhgMQ3GG X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040375)(601004)(2401047)(5005006)(8121501046)(13023025)(13024025)(13015025)(13017025)(13018025)(10201501046)(3002001)(6055026)(6041248)(20161123560025)(20161123555025)(20161123562025)(20161123564025)(6072148);SRVR:DM5PR02MB2779;BCL:0;PCL:0;RULEID:;SRVR:DM5PR02MB2779; X-Microsoft-Exchange-Diagnostics: 1;DM5PR02MB2779;4:+7cq8tEkWjg0Ltknp8NyMZeOGE1yGZ26M3hM6ENcOtjGUZ77brJ5uHKtZiVP35FbzVpE1XsEqstU+6bcwUl5hmKCd4oggWB0T98kfsAGIUSnwsCdvMRhb/qqcUsPSJsoizM9aao7sliWnVxIiSg5a4OGimcDMTwIXbkLrSqUZuaENwe9Bp5vWuxsJJNgcNnYStZc39qKU0ZC88m4EXVDnEstsMgLID3SvhK/0d8dnF6w/QPHnwbEvrn0i8PnsiWARDxXp1J+zikAHbu4BeTrn9waeb888Qo+yqKRmUiYK3emPhpm3rVGmZvkI50lGhGFR43Ka147fX7iy5AAq7YGxiPwQYToO66yp5p5rMf4Dj/a0oOcDRoqFdZtbDQnXMYrOJNVpZcT1bBKitSQWdEJy70qwzJPvrPBJgpjbvKGg8PRgVJ7kole6kXCY51ZUhpfUOj0hNgHrXbSAjB1GGqGxHYKrkt0y/d21tRV5gitWu+vF0x7Z/G3VzI4FYkU0zdliC8zeUe085JuHTW7Z2NhhgBkaj9ox/LOgNAfzOegX6VERZLzVgBPxbzcxJT+na9uf1y8MD5eWP6deevvmAaTRrJTQjU3YMVATbyI0A8bHlggun2fhEBIkfT4wtYBV2Rd6G1A6Kg2jmDvjr+z+DYRW304mUk2VQy2R69mqEYXgKrOueMeKcwE/vJ/iaUxX0KC X-Forefront-PRVS: 01613DFDC8 X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;DM5PR02MB2779;23:NVCXCkFHEXJFV7NJP8HTLrcRrSH4BSMV1J/hgwjcL?= =?us-ascii?Q?J7rcgwDKtPgcIMoAxYKe/uZr4dkiJ+Cj8XZ+tMOQSxfrI4qQosXPT+VXd0Ze?= =?us-ascii?Q?1Q4DfHrCB9ij9jcBCc1EI3GGJgI4tPnYgEu5LGIc7dTx6mUjjAvQoIQHtVD6?= =?us-ascii?Q?6EyDIffk4CcT2RhmH4Ya+anjnFOtdelRgWU4lKCzBAj41THS/7wBgLkYYShM?= =?us-ascii?Q?Jo1GDJEmHywkpyJLAoFAeTHoS4wei/O4Ejn2F+Yso+o6V2F9pBzTlI5YiTkz?= =?us-ascii?Q?eKWQs8XjRryu+T6Ea4U/gJ3TJh9Y14gs3SafTQ/nB4dypxRxjz//uI+7a1iO?= =?us-ascii?Q?6hKjJnjMvG+98yGez9J9uLmEiYKKoPfMIfNKYQZrUSoq+xR0fJrgVgG/m1kI?= =?us-ascii?Q?vCADJxk9UaTCBMSTjqt16bb1xwpNoCfzzD3A/89aofb5txIuEa9d0h5K4qKe?= =?us-ascii?Q?a743Oig5rHLn2CBYLLEQK/UMTljH9H0KvPeKplYlfYt1gKcpAs1KX11I/7R+?= =?us-ascii?Q?4Sy6/gflwBjFSCvsu34CT6I6zsvFcY0u2PZqGFMoA1jugxKeBxRClU/jV2yb?= =?us-ascii?Q?Ay/Xbfm8xabkyMucNEV9cd6KtjhVfFTs6yxooJWpWFf/V57ed0BFFvOH1KZz?= =?us-ascii?Q?2RI63Z86nkBKF0uNgWCyN9r9cNYODITxFeg1Th2T/UKLCPBYFE00xzbhslQM?= =?us-ascii?Q?GRzHB0jZpAD50JK69vqBCCJYw1SAaDWhlK8aliVX2OTlEg/sA7SQTerpfQYU?= =?us-ascii?Q?o6x77BQ9k+oT/6z+CWpVfh0B1HlvIk9uXpMzlIZtG8LehSLKpstSfCdc/hqZ?= =?us-ascii?Q?Lv4as75EjU9Cun6ZqRnqE33kqLDBKkxqNvpVbMWhWcd0Tgy8aXP0TcItzBjH?= =?us-ascii?Q?z1hcPP9dcmgwZWfzkwkpNuup+KTUoBGiqB1RSo51JKFgw/vJEi2kpkuVpkdQ?= =?us-ascii?Q?JaBcBkW8TY9rp7F9QWIGk1oQzLbQVJIHU1Kx2L972MDzMHDsLU/uLq0/ox1J?= =?us-ascii?Q?syWSdGm5QK4aDcZhJbAs7sDHnLwNEC+3PPzy4N5ZtS3bIgIzq/8vFMf7saD/?= =?us-ascii?Q?ejA0X4XR7xD6+BTZ4M/3gh1j6n9iqv9qHj5axp9yd8LMUlwqmFpsdg1vMUTM?= =?us-ascii?Q?mYVIOfjww4UUC2ybL3e25O9w+A/HYhPoIkJQmPjZgwpsKaJ1C239+6SpU6P/?= =?us-ascii?Q?R6FCLnE50R+LBW1Ae7DjATV+w44ttEY0kDoJexHSigRUKev+EknrCbr9fZ2m?= =?us-ascii?Q?3zXBkpd4cSDn+LaySXAmd7q6yl/dmt4qeeFX33HHEUorRpdhRPx1wVV1G8CS?= =?us-ascii?B?Zz09?= X-Microsoft-Exchange-Diagnostics: 1;DM5PR02MB2779;6:gIi2daT41bEMvcbacm561TEHSMVP2GWGcK7DneSiylmGbzonNVIPDq3XzSxKwaAL+U+wsXK4fQzf47US3ltad5OddXze4XlDKsjsxqpKEUpZLTqYtibPmwo4yNWWryCQIc4KFLP30Nwei1fsF0v87nROw1qNknTf5Slg4dFO2wsXgDzJuNhr4AgEwwDDLKhvuNiMA74nbu4lRtlNa5fnwqopoxbqxptk/SrhoP89S0W2Am5lgzWpfaFU9ck8iNaSkIAaTzwntCZ+YplP7gb69uiGpD6UZM3ftSXYVlsR8DtubrDQz0PGQ4MRGe41Lp4psFYM1qw0qvJt/FEecaga335SlmifG9X1ZdZQHY70XJmhQbo9BNotB9YQXuLsRJZQPatTozMIhmrUwJQcyEn+wRdazeyicijgnGz1+4nEhQGXDJblu9HS+svONgH9kK6sDyzh5pFraWTND3++Bujywg==;5:ImhlbUniYRsQhBWNolwJoYWtTiB/wijldu7PesdfsE8l9MU9n6ahBFAm9CXm9NlOD0F9iChzloGgK/A2pyLTiLT+2tD6oSgtx+PUV/yla0t7bcQDnH8AUtFWtbKy7g0RE6XG4071RyqCo8flNtUiIA==;24:ViWc8AZYjAm2+RJ/zFxfZlYwCypS1HEJps0vKKSTwd8pJDJ51SoqL6CyDwWFf/1WZvNOumlU5XcHFdnEmsjsTrZROSwUNjwvY5pSgw+jSw0= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;DM5PR02MB2779;7:vQc/N/v8pttSxO2UvyRN4HqWp562jigDyoy+iLxw1ZtO/hH3q3OsQqcjkIp1Mr4W9KVjkT77NH+OUthXSqgaiMiGo+M0LGEcQUwXYDyb+NMZUQgtOu4+12AIdJmnavHEefshDFhOtR2TgoY8JyTbWq2Iehf/L9DqlBDJ753AGrZngxxvWhEzLifG9hfMA4X0YE/q/aEa3/7YKXbnGQpEiAM0FeaO0a8NNp67b1RooJQmixR5dQdMita4E2De5Vjpv2sXHQVUsz3Iw0hrreEfeMrG9SIdkuEw0FK+eRncP4c23X5uAmoV8b6LrNCjwM1op5D3+u2CJSmHiBgXOjTrvhHsHyTaGUGKPDOpyv6nzatPwoR019dhgZacZlGrdCsU0OmCHBu1t1SDNU9ryRwsI57i5utotyiSuszh8UEzesm2gx+krmLlA4Wp+sOL/sTBprE/9Wu0r8dRmhFohOho2Q== X-OriginatorOrg: xilinx.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Dec 2016 15:41:29.7717 (UTC) X-MS-Exchange-CrossTenant-Id: 657af505-d5df-48d0-8300-c31994686c5c X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=657af505-d5df-48d0-8300-c31994686c5c;Ip=[149.199.60.100];Helo=[xsj-pvapsmtpgw02] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR02MB2779 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id uBJFff05029804 Content-Length: 2727 Lines: 99 Hi Laurent Pinchart, Thanks for the review... > > + int i = 0, j = 0; > > > > if (chan->desc_submitcount < chan->num_frms) > > i = chan->desc_submitcount; > > I don't get this. i seems to index into a segment start address array, but gets > initialized with a variable documented as "Descriptor h/w submitted count". I'm > not familiar with the hardware, but it makes no sense to me. Here i is the h/w buffer address. For ex: If the h/w is configured for 3 frame buffers and user submits 4 desc's Then we need to submit only 3 frame buffers to the h/w and the next desc will be submitted After there is a room for buffers I mean when the free buffer is available. > > > - list_for_each_entry(segment, &desc->segments, node) { > > - if (chan->ext_addr) > > - vdma_desc_write_64(chan, > > - > XILINX_VDMA_REG_START_ADDRESS_64(i++), > > - segment->hw.buf_addr, > > - segment->hw.buf_addr_msb); > > - else > > - vdma_desc_write(chan, > > - > XILINX_VDMA_REG_START_ADDRESS(i++), > > - segment->hw.buf_addr); > > - > > - last = segment; > > Isn't it an issue to write the descriptors only after calling > xilinx_dma_start() ? Until writing to the VSIZE h/w won't get started... > > > + for (j = 0; j < chan->num_frms; ) { > > + list_for_each_entry(segment, &desc->segments, node) > { > > + if (chan->ext_addr) > > + vdma_desc_write_64(chan, > > + > XILINX_VDMA_REG_START_ADDRESS_64(i++), > > + segment->hw.buf_addr, > > + segment->hw.buf_addr_msb); > > + else > > + vdma_desc_write(chan, > > + > XILINX_VDMA_REG_START_ADDRESS(i++), > > + segment->hw.buf_addr); > > I assume the size of the start address array to be limited by the hardware, but I > don't see how this code prevents from overflowing this. > > The whole function is very difficult to understand, it probably requires a rewrite. Will fix it in v2... > > > + last = segment; > > + } > > + list_del(&desc->node); > > + list_add_tail(&desc->node, &chan->active_list); > > + j++; > > + if (list_empty(&chan->pending_list)) > > + break; > > + desc = list_first_entry(&chan->pending_list, > > + struct > xilinx_dma_tx_descriptor, > > + node); > > } > > if (!chan->has_sg) { > > - list_del(&desc->node); > > - list_add_tail(&desc->node, &chan->active_list); > > - chan->desc_submitcount++; > > - chan->desc_pendingcount--; > > if (chan->desc_submitcount == chan->num_frms) > > chan->desc_submitcount = 0; > > } else { > > While at it, can you merge this into the previous if (chan->has_sg) { ... } else { ... } > ? Having them separate is confusing. Ok sure will fix in v2... Regards, Kedar. > > > -- > Regards, > > Laurent Pinchart