Received: by 10.192.165.148 with SMTP id m20csp4490840imm; Tue, 8 May 2018 09:14:44 -0700 (PDT) X-Google-Smtp-Source: AB8JxZoA9kZDtLR/M+WKqlYjGV292c4+pIqRHnE5RqdL4go5VmDNS/xkBt2y4v4r80iaJFW/26tG X-Received: by 2002:a17:902:b589:: with SMTP id a9-v6mr3753804pls.161.1525796084038; Tue, 08 May 2018 09:14:44 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1525796083; cv=none; d=google.com; s=arc-20160816; b=WgEM0/iwbApoWGHq1gWGluq3k7Nq0CfYfr0vBbj8hInKJPUQ0NvfAgo8w04B6srk1q zlgkiybgyTU6DEnBoiRNPwZ0o2VVswhV3/sneygb7N8NYQFEp3rMSSNnjuFA0kMnTlVF nraPEDprg7qP4KhD4I71KpJQ/9B94Xu+kuNhIc78akSviQTvCYvXFyx6wXGR5Ym4kptp DR5lDtcuQJBhRxStukt/s04b7uYPM5XXWA55IWN08hvF8nnek8jQKpXwmIkm+uR57dEs wdc0FR3jMNza81q58x1LUnl1UvlQBbi33arf+afW8FNFrHIyuH9p0R7VV1k7O3+sMEEL oGqQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :references:in-reply-to:mime-version:dkim-signature :arc-authentication-results; bh=BJTd0NuwCybZOmTUvNlW8cNQIj/MQSaJ8KDHrgB6BYU=; b=I05qec855y/DMSBm9gN+B6UOl1pTTrPBVMJgH3DRJQ2L0CiViAJeswhTsFQW1yZdIw wPoMHIc0GPGObC/e/otEznxZaSfFla5AVeqBYwn7ZOBN+jMUS7PZjIrBdd66nGCej3QT BL935pURyw9nXJz/mOEGB6PWulhKVWDg1zdmWASuQI3yoce4VHh8xD6Z9CnyvBw+3mFp 6H/1MXgwxIFnyuXoosnlrig4HIzerQYlZ+aExc1kOi+CvxU60D7QWQeb7bSwqmc3iGdC HeqqRfOslIXAYRYpyAKmTIZGE8OOd5fNyLeCEZnz9p4L6i7Ec0gsObrAo7p5069DxvW5 /wkw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=VWTzkgKA; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id p13-v6si18969055pgv.182.2018.05.08.09.14.29; Tue, 08 May 2018 09:14:43 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=VWTzkgKA; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932822AbeEHQN6 (ORCPT + 99 others); Tue, 8 May 2018 12:13:58 -0400 Received: from mail-wm0-f65.google.com ([74.125.82.65]:32886 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932737AbeEHQN4 (ORCPT ); Tue, 8 May 2018 12:13:56 -0400 Received: by mail-wm0-f65.google.com with SMTP id x12-v6so19800371wmc.0 for ; Tue, 08 May 2018 09:13:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=BJTd0NuwCybZOmTUvNlW8cNQIj/MQSaJ8KDHrgB6BYU=; b=VWTzkgKA55VpiJkjMztqfbZEuqeEGarErQC0CFj3Xe+WdrXWRq/EZBsMSyoK0kmsFA oGoyWWTsj4z/oi4NxSd101GHhfN3u+EgRtno5lChob+W+iiLk45COkz3m46U+wD7iVif yQFcSL8BPtW3VC+ZH6kxEHHse6XQH26ls+I54= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=BJTd0NuwCybZOmTUvNlW8cNQIj/MQSaJ8KDHrgB6BYU=; b=py9m4DiD4vY9fjehvg+PVMXb3sl59J2lbyHiS2qH/fJ8/PwTbKJG/S2QMA2rgE2sui FYz9Sekfftdi0MxcjYS2+FTxZVzD0tv1Iqa+Id6QQJe681hJSTxqfyzPl44veui9ExxY ApVLN/rd6qyyRf7W3VEwndox55fuJOXMQKVDiQ8J3OO0GnoOlKr2EBvTJNX4IgdGUQlk NBbmc3JOJPOZMy/GjI8V08EVrx/CvCztwxEHZdn7o0llKxRt2y4fyz4tamUdb4zlqVku KMtq2RRuoBMpQkfUi24PuJo9Ssx+gq2g73rLpHxc2RamoXDCtwWgYzqQDt2tJEJkXwAA O+AA== X-Gm-Message-State: ALQs6tBVcPb7Eska7t/KrvZRcqWakQADPUE3LdSrPJJ81HP4za5v0cu3 jJ6xDXF7ODrb8GzIUaayFqwbjTTqSphb/TjTp36XdQ== X-Received: by 2002:a50:a245:: with SMTP id 63-v6mr57219021edl.295.1525796035271; Tue, 08 May 2018 09:13:55 -0700 (PDT) MIME-Version: 1.0 Received: by 10.80.164.161 with HTTP; Tue, 8 May 2018 09:13:54 -0700 (PDT) In-Reply-To: References: <1525165857-11096-1-git-send-email-suzuki.poulose@arm.com> <1525165857-11096-19-git-send-email-suzuki.poulose@arm.com> <20180507202517.GC32594@xps15> From: Mathieu Poirier Date: Tue, 8 May 2018 10:13:54 -0600 Message-ID: Subject: Re: [PATCH v2 18/27] coresight: catu: Add support for scatter gather tables To: Suzuki K Poulose Cc: linux-arm-kernel , linux-kernel@vger.kernel.org, Mike Leach , Robert Walker , Mark Rutland , Will Deacon , Robin Murphy , Sudeep Holla , Frank Rowand , Rob Herring , John Horley Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 8 May 2018 at 09:56, Suzuki K Poulose wrote: > On 07/05/18 21:25, Mathieu Poirier wrote: >> >> On Tue, May 01, 2018 at 10:10:48AM +0100, Suzuki K Poulose wrote: >>> >>> This patch adds the support for setting up a SG table for use >>> by the CATU. We reuse the tmc_sg_table to represent the table/data >>> pages, even though the table format is different. >>> > > ... > >>> >>> diff --git a/drivers/hwtracing/coresight/coresight-catu.c >>> b/drivers/hwtracing/coresight/coresight-catu.c >>> index 2cd69a6..4cc2928 100644 >>> --- a/drivers/hwtracing/coresight/coresight-catu.c >>> +++ b/drivers/hwtracing/coresight/coresight-catu.c >>> @@ -16,10 +16,419 @@ > > > ... > >>> + >>> +/* >>> + * Update the valid bit for a given range of indices [start, end) >>> + * in the given table @table. >>> + */ >>> +static inline void catu_update_state_range(cate_t *table, int start, >>> + int end, int valid) >> >> >> Indentation >> > > ... > >>> +#ifdef CATU_DEBUG >>> +static void catu_dump_table(struct tmc_sg_table *catu_table) >>> +{ >>> + int i; >>> + cate_t *table; >>> + unsigned long table_end, buf_size, offset = 0; >>> + >>> + buf_size = tmc_sg_table_buf_size(catu_table); >>> + dev_dbg(catu_table->dev, >>> + "Dump table %p, tdaddr: %llx\n", >>> + catu_table, catu_table->table_daddr); >>> + >>> + while (offset < buf_size) { >>> + table_end = offset + SZ_1M < buf_size ? >>> + offset + SZ_1M : buf_size; >>> + table = catu_get_table(catu_table, offset, NULL); >>> + for (i = 0; offset < table_end; i++, offset += >>> CATU_PAGE_SIZE) >>> + dev_dbg(catu_table->dev, "%d: %llx\n", i, >>> table[i]); >>> + dev_dbg(catu_table->dev, "Prev : %llx, Next: %llx\n", >>> + table[CATU_LINK_PREV], table[CATU_LINK_NEXT]); >>> + dev_dbg(catu_table->dev, "== End of sub-table ==="); >>> + } >>> + dev_dbg(catu_table->dev, "== End of Table ==="); >>> +} >>> + >>> +#else >>> +static inline void catu_dump_table(struct tmc_sg_table *catu_table) >>> +{ >>> +} >>> +#endif >> >> >> I think this approach is better than peppering the code with #ifdefs as it >> was >> done for ETR. Please fix that to replicate what you've done here. >> > > OK > >>> + >>> +/* >>> + * catu_populate_table : Populate the given CATU table. >>> + * The table is always populated as a circular table. >>> + * i.e, the "prev" link of the "first" table points to the "last" >>> + * table and the "next" link of the "last" table points to the >>> + * "first" table. The buffer should be made linear by calling >>> + * catu_set_table(). >>> + */ >>> +static void >>> +catu_populate_table(struct tmc_sg_table *catu_table) >>> +{ > > > ... > >>> + while (offset < buf_size) { >>> + /* >>> + * The @offset is always 1M aligned here and we have an >>> + * empty table @table_ptr to fill. Each table can address >>> + * upto 1MB data buffer. The last table may have fewer >>> + * entries if the buffer size is not aligned. >>> + */ >>> + last_offset = (offset + SZ_1M) < buf_size ? >>> + (offset + SZ_1M) : buf_size; >>> + for (i = 0; offset < last_offset; i++) { >>> + >>> + data_daddr = catu_table->data_pages.daddrs[dpidx] >>> + >>> + s_dpidx * CATU_PAGE_SIZE; >>> +#ifdef CATU_DEBUG >>> + dev_dbg(catu_table->dev, >>> + "[table %5d:%03d] 0x%llx\n", >>> + (offset >> 20), i, data_daddr); >>> +#endif >> >> >> I'm not a fan of adding #ifdefs in the code like this. I think it is >> better to >> have a wrapper (that resolves to nothing if CATU_DEBUG is not defined) and >> handle the output in there. >> > > >>> + >>> + catu_populate_table(catu_table); >>> + /* Make the buf linear from offset 0 */ >>> + (void)catu_set_table(catu_table, 0, size); >>> + >>> + dev_dbg(catu_dev, >>> + "Setup table %p, size %ldKB, %d table pages\n", >>> + catu_table, (unsigned long)size >> 10, nr_tpages); >> >> >> I think this should also be wrapped in a special output debug function. >> > > I could do something like : > > #ifdef CATU_DEBUG > #define catu_dbg(fmt, ...) dev_dbg(fmt, __VA_ARGS__) > #else > #define catu_dbg(fmt, ...) do { } while (0) > #endif > > And use catu_dbg() for the sprinkled prints. Yes, that is exactly what I had in mind. > > Cheers > Suzuki