From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sinan Kaya Subject: Re: [PATCH 2/2] dmaengine: dmatest: add support for memset test Date: Wed, 28 Jun 2017 21:54:14 -0400 Message-ID: <1e6151c9-693b-335a-2e4f-bcf201e77b6d@codeaurora.org> References: <1498610786-6090-1-git-send-email-okaya@codeaurora.org> <1498610786-6090-2-git-send-email-okaya@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: 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 In-Reply-To: Content-Language: en-US Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org 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" 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. From mboxrd@z Thu Jan 1 00:00:00 1970 From: okaya@codeaurora.org (Sinan Kaya) Date: Wed, 28 Jun 2017 21:54:14 -0400 Subject: [PATCH 2/2] dmaengine: dmatest: add support for memset test In-Reply-To: References: <1498610786-6090-1-git-send-email-okaya@codeaurora.org> <1498610786-6090-2-git-send-email-okaya@codeaurora.org> Message-ID: <1e6151c9-693b-335a-2e4f-bcf201e77b6d@codeaurora.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.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.