From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751943AbdF2By1 (ORCPT ); Wed, 28 Jun 2017 21:54:27 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:35870 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751544AbdF2ByT (ORCPT ); Wed, 28 Jun 2017 21:54:19 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 221D760A3B Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=okaya@codeaurora.org Subject: Re: [PATCH 2/2] dmaengine: dmatest: add support for memset test To: Andy Shevchenko Cc: dmaengine , Timur Tabi , linux-arm-msm@vger.kernel.org, linux-arm Mailing List , Dan Williams , Vinod Koul , "linux-kernel@vger.kernel.org" References: <1498610786-6090-1-git-send-email-okaya@codeaurora.org> <1498610786-6090-2-git-send-email-okaya@codeaurora.org> From: Sinan Kaya Message-ID: <1e6151c9-693b-335a-2e4f-bcf201e77b6d@codeaurora.org> Date: Wed, 28 Jun 2017 21:54:14 -0400 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Andy, On 6/28/2017 4:58 AM, Andy Shevchenko wrote: > On Wed, Jun 28, 2017 at 3:46 AM, Sinan Kaya wrote: >> >> echo dma0chan0 > /sys/module/dmatest/parameters/channel >> echo 2 > /sys/module/dmatest/parameters/dmatest >> echo 2000 > /sys/module/dmatest/parameters/timeout >> echo 10 > /sys/module/dmatest/parameters/iterations >> echo 1 > /sys/module/dmatest/parameters/run > > Good someone is developing tests! > My comments below. Yeah, no problem. > >> static void dmatest_init_srcs(u8 **bufs, unsigned int start, unsigned int len, >> + unsigned int buf_size, bool is_memset) >> { >> unsigned int i; >> u8 *buf; >> >> for (; (buf = *bufs); bufs++) { >> + for (i = 0; i < start; i++) { > >> + unsigned int val = is_memset ? PATTERN_MEMSET_IDX : i; >> + >> + buf[i] = PATTERN_SRC | (~val & PATTERN_COUNT_MASK); > >> + unsigned int val = is_memset ? PATTERN_MEMSET_IDX : i; >> + >> buf[i] = PATTERN_SRC | PATTERN_COPY >> + | (~val & PATTERN_COUNT_MASK); > >> + unsigned int val = is_memset ? PATTERN_MEMSET_IDX : i; >> + >> + buf[i] = PATTERN_SRC | (~val & PATTERN_COUNT_MASK); > > Looks like a candidate now for a helper function > > static inline u8 gen_src_value(u8 index, bool is_memset) > { > u8 val = is_memset ? PATTERN_MEMSET_IDX : i; > return PATTERN_SRC | (~val & PATTERN_COUNT_MASK); > } > > buf[i] = gen_src_value(i, is_memset); > buf[i] = gen_src_value(i, is_memset) | PATTERN_COPY; > buf[i] = gen_src_value(i, is_memset); will change as you recommended. > >> + unsigned int val = is_memset ? PATTERN_MEMSET_IDX : i; >> + >> + buf[i] = PATTERN_DST | (~val & PATTERN_COUNT_MASK); > >> + unsigned int val = is_memset ? PATTERN_MEMSET_IDX : i; >> + >> buf[i] = PATTERN_DST | PATTERN_OVERWRITE >> + | (~val & PATTERN_COUNT_MASK); > >> + unsigned int val = is_memset ? PATTERN_MEMSET_IDX : i; >> + >> + buf[i] = PATTERN_DST | (~val & PATTERN_COUNT_MASK); > > Ditto for DST. done. > >> + unsigned int counter, bool is_srcbuf, bool is_memset) > > This is a signal to replace it with > > unsigned int flags > > #define DMATEST_FL_SRCBUF BIT(0) > #define DMATEST_FL_MEMSET BIT(1) > > or alike. > > I'm not insisting, just consider an opportunity. OK. I'm not touching it for the moment. Maybe, we can have another patch later to get rid of flags. > >> + bool is_srcbuf, bool is_memset) > > Ditto. > >> start = ktime_get(); >> + >> pr_debug("%s: verifying source buffer...\n", current->comm); > > The change doesn't belong to the patch. > >> + >> error_count += dmatest_verify(thread->dsts, dst_off + len, > > Ditto. > Posted V2 with the change now. Sinan -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.